From 4103c0ffbbd44b33dc83fbe080d332d57be75f50 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Fri, 17 Apr 2015 02:51:31 -0700 Subject: [PATCH] Validate capability in properties and instance_info Validate the value of a capability in node/properties and node/instance_info Closes-Bug: 1441114 Change-Id: Ibc8acdc1fded2478f558a427fd76fc46fc06b92e --- ironic/drivers/modules/deploy_utils.py | 45 +++++++++++++++++ ironic/drivers/modules/ilo/deploy.py | 8 +-- ironic/drivers/modules/pxe.py | 6 +-- ironic/drivers/utils.py | 54 -------------------- ironic/tests/drivers/ilo/test_deploy.py | 20 +++----- ironic/tests/drivers/test_deploy_utils.py | 36 +++++++++++++ ironic/tests/drivers/test_utils.py | 61 ----------------------- 7 files changed, 91 insertions(+), 139 deletions(-) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 8e6470233e..46f6c9519c 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -72,6 +72,10 @@ LOG = logging.getLogger(__name__) VALID_ROOT_DEVICE_HINTS = set(('size', 'model', 'wwn', 'serial', 'vendor')) +SUPPORTED_CAPABILITIES = {'boot_option': ('local', 'netboot'), + 'boot_mode': ('bios', 'uefi'), + 'secure_boot': ('true', 'false')} + # All functions are called from deploy() directly or indirectly. # They are split for stub-out. @@ -1077,3 +1081,44 @@ def get_boot_mode_for_deploy(node): {'boot_mode': boot_mode, 'node': node.uuid}) return boot_mode.lower() if boot_mode else boot_mode + + +def validate_capabilities(node): + """Validates that specified supported capabilities have valid value + + This method checks if the any of the supported capability is present in + Node capabilities. For all supported capabilities specified for a Node, + it validates that it has a valid value. + The node can have capability as part of the 'properties' or + 'instance_info' or both. + Note that the actual value of a capability does not need to be the same + in the node's 'properties' and 'instance_info'. + + :param node: an ironic node object. + :raises: InvalidParameterValue, if the capability is not set to a + valid value. + """ + exp_str = _("The parameter '%(capability)s' from %(field)s has an " + "invalid value: '%(value)s'. Acceptable values are: " + "%(valid_values)s.") + + for capability_name, valid_values in SUPPORTED_CAPABILITIES.items(): + # Validate capability_name in node's properties/capabilities + value = driver_utils.get_node_capability(node, capability_name) + if value and (value not in valid_values): + field = "properties/capabilities" + raise exception.InvalidParameterValue( + exp_str % + {'capability': capability_name, 'field': field, + 'value': value, 'valid_values': ', '.join(valid_values)}) + + # Validate capability_name in node's instance_info/['capabilities'] + capabilities = parse_instance_info_capabilities(node) + value = capabilities.get(capability_name) + + if value and (value not in valid_values): + field = "instance_info['capabilities']" + raise exception.InvalidParameterValue( + exp_str % + {'capability': capability_name, 'field': field, + 'value': value, 'valid_values': ', '.join(valid_values)}) diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index f7f221894e..153fdf7568 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -43,7 +43,6 @@ from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules import ipmitool from ironic.drivers.modules import iscsi_deploy from ironic.drivers.modules import pxe -from ironic.drivers import utils as driver_utils LOG = logging.getLogger(__name__) @@ -392,9 +391,7 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface): else: props = ['kernel', 'ramdisk'] iscsi_deploy.validate_image_properties(task.context, d_info, props) - driver_utils.validate_boot_mode_capability(node) - driver_utils.validate_boot_option_capability(node) - driver_utils.validate_secure_boot_capability(node) + deploy_utils.validate_capabilities(node) @task_manager.require_exclusive_lock def deploy(self, task): @@ -490,8 +487,7 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface): :raises: MissingParameterValue if some parameters are missing. """ - driver_utils.validate_boot_mode_capability(task.node) - driver_utils.validate_secure_boot_capability(task.node) + deploy_utils.validate_capabilities(task.node) _parse_driver_info(task.node) @task_manager.require_exclusive_lock diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 92d73acfe4..135f0d54d6 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -43,7 +43,6 @@ from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_cache from ironic.drivers.modules import iscsi_deploy -from ironic.drivers import utils as driver_utils from ironic.openstack.common import fileutils @@ -303,8 +302,7 @@ class PXEDeploy(base.DeployInterface): node = task.node # Check the boot_mode and boot_option capabilities values. - driver_utils.validate_boot_mode_capability(node) - driver_utils.validate_boot_option_capability(node) + deploy_utils.validate_capabilities(node) boot_mode = deploy_utils.get_boot_mode_for_deploy(task.node) @@ -503,7 +501,7 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): :raises: InvalidParameterValue if any parameters is invalid. """ if method == 'pass_deploy_info': - driver_utils.validate_boot_option_capability(task.node) + deploy_utils.validate_capabilities(task.node) iscsi_deploy.get_deploy_info(task.node, **kwargs) elif method == 'pass_bootloader_install_info': iscsi_deploy.validate_pass_bootloader_info_input(task, kwargs) diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py index 441dd31485..cc9ccb2b75 100644 --- a/ironic/drivers/utils.py +++ b/ironic/drivers/utils.py @@ -170,57 +170,3 @@ def add_node_capability(task, capability, value): properties['capabilities'] = capabilities node.properties = properties node.save() - - -def validate_capability(node, capability_name, valid_values): - """Validate a capabability set in node property - - :param node: an ironic node object. - :param capability_name: the name of the capability. - :parameter valid_values: an iterable with valid values expected for - that capability. - :raises: InvalidParameterValue, if the capability is not set to the - expected values. - """ - value = get_node_capability(node, capability_name) - - if value and value not in valid_values: - valid_value_str = ', '.join(valid_values) - raise exception.InvalidParameterValue( - _("Invalid %(capability)s parameter '%(value)s'. " - "Acceptable values are: %(valid_values)s.") % - {'capability': capability_name, 'value': value, - 'valid_values': valid_value_str}) - - -def validate_boot_mode_capability(node): - """Validate the boot_mode capability set in node properties. - - :param node: an ironic node object. - :raises: InvalidParameterValue, if 'boot_mode' capability is set - other than 'bios' or 'uefi' or None. - - """ - validate_capability(node, 'boot_mode', ('bios', 'uefi')) - - -def validate_boot_option_capability(node): - """Validate the boot_option capability set in node properties. - - :param node: an ironic node object. - :raises: InvalidParameterValue, if 'boot_option' capability is set - other than 'local' or 'netboot' or None. - - """ - validate_capability(node, 'boot_option', ('local', 'netboot')) - - -def validate_secure_boot_capability(node): - """Validate the secure_boot capability set in node property. - - :param node: an ironic node object. - :raises: InvalidParameterValue, if 'secure_boot' capability is set - other than 'true' or 'false' or None. - - """ - validate_capability(node, 'secure_boot', ('true', 'false')) diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index af0f839f59..496b3b14c7 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -474,9 +474,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='iscsi_ilo', driver_info=INFO_DICT) - @mock.patch.object(driver_utils, 'validate_secure_boot_capability', - spec_set=True, autospec=True) - @mock.patch.object(driver_utils, 'validate_boot_mode_capability', + @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, autospec=True) @mock.patch.object(iscsi_deploy, 'validate_image_properties', spec_set=True, autospec=True) @@ -486,8 +484,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): def _test_validate(self, validate_mock, deploy_info_mock, validate_prop_mock, - validate_boot_mode_mock, - validate_secure_boot_mock, + validate_capability_mock, props_expected): d_info = {'image_source': 'uuid'} deploy_info_mock.return_value = d_info @@ -498,8 +495,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): deploy_info_mock.assert_called_once_with(task.node) validate_prop_mock.assert_called_once_with( task.context, d_info, props_expected) - validate_boot_mode_mock.assert_called_once_with(task.node) - validate_secure_boot_mock.assert_called_once_with(task.node) + validate_capability_mock.assert_called_once_with(task.node) @mock.patch.object(iscsi_deploy, 'validate_image_properties', spec_set=True, autospec=True) @@ -675,22 +671,18 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='agent_ilo', driver_info=INFO_DICT) - @mock.patch.object(driver_utils, 'validate_secure_boot_capability', - spec_set=True, autospec=True) - @mock.patch.object(driver_utils, 'validate_boot_mode_capability', + @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, autospec=True) @mock.patch.object(ilo_deploy, '_parse_driver_info', spec_set=True, autospec=True) def test_validate(self, parse_driver_info_mock, - validate_boot_mode_mock, - validate_secure_boot_mock): + validate_capability_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.deploy.validate(task) parse_driver_info_mock.assert_called_once_with(task.node) - validate_boot_mode_mock.assert_called_once_with(task.node) - validate_secure_boot_mock.assert_called_once_with(task.node) + validate_capability_mock.assert_called_once_with(task.node) @mock.patch.object(ilo_deploy, '_prepare_agent_vmedia_boot', spec_set=True, autospec=True) diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py index 885fd4e1fd..1b925e31cb 100644 --- a/ironic/tests/drivers/test_deploy_utils.py +++ b/ironic/tests/drivers/test_deploy_utils.py @@ -1525,6 +1525,42 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase): result = utils.get_boot_mode_for_deploy(self.node) self.assertEqual('bios', result) + def test_validate_boot_mode_capability(self): + prop = {'capabilities': 'boot_mode:uefi,cap2:value2'} + self.node.properties = prop + + result = utils.validate_capabilities(self.node) + self.assertIsNone(result) + + def test_validate_boot_mode_capability_with_exc(self): + prop = {'capabilities': 'boot_mode:UEFI,cap2:value2'} + self.node.properties = prop + + self.assertRaises(exception.InvalidParameterValue, + utils.validate_capabilities, self.node) + + def test_validate_boot_mode_capability_instance_info(self): + inst_info = {'capabilities': {"boot_mode": "uefi", "cap2": "value2"}} + self.node.instance_info = inst_info + + result = utils.validate_capabilities(self.node) + self.assertIsNone(result) + + def test_validate_boot_mode_capability_instance_info_with_exc(self): + inst_info = {'capabilities': {"boot_mode": "UEFI", "cap2": "value2"}} + self.node.instance_info = inst_info + + self.assertRaises(exception.InvalidParameterValue, + utils.validate_capabilities, self.node) + + def test_all_supported_capabilities(self): + self.assertEqual(('local', 'netboot'), + utils.SUPPORTED_CAPABILITIES['boot_option']) + self.assertEqual(('bios', 'uefi'), + utils.SUPPORTED_CAPABILITIES['boot_mode']) + self.assertEqual(('true', 'false'), + utils.SUPPORTED_CAPABILITIES['secure_boot']) + class TrySetBootDeviceTestCase(db_base.DbTestCase): diff --git a/ironic/tests/drivers/test_utils.py b/ironic/tests/drivers/test_utils.py index 3513825e86..b540fd6908 100644 --- a/ironic/tests/drivers/test_utils.py +++ b/ironic/tests/drivers/test_utils.py @@ -114,64 +114,3 @@ class UtilsTestCase(db_base.DbTestCase): driver_utils.add_node_capability(task, 'a', 'b') self.assertEqual('a:b,c:d,a:b', task.node.properties['capabilities']) - - def test_validate_capability(self): - properties = {'capabilities': 'cat:meow,cap2:value2'} - self.node.properties = properties - - result = driver_utils.validate_capability( - self.node, 'cat', ['meow', 'purr']) - self.assertIsNone(result) - - def test_validate_capability_with_exception(self): - properties = {'capabilities': 'cat:bark,cap2:value2'} - self.node.properties = properties - - self.assertRaises(exception.InvalidParameterValue, - driver_utils.validate_capability, - self.node, 'cat', ['meow', 'purr']) - - def test_validate_boot_mode_capability(self): - properties = {'capabilities': 'boot_mode:uefi,cap2:value2'} - self.node.properties = properties - - result = driver_utils.validate_boot_mode_capability(self.node) - self.assertIsNone(result) - - def test_validate_boot_mode_capability_with_exception(self): - properties = {'capabilities': 'boot_mode:foo,cap2:value2'} - self.node.properties = properties - - self.assertRaises(exception.InvalidParameterValue, - driver_utils.validate_boot_mode_capability, - self.node) - - def test_validate_boot_option_capability(self): - properties = {'capabilities': 'boot_option:netboot,cap2:value2'} - self.node.properties = properties - - result = driver_utils.validate_boot_option_capability(self.node) - self.assertIsNone(result) - - def test_validate_boot_option_capability_with_exception(self): - properties = {'capabilities': 'boot_option:foo,cap2:value2'} - self.node.properties = properties - - self.assertRaises(exception.InvalidParameterValue, - driver_utils.validate_boot_option_capability, - self.node) - - def test_validate_secure_boot_capability(self): - properties = {'capabilities': 'secure_boot:true,cap2:value2'} - self.node.properties = properties - - result = driver_utils.validate_secure_boot_capability(self.node) - self.assertIsNone(result) - - def test_validate_secure_boot_capability_with_exception(self): - properties = {'capabilities': 'secure_boot:foo,cap2:value2'} - self.node.properties = properties - - self.assertRaises(exception.InvalidParameterValue, - driver_utils.validate_secure_boot_capability, - self.node)