diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 6d0f8381f6..aca403313b 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -2959,7 +2959,8 @@ class ConductorManager(base_manager.BaseConductorManager): # either tokens are required and they are present, # or a token is present in general and needs to be # validated. - if token_required or utils.is_agent_token_present(task.node): + if (token_required + or (utils.is_agent_token_present(task.node) and agent_token)): if not utils.is_agent_token_valid(task.node, agent_token): LOG.error('Invalid agent_token receieved for node ' '%(node)s', {'node': node_id}) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 7f9cea147c..2d97d655c3 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1010,13 +1010,22 @@ def get_node_next_deploy_steps(task, skip_current_step=True): skip_current_step=skip_current_step) -def add_secret_token(node): - """Adds a secret token to driver_internal_info for IPA verification.""" +def add_secret_token(node, pregenerated=False): + """Adds a secret token to driver_internal_info for IPA verification. + + :param node: Node object + :param pregenerated: Boolean value, default False, which indicates if + the token should be marked as "pregenerated" in + order to facilitate virtual media booting where + the token is embedded into the configuration. + """ characters = string.ascii_letters + string.digits token = ''.join( random.SystemRandom().choice(characters) for i in range(128)) i_info = node.driver_internal_info i_info['agent_secret_token'] = token + if pregenerated: + i_info['agent_secret_token_pregenerated'] = True node.driver_internal_info = i_info diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index bd5ab0d042..b6142bb8be 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -509,6 +509,13 @@ class IloVirtualMediaBoot(base.BootInterface): # during boot. ilo_common.eject_vmedia_devices(task) + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(task.node, pregenerated=True) + ramdisk_params['ipa-agent-token'] = \ + task.node.driver_internal_info['agent_secret_token'] + task.node.save() + deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac if node.provision_state == states.RESCUING: diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index c7bb45c783..e23ee49610 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -990,6 +990,13 @@ class IRMCVirtualMediaBoot(base.BootInterface, IRMCVolumeBootMixIn): {'node': task.node.uuid}) return + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(task.node, pregenerated=True) + ramdisk_params['ipa-agent-token'] = \ + task.node.driver_internal_info['agent_secret_token'] + task.node.save() + deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index f3538b6ebd..005f77d371 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -682,6 +682,13 @@ class RedfishVirtualMediaBoot(base.BootInterface): states.INSPECTING): return + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(node, pregenerated=True) + node.save() + ramdisk_params['ipa-agent-token'] = \ + node.driver_internal_info['agent_secret_token'] + manager_utils.node_power_action(task, states.POWER_OFF) d_info = self._parse_driver_info(node) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index b6eecf3b88..eba6766ec2 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -7082,6 +7082,30 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, 'http://callback', '1.4.1') + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', + autospec=True) + @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', + autospec=True) + def test_heartbeat_with_agent_pregenerated_token( + self, mock_spawn, mock_heartbeat): + """Test heartbeating.""" + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.DEPLOYING, + target_provision_state=states.ACTIVE, + driver_internal_info={'agent_secret_token': 'a secret'}) + + self._start_service() + + mock_spawn.reset_mock() + + mock_spawn.side_effect = self._fake_spawn + self.service.heartbeat( + self.context, node.uuid, 'http://callback', '6.0.1', + agent_token=None) + mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, + 'http://callback', '6.0.1') + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', @@ -7209,32 +7233,6 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): agent_token='evil', agent_version='4.0.0') self.assertFalse(mock_heartbeat.called) - @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', - autospec=True) - @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', - autospec=True) - def test_heartbeat_invalid_when_token_on_file_older_agent( - self, mock_spawn, mock_heartbeat): - """Heartbeat rejected if a token is on file.""" - self.config(require_agent_token=False) - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.DEPLOYING, - target_provision_state=states.ACTIVE, - driver_internal_info={'agent_secret_token': 'a secret'}) - - self._start_service() - - mock_spawn.reset_mock() - - mock_spawn.side_effect = self._fake_spawn - - self.assertRaises(exception.InvalidParameterValue, - self.service.heartbeat, self.context, - node.uuid, 'http://callback', - agent_token=None, agent_version='4.0.0') - self.assertFalse(mock_heartbeat.called) - @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 46d5195adb..1e133a5402 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -821,7 +821,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): prepare_node_for_deploy_mock.assert_called_once_with(task) eject_mock.assert_called_once_with(task) - expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab'} + expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab', + 'ipa-agent-token': mock.ANY} get_nic_mock.assert_called_once_with(task) setup_vmedia_mock.assert_called_once_with(task, iso, expected_ramdisk_opts) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 68ba242974..e225f7bb92 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -990,7 +990,9 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): shared=False) as task: task.driver.boot.prepare_ramdisk(task, ramdisk_params) - expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab'} + expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab', + 'ipa-agent-token': mock.ANY} + get_single_nic_with_vif_port_id_mock.assert_called_once_with( task) _setup_vmedia_mock.assert_called_once_with( diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 4a759b6cf8..aa072fa0f7 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -552,6 +552,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertRaises(exception.UnsupportedDriverExtension, task.driver.boot.validate_inspection, task) + @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -560,15 +562,16 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_with_params( - self, mock_boot_mode_utils, mock_manager_utils, + self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso): + mock__prepare_deploy_iso, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} @@ -576,7 +579,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__eject_vmedia.assert_called_once_with( @@ -587,17 +590,20 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): expected_params = { 'BOOTIF': None, + 'ipa-agent-token': mock.ANY, 'ipa-debug': '1', } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -606,12 +612,13 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_no_debug( - self, mock_boot_mode_utils, mock_manager_utils, + self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso): + mock__prepare_deploy_iso, mock_node_set_boot_device): self.config(debug=False) with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -622,7 +629,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__eject_vmedia.assert_called_once_with( @@ -633,16 +640,19 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): expected_params = { 'BOOTIF': None, + 'ipa-agent-token': mock.ANY, } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_floppy_image', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -655,16 +665,17 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_with_floppy( - self, mock_boot_mode_utils, mock_manager_utils, + self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, mock__has_vmedia_device, mock__prepare_deploy_iso, - mock__prepare_floppy_image): + mock__prepare_floppy_image, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = { @@ -677,7 +688,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__has_vmedia_device.assert_called_once_with( @@ -703,12 +714,13 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'BOOTIF': None, 'boot_method': 'vmedia', 'ipa-debug': '1', + 'ipa-agent-token': mock.ANY, } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task)