Validate the generated swift temp url

During deployment, a swift temporary URL may be generated,
to be used for getting the image for an instance.

This validates the generated URL to make sure that it
can be accessed.
If validation fails, an exception is raised and the error
is logged. This is helpful when diagnosing the problem with
a deployment.

Change-Id: I1f62bc9b5646d173ec6a3beb6282a5e6002c2030
Closes-Bug: 1512186

5 files changed, 58 insertions, 19 deletions

diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py
index 6016ac11e..d54afa8b4 100644
--- a/ironic/common/image_service.py
+++ b/ironic/common/image_service.py
@@ -100,23 +100,26 @@ class BaseImageService(object):
 class HttpImageService(BaseImageService):
     """Provides retrieval of disk images using HTTP."""
 
-    def validate_href(self, image_href):
+    def validate_href(self, image_href, secret=False):
         """Validate HTTP image reference.
 
         :param image_href: Image reference.
+        :param secret: Specify if image_href being validated should not be
+            shown in exception message.
         :raises: exception.ImageRefValidationFailed if HEAD request failed or
             returned response code not equal to 200.
         :returns: Response to HEAD request.
         """
+        output_url = 'secreturl' if secret else image_href
         try:
             response = requests.head(image_href)
             if response.status_code != http_client.OK:
                 raise exception.ImageRefValidationFailed(
-                    image_href=image_href,
+                    image_href=output_url,
                     reason=_("Got HTTP code %s instead of 200 in response to "
                              "HEAD request.") % response.status_code)
         except requests.RequestException as e:
-            raise exception.ImageRefValidationFailed(image_href=image_href,
+            raise exception.ImageRefValidationFailed(image_href=output_url,
                                                      reason=e)
         return response
 
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 40f8d56d9..9d7679fc8 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1272,6 +1272,23 @@ def build_instance_info_for_deploy(task):
     :raises: exception.ImageRefValidationFailed if image_source is not
         Glance href and is not HTTP(S) URL.
     """
+    def validate_image_url(url, secret=False):
+        """Validates image URL through the HEAD request.
+
+        :param url: URL to be validated
+        :param secret: if URL is secret (e.g. swift temp url),
+            it will not be shown in logs.
+        """
+        try:
+            image_service.HttpImageService().validate_href(url, secret)
+        except exception.ImageRefValidationFailed as e:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE("Agent deploy supports only HTTP(S) URLs as "
+                              "instance_info['image_source'] or swift "
+                              "temporary URL. Either the specified URL is not "
+                              "a valid HTTP(S) URL or is not reachable "
+                              "for node %(node)s. Error: %(msg)s"),
+                          {'node': node.uuid, 'msg': e})
     node = task.node
     instance_info = node.instance_info
     iwdi = node.driver_internal_info.get('is_whole_disk_image')
@@ -1280,9 +1297,10 @@ def build_instance_info_for_deploy(task):
         glance = image_service.GlanceImageService(version=2,
                                                   context=task.context)
         image_info = glance.show(image_source)
-        swift_temp_url = glance.swift_temp_url(image_info)
         LOG.debug('Got image info: %(info)s for node %(node)s.',
                   {'info': image_info, 'node': node.uuid})
+        swift_temp_url = glance.swift_temp_url(image_info)
+        validate_image_url(swift_temp_url, secret=True)
         instance_info['image_url'] = swift_temp_url
         instance_info['image_checksum'] = image_info['checksum']
         instance_info['image_disk_format'] = image_info['disk_format']
@@ -1295,14 +1313,7 @@ def build_instance_info_for_deploy(task):
             instance_info['kernel'] = image_info['properties']['kernel_id']
             instance_info['ramdisk'] = image_info['properties']['ramdisk_id']
     else:
-        try:
-            image_service.HttpImageService().validate_href(image_source)
-        except exception.ImageRefValidationFailed:
-            with excutils.save_and_reraise_exception():
-                LOG.error(_LE("Agent deploy supports only HTTP(S) URLs as "
-                              "instance_info['image_source']. Either %s "
-                              "is not a valid HTTP(S) URL or "
-                              "is not reachable."), image_source)
+        validate_image_url(image_source)
         instance_info['image_url'] = image_source
 
     if not iwdi:
diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py
index 57ac3be83..fb8160421 100644
--- a/ironic/tests/unit/common/test_image_service.py
+++ b/ironic/tests/unit/common/test_image_service.py
@@ -68,6 +68,17 @@ class HttpImageServiceTestCase(base.TestCase):
         head_mock.assert_called_once_with(self.href)
 
     @mock.patch.object(requests, 'head', autospec=True)
+    def test_validate_href_error_with_secret_parameter(self, head_mock):
+        head_mock.return_value.status_code = 204
+        e = self.assertRaises(exception.ImageRefValidationFailed,
+                              self.service.validate_href,
+                              self.href,
+                              True)
+        self.assertIn('secreturl', six.text_type(e))
+        self.assertNotIn(self.href, six.text_type(e))
+        head_mock.assert_called_once_with(self.href)
+
+    @mock.patch.object(requests, 'head', autospec=True)
     def _test_show(self, head_mock, mtime, mtime_date):
         head_mock.return_value.status_code = http_client.OK
         head_mock.return_value.headers = {
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index 0aa1763b5..d0f49da62 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -2617,8 +2617,11 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(self.context,
                                                driver='fake_agent')
 
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
     @mock.patch.object(image_service, 'GlanceImageService', autospec=True)
-    def test_build_instance_info_for_deploy_glance_image(self, glance_mock):
+    def test_build_instance_info_for_deploy_glance_image(self, glance_mock,
+                                                         validate_mock):
         i_info = self.node.instance_info
         i_info['image_source'] = '733d1c44-a2ea-414b-aca7-69decf20d810'
         driver_internal_info = self.node.driver_internal_info
@@ -2631,7 +2634,8 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
                       'container_format': 'bare', 'properties': {}}
         glance_mock.return_value.show = mock.MagicMock(spec_set=[],
                                                        return_value=image_info)
-
+        glance_mock.return_value.swift_temp_url.return_value = (
+            'http://temp-url')
         mgr_utils.mock_the_extension_manager(driver='fake_agent')
         with task_manager.acquire(
                 self.context, self.node.uuid, shared=False) as task:
@@ -2644,11 +2648,15 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
                 self.node.instance_info['image_source'])
             glance_mock.return_value.swift_temp_url.assert_called_once_with(
                 image_info)
+            validate_mock.assert_called_once_with(mock.ANY, 'http://temp-url',
+                                                  secret=True)
 
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
     @mock.patch.object(utils, 'parse_instance_info', autospec=True)
     @mock.patch.object(image_service, 'GlanceImageService', autospec=True)
     def test_build_instance_info_for_deploy_glance_partition_image(
-            self, glance_mock, parse_instance_info_mock):
+            self, glance_mock, parse_instance_info_mock, validate_mock):
         i_info = {}
         i_info['image_source'] = '733d1c44-a2ea-414b-aca7-69decf20d810'
         i_info['kernel'] = '13ce5a56-1de3-4916-b8b2-be778645d003'
@@ -2671,7 +2679,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
         glance_mock.return_value.show = mock.MagicMock(spec_set=[],
                                                        return_value=image_info)
         glance_obj_mock = glance_mock.return_value
-        glance_obj_mock.swift_temp_url.return_value = 'temp-url'
+        glance_obj_mock.swift_temp_url.return_value = 'http://temp-url'
         parse_instance_info_mock.return_value = {'swap_mb': 4}
         image_source = '733d1c44-a2ea-414b-aca7-69decf20d810'
         expected_i_info = {'root_gb': 5,
@@ -2680,7 +2688,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
                            'ephemeral_format': None,
                            'configdrive': 'configdrive',
                            'image_source': image_source,
-                           'image_url': 'temp-url',
+                           'image_url': 'http://temp-url',
                            'kernel': 'kernel',
                            'ramdisk': 'ramdisk',
                            'image_type': 'partition',
@@ -2702,6 +2710,8 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
                 self.node.instance_info['image_source'])
             glance_mock.return_value.swift_temp_url.assert_called_once_with(
                 image_info)
+            validate_mock.assert_called_once_with(
+                mock.ANY, 'http://temp-url', secret=True)
             image_type = task.node.instance_info['image_type']
             self.assertEqual('partition', image_type)
             self.assertEqual('kernel', info['kernel'])
@@ -2733,7 +2743,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
             self.assertEqual(self.node.instance_info['image_source'],
                              info['image_url'])
             validate_href_mock.assert_called_once_with(
-                mock.ANY, 'http://image-ref')
+                mock.ANY, 'http://image-ref', False)
 
     @mock.patch.object(utils, 'parse_instance_info', autospec=True)
     @mock.patch.object(image_service.HttpImageService, 'validate_href',
@@ -2775,7 +2785,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
             self.assertEqual(self.node.instance_info['image_source'],
                              info['image_url'])
             validate_href_mock.assert_called_once_with(
-                mock.ANY, 'http://image-ref')
+                mock.ANY, 'http://image-ref', False)
             self.assertEqual('partition', info['image_type'])
             self.assertEqual(expected_i_info, info)
             parse_instance_info_mock.assert_called_once_with(task.node)
diff --git a/releasenotes/notes/validate-image-url-wnen-deploying-8820f4398ea9de9f.yaml b/releasenotes/notes/validate-image-url-wnen-deploying-8820f4398ea9de9f.yaml
new file mode 100644
index 000000000..950950cc3
--- /dev/null
+++ b/releasenotes/notes/validate-image-url-wnen-deploying-8820f4398ea9de9f.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - Ironic now validates any swift temporary URL
+    when preparing for deployment of nodes.