Add an allow_version_hack flag to session and identity plugins.
Whilst historically we always wanted keystoneauth to do the most permissive thing and allow a versioned or unversioned entry in a service catalog there are now cases where we would prefer to fail when the catalog is misconfigured. This will allow a client to opt out of versioned catalog endpoints to insist that the deployment is correctly configured. Closes-Bug: #1668484 Change-Id: Ided0e0c7409994f703175fe61bd4043b840bcf1e
This commit is contained in:
Jamie Lennox
parent
3364703d3b
commit
3ce5cb4bf6
@ -57,6 +57,9 @@ class Adapter(object):
|
||||
:param str client_version: The version of the client that created the
|
||||
adapter. This will be used to create the
|
||||
user_agent.
|
||||
:param bool allow_version_hack: Allow keystoneauth to hack up catalog
|
||||
URLS to support older schemes.
|
||||
(optional, default True)
|
||||
"""
|
||||
|
||||
client_name = None
|
||||
@ -68,7 +71,7 @@ class Adapter(object):
|
||||
version=None, auth=None, user_agent=None,
|
||||
connect_retries=None, logger=None, allow={},
|
||||
additional_headers=None, client_name=None,
|
||||
client_version=None):
|
||||
client_version=None, allow_version_hack=None):
|
||||
# NOTE(jamielennox): when adding new parameters to adapter please also
|
||||
# add them to the adapter call in httpclient.HTTPClient.__init__ as
|
||||
# well as to load_adapter_from_argparse below if the argument is
|
||||
@ -87,6 +90,7 @@ class Adapter(object):
|
||||
self.logger = logger
|
||||
self.allow = allow
|
||||
self.additional_headers = additional_headers or {}
|
||||
self.allow_version_hack = allow_version_hack
|
||||
|
||||
if client_name:
|
||||
self.client_name = client_name
|
||||
@ -104,6 +108,8 @@ class Adapter(object):
|
||||
kwargs.setdefault('region_name', self.region_name)
|
||||
if self.version:
|
||||
kwargs.setdefault('version', self.version)
|
||||
if self.allow_version_hack is not None:
|
||||
kwargs.setdefault('allow_version_hack', self.allow_version_hack)
|
||||
|
||||
def request(self, url, method, **kwargs):
|
||||
endpoint_filter = kwargs.setdefault('endpoint_filter', {})
|
||||
|
||||
@ -159,7 +159,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
||||
|
||||
def get_endpoint(self, session, service_type=None, interface=None,
|
||||
region_name=None, service_name=None, version=None,
|
||||
allow={}, **kwargs):
|
||||
allow={}, allow_version_hack=True, **kwargs):
|
||||
"""Return a valid endpoint for a service.
|
||||
|
||||
If a valid token is not present then a new one will be fetched using
|
||||
@ -183,6 +183,9 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
||||
endpoint. (optional)
|
||||
:param dict allow: Extra filters to pass when discovering API
|
||||
versions. (optional)
|
||||
:param bool allow_version_hack: Allow keystoneauth to hack up catalog
|
||||
URLS to support older schemes.
|
||||
(optional, default True)
|
||||
|
||||
:raises keystoneauth1.exceptions.http.HttpError: An error from an
|
||||
invalid HTTP response.
|
||||
@ -226,19 +229,38 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
||||
# other endpoint versions. So we support a list of client defined
|
||||
# situations where we can strip the version component from a URL before
|
||||
# doing discovery.
|
||||
hacked_url = discover._get_catalog_discover_hack(service_type, url)
|
||||
if allow_version_hack:
|
||||
vers_url = discover._get_catalog_discover_hack(service_type, url)
|
||||
else:
|
||||
vers_url = url
|
||||
|
||||
try:
|
||||
disc = self.get_discovery(session, hacked_url, authenticated=False)
|
||||
disc = self.get_discovery(session, vers_url, authenticated=False)
|
||||
except (exceptions.DiscoveryFailure,
|
||||
exceptions.HttpError,
|
||||
exceptions.ConnectionError):
|
||||
# NOTE(jamielennox): Again if we can't contact the server we fall
|
||||
# back to just returning the URL from the catalog. This may not be
|
||||
# the best default but we need it for now.
|
||||
LOG.warning('Failed to contact the endpoint at %s for discovery. '
|
||||
'Fallback to using that endpoint as the base url.',
|
||||
url)
|
||||
# NOTE(jamielennox): The logic here is required for backwards
|
||||
# compatibility. By itself it is not ideal.
|
||||
|
||||
if allow_version_hack:
|
||||
# NOTE(jamielennox): Again if we can't contact the server we
|
||||
# fall back to just returning the URL from the catalog. This
|
||||
# is backwards compatible behaviour and used when there is no
|
||||
# other choice. Realistically if you have provided a version
|
||||
# you should be able to rely on that version being returned or
|
||||
# the request failing.
|
||||
LOG.warning('Failed to contact the endpoint at %s for '
|
||||
'discovery. Fallback to using that endpoint as '
|
||||
'the base url.', url)
|
||||
|
||||
else:
|
||||
# NOTE(jamielennox): If you've said no to allow_version_hack
|
||||
# and you can't determine the actual URL this is a failure
|
||||
# because we are specifying that the deployment must be up to
|
||||
# date enough to properly specify a version and keystoneauth
|
||||
# can't deliver.
|
||||
return None
|
||||
|
||||
else:
|
||||
# NOTE(jamielennox): urljoin allows the url to be relative or even
|
||||
# protocol-less. The additional trailing '/' make urljoin respect
|
||||
@ -249,7 +271,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
||||
url = disc.url_for(version, **allow)
|
||||
|
||||
if url:
|
||||
url = urllib.parse.urljoin(hacked_url.rstrip('/') + '/', url)
|
||||
url = urllib.parse.urljoin(vers_url.rstrip('/') + '/', url)
|
||||
|
||||
return url
|
||||
|
||||
|
||||
@ -562,6 +562,94 @@ class CatalogHackTests(utils.TestCase):
|
||||
|
||||
self.assertEqual(self.V3_URL, endpoint)
|
||||
|
||||
def test_setting_no_discover_hack(self):
|
||||
v2_disc = fixture.V2Discovery(self.V2_URL)
|
||||
common_disc = fixture.DiscoveryList(href=self.BASE_URL)
|
||||
|
||||
v2_m = self.stub_url('GET',
|
||||
['v2.0'],
|
||||
base_url=self.BASE_URL,
|
||||
status_code=200,
|
||||
json=v2_disc)
|
||||
|
||||
common_m = self.stub_url('GET',
|
||||
[],
|
||||
base_url=self.BASE_URL,
|
||||
status_code=300,
|
||||
json=common_disc)
|
||||
|
||||
resp_text = uuid.uuid4().hex
|
||||
|
||||
resp_m = self.stub_url('GET',
|
||||
['v3', 'path'],
|
||||
base_url=self.BASE_URL,
|
||||
status_code=200,
|
||||
text=resp_text)
|
||||
|
||||
# it doesn't matter that we auth with v2 here, discovery hack is in
|
||||
# base. All identity endpoints point to v2 urls.
|
||||
token = fixture.V2Token()
|
||||
service = token.add_service(self.IDENTITY)
|
||||
service.add_endpoint(public=self.V2_URL,
|
||||
admin=self.V2_URL,
|
||||
internal=self.V2_URL)
|
||||
|
||||
self.stub_url('POST',
|
||||
['tokens'],
|
||||
base_url=self.V2_URL,
|
||||
json=token)
|
||||
|
||||
v2_auth = identity.V2Password(self.V2_URL,
|
||||
username=uuid.uuid4().hex,
|
||||
password=uuid.uuid4().hex)
|
||||
|
||||
sess = session.Session(auth=v2_auth)
|
||||
|
||||
# v2 auth with v2 url doesn't make any discovery calls.
|
||||
self.assertFalse(v2_m.called)
|
||||
self.assertFalse(common_m.called)
|
||||
|
||||
# v3 endpoint with hack will strip v2 suffix and call root discovery
|
||||
endpoint = sess.get_endpoint(service_type=self.IDENTITY,
|
||||
version=(3, 0),
|
||||
allow_version_hack=True)
|
||||
|
||||
# got v3 url
|
||||
self.assertEqual(self.V3_URL, endpoint)
|
||||
|
||||
# only called root discovery.
|
||||
self.assertFalse(v2_m.called)
|
||||
self.assertTrue(common_m.called_once)
|
||||
|
||||
# with hack turned off it calls v2 discovery and finds nothing
|
||||
endpoint = sess.get_endpoint(service_type=self.IDENTITY,
|
||||
version=(3, 0),
|
||||
allow_version_hack=False)
|
||||
self.assertIsNone(endpoint)
|
||||
|
||||
# this one called v2
|
||||
self.assertTrue(v2_m.called_once)
|
||||
self.assertTrue(common_m.called_once)
|
||||
|
||||
# get_endpoint returning None raises EndpointNotFound when requesting
|
||||
self.assertRaises(exceptions.EndpointNotFound,
|
||||
sess.get,
|
||||
'/path',
|
||||
endpoint_filter={'service_type': 'identity',
|
||||
'version': (3, 0),
|
||||
'allow_version_hack': False})
|
||||
|
||||
self.assertFalse(resp_m.called)
|
||||
|
||||
# works when allow_version_hack is set
|
||||
resp = sess.get('/path',
|
||||
endpoint_filter={'service_type': 'identity',
|
||||
'version': (3, 0),
|
||||
'allow_version_hack': True})
|
||||
|
||||
self.assertTrue(resp_m.called_once)
|
||||
self.assertEqual(resp_text, resp.text)
|
||||
|
||||
|
||||
class GenericPlugin(plugin.BaseAuthPlugin):
|
||||
|
||||
|
||||
@ -60,7 +60,7 @@ class TestCase(testtools.TestCase):
|
||||
url = base_url
|
||||
|
||||
url = url.replace("/?", "?")
|
||||
self.requests_mock.register_uri(method, url, **kwargs)
|
||||
return self.requests_mock.register_uri(method, url, **kwargs)
|
||||
|
||||
def assertRequestBodyIs(self, body=None, json=None):
|
||||
last_request_body = self.requests_mock.last_request.body
|
||||
|
||||
@ -0,0 +1,9 @@
|
||||
---
|
||||
features:
|
||||
- A new flag `allow_version_hack` was added to identity plugins and the
|
||||
adapter which will allow a client to opt out of making guesses at the
|
||||
version url page of a service. This means that if a deployment is
|
||||
misconfigured and the service catalog contains a versioned endpoint that
|
||||
does not match the requested version the request will fail. This will be
|
||||
useful in beginning to require correctly deployed catalogs rather than
|
||||
continue to hide the problem.
|
||||
Loading…
x
Reference in New Issue
Block a user