openstack/shade
Code Issues Proposed changes

Handle glance image pagination links better

Browse Source 
Glance returns pagination links like "/v2/images?marker=". keystoneauth
Adapter treats absolute links like that as if they are rooted on the
service url. Since the catalog has https://image.example.com/v2 in it,
the Adapter will be mounted on that, which means /v2/images will be
treated as "https://image.example.com/v2/v2/iamges - which is not
correct.

Doing a urljoin would also be incorrect in an OpenStack context, because
the service could be on a sub-url, such as https://example.com/image,
meaning the v2 API endpoint would be https://example.com/image/v2 -
which means a straight urljoin of https://example.com/image/v2 and
/v2/images?marker= would result in https://example.com/v2/images?marker=
which is wrong since it strips the /image path prefix.

The most correct things for glance to do would be to return a full
absolute url - or a full relative URL. Constructing the absolute url
shade-side is hard for the above reasons - we don't have the right
context.

Strip /v1/ or /v2/ from the front of the next links (yay!) to turn
it into a relative url - thereby letting adapter.get(endpoint) do the
correct thing.

While doing this, update the catalog fixture in the test suite to have a
trailing /v2 so that we can be sure we're doing the correct thing in our
unit tests. Also add suburl fixtures to ensure that we work with
suburls.

Change-Id: I6fc8484ed62a029a8ba8ff1d31a37ba69e3000cf
This commit is contained in:
Monty Taylor 2017-10-13 16:19:44 -05:00
parent 8a6f0f9bc3
commit 1d16f64fbf
No known key found for this signature in database
GPG Key ID: 7BAE94BC7141A594
6 changed files with 306 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
shade/openstackcloud.py
View File

@ -2146,9 +2146,13 @@ class OpenStackCloud(
while 'next' in response:
image_list.extend(meta.obj_list_to_munch(response['images']))
endpoint = response['next']
# Use the raw endpoint from the catalog not the one from
# version discovery so that the next links will work right
response = self._raw_image_client.get(endpoint)
# next links from glance have the version prefix. If the catalog
# has a versioned endpoint, then we can't append the next link to
# it. Strip the absolute prefix (/v1/ or /v2/ to turn it into
# a proper relative link.
if endpoint.startswith('/v'):
endpoint = endpoint[4:]
response = self._image_client.get(endpoint)
if 'images' in response:
image_list.extend(meta.obj_list_to_munch(response['images']))
else:

13
shade/tests/unit/base.py
View File

@ -454,10 +454,12 @@ class RequestsMockTestCase(BaseTestCase):
log_inner_exceptions=True)
def get_glance_discovery_mock_dict(
self, image_version_json='image-version.json'):
self,
image_version_json='image-version.json',
image_discovery_url='https://image.example.com/'):
discovery_fixture = os.path.join(
self.fixtures_directory, image_version_json)
return dict(method='GET', uri='https://image.example.com/',
return dict(method='GET', uri=image_discovery_url,
status_code=300,
text=open(discovery_fixture, 'r').read())
@ -473,14 +475,17 @@ class RequestsMockTestCase(BaseTestCase):
return dict(method='GET', uri="https://bare-metal.example.com/",
text=open(discovery_fixture, 'r').read())
def use_glance(self, image_version_json='image-version.json'):
def use_glance(
self, image_version_json='image-version.json',
image_discovery_url='https://image.example.com/'):
# NOTE(notmorgan): This method is only meant to be used in "setUp"
# where the ordering of the url being registered is tightly controlled
# if the functionality of .use_glance is meant to be used during an
# actual test case, use .get_glance_discovery_mock and apply to the
# right location in the mock_uris when calling .register_uris
self.__do_register_uris([
self.get_glance_discovery_mock_dict(image_version_json)])
self.get_glance_discovery_mock_dict(
image_version_json, image_discovery_url)])
def use_designate(self):
# NOTE(slaweq): This method is only meant to be used in "setUp"

6
shade/tests/unit/fixtures/catalog-v2.json
View File

@ -47,10 +47,10 @@
"endpoints_links": [],
"endpoints": [
{
"adminURL": "https://image.example.com",
"adminURL": "https://image.example.com/v2",
"region": "RegionOne",
"publicURL": "https://image.example.com",
"internalURL": "https://image.example.com",
"publicURL": "https://image.example.com/v2",
"internalURL": "https://image.example.com/v2",
"id": "5a64de3c4a614d8d8f8d1ba3dee5f45f"
}
],

185
shade/tests/unit/fixtures/catalog-v3-suburl.json Normal file
View File

@ -0,0 +1,185 @@
{
"token": {
"audit_ids": [
"Rvn7eHkiSeOwucBIPaKdYA"
],
"catalog": [
{
"endpoints": [
{
"id": "32466f357f3545248c47471ca51b0d3a",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/compute/v2.1/"
}
],
"name": "nova",
"type": "compute"
},
{
"endpoints": [
{
"id": "1e875ca2225b408bbf3520a1b8e1a537",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/volumev2/v2/1c36b64c840a42cd9e9b931a369337f0"
}
],
"name": "cinderv2",
"type": "volumev2"
},
{
"endpoints": [
{
"id": "5a64de3c4a614d8d8f8d1ba3dee5f45f",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/image"
}
],
"name": "glance",
"type": "image"
},
{
"endpoints": [
{
"id": "3d15fdfc7d424f3c8923324417e1a3d1",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/volume/v1/1c36b64c840a42cd9e9b931a369337f0"
}
],
"name": "cinder",
"type": "volume"
},
{
"endpoints": [
{
"id": "4deb4d0504a044a395d4480741ba628c",
"interface": "public",
"region": "RegionOne",
"url": "https://identity.example.com"
},
{
"id": "012322eeedcd459edabb4933021112bc",
"interface": "admin",
"region": "RegionOne",
"url": "https://example.com/identity"
}
],
"endpoints_links": [],
"name": "keystone",
"type": "identity"
},
{
"endpoints": [
{
"id": "4deb4d0504a044a395d4480741ba628d",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/example"
}
],
"endpoints_links": [],
"name": "neutron",
"type": "network"
},
{
"endpoints": [
{
"id": "4deb4d0504a044a395d4480741ba628e",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/container-infra/v1"
}
],
"endpoints_links": [],
"name": "magnum",
"type": "container-infra"
},
{
"endpoints": [
{
"id": "4deb4d0504a044a395d4480741ba628c",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/object-store/v1/1c36b64c840a42cd9e9b931a369337f0"
}
],
"endpoints_links": [],
"name": "swift",
"type": "object-store"
},
{
"endpoints": [
{
"id": "652f0612744042bfbb8a8bb2c777a16d",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/bare-metal"
}
],
"endpoints_links": [],
"name": "ironic",
"type": "baremetal"
},
{
"endpoints": [
{
"id": "4deb4d0504a044a395d4480741ba628c",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/orchestration/v1/1c36b64c840a42cd9e9b931a369337f0"
}
],
"endpoints_links": [],
"name": "heat",
"type": "orchestration"
},
{
"endpoints": [
{
"id": "10c76ffd2b744a67950ed1365190d352",
"interface": "public",
"region": "RegionOne",
"url": "https://example.com/dns"
}
],
"endpoints_links": [],
"name": "designate",
"type": "dns"
}
],
"expires_at": "9999-12-31T23:59:59Z",
"issued_at": "2016-12-17T14:25:05.000000Z",
"methods": [
"password"
],
"project": {
"domain": {
"id": "default",
"name": "default"
},
"id": "1c36b64c840a42cd9e9b931a369337f0",
"name": "Default Project"
},
"roles": [
{
"id": "9fe2ff9ee4384b1894a90878d3e92bab",
"name": "_member_"
},
{
"id": "37071fc082e14c2284c32a2761f71c63",
"name": "swiftoperator"
}
],
"user": {
"domain": {
"id": "default",
"name": "default"
},
"id": "c17534835f8f42bf98fc367e0bf35e09",
"name": "mordred"
}
}
}

64
shade/tests/unit/fixtures/image-version-suburl.json Normal file
View File

@ -0,0 +1,64 @@
{
"versions": [
{
"status": "CURRENT",
"id": "v2.3",
"links": [
{
"href": "http://example.com/image/v2/",
"rel": "self"
}
]
},
{
"status": "SUPPORTED",
"id": "v2.2",
"links": [
{
"href": "http://example.com/image/v2/",
"rel": "self"
}
]
},
{
"status": "SUPPORTED",
"id": "v2.1",
"links": [
{
"href": "http://example.com/image/v2/",
"rel": "self"
}
]
},
{
"status": "SUPPORTED",
"id": "v2.0",
"links": [
{
"href": "http://example.com/image/v2/",
"rel": "self"
}
]
},
{
"status": "SUPPORTED",
"id": "v1.1",
"links": [
{
"href": "http://example.com/image/v1/",
"rel": "self"
}
]
},
{
"status": "SUPPORTED",
"id": "v1.0",
"links": [
{
"href": "http://example.com/image/v1/",
"rel": "self"
}
]
}
]
}

38
shade/tests/unit/test_image.py
View File

@ -687,6 +687,44 @@ class TestImage(BaseTestImage):
self.assert_calls()
class TestImageSuburl(BaseTestImage):
def setUp(self):
super(TestImageSuburl, self).setUp()
self.use_keystone_v3(catalog='catalog-v3-suburl.json')
self.use_glance(
image_version_json='image-version-suburl.json',
image_discovery_url='https://example.com/image')
def test_list_images(self):
self.register_uris([
dict(method='GET', uri='https://example.com/image/v2/images',
json=self.fake_search_return)
])
self.assertEqual(
self.cloud._normalize_images([self.fake_image_dict]),
self.cloud.list_images())
self.assert_calls()
def test_list_images_paginated(self):
marker = str(uuid.uuid4())
self.register_uris([
dict(method='GET', uri='https://example.com/image/v2/images',
json={'images': [self.fake_image_dict],
'next': '/v2/images?marker={marker}'.format(
marker=marker)}),
dict(method='GET',
uri=('https://example.com/image/v2/images?'
'marker={marker}'.format(marker=marker)),
json=self.fake_search_return)
])
self.assertEqual(
self.cloud._normalize_images([
self.fake_image_dict, self.fake_image_dict]),
self.cloud.list_images())
self.assert_calls()
class TestImageV1Only(base.RequestsMockTestCase):
def setUp(self):