diff --git a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py index 367a0084e..6f41660e3 100644 --- a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py +++ b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py @@ -127,39 +127,40 @@ class TestDeployUndercloud(TestPluginV1): self.assertRaises(exceptions.NotFound, self.cmd._populate_templates_dir, '/foo') + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('os.chmod') @mock.patch('os.path.exists') + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('subprocess.check_call', autospec=True) @mock.patch('tripleo_common.utils.passwords.generate_passwords') @mock.patch('yaml.safe_dump') - def test_update_passwords_env_init(self, mock_dump, mock_pw, - mock_exists, mock_chmod): + def test_update_passwords_env_init(self, mock_dump, mock_pw, mock_cc, + mock_exists, mock_chmod, mock_user): pw_dict = {"GeneratedPassword": 123} - pw_conf_path = os.path.join(self.temp_homedir, - 'undercloud-passwords.conf') - t_pw_conf_path = os.path.join( - self.temp_homedir, 'tripleo-undercloud-passwords.yaml') mock_pw.return_value = pw_dict mock_exists.return_value = False mock_open_context = mock.mock_open() with mock.patch('six.moves.builtins.open', mock_open_context): - self.cmd._update_passwords_env(self.temp_homedir) + self.cmd._update_passwords_env(self.temp_homedir, 'stack') mock_open_handle = mock_open_context() mock_dump.assert_called_once_with({'parameter_defaults': pw_dict}, mock_open_handle, default_flow_style=False) - chmod_calls = [mock.call(t_pw_conf_path, 0o600), - mock.call(pw_conf_path, 0o600)] - mock_chmod.assert_has_calls(chmod_calls) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('os.chmod') @mock.patch('os.path.exists') + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('subprocess.check_call', autospec=True) @mock.patch('tripleo_common.utils.passwords.generate_passwords') @mock.patch('yaml.safe_dump') - def test_update_passwords_env_update(self, mock_dump, mock_pw, - mock_exists, mock_chmod): + def test_update_passwords_env_update(self, mock_dump, mock_pw, mock_cc, + mock_exists, mock_chmod, mock_user): pw_dict = {"GeneratedPassword": 123} pw_conf_path = os.path.join(self.temp_homedir, 'undercloud-passwords.conf') @@ -175,6 +176,7 @@ class TestDeployUndercloud(TestPluginV1): t_pw.write('[auth]\nundercloud_db_password = abc\n') self.cmd._update_passwords_env(self.temp_homedir, + 'stack', passwords={'ADefault': 456, 'ExistingKey': 'dontupdate'}) @@ -185,9 +187,6 @@ class TestDeployUndercloud(TestPluginV1): mock_dump.assert_called_once_with(expected_dict, mock.ANY, default_flow_style=False) - chmod_calls = [mock.call(t_pw_conf_path, 0o600), - mock.call(pw_conf_path, 0o600)] - mock_chmod.assert_has_calls(chmod_calls) @mock.patch('heatclient.common.template_utils.' 'process_environment_and_files', return_value=({}, {}), @@ -604,6 +603,12 @@ class TestDeployUndercloud(TestPluginV1): env ) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('os.chmod') + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('subprocess.check_call', autospec=True) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('os.mkdir') @mock.patch('six.moves.builtins.open') @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.' @@ -641,7 +646,8 @@ class TestDeployUndercloud(TestPluginV1): mock_wait_for_port, mock_createdirs, mock_cleanupdirs, mock_launchansible, mock_tarball, mock_templates_dir, - mock_open, mock_os): + mock_open, mock_os, mock_user, mock_cc, + mock_chmod): parsed_args = self.check_parser(self.cmd, ['--local-ip', '127.0.0.1', @@ -649,6 +655,9 @@ class TestDeployUndercloud(TestPluginV1): '--stack', 'undercloud', '--output-dir', '/my', '--standalone-role', 'Undercloud', + # TODO(cjeanner) drop once we have + # proper oslo.privsep + '--deployment-user', 'stack', '-e', '/tmp/thtroot/puppet/foo.yaml', '-e', '/tmp/thtroot//docker/bar.yaml', '-e', '/tmp/thtroot42/notouch.yaml', diff --git a/tripleoclient/tests/v1/undercloud/test_undercloud.py b/tripleoclient/tests/v1/undercloud/test_undercloud.py index bf866972f..ddaf58f01 100644 --- a/tripleoclient/tests/v1/undercloud/test_undercloud.py +++ b/tripleoclient/tests/v1/undercloud/test_undercloud.py @@ -69,13 +69,15 @@ class TestUndercloudInstall(TestPluginV1): mock_subprocess.assert_called_with(['instack-install-undercloud']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_heat_customized(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_copy, mock_user): self.conf.config(output_dir='/foo') self.conf.config(templates='/usertht') self.conf.config(roles_file='foo/roles.yaml') @@ -115,6 +117,8 @@ class TestUndercloudInstall(TestPluginV1): '/usertht/environments/use-dns-for-vips.yaml', '-e', '/usertht/environments/services/undercloud-haproxy.yaml', '-e', '/usertht/environments/services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', '--output-dir=/foo', '--cleanup', '-e', '/foo/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -122,6 +126,8 @@ class TestUndercloudInstall(TestPluginV1): '/usertht/undercloud-stack-vstate-dropin.yaml', '--force-stack-update']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @@ -143,7 +149,7 @@ class TestUndercloudInstall(TestPluginV1): mock_sroutes, mock_masq, mock_wr, mock_os, - mock_copy): + mock_copy, mock_user): self.conf.config(net_config_override='/foo/net-config.json') self.conf.config(local_interface='ethX') self.conf.config(undercloud_public_host='4.3.2.1') @@ -277,7 +283,10 @@ class TestUndercloudInstall(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' - 'services/undercloud-keepalived.yaml', '--output-dir=/home/stack', + 'services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', + '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -285,6 +294,8 @@ class TestUndercloudInstall(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('six.moves.builtins.open') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @@ -293,7 +304,7 @@ class TestUndercloudInstall(TestPluginV1): def test_undercloud_install_with_heat_and_debug(self, mock_subprocess, mock_wr, mock_os, mock_copy, - mock_open): + mock_open, mock_user): self.conf.config(undercloud_log_file='/foo/bar') arglist = ['--use-heat', '--no-validations'] verifylist = [] @@ -341,6 +352,8 @@ class TestUndercloudInstall(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -348,6 +361,8 @@ class TestUndercloudInstall(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('six.moves.builtins.open') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @@ -356,7 +371,7 @@ class TestUndercloudInstall(TestPluginV1): def test_undercloud_install_with_heat_true(self, mock_subprocess, mock_wr, mock_os, mock_copy, - mock_open): + mock_open, mock_user): self.conf.config(undercloud_log_file='/foo/bar') arglist = ['--use-heat=True', '--no-validations'] verifylist = [] @@ -401,19 +416,23 @@ class TestUndercloudInstall(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', '--log-file=/foo/bar', '-e', '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_swift_encryption(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_copy, mock_user): arglist = ['--use-heat', '--no-validations'] verifylist = [] self.conf.set_default('enable_swift_encryption', True) @@ -462,6 +481,7 @@ class TestUndercloudInstall(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -528,13 +548,15 @@ class TestUndercloudUpgrade(TestPluginV1): ] ) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_enabled(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_copy, mock_user): arglist = ['--use-heat', '--no-validations'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -581,6 +603,7 @@ class TestUndercloudUpgrade(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -588,13 +611,15 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_true(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_copy, mock_user): arglist = ['--use-heat=True', '--no-validations'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -641,6 +666,8 @@ class TestUndercloudUpgrade(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -648,13 +675,14 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_and_yes(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_user, mock_copy): arglist = ['--use-heat', '--no-validations', '-y'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -701,6 +729,8 @@ class TestUndercloudUpgrade(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + # TODO(cjeanner) drop once we have proper oslo.privsep + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', @@ -708,13 +738,15 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + # TODO(cjeanner) drop once we have proper oslo.privsep + @mock.patch('getpass.getuser', return_value='stack') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_and_debug(self, mock_subprocess, - mock_wr, - mock_os, mock_copy): + mock_wr, mock_os, + mock_copy, mock_user): arglist = ['--use-heat', '--no-validations'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -764,6 +796,7 @@ class TestUndercloudUpgrade(TestPluginV1): 'services/undercloud-haproxy.yaml', '-e', '/usr/share/openstack-tripleo-heat-templates/environments/' 'services/undercloud-keepalived.yaml', + '--deployment-user', 'stack', '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', diff --git a/tripleoclient/v1/tripleo_deploy.py b/tripleoclient/v1/tripleo_deploy.py index 61314dbcb..f94ba4e12 100644 --- a/tripleoclient/v1/tripleo_deploy.py +++ b/tripleoclient/v1/tripleo_deploy.py @@ -22,6 +22,7 @@ import pwd import re import shutil import six +import subprocess import sys import tarfile import tempfile @@ -85,6 +86,38 @@ class Deploy(command.Command): roles_data = None stack_update_mark = None stack_action = 'CREATE' + deployment_user = None + + # NOTE(cjeanner) Quick'n'dirty way before we have proper + # escalation support through oslo.privsep + def _set_data_rights(self, file_name, user=None, + mode=0o600): + + u = user or self.deployment_user + u_flag = None + f_flag = None + if u: + if os.path.exists(file_name): + try: + pwd.getpwnam(u) + cmd = 'sudo chown -R %s %s' % (u, file_name) + subprocess.check_call(cmd.split()) + except KeyError: + u_flag = 'Unknown' + else: + f_flag = "Absent" + else: + u_flag = 'Undefined' + + if u_flag: + self.log.warning(_('%(u_f)s user "%(u)s". You might need to ' + 'manually set ownership after the deploy') + % {'u_f': u_flag, 'u': user}) + if f_flag: + self.log.warning(_('%(f)s file is %(f_f)s.') + % {'f': file_name, 'f_f': f_flag}) + else: + os.chmod(file_name, mode) def _set_roles_file(self, file_name=None, templates_dir=None): """Set the roles file for the deployment @@ -136,7 +169,7 @@ class Deploy(command.Command): (self.output_dir, datetime.utcnow().strftime('%Y%m%d%H%M%S')) - def _create_install_artifact(self): + def _create_install_artifact(self, user): """Create a tarball of the temporary folders used""" self.log.debug(_("Preserving deployment artifacts")) @@ -160,6 +193,8 @@ class Deploy(command.Command): msg = _("Unable to create artifact tarball, %s") % ex.message self.log.error(msg) raise exceptions.DeploymentError(msg) + # TODO(cjeanner) drop that once using oslo.privsep + self._set_data_rights(tar_filename, user=user) return tar_filename def _create_persistent_dirs(self): @@ -200,7 +235,7 @@ class Deploy(command.Command): shutil.copytree(source_templates_dir, self.tht_render, symlinks=True) - def _cleanup_working_dirs(self, cleanup=False): + def _cleanup_working_dirs(self, cleanup=False, user=None): """Cleanup temporary working directories :param cleanup: Set to true if you DO want to cleanup the dirs @@ -216,8 +251,12 @@ class Deploy(command.Command): else: self.log.warning(_("Not cleaning working directory %s") % self.tht_render) + # TODO(cjeanner) drop that once using oslo.privsep + self._set_data_rights(self.tht_render, user=user, mode=0o700) self.log.warning(_("Not cleaning ansible directory %s") % self.tmp_ansible_dir) + # TODO(cjeanner) drop that once using oslo.privsep + self._set_data_rights(self.tmp_ansible_dir, user=user, mode=0o700) def _configure_puppet(self): self.log.info(_('Configuring puppet modules symlinks ...')) @@ -225,7 +264,7 @@ class Deploy(command.Command): constants.PUPPET_MODULES, constants.PUPPET_BASE) - def _update_passwords_env(self, output_dir, passwords=None): + def _update_passwords_env(self, output_dir, user, passwords=None): pw_file = os.path.join(output_dir, 'tripleo-undercloud-passwords.yaml') undercloud_pw_file = os.path.join(output_dir, 'undercloud-passwords.conf') @@ -288,9 +327,9 @@ class Deploy(command.Command): # This contains sensitive data so ensure it's not world-readable with open(pw_file, 'w') as pf: yaml.safe_dump(stack_env, pf, default_flow_style=False) - # Using chmod here instead of permissions on the open above so we don't - # have to fight with umask. - os.chmod(pw_file, 0o600) + # TODO(cjeanner) drop that once using oslo.privsep + # Do not forget to re-add os.chmod 0o600 on that one! + self._set_data_rights(pw_file, user=user) # Write out an instack undercloud compatible version. # This contains sensitive data so ensure it's not world-readable with open(undercloud_pw_file, 'w') as pf: @@ -303,7 +342,10 @@ class Deploy(command.Command): pw_key = re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower() pf.write('undercloud_%s: %s\n' % (pw_key, v)) - os.chmod(undercloud_pw_file, 0o600) + + # TODO(cjeanner) drop that once using oslo.privsep + # Do not forget to re-add os.chmod 0o600 on that one! + self._set_data_rights(undercloud_pw_file, user=user) return pw_file @@ -519,7 +561,8 @@ class Deploy(command.Command): for e in plan_env_data.get('environments', {})] # this will allow the user to overwrite passwords with custom envs - pw_file = self._update_passwords_env(self.output_dir) + pw_file = self._update_passwords_env(self.output_dir, + parsed_args.deployment_user) environments.append(pw_file) # use deployed-server because we run os-collect-config locally @@ -769,6 +812,14 @@ class Deploy(command.Command): help=_('User to execute the non-priveleged heat-all process. ' 'Defaults to heat.') ) + # TODO(cjeanner) drop that once using oslo.privsep + parser.add_argument( + '--deployment-user', + dest='deployment_user', + default='stack', + help=_('User who executes the tripleo deploy command. ' + 'Defaults to stack.') + ) parser.add_argument( '--heat-container-image', metavar='', dest='heat_container_image', @@ -977,8 +1028,12 @@ class Deploy(command.Command): finally: if not parsed_args.keep_running: self._kill_heat(parsed_args) - tar_filename = self._create_install_artifact() - self._cleanup_working_dirs(cleanup=parsed_args.cleanup) + tar_filename = \ + self._create_install_artifact(parsed_args.deployment_user) + self._cleanup_working_dirs( + cleanup=parsed_args.cleanup, + user=parsed_args.deployment_user + ) if tar_filename: self.log.warning('Install artifact is located at %s' % tar_filename) diff --git a/tripleoclient/v1/undercloud_config.py b/tripleoclient/v1/undercloud_config.py index 134af9940..8500874f4 100644 --- a/tripleoclient/v1/undercloud_config.py +++ b/tripleoclient/v1/undercloud_config.py @@ -467,6 +467,8 @@ def prepare_undercloud_deploy(upgrade=False, no_validations=False, u = CONF.get('deployment_user') or utils.get_deployment_user() env_data['DeploymentUser'] = u + # TODO(cjeanner) drop that once using oslo.privsep + deploy_args += ['--deployment-user', u] deploy_args += ['--output-dir=%s' % CONF['output_dir']] if not os.path.isdir(CONF['output_dir']):