diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index 6016ac11e9..d54afa8b42 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 40f8d56d93..9d7679fc85 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 57ac3be832..fb81604218 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/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 diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 0aa1763b51..d0f49da62e 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 0000000000..950950cc3d --- /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.