Merge "Allow multiple HTTP redirects for image source"

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

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

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=<URI>). 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).