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

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
 

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:

ironic/tests/unit/common/test_image_service.py

@@ -67,6 +67,17 @@ class HttpImageServiceTestCase(base.TestCase):
                           self.service.validate_href, self.href)
         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

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)

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.