mirror of
https://opendev.org/openstack/ironic.git
synced 2026-01-16 23:01:47 +00:00
Remove bespoke logic for handling redirects while validating URLs
This logic is broken in a few places when dealing with real world redirect cases, such as Debian Cloud images redirecting to mirrors. It seems that the code was written with an incorrect assumption that requests does not limit redirects by default. It does, the default max_redirects seems to be 30. We can change it on Session if we need. The anaconda case is now handled by checking the url field of the response. Closes-Bug: #2127154 Change-Id: I200d631e166075ceab80dcd4b0ff596d1860aa3b Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>
This commit is contained in:
Dmitry Tantsur
parent
b8cc6447c1
commit
5482045e88
8 changed files with 102 additions and 231 deletions
|
|
@ -953,21 +953,6 @@ class NodeVerifyFailure(IronicException):
|
|||
_msg_fmt = _("Failed to verify node %(node)s: %(reason)s")
|
||||
|
||||
|
||||
class ImageRefIsARedirect(IronicException):
|
||||
_msg_fmt = _("Received a URL redirect when attempting to evaluate "
|
||||
"image reference %(image_ref)s pointing to "
|
||||
"%(redirect_url)s. This may, or may not be recoverable.")
|
||||
redirect_url = None
|
||||
|
||||
def __init__(self, image_ref=None, redirect_url=None, msg=None):
|
||||
self.redirect_url = redirect_url
|
||||
# Kwargs are expected by IronicException to convert the message.
|
||||
super(ImageRefIsARedirect, self).__init__(
|
||||
message=msg,
|
||||
image_ref=image_ref,
|
||||
redirect_url=redirect_url)
|
||||
|
||||
|
||||
class ConcurrentActionLimit(TemporaryFailure):
|
||||
# NOTE(TheJulia): We explicitly don't report the concurrent
|
||||
# action limit configuration value as a security guard since
|
||||
|
|
|
|||
|
|
@ -153,9 +153,6 @@ class HttpImageService(BaseImageService):
|
|||
shown in exception message.
|
||||
:raises: exception.ImageRefValidationFailed if HEAD request failed or
|
||||
returned response code not equal to 200.
|
||||
:raises: exception.ImageRefIsARedirect if the supplied URL is a
|
||||
redirect to a different URL. The caller may be able to handle
|
||||
this.
|
||||
:returns: Response to HEAD request.
|
||||
"""
|
||||
output_url = 'secreturl' if secret else image_href
|
||||
|
|
@ -170,32 +167,9 @@ class HttpImageService(BaseImageService):
|
|||
auth = HttpImageService.gen_auth_from_conf_user_pass(image_href)
|
||||
# NOTE(TheJulia): Head requests do not work on things that are not
|
||||
# files, but they can be responded with redirects or a 200 OK....
|
||||
# We don't want to permit endless redirects either, thus not
|
||||
# request an override to the requests default to try and resolve
|
||||
# redirects as otherwise we might end up with something like
|
||||
# HTTPForbidden or a list of files. Both should be okay to at
|
||||
# least know things are okay in a limited fashion.
|
||||
response = requests.head(image_href, verify=verify,
|
||||
timeout=CONF.webserver_connection_timeout,
|
||||
auth=auth)
|
||||
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
|
||||
# redirect to a target path, and the caller *may* actually
|
||||
# care about that.
|
||||
redirect = requests.Session().get_redirect_target(response)
|
||||
|
||||
# Extra guard because this is pointless if there is no
|
||||
# location in the field. Requests also properly formats
|
||||
# our string for us, or gives us None.
|
||||
if redirect:
|
||||
raise exception.ImageRefIsARedirect(
|
||||
image_ref=image_href,
|
||||
redirect_url=redirect)
|
||||
auth=auth, allow_redirects=True)
|
||||
|
||||
if (response.status_code == http_client.FORBIDDEN
|
||||
and str(image_href).endswith('/')):
|
||||
|
|
|
|||
|
|
@ -790,24 +790,12 @@ def is_source_a_path(ctx, image_source):
|
|||
context=ctx)
|
||||
try:
|
||||
res = image_service.validate_href(image_source)
|
||||
if 'headers' in dir(res):
|
||||
# response/result is from the HTTP check path.
|
||||
headers = res.headers
|
||||
else:
|
||||
# We have no headers.
|
||||
headers = {}
|
||||
except exception.ImageRefIsARedirect as e:
|
||||
# Our exception handling formats this for us in this
|
||||
# case. \o/
|
||||
LOG.debug(str(e))
|
||||
# Servers redirect to a proper folder ending in a slash if
|
||||
# not supplied originally.
|
||||
if e.redirect_url and e.redirect_url.endswith('/'):
|
||||
return True
|
||||
headers = getattr(res, 'headers', {})
|
||||
except Exception:
|
||||
# NOTE(TheJulia): I don't really like this pattern, *but*
|
||||
# the wholedisk image support is similar.
|
||||
return
|
||||
|
||||
# NOTE(TheJulia): Files should have been caught almost exclusively
|
||||
# before with the Content-Length check.
|
||||
# When the ISO is mounted and the webserver mount point url is
|
||||
|
|
@ -833,7 +821,8 @@ def is_source_a_path(ctx, image_source):
|
|||
# NOTE(TheJulia): Files on a webserver have a length which is returned
|
||||
# when headres are queried.
|
||||
return False
|
||||
if image_source.endswith('/'):
|
||||
# In case of redirects, URL may be different from image_source.
|
||||
if res.url.endswith('/'):
|
||||
# If all else fails, looks like a URL, and the server didn't give
|
||||
# us any hints.
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -1210,7 +1210,10 @@ def _validate_image_url(node, url, secret=False, inspect_image=None,
|
|||
oci.set_image_auth(url, image_auth)
|
||||
oci.validate_href(url, secret)
|
||||
else:
|
||||
image_service.HttpImageService().validate_href(url, secret)
|
||||
resp = image_service.HttpImageService().validate_href(url, secret)
|
||||
if resp and resp.url != url:
|
||||
LOG.debug('URL %s redirected to %s', url, resp.url)
|
||||
image_info['image_source'] = resp.url
|
||||
except exception.ImageRefValidationFailed as e:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error("The specified URL is not a valid HTTP(S) URL or is "
|
||||
|
|
@ -1540,73 +1543,48 @@ def build_instance_info_for_deploy(task):
|
|||
# and not a file, or where a user may be supplying a full URL to
|
||||
# a remotely hosted image, we at a minimum need to check if the
|
||||
# url is valid, and address any redirects upfront.
|
||||
try:
|
||||
# NOTE(TheJulia): In the case we're here, we not doing an
|
||||
# integrated image based deploy, but we may also be doing
|
||||
# a path based anaconda base deploy, in which case we have
|
||||
# no backing image, but we need to check for a URL
|
||||
# redirection. So, if the source is a path (i.e. isap),
|
||||
# we don't need to inspect the image as there is no image
|
||||
# in the case for the deployment to drive.
|
||||
validated_results = {}
|
||||
if isap:
|
||||
# This is if the source is a path url, such as one used by
|
||||
# anaconda templates to to rely upon bootstrapping
|
||||
# defaults.
|
||||
_validate_image_url(node, image_source,
|
||||
inspect_image=False)
|
||||
else:
|
||||
# When not isap, we can just let _validate_image_url make
|
||||
# the required decision on if contents need to be sampled,
|
||||
# or not. We try to pass the image_disk_format which may
|
||||
# be declared by the user, and if not we set
|
||||
# expected_format to None.
|
||||
validate_results = _validate_image_url(
|
||||
node,
|
||||
image_source,
|
||||
expected_format=instance_info.get(
|
||||
'image_disk_format',
|
||||
None))
|
||||
|
||||
# NOTE(TheJulia): In the case we're here, we not doing an
|
||||
# integrated image based deploy, but we may also be doing
|
||||
# a path based anaconda base deploy, in which case we have
|
||||
# no backing image, but we need to check for a URL
|
||||
# redirection. So, if the source is a path (i.e. isap),
|
||||
# we don't need to inspect the image as there is no image
|
||||
# in the case for the deployment to drive.
|
||||
if isap:
|
||||
# This is if the source is a path url, such as one used by
|
||||
# anaconda templates to to rely upon bootstrapping
|
||||
# defaults.
|
||||
validate_results = _validate_image_url(node, image_source,
|
||||
inspect_image=False)
|
||||
else:
|
||||
# When not isap, we can just let _validate_image_url make
|
||||
# the required decision on if contents need to be sampled,
|
||||
# or not. We try to pass the image_disk_format which may
|
||||
# be declared by the user, and if not we set
|
||||
# expected_format to None.
|
||||
validate_results = _validate_image_url(
|
||||
node,
|
||||
image_source,
|
||||
expected_format=instance_info.get(
|
||||
'image_disk_format',
|
||||
None))
|
||||
# image_url is internal, and used by IPA and some boot
|
||||
# templates. In most cases, it needs to come from image_source
|
||||
# explicitly.
|
||||
if 'disk_format' in validated_results:
|
||||
if 'disk_format' in validate_results:
|
||||
# Ensure IPA has the value available, so write what we
|
||||
# detect, if anything. This is also an item which might be
|
||||
# needful with ansible deploy interface, when used in
|
||||
# standalone mode.
|
||||
instance_info['image_disk_format'] = \
|
||||
validate_results.get('disk_format')
|
||||
if not instance_info.get('image_url'):
|
||||
instance_info['image_url'] = image_source
|
||||
except exception.ImageRefIsARedirect as e:
|
||||
# At this point, we've got a redirect response from the
|
||||
# webserver, and we're going to try to handle it as a single
|
||||
# redirect action, as requests, by default, only lets a single
|
||||
# redirect to occur. This is likely a URL pathing fix, like a
|
||||
# trailing / on a path,
|
||||
# or move to HTTPS from a user supplied HTTP url.
|
||||
if e.redirect_url:
|
||||
# Since we've got a redirect, we need to carry the rest of
|
||||
# the request logic as well, which includes recording a
|
||||
# disk format, if applicable.
|
||||
instance_info['image_url'] = e.redirect_url
|
||||
# We need to save the image_source back out so it caches
|
||||
instance_info['image_source'] = e.redirect_url
|
||||
task.node.instance_info = instance_info
|
||||
if not isap:
|
||||
# The redirect doesn't relate to a path being used, so
|
||||
# the target is a filename, likely cause is webserver
|
||||
# telling the client to use HTTPS.
|
||||
validated_results = _validate_image_url(
|
||||
node, e.redirect_url,
|
||||
expected_format=instance_info.get(
|
||||
'image_disk_format', None))
|
||||
if 'disk_format' in validated_results:
|
||||
instance_info['image_disk_format'] = \
|
||||
validated_results.get('disk_format')
|
||||
else:
|
||||
raise
|
||||
redirect_target = validate_results.get('image_source')
|
||||
if redirect_target:
|
||||
instance_info['image_source'] = redirect_target
|
||||
instance_info['image_url'] = redirect_target
|
||||
if not instance_info.get('image_url'):
|
||||
instance_info['image_url'] = image_source
|
||||
|
||||
if not isap:
|
||||
if not iwdi:
|
||||
|
|
|
|||
|
|
@ -44,7 +44,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.service.validate_href(self.href)
|
||||
path_mock.assert_not_called()
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -62,7 +63,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
response.status_code = http_client.OK
|
||||
self.service.validate_href(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=False,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -79,7 +81,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=False,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
head_mock.side_effect = requests.RequestException()
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
|
|
@ -92,7 +95,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
response.status_code = http_client.OK
|
||||
self.service.validate_href(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -110,7 +114,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
head_mock.side_effect = requests.RequestException()
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
|
|
@ -124,7 +129,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
|
||||
self.service.validate_href(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify='/some/path',
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -150,7 +156,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
|
||||
self.service.validate_href(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify='/some/path',
|
||||
timeout=60, auth=auth_creds)
|
||||
timeout=60, auth=auth_creds,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -179,7 +186,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
response.status_code = http_client.OK
|
||||
self.service.validate_href(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=15, auth=None)
|
||||
timeout=15, auth=None,
|
||||
allow_redirects=True)
|
||||
response.status_code = http_client.NO_CONTENT
|
||||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href,
|
||||
|
|
@ -199,7 +207,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify='/some/path',
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
|
||||
@mock.patch.object(requests, 'head', autospec=True)
|
||||
def test_validate_href_verify_error(self, head_mock):
|
||||
|
|
@ -208,7 +217,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify='/some/path',
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
|
||||
@mock.patch.object(requests, 'head', autospec=True)
|
||||
def test_validate_href_verify_os_error(self, head_mock):
|
||||
|
|
@ -217,7 +227,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.validate_href, self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify='/some/path',
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
|
||||
@mock.patch.object(requests, 'head', autospec=True)
|
||||
def test_validate_href_error_with_secret_parameter(self, head_mock):
|
||||
|
|
@ -230,7 +241,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertIn('secreturl', str(e))
|
||||
self.assertNotIn(self.href, str(e))
|
||||
head_mock.assert_called_once_with(self.href, verify=False,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
|
||||
@mock.patch.object(requests, 'head', autospec=True)
|
||||
def test_validate_href_path_forbidden(self, head_mock):
|
||||
|
|
@ -241,73 +253,10 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
url = self.href + '/'
|
||||
resp = self.service.validate_href(url)
|
||||
head_mock.assert_called_once_with(url, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
self.assertEqual(http_client.FORBIDDEN, resp.status_code)
|
||||
|
||||
@mock.patch.object(requests, 'head', autospec=True)
|
||||
def test_validate_href_path_moved_permanently(self, head_mock):
|
||||
cfg.CONF.set_override('webserver_verify_ca', 'True')
|
||||
|
||||
response = head_mock.return_value
|
||||
response.status_code = http_client.MOVED_PERMANENTLY
|
||||
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_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
|
||||
|
|
@ -373,7 +322,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
}
|
||||
result = self.service.show(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
self.assertEqual({'size': 100, 'updated_at': mtime_date,
|
||||
'properties': {}, 'no_cache': False}, result)
|
||||
|
||||
|
|
@ -399,7 +349,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
}
|
||||
result = self.service.show(self.href)
|
||||
head_mock.assert_called_once_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
self.assertEqual({
|
||||
'size': 100,
|
||||
'updated_at': datetime.datetime(2014, 11, 15, 8, 12, 31),
|
||||
|
|
@ -423,7 +374,8 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||
self.assertRaises(exception.ImageRefValidationFailed,
|
||||
self.service.show, self.href)
|
||||
head_mock.assert_called_with(self.href, verify=True,
|
||||
timeout=60, auth=None)
|
||||
timeout=60, auth=None,
|
||||
allow_redirects=True)
|
||||
|
||||
@mock.patch.object(shutil, 'copyfileobj', autospec=True)
|
||||
@mock.patch.object(requests, 'get', autospec=True)
|
||||
|
|
|
|||
|
|
@ -624,6 +624,7 @@ class IronicImagesTestCase(base.TestCase):
|
|||
mock_gip,
|
||||
mock_validate):
|
||||
mock_igi.return_value = False
|
||||
mock_validate.return_value.url = 'http://whole-disk-image'
|
||||
instance_info = {
|
||||
'image_source': 'http://whole-disk-image'}
|
||||
is_whole_disk_image = images.is_whole_disk_image('context',
|
||||
|
|
@ -665,22 +666,6 @@ class IronicImagesTestCase(base.TestCase):
|
|||
self.assertTrue(images.is_source_a_path('context', 'http://foo/bar'))
|
||||
validate_mock.assert_called_once_with(mock.ANY, 'http://foo/bar')
|
||||
|
||||
@mock.patch.object(images, 'LOG', autospec=True)
|
||||
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||
autospec=True)
|
||||
def test_is_source_a_path_redirect(self, validate_mock, log_mock):
|
||||
url = 'http://foo/bar'
|
||||
redirect_url = url + '/'
|
||||
validate_mock.side_effect = exception.ImageRefIsARedirect(
|
||||
url, redirect_url)
|
||||
self.assertTrue(images.is_source_a_path('context', url))
|
||||
log_mock.debug.assert_called_once_with('Received a URL redirect when '
|
||||
'attempting to evaluate image '
|
||||
'reference http://foo/bar '
|
||||
'pointing to http://foo/bar/. '
|
||||
'This may, or may not be '
|
||||
'recoverable.')
|
||||
|
||||
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||
autospec=True)
|
||||
def test_is_source_a_path_other_error(self, validate_mock):
|
||||
|
|
|
|||
|
|
@ -2361,6 +2361,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
autospec=True)
|
||||
def test_build_instance_info_for_deploy_nonglance_image(
|
||||
self, validate_href_mock, mock_cache_image):
|
||||
validate_href_mock.return_value.url = 'http://image-ref'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-ref'
|
||||
|
|
@ -2392,6 +2393,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self, validate_href_mock, mock_cache_image):
|
||||
cfg.CONF.set_override('conductor_always_validates_images', True,
|
||||
group='conductor')
|
||||
validate_href_mock.return_value.url = 'http://image-ref'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-ref'
|
||||
|
|
@ -2426,6 +2428,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
group='conductor')
|
||||
cfg.CONF.set_override('disable_deep_image_inspection', True,
|
||||
group='conductor')
|
||||
validate_href_mock.return_value.url = 'http://image-ref'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-ref'
|
||||
|
|
@ -2473,11 +2476,14 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self.node.driver_internal_info = driver_internal_info
|
||||
self.node.save()
|
||||
|
||||
validate_href_mock.side_effect = ['http://image-ref',
|
||||
'http://kernel-ref',
|
||||
'http://ramdisk-ref']
|
||||
validate_href_mock.side_effect = [
|
||||
mock.Mock(url='http://image-ref'),
|
||||
mock.Mock(url='http://kernel-ref'),
|
||||
mock.Mock(url='http://ramdisk-ref'),
|
||||
]
|
||||
parse_instance_info_mock.return_value = {'swap_mb': 5}
|
||||
expected_i_info = {'image_source': 'http://image-ref',
|
||||
expected_i_info = {'image_disk_format': 'qcow2',
|
||||
'image_source': 'http://image-ref',
|
||||
'image_url': 'http://image-ref',
|
||||
'image_type': 'partition',
|
||||
'kernel': 'http://kernel-ref',
|
||||
|
|
@ -2524,9 +2530,11 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self.node.driver_internal_info = driver_internal_info
|
||||
self.node.save()
|
||||
|
||||
validate_href_mock.side_effect = ['http://image-ref',
|
||||
'http://kernel-ref',
|
||||
'http://ramdisk-ref']
|
||||
validate_href_mock.side_effect = [
|
||||
mock.Mock(url='http://image-ref'),
|
||||
mock.Mock(url='http://kernel-ref'),
|
||||
mock.Mock(url='http://ramdisk-ref'),
|
||||
]
|
||||
parse_instance_info_mock.return_value = {'swap_mb': 5}
|
||||
expected_i_info = {'image_source': 'http://image-ref',
|
||||
'image_url': 'http://image-ref',
|
||||
|
|
@ -2576,6 +2584,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
autospec=True)
|
||||
def test_build_instance_info_for_deploy_source_is_a_path(
|
||||
self, validate_href_mock):
|
||||
validate_href_mock.return_value.url = 'http://image-url/folder/'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-url/folder/'
|
||||
|
|
@ -2611,9 +2620,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self.node.instance_info = i_info
|
||||
self.node.driver_internal_info = driver_internal_info
|
||||
self.node.save()
|
||||
validate_href_mock.side_effect = exception.ImageRefIsARedirect(
|
||||
image_ref=url,
|
||||
redirect_url=r_url)
|
||||
validate_href_mock.return_value = mock.Mock(url=r_url)
|
||||
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
|
|
@ -2643,11 +2650,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self.node.instance_info = i_info
|
||||
self.node.driver_internal_info = driver_internal_info
|
||||
self.node.save()
|
||||
validate_href_mock.side_effect = iter([
|
||||
exception.ImageRefIsARedirect(
|
||||
image_ref=url,
|
||||
redirect_url=r_url),
|
||||
None])
|
||||
validate_href_mock.return_value = mock.Mock(url=r_url)
|
||||
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
|
|
@ -2657,10 +2660,8 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
|
|||
self.assertNotEqual(self.node.instance_info['image_source'],
|
||||
info['image_url'])
|
||||
self.assertEqual(r_url, info['image_url'])
|
||||
validate_href_mock.assert_has_calls([
|
||||
mock.call(mock.ANY, 'http://image-url/file', False),
|
||||
mock.call(mock.ANY, 'https://image-url/file', False)
|
||||
])
|
||||
validate_href_mock.assert_called_once_with(
|
||||
mock.ANY, 'http://image-url/file', False)
|
||||
mock_cache_image.assert_called_once_with(mock.ANY,
|
||||
task.node,
|
||||
force_raw=False,
|
||||
|
|
@ -2849,6 +2850,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||
autospec=True)
|
||||
def test_build_instance_info_local_image(self, validate_href_mock):
|
||||
cfg.CONF.set_override('image_download_source', 'local', group='agent')
|
||||
validate_href_mock.return_value.url = 'http://image-ref'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-ref'
|
||||
|
|
@ -2926,6 +2928,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||
cfg.CONF.set_override('image_download_source', 'http', group='agent')
|
||||
cfg.CONF.set_override('conductor_always_validates_images', True,
|
||||
group='conductor')
|
||||
validate_href_mock.return_value.url = 'http://image-ref'
|
||||
i_info = self.node.instance_info
|
||||
driver_internal_info = self.node.driver_internal_info
|
||||
i_info['image_source'] = 'http://image-ref'
|
||||
|
|
|
|||
5
releasenotes/notes/redirect-f2f1bc4079763e7e.yaml
Normal file
5
releasenotes/notes/redirect-f2f1bc4079763e7e.yaml
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes validation of image source URLs that are redirects to another URL.
|
||||
Previously, it would raise UnboundLocalError.
|
||||
Loading…
Add table
Reference in a new issue