From e02e905f4bc01aad675f186421b69cb0979cf0a9 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Mon, 27 Jun 2016 21:32:57 -0700 Subject: [PATCH] Add validation of 'ilo_deploy_iso' in deploy.validate() iLO virtual media based drivers use ISO image supplied as 'ilo_deploy_iso' in node's 'driver_info' to boot node during provisioning process. This parameter was not getting validated during deploy.validate() call. This change fixes the issue. Change-Id: I8fc4a89621e09281349f88c32ed77d24aa11355b Closes-Bug: #1592335 (cherry picked from commit 2eab1ee09ac525e086b59c43c2581b9087b5a257) --- ironic/drivers/modules/ilo/deploy.py | 58 ++++++++++++ .../unit/drivers/modules/ilo/test_deploy.py | 94 +++++++++++++++++++ .../notes/bug-1592335-7c5835868fe364ea.yaml | 5 + 3 files changed, 157 insertions(+) create mode 100644 releasenotes/notes/bug-1592335-7c5835868fe364ea.yaml diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 2da4f41e67..4f9f3681dd 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -20,8 +20,10 @@ from oslo_log import log as logging from ironic.common import boot_devices from ironic.common import exception +from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common.i18n import _LW +from ironic.common import image_service from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils @@ -143,11 +145,53 @@ def _disable_secure_boot_if_supported(task): task.node.uuid) +def _validate(task): + """Validate the prerequisites for virtual media based deploy. + + This method validates whether the 'driver_info' property of the + supplied node contains the required information for this driver. + + :param task: a TaskManager instance containing the node to act on. + :raises: InvalidParameterValue if any parameters are incorrect + :raises: MissingParameterValue if some mandatory information + is missing on the node + """ + node = task.node + ilo_common.parse_driver_info(node) + if 'ilo_deploy_iso' not in node.driver_info: + raise exception.MissingParameterValue(_( + "Missing 'ilo_deploy_iso' parameter in node's 'driver_info'.")) + deploy_iso = node.driver_info['ilo_deploy_iso'] + if not service_utils.is_glance_image(deploy_iso): + try: + image_service.HttpImageService().validate_href(deploy_iso) + except exception.ImageRefValidationFailed: + raise exception.InvalidParameterValue(_( + "Virtual media deploy accepts only Glance images or " + "HTTP(S) as driver_info['ilo_deploy_iso']. Either '%s' " + "is not a glance UUID or not a valid HTTP(S) URL or " + "the given URL is not reachable.") % deploy_iso) + + class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy): def get_properties(self): return {} + def validate(self, task): + """Validate the prerequisites for virtual media based deploy. + + This method validates whether the 'driver_info' property of the + supplied node contains the required information for this driver. + + :param task: a TaskManager instance containing the node to act on. + :raises: InvalidParameterValue if any parameters are incorrect + :raises: MissingParameterValue if some mandatory information + is missing on the node + """ + _validate(task) + super(IloVirtualMediaIscsiDeploy, self).validate(task) + @task_manager.require_exclusive_lock def tear_down(self, task): """Tear down a previous deployment on the task's node. @@ -200,6 +244,20 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy): """ return ilo_boot.COMMON_PROPERTIES + def validate(self, task): + """Validate the prerequisites for virtual media based deploy. + + This method validates whether the 'driver_info' property of the + supplied node contains the required information for this driver. + + :param task: a TaskManager instance containing the node to act on. + :raises: InvalidParameterValue if any parameters are incorrect + :raises: MissingParameterValue if some mandatory information + is missing on the node + """ + _validate(task) + super(IloVirtualMediaAgentDeploy, self).validate(task) + @task_manager.require_exclusive_lock def tear_down(self, task): """Tear down a previous deployment on the task's node. diff --git a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py index dee0851388..c1fb7d0ad1 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py @@ -21,6 +21,8 @@ import six from ironic.common import boot_devices from ironic.common import exception +from ironic.common.glance_service import service_utils +from ironic.common import image_service from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils @@ -192,6 +194,72 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): deploy_boot_mode = task.node.instance_info.get('deploy_boot_mode') self.assertIsNone(deploy_boot_mode) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_MissingParam(self, mock_parse_driver_info): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaisesRegex(exception.MissingParameterValue, + "Missing 'ilo_deploy_iso'", + ilo_deploy._validate, task) + mock_parse_driver_info.assert_called_once_with(task.node) + + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_valid_uuid(self, mock_parse_driver_info, + mock_is_glance_image): + mock_is_glance_image.return_value = True + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + deploy_iso = '8a81759a-f29b-454b-8ab3-161c6ca1882c' + task.node.driver_info['ilo_deploy_iso'] = deploy_iso + ilo_deploy._validate(task) + mock_parse_driver_info.assert_called_once_with(task.node) + mock_is_glance_image.assert_called_once_with(deploy_iso) + + @mock.patch.object(image_service.HttpImageService, 'validate_href', + spec_set=True, autospec=True) + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_InvalidParam(self, mock_parse_driver_info, + mock_is_glance_image, + mock_validate_href): + deploy_iso = 'http://abc.org/image/qcow2' + mock_validate_href.side_effect = iter( + [exception.ImageRefValidationFailed( + image_href='http://abc.org/image/qcow2', reason='fail')]) + mock_is_glance_image.return_value = False + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.driver_info['ilo_deploy_iso'] = deploy_iso + self.assertRaisesRegex(exception.InvalidParameterValue, + "Virtual media deploy accepts", + ilo_deploy._validate, task) + mock_parse_driver_info.assert_called_once_with(task.node) + mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) + + @mock.patch.object(image_service.HttpImageService, 'validate_href', + spec_set=True, autospec=True) + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_valid_url(self, mock_parse_driver_info, + mock_is_glance_image, + mock_validate_href): + deploy_iso = 'http://abc.org/image/deploy.iso' + mock_is_glance_image.return_value = False + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.driver_info['ilo_deploy_iso'] = deploy_iso + ilo_deploy._validate(task) + mock_parse_driver_info.assert_called_once_with(task.node) + mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) + class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): @@ -201,6 +269,19 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='iscsi_ilo', driver_info=INFO_DICT) + @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'validate', spec_set=True, + autospec=True) + @mock.patch.object(ilo_deploy, '_validate', spec_set=True, + autospec=True) + def test_validate(self, + mock_validate, + iscsi_validate): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.deploy.validate(task) + mock_validate.assert_called_once_with(task) + iscsi_validate.assert_called_once_with(mock.ANY, task) + @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, autospec=True) @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'tear_down', spec_set=True, @@ -307,6 +388,19 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='agent_ilo', driver_info=INFO_DICT) + @mock.patch.object(agent.AgentDeploy, 'validate', spec_set=True, + autospec=True) + @mock.patch.object(ilo_deploy, '_validate', spec_set=True, + autospec=True) + def test_validate(self, + mock_validate, + agent_validate): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.deploy.validate(task) + mock_validate.assert_called_once_with(task) + agent_validate.assert_called_once_with(mock.ANY, task) + @mock.patch.object(agent.AgentDeploy, 'tear_down', spec_set=True, autospec=True) @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, diff --git a/releasenotes/notes/bug-1592335-7c5835868fe364ea.yaml b/releasenotes/notes/bug-1592335-7c5835868fe364ea.yaml new file mode 100644 index 0000000000..371d9eb317 --- /dev/null +++ b/releasenotes/notes/bug-1592335-7c5835868fe364ea.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - A node using 'agent_ilo' or 'iscsi_ilo' driver has + their 'driver_info/ilo_deploy_iso' field validated + during node validate. This closes bug #1592335.