From 20ca23cab0bdfdffa567f8fb4b49f3727bac6444 Mon Sep 17 00:00:00 2001 From: Andrew Jorgensen Date: Mon, 14 Aug 2017 17:08:37 +0000 Subject: Log a helpful message if a user script does not include shebang. A patch to allow scripts missing a #! to run by using shell=True was proposed but rejected. Instead we emit a log message to help the user understand what went wrong. --- cloudinit/util.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index ce2c6034..609e94c8 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -12,7 +12,6 @@ import contextlib import copy as obj_copy import ctypes import email -import errno import glob import grp import gzip @@ -34,6 +33,8 @@ import sys import tempfile import time +from errno import ENOENT, ENOEXEC + from base64 import b64decode, b64encode from six.moves.urllib import parse as urlparse @@ -239,7 +240,10 @@ class ProcessExecutionError(IOError): self.cmd = cmd if not description: - self.description = 'Unexpected error while running command.' + if not exit_code and errno == ENOEXEC: + self.description = 'Exec format error. Missing #! in script?' + else: + self.description = 'Unexpected error while running command.' else: self.description = description @@ -433,7 +437,7 @@ def read_conf(fname): try: return load_yaml(load_file(fname), default={}) except IOError as e: - if e.errno == errno.ENOENT: + if e.errno == ENOENT: return {} else: raise @@ -901,7 +905,7 @@ def read_file_or_url(url, timeout=5, retries=10, contents = load_file(file_path, decode=False) except IOError as e: code = e.errno - if e.errno == errno.ENOENT: + if e.errno == ENOENT: code = url_helper.NOT_FOUND raise url_helper.UrlError(cause=e, code=code, headers=None, url=url) @@ -1247,7 +1251,7 @@ def find_devs_with(criteria=None, oformat='device', try: (out, _err) = subp(cmd, rcs=[0, 2]) except ProcessExecutionError as e: - if e.errno == errno.ENOENT: + if e.errno == ENOENT: # blkid not found... out = "" else: @@ -1285,7 +1289,7 @@ def load_file(fname, read_cb=None, quiet=False, decode=True): except IOError as e: if not quiet: raise - if e.errno != errno.ENOENT: + if e.errno != ENOENT: raise contents = ofh.getvalue() LOG.debug("Read %s bytes from %s", len(contents), fname) @@ -1653,7 +1657,7 @@ def del_file(path): try: os.unlink(path) except OSError as e: - if e.errno != errno.ENOENT: + if e.errno != ENOENT: raise e @@ -2281,7 +2285,7 @@ def pathprefix2dict(base, required=None, optional=None, delim=os.path.sep): try: ret[f] = load_file(base + delim + f, quiet=False, decode=False) except IOError as e: - if e.errno != errno.ENOENT: + if e.errno != ENOENT: raise if f in required: missing.append(f) -- cgit v1.2.3 From 409918f9ba83e45e9bc5cc0b6c589e2fc8ae9b60 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 29 Aug 2017 09:59:20 -0400 Subject: Use /run/cloud-init for tempfile operations. During boot, the usage of /tmp is not safe. In systemd systems, systemd-tmpfiles-clean may run at any point and clear out a temp file while cloud-init is using it. The solution here is to use /run/cloud-init/tmp. LP: #1707222 --- cloudinit/config/cc_bootcmd.py | 3 +- cloudinit/config/cc_chef.py | 3 +- cloudinit/config/cc_snappy.py | 4 +- cloudinit/net/dhcp.py | 3 +- cloudinit/sources/helpers/azure.py | 4 +- cloudinit/temp_utils.py | 93 ++++++++++++++++++++++ cloudinit/util.py | 36 +-------- packages/bddeb | 5 +- .../unittests/test_datasource/test_azure_helper.py | 4 +- tests/unittests/test_net.py | 3 +- 10 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 cloudinit/temp_utils.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py index 604f93b0..9c0476af 100644 --- a/cloudinit/config/cc_bootcmd.py +++ b/cloudinit/config/cc_bootcmd.py @@ -37,6 +37,7 @@ specified either as lists or strings. For invocation details, see ``runcmd``. import os from cloudinit.settings import PER_ALWAYS +from cloudinit import temp_utils from cloudinit import util frequency = PER_ALWAYS @@ -49,7 +50,7 @@ def handle(name, cfg, cloud, log, _args): " no 'bootcmd' key in configuration"), name) return - with util.ExtendedTemporaryFile(suffix=".sh") as tmpf: + with temp_utils.ExtendedTemporaryFile(suffix=".sh") as tmpf: try: content = util.shellify(cfg["bootcmd"]) tmpf.write(util.encode_text(content)) diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 02c70b10..c192dd32 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -71,6 +71,7 @@ import itertools import json import os +from cloudinit import temp_utils from cloudinit import templater from cloudinit import url_helper from cloudinit import util @@ -303,7 +304,7 @@ def install_chef(cloud, chef_cfg, log): "omnibus_url_retries", default=OMNIBUS_URL_RETRIES)) content = url_helper.readurl(url=url, retries=retries).contents - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: # Use tmpdir over tmpfile to avoid 'text file busy' on execute tmpf = "%s/chef-omnibus-install" % tmpd util.write_file(tmpf, content, mode=0o700) diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py index a9682f19..eecb8178 100644 --- a/cloudinit/config/cc_snappy.py +++ b/cloudinit/config/cc_snappy.py @@ -63,11 +63,11 @@ is ``auto``. Options are: from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE +from cloudinit import temp_utils from cloudinit import util import glob import os -import tempfile LOG = logging.getLogger(__name__) @@ -183,7 +183,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None): # config # Note, however, we do not touch config files on disk. nested_cfg = {'config': {shortname: config}} - (fd, cfg_tmpf) = tempfile.mkstemp() + (fd, cfg_tmpf) = temp_utils.mkstemp() os.write(fd, util.yaml_dumps(nested_cfg).encode()) os.close(fd) cfgfile = cfg_tmpf diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index c7febc57..c842c839 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -9,6 +9,7 @@ import os import re from cloudinit.net import find_fallback_nic, get_devicelist +from cloudinit import temp_utils from cloudinit import util LOG = logging.getLogger(__name__) @@ -47,7 +48,7 @@ def maybe_perform_dhcp_discovery(nic=None): if not dhclient_path: LOG.debug('Skip dhclient configuration: No dhclient command found.') return {} - with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: + with temp_utils.tempdir(prefix='cloud-init-dhcp-') as tmpdir: return dhcp_discovery(dhclient_path, nic, tmpdir) diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index e22409d1..28ed0ae2 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -6,10 +6,10 @@ import os import re import socket import struct -import tempfile import time from cloudinit import stages +from cloudinit import temp_utils from contextlib import contextmanager from xml.etree import ElementTree @@ -111,7 +111,7 @@ class OpenSSLManager(object): } def __init__(self): - self.tmpdir = tempfile.mkdtemp() + self.tmpdir = temp_utils.mkdtemp() self.certificate = None self.generate_certificate() diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py new file mode 100644 index 00000000..0355f19d --- /dev/null +++ b/cloudinit/temp_utils.py @@ -0,0 +1,93 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import contextlib +import errno +import os +import shutil +import tempfile + +_TMPDIR = None +_ROOT_TMPDIR = "/run/cloud-init/tmp" + + +def _tempfile_dir_arg(odir=None): + """Return the proper 'dir' argument for tempfile functions. + + When root, cloud-init will use /run/cloud-init/tmp to avoid + any cleaning that a distro boot might do on /tmp (such as + systemd-tmpfiles-clean). + + If the caller of this function (mkdtemp or mkstemp) was provided + with a 'dir' argument, then that is respected. + + @param odir: original 'dir' arg to 'mkdtemp' or other.""" + + if odir is not None: + return odir + + global _TMPDIR + if _TMPDIR: + return _TMPDIR + + if os.getuid() == 0: + tdir = _ROOT_TMPDIR + else: + tdir = os.environ.get('TMPDIR', '/tmp') + if not os.path.isdir(tdir): + os.makedirs(tdir) + os.chmod(tdir, 0o1777) + + _TMPDIR = tdir + return tdir + + +def ExtendedTemporaryFile(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + fh = tempfile.NamedTemporaryFile(**kwargs) + # Replace its unlink with a quiet version + # that does not raise errors when the + # file to unlink has been unlinked elsewhere.. + + def _unlink_if_exists(path): + try: + os.unlink(path) + except OSError as e: + if e.errno != errno.ENOENT: + raise e + + fh.unlink = _unlink_if_exists + + # Add a new method that will unlink + # right 'now' but still lets the exit + # method attempt to remove it (which will + # not throw due to our del file being quiet + # about files that are not there) + def unlink_now(): + fh.unlink(fh.name) + + setattr(fh, 'unlink_now', unlink_now) + return fh + + +@contextlib.contextmanager +def tempdir(**kwargs): + # This seems like it was only added in python 3.2 + # Make it since its useful... + # See: http://bugs.python.org/file12970/tempdir.patch + tdir = mkdtemp(**kwargs) + try: + yield tdir + finally: + shutil.rmtree(tdir) + + +def mkdtemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkdtemp(**kwargs) + + +def mkstemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkstemp(**kwargs) + +# vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 609e94c8..ae5cda8d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -30,7 +30,6 @@ import stat import string import subprocess import sys -import tempfile import time from errno import ENOENT, ENOEXEC @@ -45,6 +44,7 @@ from cloudinit import importer from cloudinit import log as logging from cloudinit import mergers from cloudinit import safeyaml +from cloudinit import temp_utils from cloudinit import type_utils from cloudinit import url_helper from cloudinit import version @@ -349,26 +349,6 @@ class DecompressionError(Exception): pass -def ExtendedTemporaryFile(**kwargs): - fh = tempfile.NamedTemporaryFile(**kwargs) - # Replace its unlink with a quiet version - # that does not raise errors when the - # file to unlink has been unlinked elsewhere.. - LOG.debug("Created temporary file %s", fh.name) - fh.unlink = del_file - - # Add a new method that will unlink - # right 'now' but still lets the exit - # method attempt to remove it (which will - # not throw due to our del file being quiet - # about files that are not there) - def unlink_now(): - fh.unlink(fh.name) - - setattr(fh, 'unlink_now', unlink_now) - return fh - - def fork_cb(child_cb, *args, **kwargs): fid = os.fork() if fid == 0: @@ -790,18 +770,6 @@ def umask(n_msk): os.umask(old) -@contextlib.contextmanager -def tempdir(**kwargs): - # This seems like it was only added in python 3.2 - # Make it since its useful... - # See: http://bugs.python.org/file12970/tempdir.patch - tdir = tempfile.mkdtemp(**kwargs) - try: - yield tdir - finally: - del_dir(tdir) - - def center(text, fill, max_len): return '{0:{fill}{align}{size}}'.format(text, fill=fill, align="^", size=max_len) @@ -1587,7 +1555,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): mtypes = [''] mounted = mounts() - with tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: umount = False if os.path.realpath(device) in mounted: mountpoint = mounted[os.path.realpath(device)]['mountpoint'] diff --git a/packages/bddeb b/packages/bddeb index 7c123548..4f2e2ddf 100755 --- a/packages/bddeb +++ b/packages/bddeb @@ -21,8 +21,9 @@ def find_root(): if "avoid-pep8-E402-import-not-top-of-file": # Use the util functions from cloudinit sys.path.insert(0, find_root()) - from cloudinit import templater from cloudinit import util + from cloudinit import temp_utils + from cloudinit import templater DEBUILD_ARGS = ["-S", "-d"] @@ -148,7 +149,7 @@ def main(): capture = False templ_data = {'debian_release': args.release} - with util.tempdir() as tdir: + with temp_utils.tempdir() as tdir: # output like 0.7.6-1022-g36e92d3 ver_data = read_version() diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 80ce003d..44b99eca 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -275,7 +275,7 @@ class TestOpenSSLManager(TestCase): mock.patch('builtins.open')) @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp') + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp') def test_openssl_manager_creates_a_tmpdir(self, mkdtemp): manager = azure_helper.OpenSSLManager() self.assertEqual(mkdtemp.return_value, manager.tmpdir) @@ -292,7 +292,7 @@ class TestOpenSSLManager(TestCase): manager.clean_up() @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp', mock.MagicMock()) + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp', mock.MagicMock()) @mock.patch.object(azure_helper.util, 'del_dir') def test_clean_up(self, del_dir): manager = azure_helper.OpenSSLManager() diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index c10ef905..f2496151 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -9,6 +9,7 @@ from cloudinit.net import network_state from cloudinit.net import renderers from cloudinit.net import sysconfig from cloudinit.sources.helpers import openstack +from cloudinit import temp_utils from cloudinit import util from cloudinit.tests.helpers import CiTestCase @@ -2150,7 +2151,7 @@ class TestCmdlineConfigParsing(CiTestCase): static['mac_address'] = macs['eth1'] expected = {'version': 1, 'config': [dhcp, static]} - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: for fname, content in pairs: fp = os.path.join(tmpd, fname) files.append(fp) -- cgit v1.2.3 From cf10a2ff2e2f666d9370f38297a5a105e809ea3c Mon Sep 17 00:00:00 2001 From: Ethan Apodaca Date: Wed, 13 Sep 2017 22:18:26 -0600 Subject: chef: Add option to pin chef omnibus install version Most users of chef will want to pin the version that is installed. Typically new versions of chef have to be evaluated for breakage etc. This change proposes a new optional `omnibus_version` field to the chef configuration. The changeset also adds documentation referencing the new field. LP: #1462693 --- cloudinit/config/cc_chef.py | 45 ++++++++---- cloudinit/util.py | 25 +++++++ doc/examples/cloud-config-chef.txt | 4 ++ tests/unittests/test_handler/test_handler_chef.py | 88 +++++++++++++++++++---- 4 files changed, 138 insertions(+), 24 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index c192dd32..46abedd1 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -58,6 +58,9 @@ file). log_level: log_location: node_name: + omnibus_url: + omnibus_url_retries: + omnibus_version: pid_file: server_url: show_time: @@ -71,7 +74,6 @@ import itertools import json import os -from cloudinit import temp_utils from cloudinit import templater from cloudinit import url_helper from cloudinit import util @@ -280,6 +282,31 @@ def run_chef(chef_cfg, log): util.subp(cmd, capture=False) +def install_chef_from_omnibus(url=None, retries=None, omnibus_version=None): + """Install an omnibus unified package from url. + + @param url: URL where blob of chef content may be downloaded. Defaults to + OMNIBUS_URL. + @param retries: Number of retries to perform when attempting to read url. + Defaults to OMNIBUS_URL_RETRIES + @param omnibus_version: Optional version string to require for omnibus + install. + """ + if url is None: + url = OMNIBUS_URL + if retries is None: + retries = OMNIBUS_URL_RETRIES + + if omnibus_version is None: + args = [] + else: + args = ['-v', omnibus_version] + content = url_helper.readurl(url=url, retries=retries).contents + return util.subp_blob_in_tempfile( + blob=content, args=args, + basename='chef-omnibus-install', capture=False) + + def install_chef(cloud, chef_cfg, log): # If chef is not installed, we install chef based on 'install_type' install_type = util.get_cfg_option_str(chef_cfg, 'install_type', @@ -298,17 +325,11 @@ def install_chef(cloud, chef_cfg, log): # This will install and run the chef-client from packages cloud.distro.install_packages(('chef',)) elif install_type == 'omnibus': - # This will install as a omnibus unified package - url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL) - retries = max(0, util.get_cfg_option_int(chef_cfg, - "omnibus_url_retries", - default=OMNIBUS_URL_RETRIES)) - content = url_helper.readurl(url=url, retries=retries).contents - with temp_utils.tempdir() as tmpd: - # Use tmpdir over tmpfile to avoid 'text file busy' on execute - tmpf = "%s/chef-omnibus-install" % tmpd - util.write_file(tmpf, content, mode=0o700) - util.subp([tmpf], capture=False) + omnibus_version = util.get_cfg_option_str(chef_cfg, "omnibus_version") + install_chef_from_omnibus( + url=util.get_cfg_option_str(chef_cfg, "omnibus_url"), + retries=util.get_cfg_option_int(chef_cfg, "omnibus_url_retries"), + omnibus_version=omnibus_version) else: log.warn("Unknown chef install type '%s'", install_type) run = False diff --git a/cloudinit/util.py b/cloudinit/util.py index ae5cda8d..7e9d94fc 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1742,6 +1742,31 @@ def delete_dir_contents(dirname): del_file(node_fullpath) +def subp_blob_in_tempfile(blob, *args, **kwargs): + """Write blob to a tempfile, and call subp with args, kwargs. Then cleanup. + + 'basename' as a kwarg allows providing the basename for the file. + The 'args' argument to subp will be updated with the full path to the + filename as the first argument. + """ + basename = kwargs.pop('basename', "subp_blob") + + if len(args) == 0 and 'args' not in kwargs: + args = [tuple()] + + # Use tmpdir over tmpfile to avoid 'text file busy' on execute + with temp_utils.tempdir() as tmpd: + tmpf = os.path.join(tmpd, basename) + if 'args' in kwargs: + kwargs['args'] = [tmpf] + list(kwargs['args']) + else: + args = list(args) + args[0] = [tmpf] + args[0] + + write_file(tmpf, blob, mode=0o700) + return subp(*args, **kwargs) + + def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, logstring=False, decode="replace", target=None, update_env=None): diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt index 9d235817..58d5fdc7 100644 --- a/doc/examples/cloud-config-chef.txt +++ b/doc/examples/cloud-config-chef.txt @@ -94,6 +94,10 @@ chef: # if install_type is 'omnibus', change the url to download omnibus_url: "https://www.chef.io/chef/install.sh" + # if install_type is 'omnibus', pass pinned version string + # to the install script + omnibus_version: "12.3.0" + # Capture all subprocess output into a logfile # Useful for troubleshooting cloud-init issues diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index e5785cfd..0136a93d 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -1,11 +1,10 @@ # This file is part of cloud-init. See LICENSE file for license information. +import httpretty import json import logging import os -import shutil import six -import tempfile from cloudinit import cloud from cloudinit.config import cc_chef @@ -14,18 +13,83 @@ from cloudinit import helpers from cloudinit.sources import DataSourceNone from cloudinit import util -from cloudinit.tests import helpers as t_help +from cloudinit.tests.helpers import ( + CiTestCase, FilesystemMockingTestCase, mock, skipIf) LOG = logging.getLogger(__name__) CLIENT_TEMPL = os.path.sep.join(["templates", "chef_client.rb.tmpl"]) -class TestChef(t_help.FilesystemMockingTestCase): +class TestInstallChefOmnibus(CiTestCase): + + def setUp(self): + self.new_root = self.tmp_dir() + + @httpretty.activate + def test_install_chef_from_omnibus_runs_chef_url_content(self): + """install_chef_from_omnibus runs downloaded OMNIBUS_URL as script.""" + chef_outfile = self.tmp_path('chef.out', self.new_root) + response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile) + httpretty.register_uri( + httpretty.GET, cc_chef.OMNIBUS_URL, body=response, status=200) + cc_chef.install_chef_from_omnibus() + self.assertEqual('Hi Mom\n', util.load_file(chef_outfile)) + + @mock.patch('cloudinit.config.cc_chef.url_helper.readurl') + @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile') + def test_install_chef_from_omnibus_retries_url(self, m_subp_blob, m_rdurl): + """install_chef_from_omnibus retries OMNIBUS_URL upon failure.""" + + class FakeURLResponse(object): + contents = '#!/bin/bash\necho "Hi Mom" > {0}/chef.out'.format( + self.new_root) + + m_rdurl.return_value = FakeURLResponse() + + cc_chef.install_chef_from_omnibus() + expected_kwargs = {'retries': cc_chef.OMNIBUS_URL_RETRIES, + 'url': cc_chef.OMNIBUS_URL} + self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[0][1]) + cc_chef.install_chef_from_omnibus(retries=10) + expected_kwargs = {'retries': 10, + 'url': cc_chef.OMNIBUS_URL} + self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[1][1]) + expected_subp_kwargs = { + 'args': ['-v', '2.0'], + 'basename': 'chef-omnibus-install', + 'blob': m_rdurl.return_value.contents, + 'capture': False + } + self.assertItemsEqual( + expected_subp_kwargs, + m_subp_blob.call_args_list[0][1]) + + @httpretty.activate + @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile') + def test_install_chef_from_omnibus_has_omnibus_version(self, m_subp_blob): + """install_chef_from_omnibus provides version arg to OMNIBUS_URL.""" + chef_outfile = self.tmp_path('chef.out', self.new_root) + response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile) + httpretty.register_uri( + httpretty.GET, cc_chef.OMNIBUS_URL, body=response) + cc_chef.install_chef_from_omnibus(omnibus_version='2.0') + + called_kwargs = m_subp_blob.call_args_list[0][1] + expected_kwargs = { + 'args': ['-v', '2.0'], + 'basename': 'chef-omnibus-install', + 'blob': response, + 'capture': False + } + self.assertItemsEqual(expected_kwargs, called_kwargs) + + +class TestChef(FilesystemMockingTestCase): + def setUp(self): super(TestChef, self).setUp() - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp) + self.tmp = self.tmp_dir() def fetch_cloud(self, distro_kind): cls = distros.fetch(distro_kind) @@ -43,8 +107,8 @@ class TestChef(t_help.FilesystemMockingTestCase): for d in cc_chef.CHEF_DIRS: self.assertFalse(os.path.isdir(d)) - @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), - CLIENT_TEMPL + " is not available") + @skipIf(not os.path.isfile(CLIENT_TEMPL), + CLIENT_TEMPL + " is not available") def test_basic_config(self): """ test basic config looks sane @@ -122,8 +186,8 @@ class TestChef(t_help.FilesystemMockingTestCase): 'c': 'd', }, json.loads(c)) - @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), - CLIENT_TEMPL + " is not available") + @skipIf(not os.path.isfile(CLIENT_TEMPL), + CLIENT_TEMPL + " is not available") def test_template_deletes(self): tpl_file = util.load_file('templates/chef_client.rb.tmpl') self.patchUtils(self.tmp) @@ -143,8 +207,8 @@ class TestChef(t_help.FilesystemMockingTestCase): self.assertNotIn('json_attribs', c) self.assertNotIn('Formatter.show_time', c) - @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL), - CLIENT_TEMPL + " is not available") + @skipIf(not os.path.isfile(CLIENT_TEMPL), + CLIENT_TEMPL + " is not available") def test_validation_cert_and_validation_key(self): # test validation_cert content is written to validation_key path tpl_file = util.load_file('templates/chef_client.rb.tmpl') -- cgit v1.2.3 From 7eb3460b0d6d3e362a246958a7ea0a9ee5d91d5e Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 15 Sep 2017 20:07:11 -0600 Subject: ec2: Fix maybe_perform_dhcp_discovery to use /var/tmp as a tmpdir /run/cloud-init/tmp is on a filesystem mounted noexec, so running dchlient in Ec2Local during discovery breaks with 'Permission denied'. This branch allows us to run from a different tmp dir so we have exec rights. LP: #1717627 --- cloudinit/net/dhcp.py | 5 +- cloudinit/net/tests/test_dhcp.py | 18 ++++--- cloudinit/temp_utils.py | 22 +++++--- cloudinit/tests/test_temp_utils.py | 101 +++++++++++++++++++++++++++++++++++++ cloudinit/util.py | 2 +- 5 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 cloudinit/tests/test_temp_utils.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index c842c839..05350639 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -48,8 +48,9 @@ def maybe_perform_dhcp_discovery(nic=None): if not dhclient_path: LOG.debug('Skip dhclient configuration: No dhclient command found.') return {} - with temp_utils.tempdir(prefix='cloud-init-dhcp-') as tmpdir: - return dhcp_discovery(dhclient_path, nic, tmpdir) + with temp_utils.tempdir(prefix='cloud-init-dhcp-', needs_exe=True) as tdir: + # Use /var/tmp because /run/cloud-init/tmp is mounted noexec + return dhcp_discovery(dhclient_path, nic, tdir) def parse_dhcp_lease_file(lease_file): diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 4a37e98a..1324c3d0 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -8,7 +8,7 @@ from cloudinit.net.dhcp import ( InvalidDHCPLeaseFileError, maybe_perform_dhcp_discovery, parse_dhcp_lease_file, dhcp_discovery) from cloudinit.util import ensure_file, write_file -from cloudinit.tests.helpers import CiTestCase +from cloudinit.tests.helpers import CiTestCase, wrap_and_call class TestParseDHCPLeasesFile(CiTestCase): @@ -91,21 +91,27 @@ class TestDHCPDiscoveryClean(CiTestCase): 'Skip dhclient configuration: No dhclient command found.', self.logs.getvalue()) + @mock.patch('cloudinit.temp_utils.os.getuid') @mock.patch('cloudinit.net.dhcp.dhcp_discovery') @mock.patch('cloudinit.net.dhcp.util.which') @mock.patch('cloudinit.net.dhcp.find_fallback_nic') - def test_dhclient_run_with_tmpdir(self, m_fallback, m_which, m_dhcp): + def test_dhclient_run_with_tmpdir(self, m_fback, m_which, m_dhcp, m_uid): """maybe_perform_dhcp_discovery passes tmpdir to dhcp_discovery.""" - m_fallback.return_value = 'eth9' + m_uid.return_value = 0 # Fake root user for tmpdir + m_fback.return_value = 'eth9' m_which.return_value = '/sbin/dhclient' m_dhcp.return_value = {'address': '192.168.2.2'} - self.assertEqual( - {'address': '192.168.2.2'}, maybe_perform_dhcp_discovery()) + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'_TMPDIR': {'new': None}, + 'os.getuid': 0}, + maybe_perform_dhcp_discovery) + self.assertEqual({'address': '192.168.2.2'}, retval) m_dhcp.assert_called_once() call = m_dhcp.call_args_list[0] self.assertEqual('/sbin/dhclient', call[0][0]) self.assertEqual('eth9', call[0][1]) - self.assertIn('/tmp/cloud-init-dhcp-', call[0][2]) + self.assertIn('/var/tmp/cloud-init/cloud-init-dhcp-', call[0][2]) @mock.patch('cloudinit.net.dhcp.util.subp') def test_dhcp_discovery_run_in_sandbox(self, m_subp): diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py index 0355f19d..5d7adf70 100644 --- a/cloudinit/temp_utils.py +++ b/cloudinit/temp_utils.py @@ -8,9 +8,10 @@ import tempfile _TMPDIR = None _ROOT_TMPDIR = "/run/cloud-init/tmp" +_EXE_ROOT_TMPDIR = "/var/tmp/cloud-init" -def _tempfile_dir_arg(odir=None): +def _tempfile_dir_arg(odir=None, needs_exe=False): """Return the proper 'dir' argument for tempfile functions. When root, cloud-init will use /run/cloud-init/tmp to avoid @@ -20,8 +21,10 @@ def _tempfile_dir_arg(odir=None): If the caller of this function (mkdtemp or mkstemp) was provided with a 'dir' argument, then that is respected. - @param odir: original 'dir' arg to 'mkdtemp' or other.""" - + @param odir: original 'dir' arg to 'mkdtemp' or other. + @param needs_exe: Boolean specifying whether or not exe permissions are + needed for tempdir. This is needed because /run is mounted noexec. + """ if odir is not None: return odir @@ -29,7 +32,9 @@ def _tempfile_dir_arg(odir=None): if _TMPDIR: return _TMPDIR - if os.getuid() == 0: + if needs_exe: + tdir = _EXE_ROOT_TMPDIR + elif os.getuid() == 0: tdir = _ROOT_TMPDIR else: tdir = os.environ.get('TMPDIR', '/tmp') @@ -42,7 +47,8 @@ def _tempfile_dir_arg(odir=None): def ExtendedTemporaryFile(**kwargs): - kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + kwargs['dir'] = _tempfile_dir_arg( + kwargs.pop('dir', None), kwargs.pop('needs_exe', False)) fh = tempfile.NamedTemporaryFile(**kwargs) # Replace its unlink with a quiet version # that does not raise errors when the @@ -82,12 +88,14 @@ def tempdir(**kwargs): def mkdtemp(**kwargs): - kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + kwargs['dir'] = _tempfile_dir_arg( + kwargs.pop('dir', None), kwargs.pop('needs_exe', False)) return tempfile.mkdtemp(**kwargs) def mkstemp(**kwargs): - kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + kwargs['dir'] = _tempfile_dir_arg( + kwargs.pop('dir', None), kwargs.pop('needs_exe', False)) return tempfile.mkstemp(**kwargs) # vi: ts=4 expandtab diff --git a/cloudinit/tests/test_temp_utils.py b/cloudinit/tests/test_temp_utils.py new file mode 100644 index 00000000..ffbb92cd --- /dev/null +++ b/cloudinit/tests/test_temp_utils.py @@ -0,0 +1,101 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +"""Tests for cloudinit.temp_utils""" + +from cloudinit.temp_utils import mkdtemp, mkstemp +from cloudinit.tests.helpers import CiTestCase, wrap_and_call + + +class TestTempUtils(CiTestCase): + + def test_mkdtemp_default_non_root(self): + """mkdtemp creates a dir under /tmp for the unprivileged.""" + calls = [] + + def fake_mkdtemp(*args, **kwargs): + calls.append(kwargs) + return '/fake/return/path' + + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'os.getuid': 1000, + 'tempfile.mkdtemp': {'side_effect': fake_mkdtemp}, + '_TMPDIR': {'new': None}, + 'os.path.isdir': True}, + mkdtemp) + self.assertEqual('/fake/return/path', retval) + self.assertEqual([{'dir': '/tmp'}], calls) + + def test_mkdtemp_default_non_root_needs_exe(self): + """mkdtemp creates a dir under /var/tmp/cloud-init when needs_exe.""" + calls = [] + + def fake_mkdtemp(*args, **kwargs): + calls.append(kwargs) + return '/fake/return/path' + + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'os.getuid': 1000, + 'tempfile.mkdtemp': {'side_effect': fake_mkdtemp}, + '_TMPDIR': {'new': None}, + 'os.path.isdir': True}, + mkdtemp, needs_exe=True) + self.assertEqual('/fake/return/path', retval) + self.assertEqual([{'dir': '/var/tmp/cloud-init'}], calls) + + def test_mkdtemp_default_root(self): + """mkdtemp creates a dir under /run/cloud-init for the privileged.""" + calls = [] + + def fake_mkdtemp(*args, **kwargs): + calls.append(kwargs) + return '/fake/return/path' + + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'os.getuid': 0, + 'tempfile.mkdtemp': {'side_effect': fake_mkdtemp}, + '_TMPDIR': {'new': None}, + 'os.path.isdir': True}, + mkdtemp) + self.assertEqual('/fake/return/path', retval) + self.assertEqual([{'dir': '/run/cloud-init/tmp'}], calls) + + def test_mkstemp_default_non_root(self): + """mkstemp creates secure tempfile under /tmp for the unprivileged.""" + calls = [] + + def fake_mkstemp(*args, **kwargs): + calls.append(kwargs) + return '/fake/return/path' + + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'os.getuid': 1000, + 'tempfile.mkstemp': {'side_effect': fake_mkstemp}, + '_TMPDIR': {'new': None}, + 'os.path.isdir': True}, + mkstemp) + self.assertEqual('/fake/return/path', retval) + self.assertEqual([{'dir': '/tmp'}], calls) + + def test_mkstemp_default_root(self): + """mkstemp creates a secure tempfile in /run/cloud-init for root.""" + calls = [] + + def fake_mkstemp(*args, **kwargs): + calls.append(kwargs) + return '/fake/return/path' + + retval = wrap_and_call( + 'cloudinit.temp_utils', + {'os.getuid': 0, + 'tempfile.mkstemp': {'side_effect': fake_mkstemp}, + '_TMPDIR': {'new': None}, + 'os.path.isdir': True}, + mkstemp) + self.assertEqual('/fake/return/path', retval) + self.assertEqual([{'dir': '/run/cloud-init/tmp'}], calls) + +# vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 7e9d94fc..4c01f449 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1755,7 +1755,7 @@ def subp_blob_in_tempfile(blob, *args, **kwargs): args = [tuple()] # Use tmpdir over tmpfile to avoid 'text file busy' on execute - with temp_utils.tempdir() as tmpd: + with temp_utils.tempdir(needs_exe=True) as tmpd: tmpf = os.path.join(tmpd, basename) if 'args' in kwargs: kwargs['args'] = [tmpf] + list(kwargs['args']) -- cgit v1.2.3 From 0451a9f60960da56e3af4f97bbcece3d98482f86 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 21 Sep 2017 07:38:48 -0400 Subject: suse: updates to templates to support openSUSE and SLES. Things done here: - identify 'suse' as a variant in util.system_info and also tools/render-cloudcfg. - update systemd and cloud.cfg templates for suse specific changes. LP: #1718640 --- cloudinit/util.py | 2 ++ config/cloud.cfg.tmpl | 8 ++++++-- systemd/cloud-init-local.service.tmpl | 6 ++++++ systemd/cloud-init.service.tmpl | 10 ++++++++++ tools/render-cloudcfg | 5 +++-- 5 files changed, 27 insertions(+), 4 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 4c01f449..e1290aa8 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -598,6 +598,8 @@ def system_info(): var = 'ubuntu' elif linux_dist == 'redhat': var = 'rhel' + elif linux_dist == 'suse': + var = 'suse' else: var = 'linux' elif system in ('windows', 'darwin', "freebsd"): diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl index a537d65a..50e3bd86 100644 --- a/config/cloud.cfg.tmpl +++ b/config/cloud.cfg.tmpl @@ -127,7 +127,7 @@ cloud_final_modules: # (not accessible to handlers/transforms) system_info: # This will affect which distro class gets used -{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu", "freebsd"] %} +{% if variant in ["centos", "debian", "fedora", "rhel", "suse", "ubuntu", "freebsd"] %} distro: {{ variant }} {% else %} # Unknown/fallback distro. @@ -163,13 +163,17 @@ system_info: primary: http://ports.ubuntu.com/ubuntu-ports security: http://ports.ubuntu.com/ubuntu-ports ssh_svcname: ssh -{% elif variant in ["centos", "rhel", "fedora"] %} +{% elif variant in ["centos", "rhel", "fedora", "suse"] %} # Default user name + that default users groups (if added/used) default_user: name: {{ variant }} lock_passwd: True gecos: {{ variant }} Cloud User +{% if variant == "suse" %} + groups: [cdrom, users] +{% else %} groups: [wheel, adm, systemd-journal] +{% endif %} sudo: ["ALL=(ALL) NOPASSWD:ALL"] shell: /bin/bash # Other config here will be given to the distro class and/or path classes diff --git a/systemd/cloud-init-local.service.tmpl b/systemd/cloud-init-local.service.tmpl index ff9c644d..bf6b2961 100644 --- a/systemd/cloud-init-local.service.tmpl +++ b/systemd/cloud-init-local.service.tmpl @@ -13,6 +13,12 @@ Before=shutdown.target Before=sysinit.target Conflicts=shutdown.target {% endif %} +{% if variant in ["suse"] %} +# Other distros use Before=sysinit.target. There is not a clearly identified +# reason for usage of basic.target instead. +Before=basic.target +Conflicts=shutdown.target +{% endif %} RequiresMountsFor=/var/lib/cloud [Service] diff --git a/systemd/cloud-init.service.tmpl b/systemd/cloud-init.service.tmpl index 2c71889d..b92e8abc 100644 --- a/systemd/cloud-init.service.tmpl +++ b/systemd/cloud-init.service.tmpl @@ -13,6 +13,13 @@ After=networking.service {% if variant in ["centos", "fedora", "redhat"] %} After=network.service {% endif %} +{% if variant in ["suse"] %} +Requires=wicked.service +After=wicked.service +# setting hostname via hostnamectl depends on dbus, which otherwise +# would not be guaranteed at this point. +After=dbus.service +{% endif %} Before=network-online.target Before=sshd-keygen.service Before=sshd.service @@ -20,6 +27,9 @@ Before=sshd.service Before=sysinit.target Conflicts=shutdown.target {% endif %} +{% if variant in ["suse"] %} +Conflicts=shutdown.target +{% endif %} Before=systemd-user-sessions.service [Service] diff --git a/tools/render-cloudcfg b/tools/render-cloudcfg index e624541a..8b7cb875 100755 --- a/tools/render-cloudcfg +++ b/tools/render-cloudcfg @@ -4,6 +4,8 @@ import argparse import os import sys +VARIANTS = ["bsd", "centos", "fedora", "rhel", "suse", "ubuntu", "unknown"] + if "avoid-pep8-E402-import-not-top-of-file": _tdir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) sys.path.insert(0, _tdir) @@ -14,11 +16,10 @@ if "avoid-pep8-E402-import-not-top-of-file": def main(): parser = argparse.ArgumentParser() - variants = ["bsd", "centos", "fedora", "rhel", "ubuntu", "unknown"] platform = util.system_info() parser.add_argument( "--variant", default=platform['variant'], action="store", - help="define the variant.", choices=variants) + help="define the variant.", choices=VARIANTS) parser.add_argument( "template", nargs="?", action="store", default='./config/cloud.cfg.tmpl', -- cgit v1.2.3