From 44d3d5eb4c57dd86c4a7e0adb11ae620f176519a Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 29 Oct 2019 10:42:00 -0400 Subject: [PATCH] Remove dead set_admin_password code to generate password The schema validation on the changePassword server action API requires adminPass as a non-None string value so the code down in the ComputeManager.set_admin_password method to check if new_pass is None and generate a password if so is dead code and is removed here along with the password=None kwarg usage in the API. Change-Id: I310df11043f8bfe78640087d9ec2848ace65edc8 --- nova/compute/api.py | 2 +- nova/compute/manager.py | 4 ---- .../unit/api/openstack/compute/test_admin_password.py | 9 ++++++--- nova/tests/unit/compute/test_compute_api.py | 4 ++-- nova/tests/unit/compute/test_compute_mgr.py | 7 +++---- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index eac37255f0c0..66db401f409f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3885,7 +3885,7 @@ class API(base.Base): @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE]) - def set_admin_password(self, context, instance, password=None): + def set_admin_password(self, context, instance, password): """Set the root/admin password for the given instance. @param context: Nova auth context. diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 79fc3058eb9c..db1253adf702 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3917,10 +3917,6 @@ class ComputeManager(manager.Manager): """ context = context.elevated() - if new_pass is None: - # Generate a random password - new_pass = utils.generate_password() - current_power_state = self._get_power_state(context, instance) expected_state = power_state.RUNNING diff --git a/nova/tests/unit/api/openstack/compute/test_admin_password.py b/nova/tests/unit/api/openstack/compute/test_admin_password.py index 0a274b243674..c23b9a73453d 100644 --- a/nova/tests/unit/api/openstack/compute/test_admin_password.py +++ b/nova/tests/unit/api/openstack/compute/test_admin_password.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. import mock +import six import webob from nova.api.openstack.compute import admin_password as admin_password_v21 @@ -122,9 +123,11 @@ class AdminPasswordTestV21(test.NoDBTestCase): def test_change_password_none(self): body = {'changePassword': {'adminPass': None}} - self.assertRaises(self.validation_error, - self._get_action(), - self.fake_req, fakes.FAKE_UUID, body=body) + ex = self.assertRaises(self.validation_error, + self._get_action(), + self.fake_req, fakes.FAKE_UUID, body=body) + self.assertIn('adminPass. Value: None. None is not of type', + six.text_type(ex)) def test_change_password_adminpass_none(self): body = {'changePassword': None} diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 54474b6c1d21..507e0f15fb47 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4912,14 +4912,14 @@ class _ComputeAPIUnitTestMixIn(object): 'set_admin_password') def do_test(compute_rpcapi_mock, record_mock, instance_save_mock): # call the API - self.compute_api.set_admin_password(self.context, instance) + self.compute_api.set_admin_password(self.context, instance, 'pass') # make our assertions instance_save_mock.assert_called_once_with( expected_task_state=[None]) record_mock.assert_called_once_with( self.context, instance, instance_actions.CHANGE_PASSWORD) compute_rpcapi_mock.assert_called_once_with( - self.context, instance=instance, new_pass=None) + self.context, instance=instance, new_pass='pass') do_test() diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 0712767f85cb..0788b593343c 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4365,9 +4365,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch('nova.compute.manager.ComputeManager._get_power_state', return_value=power_state.RUNNING) @mock.patch.object(objects.Instance, 'save') - @mock.patch('nova.utils.generate_password', return_value='fake-pass') - def test_set_admin_password(self, gen_password_mock, instance_save_mock, - power_state_mock): + def test_set_admin_password(self, instance_save_mock, power_state_mock): # Ensure instance can have its admin password set. instance = fake_instance.fake_instance_obj( self.context, @@ -4378,7 +4376,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch.object(self.compute.driver, 'set_admin_password') def do_test(driver_mock, elevated_mock): # call the manager method - self.compute.set_admin_password(self.context, instance, None) + self.compute.set_admin_password(self.context, instance, + 'fake-pass') # make our assertions self.assertEqual(vm_states.ACTIVE, instance.vm_state) self.assertIsNone(instance.task_state)