Cache root urls with and without trailing slashes
The trailng slash on a pathless url is not meaningful, but we were treating the url given to the discovery cache as if it were. In some circumstances, such as an endpoint_override that didn't match the found discovery document perfectly, a double-request could be made. Normalize root urls in the caching code so that https://example.com and https://example.com/ would be the same. Change-Id: I70a5911cf0f213a7816fe8d58c6cca4702ff71bb
This commit is contained in:
parent
58ac4c059a
commit
106d91fb41
@ -1415,6 +1415,18 @@ def get_discovery(session, url, cache=None, authenticated=False):
|
||||
if session.auth and hasattr(session.auth, '_discovery_cache'):
|
||||
caches.append(session.auth._discovery_cache)
|
||||
|
||||
# https://example.com and https://example.com/ should be treated the same
|
||||
# for caching purposes.
|
||||
parsed_url = urllib.parse.urlparse(url)
|
||||
if parsed_url.path in ('', '/'):
|
||||
url = urllib.parse.ParseResult(
|
||||
parsed_url.scheme,
|
||||
parsed_url.netloc,
|
||||
'',
|
||||
parsed_url.params,
|
||||
parsed_url.query,
|
||||
parsed_url.fragment).geturl()
|
||||
|
||||
for cache in caches:
|
||||
disc = cache.get(url)
|
||||
|
||||
|
@ -341,6 +341,31 @@ class CommonIdentityTests(object):
|
||||
|
||||
self.assertIn(self.TEST_COMPUTE_ADMIN, discovery_cache.keys())
|
||||
|
||||
def test_discovery_trailing_slash(self):
|
||||
# The discovery cache should treat root urls the same whether they have
|
||||
# a slash or not. If the url is called a second time (meaning the cache
|
||||
# didn't work, we'll hit the 500 error.
|
||||
self.requests_mock.get(
|
||||
'https://example.com', [
|
||||
{'json': self.TEST_DISCOVERY},
|
||||
{'status_code': 500}
|
||||
])
|
||||
|
||||
sess = session.Session()
|
||||
discovery_cache = {}
|
||||
|
||||
expected_url = 'https://example.com/v2.0'
|
||||
|
||||
for test_endpoint in ('https://example.com', 'https://example.com/'):
|
||||
disc = discover.get_discovery(
|
||||
sess, test_endpoint, cache=discovery_cache)
|
||||
url = disc.url_for(('2', '0'))
|
||||
|
||||
self.assertEqual(expected_url, url)
|
||||
|
||||
self.assertIn('https://example.com', discovery_cache.keys())
|
||||
self.assertNotIn('https://example.com/', discovery_cache.keys())
|
||||
|
||||
def test_discovering_with_no_data(self):
|
||||
# which returns discovery information pointing to TEST_URL but there is
|
||||
# no data there.
|
||||
|
@ -0,0 +1,8 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixed an issue where https://example.com and https://example.com/ were
|
||||
being treated as different urls in the discovery cache resulting in a
|
||||
second unneeded discovery call when someone sets an ``endpoint_override``
|
||||
that didn't match the trailing-slash form given by that service's
|
||||
discovery document.
|
Loading…
x
Reference in New Issue
Block a user