diff options
| -rwxr-xr-x | bin/cfn-signal | 15 | ||||
| -rw-r--r-- | heat_cfntools/cfntools/cfn_helper.py | 235 | ||||
| -rw-r--r-- | heat_cfntools/tests/test_cfn_helper.py | 324 |
3 files changed, 329 insertions, 245 deletions
diff --git a/bin/cfn-signal b/bin/cfn-signal index a8ab7c7..7fce58e 100755 --- a/bin/cfn-signal +++ b/bin/cfn-signal @@ -100,14 +100,19 @@ body = { "UniqueId": unique_id, "Data": args.data } +data = cfn_helper.json.dumps(body) -insecure = "" +cmd = ['curl'] if args.insecure: - insecure = "--insecure" + cmd.append('--insecure') +cmd.extend([ + '-X', 'PUT', + '-H', 'Content-Type:', + '--data-binary', data, + args.url +]) -cmd_str = ("curl %s -X PUT -H \'Content-Type:\' --data-binary \'%s\' \"%s\"" % - (insecure, cfn_helper.json.dumps(body), args.url)) -command = cfn_helper.CommandRunner(cmd_str).run() +command = cfn_helper.CommandRunner(cmd).run() if command.status != 0: LOG.error(command.stderr) sys.exit(command.status) diff --git a/heat_cfntools/cfntools/cfn_helper.py b/heat_cfntools/cfntools/cfn_helper.py index 2351c1d..e241e57 100644 --- a/heat_cfntools/cfntools/cfn_helper.py +++ b/heat_cfntools/cfntools/cfn_helper.py @@ -1,4 +1,4 @@ -# + # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain # a copy of the License at @@ -19,6 +19,7 @@ Not implemented yet: - placeholders are ignored """ import atexit +import contextlib import errno import functools import grp @@ -40,6 +41,7 @@ import six.moves.configparser as ConfigParser import subprocess import tempfile + # Override BOTO_CONFIG, which makes boto look only at the specified # config file, instead of the default locations os.environ['BOTO_CONFIG'] = '/var/lib/heat-cfntools/cfn-boto-cfg' @@ -138,7 +140,7 @@ class Hook(object): def event(self, ev_name, ev_object, ev_resource): if self.resource_name_get() == ev_resource and \ ev_name in self.triggers: - CommandRunner(self.action).run(user=self.runas) + CommandRunner(self.action, shell=True).run(user=self.runas) else: LOG.debug('event: {%s, %s, %s} did not match %s' % (ev_name, ev_object, ev_resource, self.__str__())) @@ -152,11 +154,39 @@ class Hook(object): self.action) +class ControlledPrivilegesFailureException(Exception): + pass + + +@contextlib.contextmanager +def controlled_privileges(user): + orig_euid = None + try: + real = pwd.getpwnam(user) + if os.geteuid() != real.pw_uid: + orig_euid = os.geteuid() + os.seteuid(real.pw_uid) + LOG.debug("Privileges set for user %s" % user) + except Exception as e: + raise ControlledPrivilegesFailureException(e) + + try: + yield + finally: + if orig_euid is not None: + try: + os.seteuid(orig_euid) + LOG.debug("Original privileges restored.") + except Exception as e: + LOG.error("Error restoring privileges %s" % e) + + class CommandRunner(object): """Helper class to run a command and store the output.""" - def __init__(self, command, nextcommand=None): + def __init__(self, command, shell=False, nextcommand=None): self._command = command + self._shell = shell self._next = nextcommand self._stdout = None self._stderr = None @@ -182,33 +212,32 @@ class CommandRunner(object): LOG.debug("Running command: %s" % self._command) cmd = self._command - if isinstance(cmd, str): - cmd = cmd.split() + shell = self._shell - if user != 'root': - # lower the privileges - try: - real = pwd.getpwnam(user) - os.setuid(real.pw_uid) - except Exception as e: - LOG.error("Error setting privileges for user '%s': %s" - % (user, e)) - # 126: command invoked cannot be executed - self._status = 126 - return + # Ensure commands that are given as string are run on shell + assert isinstance(cmd, six.string_types) is bool(shell) - subproc = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, cwd=cwd, env=env) - output = subproc.communicate() + try: + with controlled_privileges(user): + subproc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, cwd=cwd, + env=env, shell=shell) + output = subproc.communicate() + self._status = subproc.returncode + self._stdout = output[0] + self._stderr = output[1] + except ControlledPrivilegesFailureException as e: + LOG.error("Error setting privileges for user '%s': %s" + % (user, e)) + self._status = 126 + self._stderr = six.text_type(e) - self._status = subproc.returncode - self._stdout = output[0] - self._stderr = output[1] if self._status: LOG.debug("Return code of %d after executing: '%s'\n" "stdout: '%s'\n" "stderr: '%s'" % (self._status, cmd, self._stdout, self._stderr)) + if self._next: self._next.run() return self @@ -291,7 +320,8 @@ class RpmHelper(object): e.g., httpd-2.2.22 e.g., httpd-2.2.22-1.fc16 """ - command = CommandRunner("rpm -q %s" % pkg).run() + cmd = ['rpm', '-q', pkg] + command = CommandRunner(cmd).run() return command.status == 0 @classmethod @@ -304,8 +334,8 @@ class RpmHelper(object): e.g., httpd-2.2.22 e.g., httpd-2.2.22-1.fc16 """ - cmd_str = "yum -y --showduplicates list available %s" % pkg - command = CommandRunner(cmd_str).run() + cmd = ['yum', '-y', '--showduplicates', 'list', 'available', pkg] + command = CommandRunner(cmd).run() return command.status == 0 @classmethod @@ -318,8 +348,8 @@ class RpmHelper(object): e.g., httpd-2.2.22 e.g., httpd-2.2.22-1.fc21 """ - cmd_str = "dnf -y --showduplicates list available %s" % pkg - command = CommandRunner(cmd_str).run() + cmd = ['dnf', '-y', '--showduplicates', 'list', 'available', pkg] + command = CommandRunner(cmd).run() return command.status == 0 @classmethod @@ -332,8 +362,8 @@ class RpmHelper(object): e.g., httpd-2.2.22 e.g., httpd-2.2.22-1.fc16 """ - cmd_str = "zypper -n --no-refresh search %s" % pkg - command = CommandRunner(cmd_str).run() + cmd = ['zypper', '-n', '--no-refresh', 'search', pkg] + command = CommandRunner(cmd).run() return command.status == 0 @classmethod @@ -359,22 +389,16 @@ class RpmHelper(object): * packages must be in same format as yum pkg list """ if rpms: - cmd = "rpm -U --force --nosignature " - cmd += " ".join(packages) - LOG.info("Installing packages: %s" % cmd) + cmd = ['rpm', '-U', '--force', '--nosignature'] elif zypper: - cmd = "zypper -n install " - cmd += " ".join(packages) - LOG.info("Installing packages: %s" % cmd) + cmd = ['zypper', '-n', 'install'] elif dnf: # use dnf --best to upgrade outdated-but-installed packages - cmd = "dnf -y --best install " - cmd += " ".join(packages) - LOG.info("Installing packages: %s" % cmd) + cmd = ['dnf', '-y', '--best', 'install'] else: - cmd = "yum -y install " - cmd += " ".join(packages) - LOG.info("Installing packages: %s" % cmd) + cmd = ['yum', '-y', 'install'] + cmd.extend(packages) + LOG.info("Installing packages: %s" % cmd) command = CommandRunner(cmd).run() if command.status: LOG.warn("Failed to install packages: %s" % cmd) @@ -400,22 +424,22 @@ class RpmHelper(object): if rpms: cls.install(packages) elif zypper: - cmd = "zypper -n install --oldpackage " - cmd += " ".join(packages) + cmd = ['zypper', '-n', 'install', '--oldpackage'] + cmd.extend(packages) LOG.info("Downgrading packages: %s", cmd) command = CommandRunner(cmd).run() if command.status: LOG.warn("Failed to downgrade packages: %s" % cmd) elif dnf: - cmd = "dnf -y downgrade " - cmd += " ".join(packages) + cmd = ['dnf', '-y', 'downgrade'] + cmd.extend(packages) LOG.info("Downgrading packages: %s", cmd) command = CommandRunner(cmd).run() if command.status: LOG.warn("Failed to downgrade packages: %s" % cmd) else: - cmd = "yum -y downgrade " - cmd += " ".join(packages) + cmd = ['yum', '-y', 'downgrade'] + cmd.extend(packages) LOG.info("Downgrading packages: %s" % cmd) command = CommandRunner(cmd).run() if command.status: @@ -449,22 +473,23 @@ class PackagesHandler(object): # TODO(asalkeld) support versions # -b == local & remote install # -y == install deps - opts = '-b -y' + opts = ['-b', '-y'] for pkg_name, versions in packages.items(): if len(versions) > 0: - cmd_str = 'gem install %s --version %s %s' % (opts, - versions[0], - pkg_name) - CommandRunner(cmd_str).run() + cmd = ['gem', 'install'] + opts + cmd.extend(['--version', versions[0], pkg_name]) + CommandRunner(cmd).run() else: - CommandRunner('gem install %s %s' % (opts, pkg_name)).run() + cmd = ['gem', 'install'] + opts + cmd.append(pkg_name) + CommandRunner(cmd).run() def _handle_python_packages(self, packages): """very basic support for easy_install.""" # TODO(asalkeld) support versions for pkg_name, versions in packages.items(): - cmd_str = 'easy_install %s' % (pkg_name) - CommandRunner(cmd_str).run() + cmd = ['easy_install', pkg_name] + CommandRunner(cmd).run() def _handle_zypper_packages(self, packages): """Handle installation, upgrade, or downgrade of packages via yum. @@ -576,7 +601,8 @@ class PackagesHandler(object): * if a version array is supplied, choose the highest version from the array and follow same logic for version string above """ - cmd = CommandRunner("which yum").run() + + cmd = CommandRunner(['which', 'yum']).run() if cmd.status == 1: # yum not available, use DNF if available self._handle_dnf_packages(packages) @@ -629,11 +655,11 @@ class PackagesHandler(object): def _handle_apt_packages(self, packages): """very basic support for apt.""" # TODO(asalkeld) support versions - pkg_list = ' '.join([p for p in packages]) + pkg_list = list(packages) - cmd_str = 'DEBIAN_FRONTEND=noninteractive apt-get -y install %s' % \ - pkg_list - CommandRunner(cmd_str).run() + env = {'DEBIAN_FRONTEND': 'noninteractive'} + cmd = ['apt-get', '-y', 'install'] + pkg_list + CommandRunner(cmd).run(env=env) # map of function pointers to handle different package managers _package_handlers = {"yum": _handle_yum_packages, @@ -709,7 +735,7 @@ class FilesHandler(object): .encode('UTF-8')) f.close() elif 'source' in meta: - CommandRunner('curl -o %s %s' % (dest, meta['source'])).run() + CommandRunner(['curl', '-o', dest, meta['source']]).run() else: LOG.error('%s %s' % (dest, str(meta))) continue @@ -810,8 +836,9 @@ class SourcesHandler(object): def _apply_source(self, dest, url): cmd = self._apply_source_cmd(dest, url) + #FIXME bug 1498298 if cmd != '': - runner = CommandRunner(cmd) + runner = CommandRunner(cmd, shell=True) runner.run() def apply_sources(self): @@ -833,47 +860,52 @@ class ServicesHandler(object): if os.path.exists("/bin/systemctl"): service_exe = "/bin/systemctl" service = '%s.service' % service - service_start = '%s start %s' - service_status = '%s status %s' - service_stop = '%s stop %s' + service_start = [service_exe, 'start', service] + service_status = [service_exe, 'status', service] + service_stop = [service_exe, 'stop', service] elif os.path.exists("/sbin/service"): service_exe = "/sbin/service" - service_start = '%s %s start' - service_status = '%s %s status' - service_stop = '%s %s stop' + service_start = [service_exe, service, 'start'] + service_status = [service_exe, service, 'status'] + service_stop = [service_exe, service, 'stop'] else: service_exe = "/usr/sbin/service" - service_start = '%s %s start' - service_status = '%s %s status' - service_stop = '%s %s stop' + service_start = [service_exe, service, 'start'] + service_status = [service_exe, service, 'status'] + service_stop = [service_exe, service, 'stop'] if os.path.exists("/bin/systemctl"): enable_exe = "/bin/systemctl" - enable_on = '%s enable %s' - enable_off = '%s disable %s' + enable_on = [enable_exe, 'enable', service] + enable_off = [enable_exe, 'disable', service] elif os.path.exists("/sbin/chkconfig"): enable_exe = "/sbin/chkconfig" - enable_on = '%s %s on' - enable_off = '%s %s off' + enable_on = [enable_exe, service, 'on'] + enable_off = [enable_exe, service, 'off'] + else: enable_exe = "/usr/sbin/update-rc.d" - enable_on = '%s %s enable' - enable_off = '%s %s disable' + enable_on = [enable_exe, service, 'enable'] + enable_off = [enable_exe, service, 'disable'] - cmd = "" + cmd = None if "enable" == command: - cmd = enable_on % (enable_exe, service) + cmd = enable_on elif "disable" == command: - cmd = enable_off % (enable_exe, service) + cmd = enable_off elif "start" == command: - cmd = service_start % (service_exe, service) + cmd = service_start elif "stop" == command: - cmd = service_stop % (service_exe, service) + cmd = service_stop elif "status" == command: - cmd = service_status % (service_exe, service) - command = CommandRunner(cmd) - command.run() - return command + cmd = service_status + + if cmd is not None: + command = CommandRunner(cmd) + command.run() + return command + else: + LOG.error("Unknown sysv command %s" % command) def _initialize_service(self, handler, service, properties): if "enabled" in properties: @@ -1056,7 +1088,7 @@ class CommandsHandler(object): return if "test" in properties: - test = CommandRunner(properties["test"]) + test = CommandRunner(properties["test"], shell=True) test_status = test.run('root', cwd, env).status if test_status != 0: LOG.info("%s test returns false, skipping command" @@ -1068,10 +1100,11 @@ class CommandsHandler(object): if "command" in properties: try: command = properties["command"] + #FIXME bug 1498300 if isinstance(command, list): escape = lambda x: '"%s"' % x.replace('"', '\\"') command = ' '.join(map(escape, command)) - command = CommandRunner(command) + command = CommandRunner(command, shell=True) command.run('root', cwd, env) command_status = command.status except OSError as e: @@ -1110,14 +1143,11 @@ class GroupsHandler(object): def _initialize_group(self, group, properties): gid = properties.get("gid", None) - - param_list = [] - param_list.append(group) - + cmd = ['groupadd', group] if gid is not None: - param_list.append("--gid " + gid) + cmd.extend(['--gid', str(gid)]) - command = CommandRunner("groupadd " + ' '.join(param_list)) + command = CommandRunner(cmd) command.run() command_status = command.status @@ -1156,23 +1186,23 @@ class UsersHandler(object): uid = properties.get("uid", None) homeDir = properties.get("homeDir", None) - param_list = [] - param_list.append(user) + cmd = ['useradd', user] if uid is not None: - param_list.append("--uid " + uid) + cmd.extend(['--uid', six.text_type(uid)]) if homeDir is not None: - param_list.append("--home " + homeDir) + cmd.extend(['--home', six.text_type(homeDir)]) if "groups" in properties: - param_list.append("--groups " + ','.join(properties["groups"])) + groups = ','.join(properties["groups"]) + cmd.extend(['--groups', groups]) #Users are created as non-interactive system users with a shell #of /sbin/nologin. This is by design and cannot be modified. - param_list.append("--shell /sbin/nologin") + cmd.extend(['--shell', '/sbin/nologin']) - command = CommandRunner("useradd " + ' '.join(param_list)) + command = CommandRunner(cmd) command.run() command_status = command.status @@ -1265,7 +1295,8 @@ class Metadata(object): url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json' if not os.path.exists(cache_path): - CommandRunner('curl -o %s %s' % (cache_path, url)).run() + cmd = ['curl', '-o', cache_path, url] + CommandRunner(cmd).run() try: with open(cache_path) as fd: try: diff --git a/heat_cfntools/tests/test_cfn_helper.py b/heat_cfntools/tests/test_cfn_helper.py index 15641bc..308ca03 100644 --- a/heat_cfntools/tests/test_cfn_helper.py +++ b/heat_cfntools/tests/test_cfn_helper.py @@ -26,10 +26,11 @@ import testtools.matchers as ttm from heat_cfntools.cfntools import cfn_helper -def popen_root_calls(calls): - kwargs = {'env': None, 'cwd': None, 'stderr': -1, 'stdout': -1} +def popen_root_calls(calls, shell=False): + kwargs = {'env': None, 'cwd': None, 'stderr': -1, 'stdout': -1, + 'shell': shell} return [ - mock.call(call.split(), **kwargs) + mock.call(call, **kwargs) for call in calls ] @@ -47,9 +48,12 @@ class FakePOpen(): pass +@mock.patch.object(cfn_helper.pwd, 'getpwnam') +@mock.patch.object(cfn_helper.os, 'seteuid') +@mock.patch.object(cfn_helper.os, 'geteuid') class TestCommandRunner(testtools.TestCase): - def test_command_runner(self): + def test_command_runner(self, mock_geteuid, mock_seteuid, mock_getpwnam): def returns(*args, **kwargs): if args[0][0] == '/bin/command1': return FakePOpen('All good') @@ -60,74 +64,88 @@ class TestCommandRunner(testtools.TestCase): with mock.patch('subprocess.Popen') as mock_popen: mock_popen.side_effect = returns - cmd2 = cfn_helper.CommandRunner('/bin/command2') - cmd1 = cfn_helper.CommandRunner('/bin/command1', cmd2) + cmd2 = cfn_helper.CommandRunner(['/bin/command2']) + cmd1 = cfn_helper.CommandRunner(['/bin/command1'], + nextcommand=cmd2) cmd1.run('root') self.assertEqual( - 'CommandRunner:\n\tcommand: /bin/command1\n\tstdout: All good', + 'CommandRunner:\n\tcommand: [\'/bin/command1\']\n\tstdout: ' + 'All good', str(cmd1)) self.assertEqual( - 'CommandRunner:\n\tcommand: /bin/command2\n\tstatus: -1\n' - '\tstdout: Doing something\n\tstderr: error', + 'CommandRunner:\n\tcommand: [\'/bin/command2\']\n\tstatus: ' + '-1\n\tstdout: Doing something\n\tstderr: error', str(cmd2)) - calls = popen_root_calls(['/bin/command1', '/bin/command2']) + calls = popen_root_calls([['/bin/command1'], ['/bin/command2']]) mock_popen.assert_has_calls(calls) - def test_runner_runs_command_as_list(self): - with mock.patch('subprocess.Popen') as mock_popen: - command = '/bin/command --option=value arg1 arg2' - expected_cmd = ['/bin/command', '--option=value', 'arg1', 'arg2'] - cmd = cfn_helper.CommandRunner(command) - cmd.run() - self.assertTrue(mock_popen.called) - actual_cmd = mock_popen.call_args[0][0] - self.assertTrue(isinstance(actual_cmd, list)) - self.assertItemsEqual(expected_cmd, actual_cmd) - - @mock.patch.object(cfn_helper.pwd, 'getpwnam') - @mock.patch.object(cfn_helper.os, 'setuid') - def test_privileges_are_lowered_for_non_root_user(self, mock_setuid, + def test_privileges_are_lowered_for_non_root_user(self, mock_geteuid, + mock_seteuid, mock_getpwnam): pw_entry = mock.MagicMock() pw_entry.pw_uid = 1001 mock_getpwnam.return_value = pw_entry + mock_geteuid.return_value = 0 + calls = [mock.call(1001), mock.call(0)] with mock.patch('subprocess.Popen') as mock_popen: - command = '/bin/command --option=value arg1 arg2' + command = ['/bin/command', '--option=value', 'arg1', 'arg2'] cmd = cfn_helper.CommandRunner(command) cmd.run(user='nonroot') + self.assertTrue(mock_geteuid.called) mock_getpwnam.assert_called_once_with('nonroot') - mock_setuid.assert_called_once_with(1001) + mock_seteuid.assert_has_calls(calls) self.assertTrue(mock_popen.called) - @mock.patch.object(cfn_helper.pwd, 'getpwnam') - @mock.patch.object(cfn_helper.os, 'setuid') - def test_run_returns_when_cannot_set_privileges(self, mock_setuid, + def test_run_returns_when_cannot_set_privileges(self, mock_geteuid, + mock_seteuid, mock_getpwnam): - mock_setuid.side_effect = Exception('[Error 1] Permission Denied') + msg = '[Error 1] Permission Denied' + mock_seteuid.side_effect = Exception(msg) with mock.patch('subprocess.Popen') as mock_popen: - command = '/bin/command2' + command = ['/bin/command2'] cmd = cfn_helper.CommandRunner(command) cmd.run(user='nonroot') self.assertTrue(mock_getpwnam.called) - self.assertTrue(mock_setuid.called) + self.assertTrue(mock_seteuid.called) self.assertFalse(mock_popen.called) self.assertEqual(126, cmd.status) + self.assertEqual(msg, cmd.stderr) + + def test_privileges_are_restored_for_command_failure(self, mock_geteuid, + mock_seteuid, + mock_getpwnam): + pw_entry = mock.MagicMock() + pw_entry.pw_uid = 1001 + mock_getpwnam.return_value = pw_entry + mock_geteuid.return_value = 0 + calls = [mock.call(1001), mock.call(0)] + with mock.patch('subprocess.Popen') as mock_popen: + mock_popen.side_effect = ValueError('Something wrong') + command = ['/bin/command', '--option=value', 'arg1', 'arg2'] + cmd = cfn_helper.CommandRunner(command) + self.assertRaises(ValueError, cmd.run, user='nonroot') + self.assertTrue(mock_geteuid.called) + mock_getpwnam.assert_called_once_with('nonroot') + mock_seteuid.assert_has_calls(calls) + self.assertTrue(mock_popen.called) +@mock.patch.object(cfn_helper, 'controlled_privileges') class TestPackages(testtools.TestCase): - def test_yum_install(self): + def test_yum_install(self, mock_cp): def returns(*args, **kwargs): - if (args[0][0] == 'rpm' and args[0][1] == '-q'): + if args[0][0] == 'rpm' and args[0][1] == '-q': return FakePOpen(returncode=1) else: return FakePOpen(returncode=0) - calls = ['which yum'] + calls = [['which', 'yum']] for pack in ('httpd', 'wordpress', 'mysql-server'): - calls.append('rpm -q %s' % pack) - calls.append('yum -y --showduplicates list available %s' % pack) + calls.append(['rpm', '-q', pack]) + calls.append(['yum', '-y', '--showduplicates', 'list', + 'available', pack]) calls = popen_root_calls(calls) packages = { @@ -143,7 +161,7 @@ class TestPackages(testtools.TestCase): cfn_helper.PackagesHandler(packages).apply_packages() mock_popen.assert_has_calls(calls, any_order=True) - def test_dnf_install_yum_unavailable(self): + def test_dnf_install_yum_unavailable(self, mock_cp): def returns(*args, **kwargs): if ((args[0][0] == 'rpm' and args[0][1] == '-q') @@ -152,10 +170,11 @@ class TestPackages(testtools.TestCase): else: return FakePOpen(returncode=0) - calls = ['which yum'] + calls = [['which', 'yum']] for pack in ('httpd', 'wordpress', 'mysql-server'): - calls.append('rpm -q %s' % pack) - calls.append('dnf -y --showduplicates list available %s' % pack) + calls.append(['rpm', '-q', pack]) + calls.append(['dnf', '-y', '--showduplicates', 'list', + 'available', pack]) calls = popen_root_calls(calls) packages = { @@ -171,18 +190,19 @@ class TestPackages(testtools.TestCase): cfn_helper.PackagesHandler(packages).apply_packages() mock_popen.assert_has_calls(calls, any_order=True) - def test_dnf_install(self): + def test_dnf_install(self, mock_cp): def returns(*args, **kwargs): - if (args[0][0] == 'rpm' and args[0][1] == '-q'): + if args[0][0] == 'rpm' and args[0][1] == '-q': return FakePOpen(returncode=1) else: return FakePOpen(returncode=0) calls = [] for pack in ('httpd', 'wordpress', 'mysql-server'): - calls.append('rpm -q %s' % pack) - calls.append('dnf -y --showduplicates list available %s' % pack) + calls.append(['rpm', '-q', pack]) + calls.append(['dnf', '-y', '--showduplicates', 'list', + 'available', pack]) calls = popen_root_calls(calls) packages = { @@ -198,18 +218,18 @@ class TestPackages(testtools.TestCase): cfn_helper.PackagesHandler(packages).apply_packages() mock_popen.assert_has_calls(calls, any_order=True) - def test_zypper_install(self): + def test_zypper_install(self, mock_cp): def returns(*args, **kwargs): - if (args[0][0] == 'rpm' and args[0][1] == '-q'): + if args[0][0].startswith('rpm') and args[0][1].startswith('-q'): return FakePOpen(returncode=1) else: return FakePOpen(returncode=0) calls = [] for pack in ('httpd', 'wordpress', 'mysql-server'): - calls.append('rpm -q %s' % pack) - calls.append('zypper -n --no-refresh search %s' % pack) + calls.append(['rpm', '-q', pack]) + calls.append(['zypper', '-n', '--no-refresh', 'search', pack]) calls = popen_root_calls(calls) packages = { @@ -225,7 +245,7 @@ class TestPackages(testtools.TestCase): cfn_helper.PackagesHandler(packages).apply_packages() mock_popen.assert_has_calls(calls, any_order=True) - def test_apt_install(self): + def test_apt_install(self, mock_cp): packages = { "apt": { "mysql-server": [], @@ -240,47 +260,57 @@ class TestPackages(testtools.TestCase): self.assertTrue(mock_popen.called) +@mock.patch.object(cfn_helper, 'controlled_privileges') class TestServicesHandler(testtools.TestCase): - def test_services_handler_systemd(self): + def test_services_handler_systemd(self, mock_cp): calls = [] returns = [] # apply_services - calls.append('/bin/systemctl enable httpd.service') + calls.append(['/bin/systemctl', 'enable', 'httpd.service']) returns.append(FakePOpen()) - calls.append('/bin/systemctl status httpd.service') + calls.append(['/bin/systemctl', 'status', 'httpd.service']) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start httpd.service') + calls.append(['/bin/systemctl', 'start', 'httpd.service']) returns.append(FakePOpen()) - calls.append('/bin/systemctl enable mysqld.service') + calls.append(['/bin/systemctl', 'enable', 'mysqld.service']) returns.append(FakePOpen()) - calls.append('/bin/systemctl status mysqld.service') + calls.append(['/bin/systemctl', 'status', 'mysqld.service']) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start mysqld.service') + calls.append(['/bin/systemctl', 'start', 'mysqld.service']) returns.append(FakePOpen()) # monitor_services not running - calls.append('/bin/systemctl status httpd.service') + calls.append(['/bin/systemctl', 'status', 'httpd.service']) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start httpd.service') + calls.append(['/bin/systemctl', 'start', 'httpd.service']) returns.append(FakePOpen()) - calls.append('/bin/services_restarted') + + calls = popen_root_calls(calls) + + calls.extend(popen_root_calls(['/bin/services_restarted'], shell=True)) returns.append(FakePOpen()) - calls.append('/bin/systemctl status mysqld.service') + + calls.extend(popen_root_calls([['/bin/systemctl', 'status', + 'mysqld.service']])) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start mysqld.service') + calls.extend(popen_root_calls([['/bin/systemctl', 'start', + 'mysqld.service']])) returns.append(FakePOpen()) - calls.append('/bin/services_restarted') + + calls.extend(popen_root_calls(['/bin/services_restarted'], shell=True)) returns.append(FakePOpen()) # monitor_services running - calls.append('/bin/systemctl status httpd.service') + calls.extend(popen_root_calls([['/bin/systemctl', 'status', + 'httpd.service']])) returns.append(FakePOpen()) - calls.append('/bin/systemctl status mysqld.service') + calls.extend(popen_root_calls([['/bin/systemctl', 'status', + 'mysqld.service']])) returns.append(FakePOpen()) - calls = popen_root_calls(calls) + #calls = popen_root_calls(calls) services = { "systemd": { @@ -312,16 +342,16 @@ class TestServicesHandler(testtools.TestCase): mock_popen.assert_has_calls(calls, any_order=True) mock_exists.assert_called_with('/bin/systemctl') - def test_services_handler_systemd_disabled(self): + def test_services_handler_systemd_disabled(self, mock_cp): calls = [] # apply_services - calls.append('/bin/systemctl disable httpd.service') - calls.append('/bin/systemctl status httpd.service') - calls.append('/bin/systemctl stop httpd.service') - calls.append('/bin/systemctl disable mysqld.service') - calls.append('/bin/systemctl status mysqld.service') - calls.append('/bin/systemctl stop mysqld.service') + calls.append(['/bin/systemctl', 'disable', 'httpd.service']) + calls.append(['/bin/systemctl', 'status', 'httpd.service']) + calls.append(['/bin/systemctl', 'stop', 'httpd.service']) + calls.append(['/bin/systemctl', 'disable', 'mysqld.service']) + calls.append(['/bin/systemctl', 'status', 'mysqld.service']) + calls.append(['/bin/systemctl', 'stop', 'mysqld.service']) calls = popen_root_calls(calls) services = { @@ -347,7 +377,7 @@ class TestServicesHandler(testtools.TestCase): mock_popen.assert_has_calls(calls, any_order=True) mock_exists.assert_called_with('/bin/systemctl') - def test_services_handler_sysv_service_chkconfig(self): + def test_services_handler_sysv_service_chkconfig(self, mock_cp): def exists(*args, **kwargs): return args[0] != '/bin/systemctl' @@ -356,27 +386,28 @@ class TestServicesHandler(testtools.TestCase): returns = [] # apply_services - calls.append('/sbin/chkconfig httpd on') + calls.append(['/sbin/chkconfig', 'httpd', 'on']) returns.append(FakePOpen()) - calls.append('/sbin/service httpd status') + calls.append(['/sbin/service', 'httpd', 'status']) returns.append(FakePOpen(returncode=-1)) - calls.append('/sbin/service httpd start') + calls.append(['/sbin/service', 'httpd', 'start']) returns.append(FakePOpen()) # monitor_services not running - calls.append('/sbin/service httpd status') + calls.append(['/sbin/service', 'httpd', 'status']) returns.append(FakePOpen(returncode=-1)) - calls.append('/sbin/service httpd start') + calls.append(['/sbin/service', 'httpd', 'start']) returns.append(FakePOpen()) - calls.append('/bin/services_restarted') + + calls = popen_root_calls(calls) + + calls.extend(popen_root_calls(['/bin/services_restarted'], shell=True)) returns.append(FakePOpen()) # monitor_services running - calls.append('/sbin/service httpd status') + calls.extend(popen_root_calls([['/sbin/service', 'httpd', 'status']])) returns.append(FakePOpen()) - calls = popen_root_calls(calls) - services = { "sysvinit": { "httpd": {"enabled": "true", "ensureRunning": "true"} @@ -407,16 +438,16 @@ class TestServicesHandler(testtools.TestCase): mock_exists.assert_any_call('/sbin/service') mock_exists.assert_any_call('/sbin/chkconfig') - def test_services_handler_sysv_disabled_service_chkconfig(self): + def test_services_handler_sysv_disabled_service_chkconfig(self, mock_cp): def exists(*args, **kwargs): return args[0] != '/bin/systemctl' calls = [] # apply_services - calls.append('/sbin/chkconfig httpd off') - calls.append('/sbin/service httpd status') - calls.append('/sbin/service httpd stop') + calls.append(['/sbin/chkconfig', 'httpd', 'off']) + calls.append(['/sbin/service', 'httpd', 'status']) + calls.append(['/sbin/service', 'httpd', 'stop']) calls = popen_root_calls(calls) @@ -445,31 +476,35 @@ class TestServicesHandler(testtools.TestCase): mock_exists.assert_any_call('/sbin/service') mock_exists.assert_any_call('/sbin/chkconfig') - def test_services_handler_sysv_systemctl(self): + def test_services_handler_sysv_systemctl(self, mock_cp): calls = [] returns = [] # apply_services - calls.append('/bin/systemctl enable httpd.service') + calls.append(['/bin/systemctl', 'enable', 'httpd.service']) returns.append(FakePOpen()) - calls.append('/bin/systemctl status httpd.service') + calls.append(['/bin/systemctl', 'status', 'httpd.service']) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start httpd.service') + calls.append(['/bin/systemctl', 'start', 'httpd.service']) returns.append(FakePOpen()) # monitor_services not running - calls.append('/bin/systemctl status httpd.service') + calls.append(['/bin/systemctl', 'status', 'httpd.service']) returns.append(FakePOpen(returncode=-1)) - calls.append('/bin/systemctl start httpd.service') - returns.append(FakePOpen()) - calls.append('/bin/services_restarted') + calls.append(['/bin/systemctl', 'start', 'httpd.service']) returns.append(FakePOpen()) - # monitor_services running - calls.append('/bin/systemctl status httpd.service') + shell_calls = [] + shell_calls.append('/bin/services_restarted') returns.append(FakePOpen()) calls = popen_root_calls(calls) + calls.extend(popen_root_calls(shell_calls, shell=True)) + + # monitor_services running + calls.extend(popen_root_calls([['/bin/systemctl', 'status', + 'httpd.service']])) + returns.append(FakePOpen()) services = { "sysvinit": { @@ -499,13 +534,13 @@ class TestServicesHandler(testtools.TestCase): mock_popen.assert_has_calls(calls) mock_exists.assert_called_with('/bin/systemctl') - def test_services_handler_sysv_disabled_systemctl(self): + def test_services_handler_sysv_disabled_systemctl(self, mock_cp): calls = [] # apply_services - calls.append('/bin/systemctl disable httpd.service') - calls.append('/bin/systemctl status httpd.service') - calls.append('/bin/systemctl stop httpd.service') + calls.append(['/bin/systemctl', 'disable', 'httpd.service']) + calls.append(['/bin/systemctl', 'status', 'httpd.service']) + calls.append(['/bin/systemctl', 'stop', 'httpd.service']) calls = popen_root_calls(calls) @@ -532,31 +567,35 @@ class TestServicesHandler(testtools.TestCase): mock_popen.assert_has_calls(calls) mock_exists.assert_called_with('/bin/systemctl') - def test_services_handler_sysv_service_updaterc(self): + def test_services_handler_sysv_service_updaterc(self, mock_cp): calls = [] returns = [] # apply_services - calls.append('/usr/sbin/update-rc.d httpd enable') + calls.append(['/usr/sbin/update-rc.d', 'httpd', 'enable']) returns.append(FakePOpen()) - calls.append('/usr/sbin/service httpd status') + calls.append(['/usr/sbin/service', 'httpd', 'status']) returns.append(FakePOpen(returncode=-1)) - calls.append('/usr/sbin/service httpd start') + calls.append(['/usr/sbin/service', 'httpd', 'start']) returns.append(FakePOpen()) # monitor_services not running - calls.append('/usr/sbin/service httpd status') + calls.append(['/usr/sbin/service', 'httpd', 'status']) returns.append(FakePOpen(returncode=-1)) - calls.append('/usr/sbin/service httpd start') - returns.append(FakePOpen()) - calls.append('/bin/services_restarted') + calls.append(['/usr/sbin/service', 'httpd', 'start']) returns.append(FakePOpen()) - # monitor_services running - calls.append('/usr/sbin/service httpd status') + shell_calls = [] + shell_calls.append('/bin/services_restarted') returns.append(FakePOpen()) calls = popen_root_calls(calls) + calls.extend(popen_root_calls(shell_calls, shell=True)) + + # monitor_services running + calls.extend(popen_root_calls([['/usr/sbin/service', 'httpd', + 'status']])) + returns.append(FakePOpen()) services = { "sysvinit": { @@ -588,16 +627,16 @@ class TestServicesHandler(testtools.TestCase): mock_exists.assert_any_call('/sbin/service') mock_exists.assert_any_call('/sbin/chkconfig') - def test_services_handler_sysv_disabled_service_updaterc(self): + def test_services_handler_sysv_disabled_service_updaterc(self, mock_cp): calls = [] returns = [] # apply_services - calls.append('/usr/sbin/update-rc.d httpd disable') + calls.append(['/usr/sbin/update-rc.d', 'httpd', 'disable']) returns.append(FakePOpen()) - calls.append('/usr/sbin/service httpd status') + calls.append(['/usr/sbin/service', 'httpd', 'status']) returns.append(FakePOpen()) - calls.append('/usr/sbin/service httpd stop') + calls.append(['/usr/sbin/service', 'httpd', 'stop']) returns.append(FakePOpen()) calls = popen_root_calls(calls) @@ -659,7 +698,8 @@ interval=120''' % fcreds.name).encode('UTF-8')) self.assertIn('invalid credentials file', str(e)) fcreds.close() - def test_hup_config(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_hup_config(self, mock_cp): hooks_conf = tempfile.NamedTemporaryFile() def write_hook_conf(f, name, triggers, path, action): @@ -734,11 +774,11 @@ interval=120''' % fcreds.name).encode('UTF-8')) ' root, /bin/hook3}', str(hooks[3])) calls = [] - calls.append('/bin/cfn-http-restarted') - calls.append('/bin/hook1') - calls.append('/bin/hook2') - calls.append('/bin/hook3') - calls = popen_root_calls(calls) + calls.extend(popen_root_calls(['/bin/cfn-http-restarted'], shell=True)) + calls.extend(popen_root_calls(['/bin/hook1'], shell=True)) + calls.extend(popen_root_calls(['/bin/hook2'], shell=True)) + calls.extend(popen_root_calls(['/bin/hook3'], shell=True)) + #calls = popen_root_calls(calls) with mock.patch('subprocess.Popen') as mock_popen: mock_popen.return_value = FakePOpen('All good') @@ -1070,7 +1110,8 @@ class TestMetadataRetrieve(testtools.TestCase): self.assertEqual(meta_in, meta_out) - def test_nova_meta_curl(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_nova_meta_curl(self, mock_cp): url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json' temp_home = tempfile.mkdtemp() cache_path = os.path.join(temp_home, 'meta_data.json') @@ -1103,9 +1144,10 @@ class TestMetadataRetrieve(testtools.TestCase): meta_out = md.get_nova_meta(cache_path=cache_path) self.assertEqual(meta_in, meta_out) mock_popen.assert_has_calls( - popen_root_calls(['curl -o %s %s' % (cache_path, url)])) + popen_root_calls([['curl', '-o', cache_path, url]])) - def test_nova_meta_curl_corrupt(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_nova_meta_curl_corrupt(self, mock_cp): url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json' temp_home = tempfile.mkdtemp() cache_path = os.path.join(temp_home, 'meta_data.json') @@ -1131,9 +1173,10 @@ class TestMetadataRetrieve(testtools.TestCase): meta_out = md.get_nova_meta(cache_path=cache_path) self.assertEqual(None, meta_out) mock_popen.assert_has_calls( - popen_root_calls(['curl -o %s %s' % (cache_path, url)])) + popen_root_calls([['curl', '-o', cache_path, url]])) - def test_nova_meta_curl_failed(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_nova_meta_curl_failed(self, mock_cp): url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json' temp_home = tempfile.mkdtemp() cache_path = os.path.join(temp_home, 'meta_data.json') @@ -1149,7 +1192,7 @@ class TestMetadataRetrieve(testtools.TestCase): meta_out = md.get_nova_meta(cache_path=cache_path) self.assertEqual(None, meta_out) mock_popen.assert_has_calls( - popen_root_calls(['curl -o %s %s' % (cache_path, url)])) + popen_root_calls([['curl', '-o', cache_path, url]])) def test_get_tags(self): fake_tags = {'foo': 'fee', @@ -1209,7 +1252,8 @@ class TestCfnInit(testtools.TestCase): md.cfn_init() self.assertThat(foo_file.name, ttm.FileContains('bar')) - def test_cfn_init_with_ignore_errors_false(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_cfn_init_with_ignore_errors_false(self, mock_cp): md_data = {"AWS::CloudFormation::Init": {"config": {"commands": { "00_foo": {"command": "/bin/command1", "ignoreErrors": "false"}}}}} @@ -1219,16 +1263,18 @@ class TestCfnInit(testtools.TestCase): self.assertTrue( md.retrieve(meta_str=md_data, last_path=self.last_file)) self.assertRaises(cfn_helper.CommandsHandlerRunError, md.cfn_init) - mock_popen.assert_has_calls(popen_root_calls(['/bin/command1'])) + mock_popen.assert_has_calls(popen_root_calls(['/bin/command1'], + shell=True)) - def test_cfn_init_with_ignore_errors_true(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_cfn_init_with_ignore_errors_true(self, mock_cp): calls = [] returns = [] - calls.append('/bin/command1') + calls.extend(popen_root_calls(['/bin/command1'], shell=True)) returns.append(FakePOpen('Doing something', 'error', -1)) - calls.append('/bin/command2') + calls.extend(popen_root_calls(['/bin/command2'], shell=True)) returns.append(FakePOpen('All good')) - calls = popen_root_calls(calls) + #calls = popen_root_calls(calls) md_data = {"AWS::CloudFormation::Init": {"config": {"commands": { "00_foo": {"command": "/bin/command1", @@ -1257,7 +1303,7 @@ class TestSourcesHandler(testtools.TestCase): sources = {dest: url} td = os.path.dirname(end_file) er = "mkdir -p '%s'; cd '%s'; curl -s '%s' | gunzip | tar -xvf -" - calls = popen_root_calls([er % (dest, dest, url)]) + calls = popen_root_calls([er % (dest, dest, url)], shell=True) with mock.patch.object(tempfile, 'mkdtemp') as mock_mkdtemp: mock_mkdtemp.return_value = td @@ -1268,26 +1314,28 @@ class TestSourcesHandler(testtools.TestCase): mock_popen.assert_has_calls(calls) mock_mkdtemp.assert_called_with() - def test_apply_sources_github(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_apply_sources_github(self, mock_cp): url = "https://github.com/NoSuchProject/tarball/NoSuchTarball" dest = tempfile.mkdtemp() self.addCleanup(os.rmdir, dest) sources = {dest: url} er = "mkdir -p '%s'; cd '%s'; curl -s '%s' | gunzip | tar -xvf -" - calls = popen_root_calls([er % (dest, dest, url)]) + calls = popen_root_calls([er % (dest, dest, url)], shell=True) with mock.patch('subprocess.Popen') as mock_popen: mock_popen.return_value = FakePOpen('Curl good') sh = cfn_helper.SourcesHandler(sources) sh.apply_sources() mock_popen.assert_has_calls(calls) - def test_apply_sources_general(self): + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_apply_sources_general(self, mock_cp): url = "https://website.no.existe/a/b/c/file.tar.gz" dest = tempfile.mkdtemp() self.addCleanup(os.rmdir, dest) sources = {dest: url} er = "mkdir -p '%s'; cd '%s'; curl -s '%s' | gunzip | tar -xvf -" - calls = popen_root_calls([er % (dest, dest, url)]) + calls = popen_root_calls([er % (dest, dest, url)], shell=True) with mock.patch('subprocess.Popen') as mock_popen: mock_popen.return_value = FakePOpen('Curl good') sh = cfn_helper.SourcesHandler(sources) |