From 8b8ff830e89923ca6862362a5d16e496a0c0093c Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Tue, 15 Aug 2017 09:08:10 -0500 Subject: [PATCH] Update discovery url normalization with catalog info There's an unfortunately common deployment config issue where the per-service version discovery document doesn't return scheme and netloc properly. (Especially common with glance, as there was an actually upstream bug in the version discovery documents until very recently) Since up until now nobody has actually been doing version discovery (all of the python-client libs skip it and cheat by appending strings to URLs locally) it's a pervassive issue with existing clouds. The workaround is to grab the scheme and netloc from the catalog url, since the service_url is a resource on the endpoint described by the catalog_url, and since the catalog_url has to be correct or else nothing works. Do this in the url normalization so that it persists in the cache, and so that things like endpoint_override are not affected. The need for the workaround and the description of it are documented in the API-SIG spec on consuming version discovery. Needed-By: I78019717cdee79cab43b0d11e737327aa281fd03 Change-Id: I29102e08998b662db8136bee32217532a316f263 --- keystoneauth1/discover.py | 14 ++- .../unit/identity/test_identity_common.py | 97 ++++++++++++++++--- 2 files changed, 93 insertions(+), 18 deletions(-) diff --git a/keystoneauth1/discover.py b/keystoneauth1/discover.py index 4b738a01..ff788e41 100644 --- a/keystoneauth1/discover.py +++ b/keystoneauth1/discover.py @@ -372,8 +372,18 @@ def _combine_relative_url(discovery_url, version_url): # effect on absolute urls. url = urllib.parse.urljoin(discovery_url.rstrip('/') + '/', version_url) - # Parse and recombine the result to squish double //'s from the above - return urllib.parse.urlparse(url).geturl() + # Sadly version discovery documents are common with the scheme + # and netloc broken. + parsed_version_url = urllib.parse.urlparse(url) + parsed_discovery_url = urllib.parse.urlparse(discovery_url) + + return urllib.parse.ParseResult( + parsed_discovery_url.scheme, + parsed_discovery_url.netloc, + parsed_version_url.path, + parsed_version_url.params, + parsed_version_url.query, + parsed_version_url.fragment).geturl() def _version_from_url(url): diff --git a/keystoneauth1/tests/unit/identity/test_identity_common.py b/keystoneauth1/tests/unit/identity/test_identity_common.py index 5759a87c..93f1a707 100644 --- a/keystoneauth1/tests/unit/identity/test_identity_common.py +++ b/keystoneauth1/tests/unit/identity/test_identity_common.py @@ -137,21 +137,29 @@ class CommonIdentityTests(object): """The API version being tested.""" def test_discovering(self): + disc = fixture.DiscoveryList(v2=False, v3=False) + + disc.add_nova_microversion( + href=self.TEST_COMPUTE_ADMIN, + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + self.stub_url('GET', [], base_url=self.TEST_COMPUTE_ADMIN, - json=self.TEST_DISCOVERY) + json=disc) body = 'SUCCESS' # which gives our sample values - self.stub_url('GET', ['path'], text=body) + self.stub_url('GET', ['path'], + text=body, base_url=self.TEST_COMPUTE_ADMIN) a = self.create_auth_plugin() s = session.Session(auth=a) resp = s.get('/path', endpoint_filter={'service_type': 'compute', 'interface': 'admin', - 'version': self.version}) + 'version': '2.1'}) self.assertEqual(200, resp.status_code) self.assertEqual(body, resp.text) @@ -169,13 +177,21 @@ class CommonIdentityTests(object): self.assertEqual(new_body, resp.text) def test_discovery_uses_provided_session_cache(self): + disc = fixture.DiscoveryList(v2=False, v3=False) + + disc.add_nova_microversion( + href=self.TEST_COMPUTE_ADMIN, + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + # register responses such that if the discovery URL is hit more than # once then the response will be invalid and not point to COMPUTE_ADMIN - resps = [{'json': self.TEST_DISCOVERY}, {'status_code': 500}] + resps = [{'json': disc}, {'status_code': 500}] self.requests_mock.get(self.TEST_COMPUTE_ADMIN, resps) body = 'SUCCESS' - self.stub_url('GET', ['path'], text=body) + self.stub_url('GET', ['path'], + text=body, base_url=self.TEST_COMPUTE_ADMIN) cache = {} # now either of the two plugins I use, it should not cause a second @@ -189,23 +205,32 @@ class CommonIdentityTests(object): auth=auth, endpoint_filter={'service_type': 'compute', 'interface': 'admin', - 'version': self.version}) + 'version': '2.1'}) self.assertEqual(200, resp.status_code) self.assertEqual(body, resp.text) self.assertIn(self.TEST_COMPUTE_ADMIN, cache.keys()) def test_discovery_uses_session_cache(self): + disc = fixture.DiscoveryList(v2=False, v3=False) + + disc.add_nova_microversion( + href=self.TEST_COMPUTE_ADMIN, + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + # register responses such that if the discovery URL is hit more than # once then the response will be invalid and not point to COMPUTE_ADMIN - resps = [{'json': self.TEST_DISCOVERY}, {'status_code': 500}] + resps = [{'json': disc}, {'status_code': 500}] self.requests_mock.get(self.TEST_COMPUTE_ADMIN, resps) body = 'SUCCESS' - self.stub_url('GET', ['path'], text=body) + self.stub_url('GET', ['path'], + base_url=self.TEST_COMPUTE_ADMIN, + text=body) filter = {'service_type': 'compute', 'interface': 'admin', - 'version': self.version} + 'version': '2.1'} # create a session and call the endpoint, causing its cache to be set sess = session.Session() @@ -224,13 +249,22 @@ class CommonIdentityTests(object): self.assertEqual(body, resp.text) def test_discovery_uses_plugin_cache(self): + disc = fixture.DiscoveryList(v2=False, v3=False) + + disc.add_nova_microversion( + href=self.TEST_COMPUTE_ADMIN, + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + # register responses such that if the discovery URL is hit more than # once then the response will be invalid and not point to COMPUTE_ADMIN - resps = [{'json': self.TEST_DISCOVERY}, {'status_code': 500}] + resps = [{'json': disc}, {'status_code': 500}] self.requests_mock.get(self.TEST_COMPUTE_ADMIN, resps) body = 'SUCCESS' - self.stub_url('GET', ['path'], text=body) + self.stub_url('GET', ['path'], + base_url=self.TEST_COMPUTE_ADMIN, + text=body) # now either of the two sessions I use, it should not cause a second # request to the discovery url. Calling discovery directly should also @@ -244,22 +278,31 @@ class CommonIdentityTests(object): auth=auth, endpoint_filter={'service_type': 'compute', 'interface': 'admin', - 'version': self.version}) + 'version': '2.1'}) self.assertEqual(200, resp.status_code) self.assertEqual(body, resp.text) def test_discovery_uses_session_plugin_cache(self): + disc = fixture.DiscoveryList(v2=False, v3=False) + + disc.add_nova_microversion( + href=self.TEST_COMPUTE_ADMIN, + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + # register responses such that if the discovery URL is hit more than # once then the response will be invalid and not point to COMPUTE_ADMIN - resps = [{'json': self.TEST_DISCOVERY}, {'status_code': 500}] + resps = [{'json': disc}, {'status_code': 500}] self.requests_mock.get(self.TEST_COMPUTE_ADMIN, resps) body = 'SUCCESS' - self.stub_url('GET', ['path'], text=body) + self.stub_url('GET', ['path'], + base_url=self.TEST_COMPUTE_ADMIN, + text=body) filter = {'service_type': 'compute', 'interface': 'admin', - 'version': self.version} + 'version': '2.1'} # create a plugin and call the endpoint, causing its cache to be set plugin = self.create_auth_plugin() @@ -286,7 +329,7 @@ class CommonIdentityTests(object): sb = session.Session() discovery_cache = {} - expected_url = urllib.parse.urljoin(self.TEST_ROOT_URL, '/v2.0') + expected_url = urllib.parse.urljoin(self.TEST_COMPUTE_ADMIN, '/v2.0') for sess in (sa, sb): disc = discover.get_discovery( @@ -887,6 +930,28 @@ class CommonIdentityTests(object): self.assertFalse( self.requests_mock.request_history[-1].url.endswith('/')) + def test_broken_discovery_endpoint(self): + + # Discovery document with a bogus/broken base URL + disc = fixture.DiscoveryList(v2=False, v3=False) + disc.add_nova_microversion( + href='http://internal.example.com', + id='v2.1', status='CURRENT', + min_version='2.1', version='2.38') + + a = self.create_auth_plugin() + s = session.Session(auth=a) + + self.requests_mock.get(self.TEST_COMPUTE_PUBLIC, json=disc) + + data = s.get_endpoint_data(service_type='compute', + interface='public', + min_version='2.1', + max_version='2.latest') + + # Verify that we get the versioned url based on the catalog url + self.assertTrue(data.url, self.TEST_COMPUTE_PUBLIC + '/v2.1') + def test_asking_for_auth_endpoint_ignores_checks(self): a = self.create_auth_plugin() s = session.Session(auth=a)