Merge "Add validation of 'ilo_deploy_iso' in deploy.validate()" into stable/mitaka

3 files changed, 157 insertions, 0 deletions

diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py
index 2da4f41e6..4f9f3681d 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 dee085138..c1fb7d0ad 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 000000000..371d9eb31
--- /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.