From 61169bc147259abcdf45b86b561c7b6f089e7f8e Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Sun, 13 Sep 2020 21:46:48 -0400 Subject: [PATCH] Ensure root isn't used to run updates/upgrades The updates and upgrades operations should not be executed as "root" user, and the operator should use the regular deployment user (e.g. "stack"). This enforces this expected behavior. Change-Id: If23f12d6ab571bc3a3b41aef3dcbd58f98d80977 --- .../v1/overcloud_update/test_overcloud_update.py | 13 ++++++++++--- .../v1/overcloud_upgrade/test_overcloud_upgrade.py | 10 ++++++++-- tripleoclient/v1/overcloud_external_update.py | 1 + tripleoclient/v1/overcloud_external_upgrade.py | 1 + tripleoclient/v1/overcloud_update.py | 3 +++ tripleoclient/v1/overcloud_upgrade.py | 2 ++ 6 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py index 1ef4231e6..9fd334cf7 100644 --- a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py +++ b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py @@ -36,6 +36,7 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare): self.mock_uuid4 = uuid4_patcher.start() self.addCleanup(self.mock_uuid4.stop) + @mock.patch('tripleoclient.utils.ensure_run_as_normal_user') @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', return_value=True) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' @@ -56,7 +57,7 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare): def test_update_out(self, mock_deploy, mock_open, mock_copy, mock_yaml, mock_abspath, mock_update, mock_logger, mock_get_stack, mock_get_undercloud_host_entry, - mock_confirm): + mock_confirm, mock_usercheck): mock_stack = mock.Mock(parameters={'DeployIdentifier': ''}) mock_stack.stack_name = 'overcloud' mock_get_stack.return_value = mock_stack @@ -75,11 +76,13 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare): mock_exists.return_value = True mock_isfile.return_value = True self.cmd.take_action(parsed_args) + mock_usercheck.assert_called_once() mock_update.assert_called_once_with( self.app.client_manager, container='overcloud', ) + @mock.patch('tripleoclient.utils.ensure_run_as_normal_user') @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', return_value=True) @mock.patch('tripleoclient.utils.get_stack', @@ -94,7 +97,7 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare): '_deploy_tripleo_heat_templates', autospec=True) def test_update_failed(self, mock_deploy, mock_copy, mock_yaml, mock_abspath, mock_open, mock_update, - mock_get_stack, mock_confirm): + mock_get_stack, mock_confirm, mock_usercheck): mock_stack = mock.Mock(parameters={'DeployIdentifier': ''}) mock_stack.stack_name = 'overcloud' mock_get_stack.return_value = mock_stack @@ -113,6 +116,7 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare): mock_isfile.return_value = True self.assertRaises(exceptions.DeploymentError, self.cmd.take_action, parsed_args) + mock_usercheck.assert_called_once() class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): @@ -159,11 +163,13 @@ class TestOvercloudUpdateConverge(fakes.TestOvercloudUpdateConverge): app_args.verbose_level = 1 self.cmd = overcloud_update.UpdateConverge(self.app, app_args) + @mock.patch('tripleoclient.utils.ensure_run_as_normal_user') @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', return_value=True) @mock.patch( 'tripleoclient.v1.overcloud_deploy.DeployOvercloud.take_action') - def test_update_converge(self, deploy_action, mock_confirm): + def test_update_converge(self, deploy_action, mock_confirm, + mock_usercheck): argslist = ['--templates', '--stack', 'cloud'] verifylist = [ ('stack', 'cloud') @@ -175,6 +181,7 @@ class TestOvercloudUpdateConverge(fakes.TestOvercloudUpdateConverge): mock_exists.return_value = True mock_isfile.return_value = True self.cmd.take_action(parsed_args) + mock_usercheck.assert_called_once() assert('/usr/share/openstack-tripleo-heat-templates/' 'environments/lifecycle/update-converge.yaml' in parsed_args.environment_files) diff --git a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py index 4418ff470..27c957af8 100644 --- a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py +++ b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py @@ -37,6 +37,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): uuid4_patcher = mock.patch('uuid.uuid4', return_value="UUID4") self.mock_uuid4 = uuid4_patcher.start() self.addCleanup(self.mock_uuid4.stop) + + @mock.patch('tripleoclient.utils.ensure_run_as_normal_user') @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', return_value=True) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' @@ -58,7 +60,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): add_env, mock_enable_ssh_admin, mock_overcloud_deploy, - mock_confirm): + mock_confirm, + mock_usercheck): mock_stack = mock.Mock(parameters={'DeployIdentifier': ''}) mock_stack.stack_name = 'overcloud' @@ -78,6 +81,7 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): parsed_args = self.check_parser(self.cmd, argslist, verifylist) self.cmd.take_action(parsed_args) + mock_usercheck.assert_called_once() mock_overcloud_deploy.assert_called_once_with(parsed_args) args, kwargs = mock_overcloud_deploy.call_args @@ -92,6 +96,7 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): mock.ANY ) + @mock.patch('tripleoclient.utils.ensure_run_as_normal_user') @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', return_value=True) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' @@ -103,7 +108,7 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): @mock.patch('yaml.safe_load') def test_upgrade_failed(self, mock_yaml, mock_open, add_env, mock_get_stack, mock_overcloud_deploy, - mock_confirm): + mock_confirm, mock_usercheck): mock_overcloud_deploy.side_effect = exceptions.DeploymentError() mock_yaml.return_value = {'fake_container': 'fake_value'} mock_stack = mock.Mock(parameters={'DeployIdentifier': ''}) @@ -120,6 +125,7 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare): self.assertRaises(exceptions.DeploymentError, self.cmd.take_action, parsed_args) + mock_usercheck.assert_called_once() mock_overcloud_deploy.assert_called_once_with(parsed_args) @mock.patch('tripleo_common.update.check_neutron_mechanism_drivers') diff --git a/tripleoclient/v1/overcloud_external_update.py b/tripleoclient/v1/overcloud_external_update.py index 0445f6ba3..5eecc2ae2 100644 --- a/tripleoclient/v1/overcloud_external_update.py +++ b/tripleoclient/v1/overcloud_external_update.py @@ -113,6 +113,7 @@ class ExternalUpdateRun(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( diff --git a/tripleoclient/v1/overcloud_external_upgrade.py b/tripleoclient/v1/overcloud_external_upgrade.py index c17867138..b7bffa4bf 100644 --- a/tripleoclient/v1/overcloud_external_upgrade.py +++ b/tripleoclient/v1/overcloud_external_upgrade.py @@ -113,6 +113,7 @@ class ExternalUpgradeRun(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index 02605a1ba..35a9293d5 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/tripleoclient/v1/overcloud_update.py @@ -55,6 +55,7 @@ class UpdatePrepare(DeployOvercloud): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( @@ -172,6 +173,7 @@ class UpdateRun(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( @@ -236,6 +238,7 @@ class UpdateConverge(DeployOvercloud): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( diff --git a/tripleoclient/v1/overcloud_upgrade.py b/tripleoclient/v1/overcloud_upgrade.py index bd5ee0354..130f25512 100644 --- a/tripleoclient/v1/overcloud_upgrade.py +++ b/tripleoclient/v1/overcloud_upgrade.py @@ -60,6 +60,7 @@ class UpgradePrepare(DeployOvercloud): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation( @@ -207,6 +208,7 @@ class UpgradeRun(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + oooutils.ensure_run_as_normal_user() if (not parsed_args.yes and not oooutils.prompt_user_for_confirmation(