diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index e9d24f7800..ae9d2b588a 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -178,7 +178,10 @@ class HttpImageService(BaseImageService): response = requests.head(image_href, verify=verify, timeout=CONF.webserver_connection_timeout, auth=auth) - if response.status_code == http_client.MOVED_PERMANENTLY: + if (response.status_code == http_client.MOVED_PERMANENTLY + or response.status_code == http_client.FOUND + or response.status_code == http_client.TEMPORARY_REDIRECT + or response.status_code == http_client.PERMANENT_REDIRECT): # NOTE(TheJulia): In the event we receive a redirect, we need # to notify the caller. Before this we would just fail, # but a url which is missing a trailing slash results in a diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index 6304933d31..9340237407 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -245,7 +245,7 @@ class HttpImageServiceTestCase(base.TestCase): self.assertEqual(http_client.FORBIDDEN, resp.status_code) @mock.patch.object(requests, 'head', autospec=True) - def test_validate_href_path_redirected(self, head_mock): + def test_validate_href_path_moved_permanently(self, head_mock): cfg.CONF.set_override('webserver_verify_ca', 'True') response = head_mock.return_value @@ -260,6 +260,54 @@ class HttpImageServiceTestCase(base.TestCase): head_mock.assert_called_once_with(url, verify=True, timeout=60, auth=None) + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_found(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.FOUND + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_temporary_redirect(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.TEMPORARY_REDIRECT + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_permanent_redirect(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.PERMANENT_REDIRECT + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + def test_verify_basic_auth_cred_format(self): self.assertIsNone(self .service diff --git a/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml b/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml new file mode 100644 index 0000000000..56efcb961e --- /dev/null +++ b/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed HttpImageService.validate_href() ImageRefValidationFailed exception + if protocol is HTTP/HTTPS and the HTTP header response is a redirection + other then 301 (MOVED_PERMANENTLY). HTTP/HTTPS protocol is often used + under standalone Ironic configuration to identify an image source (e.g + --instance-info image_source=). The HTTP server may use redirection + to load balance or geographically distribute the requests, or simply point + to the correct URL. The redirection may vary from 301 (MOVED_PERMANENTLY), + to 302 (FOUND), or 307 (TEMPORARY_REDIRECT), and 308 (PERMANENT_REDIRECT).