Merge "Validate the generated swift temp url"

ironic/common/image_service.py

@@ -95,23 +95,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

@@ -1160,6 +1160,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')
@@ -1168,9 +1185,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']
@@ -1183,14 +1201,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:
+        validate_image_url(image_source)
-            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)
         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

@@ -2115,8 +2115,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
@@ -2129,7 +2132,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:
@@ -2142,11 +2146,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'
@@ -2169,7 +2177,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,
@@ -2178,7 +2186,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',
@@ -2200,6 +2208,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'])
@@ -2231,7 +2241,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',
@@ -2273,7 +2283,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.