From 58f84d28549b9e06d233be6465b388e873148fd2 Mon Sep 17 00:00:00 2001 From: cid Date: Tue, 18 Jun 2024 16:20:11 +0100 Subject: [PATCH] Allow disabling specific boot modes during deployment/enrollment Allow operators to provide a list of disabled boot modes for new deployments ``disallowed_deployment_boot_modes`` and/or enrollments ``disallowed_enrollment_boot_modes``. Defaults are an empty list, [], indicating all modes are allowed. Closes-Bug: #2068530 Change-Id: I1404c81718cd6bb2977e6f298d9b7d11664226d0 --- ironic/api/controllers/v1/node.py | 12 +++ ironic/api/controllers/v1/utils.py | 16 +++ ironic/common/exception.py | 4 + ironic/conductor/deployments.py | 9 +- ironic/conf/api.py | 11 +++ ironic/conf/conductor.py | 12 +++ .../unit/api/controllers/v1/test_node.py | 98 +++++++++++++++++++ .../tests/unit/conductor/test_deployments.py | 50 ++++++++++ ...ing-bios-deployments-356f3215f80a879e.yaml | 7 ++ 9 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/allow-disabling-bios-deployments-356f3215f80a879e.yaml diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index e721ce7769..8ca25bdc5e 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2775,6 +2775,12 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted() + node_capabilities = node.get('properties', {}).get('capabilities', '') + # ``check_allow_boot_mode`` expects ``node_capabilities`` to be a list + api_utils.check_allow_boot_mode( + [node_capabilities], + CONF.api.disallowed_enrollment_boot_modes) + context = api.request.context owned_node = False if CONF.api.project_admin_can_manage_own_nodes: @@ -2870,6 +2876,12 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted() + node_capabilities = api_utils.get_patch_values( + patch, '/properties/capabilities') + api_utils.check_allow_boot_mode( + node_capabilities, + CONF.api.disallowed_enrollment_boot_modes) + api_utils.patch_validate_allowed_fields(patch, PATCH_ALLOWED_FIELDS) reject_patch_in_newer_versions(patch) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index b806888737..fa867960e5 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -2006,6 +2006,22 @@ def check_allow_deploy_steps(target, deploy_steps): msg, status_code=http_client.BAD_REQUEST) +def check_allow_boot_mode(node_capabilities, disallowed_boot_modes): + """Check if boot mode is allowed""" + + if (not node_capabilities) or (not disallowed_boot_modes): + return + + disallowed_set = set(disallowed_boot_modes) + + for capability in node_capabilities: + for item in capability.lower().split(','): + key, value = item.split(':') + if key.strip() == 'boot_mode' and value.strip() in disallowed_set: + raise exception.BootModeNotAllowed(mode=value.strip(), + op=_('provisioning')) + + def check_allow_clean_disable_ramdisk(target, disable_ramdisk): if disable_ramdisk is None: return diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 59d60f3ba0..e524948c29 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -893,3 +893,7 @@ class UnsupportedHardwareFeature(Invalid): _msg_fmt = _("Node %(node)s hardware does not support feature " "%(feature)s, which is required based upon the " "requested configuration.") + + +class BootModeNotAllowed(Invalid): + _msg_fmt = _("'%(mode)s' boot mode is not allowed for %(op)s operation.") diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 980baad03e..a14898ede4 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -43,7 +43,8 @@ def validate_node(task, event='deploy'): :param task: a TaskManager instance. :param event: event to process: deploy or rebuild. - :raises: NodeInMaintenance, NodeProtected, InvalidStateRequested + :raises: NodeInMaintenance, NodeProtected, InvalidStateRequested, + BootModeNotAllowed """ if task.node.maintenance: raise exception.NodeInMaintenance(op=_('provisioning'), @@ -56,6 +57,12 @@ def validate_node(task, event='deploy'): raise exception.InvalidStateRequested( action=event, node=task.node.uuid, state=task.node.provision_state) + disallowed_boot_modes = CONF.conductor.disallowed_deployment_boot_modes + boot_mode = task.node.properties.get('boot_mode', '').lower() + if disallowed_boot_modes and boot_mode.strip() in disallowed_boot_modes: + raise exception.BootModeNotAllowed(mode=boot_mode, + op=_('provisioning')) + @METRICS.timer('start_deploy') @task_manager.require_exclusive_lock diff --git a/ironic/conf/api.py b/ironic/conf/api.py index a8989da50b..b04048d1d8 100644 --- a/ironic/conf/api.py +++ b/ironic/conf/api.py @@ -17,6 +17,7 @@ from oslo_config import cfg from oslo_config import types as cfg_types +from ironic.common import boot_modes from ironic.common.i18n import _ @@ -92,6 +93,16 @@ opts = [ mutable=True, help=_('If a project scoped administrative user is permitted ' 'to create/delete baremetal nodes in their project.')), + cfg.ListOpt('disallowed_enrollment_boot_modes', + item_type=cfg_types.String( + choices=[ + (boot_modes.UEFI, _('UEFI boot mode')), + (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], + ), + default=[], + mutable=True, + help=_("Specifies a list of boot modes that are not allowed " + "during enrollment. Eg: ['bios']")), ] opt_group = cfg.OptGroup(name='api', diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index 01f385ba6f..976130e997 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -18,8 +18,10 @@ from oslo_config import cfg from oslo_config import types +from ironic.common import boot_modes from ironic.common.i18n import _ + opts = [ cfg.IntOpt('workers_pool_size', default=300, min=3, @@ -417,6 +419,16 @@ opts = [ 'seconds, or 30 minutes. If you need to wait longer ' 'than the maximum value, we recommend exploring ' 'hold steps.')), + cfg.ListOpt('disallowed_deployment_boot_modes', + item_type=types.String( + choices=[ + (boot_modes.UEFI, _('UEFI boot mode')), + (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], + ), + default=[], + mutable=True, + help=_("Specifies a list of boot modes that are not allowed " + "during deployment. Eg: ['bios']")), ] diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index cfd0eb9bc7..5411aa3459 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -2903,6 +2903,60 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_code) + def test_update_fails_on_disabled_bios_boot_mode(self): + self.config(disallowed_enrollment_boot_modes=['bios'], group='api') + + patch = [{ + 'path': '/properties/capabilities', + 'value': 'boot_mode:bios', + 'op': 'replace' + }] + + response = self.patch_json('/nodes/%s/' % self.node.uuid, patch, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertIn("'bios' boot mode is not allowed", + response.json['error_message']) + + def test_update_fails_on_disabled_uefi_boot_mode(self): + self.config(disallowed_enrollment_boot_modes=['uefi'], group='api') + + patch = [{ + 'path': '/properties/capabilities', + 'value': 'boot_mode:uefi', + 'op': 'replace' + }] + + response = self.patch_json('/nodes/%s/' % self.node.uuid, patch, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertIn("'uefi' boot mode is not allowed", + response.json['error_message']) + + def test_update_fails_on_invalid_boot_mode(self): + # NOTE(cid): This test might need updating if boot modes' naming + # convention changes + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['BIOS'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['Bios'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['UEFI'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['Uefi'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['blah'], + group='api') + def test_update_with_reset_interfaces(self): self.mock_update_node.return_value = self.node (self @@ -4436,6 +4490,50 @@ class TestPost(test_api_base.BaseApiTest): self.assertFalse(mock_warning.called) self.assertFalse(mock_exception.called) + def test_create_node_fails_on_disabled_bios_boot_mode(self): + self.config(disallowed_enrollment_boot_modes=['bios'], group='api') + ndict = test_api_utils.post_get_test_node() + ndict['properties'] = {'capabilities': 'boot_mode:bios'} + + response = self.post_json('/nodes', ndict, expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertIn("'bios' boot mode is not allowed", + response.json['error_message']) + + def test_create_node_fails_on_disabled_uefi_boot_mode(self): + self.config(disallowed_enrollment_boot_modes=['uefi'], group='api') + ndict = test_api_utils.post_get_test_node() + ndict['properties'] = {'capabilities': 'boot_mode:uefi'} + + response = self.post_json('/nodes', ndict, expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertIn("'uefi' boot mode is not allowed", + response.json['error_message']) + + def test_create_node_fails_on_invalid_boot_mode(self): + # NOTE(cid): This test might need updating if boot modes' naming + # convention changes + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['BIOS'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['Bios'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['UEFI'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['Uefi'], + group='api') + self.assertRaises(ValueError, + self.config, + disallowed_enrollment_boot_modes=['blah'], + group='api') + def test_create_node_chassis_uuid_always_in_response(self): result = self._test_create_node(chassis_uuid=None) self.assertIsNone(result['chassis_uuid']) diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 61de1b54af..f073460853 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -282,6 +282,56 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertIsNone(node.last_error) mock_deploy.assert_called_once_with(mock.ANY, mock.ANY) + def test_node_validation_in_disabled_bios_boot_mode_fails(self): + self.config(disallowed_deployment_boot_modes=['bios'], + group='conductor') + node = obj_utils.create_test_node(self.context, + properties={'boot_mode': 'bios'}, + driver='fake-hardware') + + with task_manager.acquire(self.context, node.uuid, + shared=False) as task: + self.assertRaises(exception.BootModeNotAllowed, + deployments.validate_node, + task, event='deploy') + + def test_node_validation_in_disabled_uefi_boot_mode_fails(self): + self.config(disallowed_deployment_boot_modes=['uefi'], + group='conductor') + node = obj_utils.create_test_node(self.context, + properties={'boot_mode': 'uefi'}, + driver='fake-hardware') + + with task_manager.acquire(self.context, node.uuid, + shared=False) as task: + self.assertRaises(exception.BootModeNotAllowed, + deployments.validate_node, + task, event='deploy') + + def test_update_fails_on_invalid_boot_mode(self): + # NOTE(cid): This test might need updating if boot modes' naming + # convention changes + self.assertRaises(ValueError, + self.config, + disallowed_deployment_boot_modes=['BIOS'], + group='conductor') + self.assertRaises(ValueError, + self.config, + disallowed_deployment_boot_modes=['Bios'], + group='conductor') + self.assertRaises(ValueError, + self.config, + disallowed_deployment_boot_modes=['UEFI'], + group='conductor') + self.assertRaises(ValueError, + self.config, + disallowed_deployment_boot_modes=['Uefi'], + group='conductor') + self.assertRaises(ValueError, + self.config, + disallowed_deployment_boot_modes=['blah'], + group='conductor') + @mock.patch.object(deployments, 'do_next_deploy_step', autospec=True) @mock.patch.object(conductor_steps, 'set_node_deployment_steps', autospec=True) diff --git a/releasenotes/notes/allow-disabling-bios-deployments-356f3215f80a879e.yaml b/releasenotes/notes/allow-disabling-bios-deployments-356f3215f80a879e.yaml new file mode 100644 index 0000000000..25e36c5d3f --- /dev/null +++ b/releasenotes/notes/allow-disabling-bios-deployments-356f3215f80a879e.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds configuration options for operators to specify any or what boot modes + to disallow for enrollment (`disallowed_enrollment_boot_modes`) and/or + deployment (`disallowed_deployment_boot_modes`). Defaults are empty lists, + indicating all boot modes are allowed.