openstack/keystone
Code Issues Proposed changes

Don't add unformatted project-specific endpoints to catalog

Browse Source 
If a token is domain-scoped, there is no project defined.
Hence for any catalog entries, we should skip any endpoints
that need a project id in order to be formatted correctly.

Co-Authored-By: Henry Nash <henryn@linux.vnet.ibm.com>
Change-Id: I3617a2509bfc4213f136b5c867c40d478a70ded8
Closes-Bug: #1261468
This commit is contained in:
Dave Chen 2015-01-04 22:18:46 +08:00
parent d68fa6b192
commit a06d5b5c24
6 changed files with 148 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
keystone/catalog/backends/sql.py
View File

@ -269,10 +269,29 @@ class Catalog(catalog.Driver):
return ref.to_dict()
def get_catalog(self, user_id, tenant_id):
"""Retrieve and format the V2 service catalog.
:param user_id: The id of the user who has been authenticated for
creating service catalog.
:param tenant_id: The id of the project. 'tenant_id' will be None
in the case this being called to create a catalog to go in a
domain scoped token. In this case, any endpoint that requires
a tenant_id as part of their URL will be skipped (as would a whole
service if, as a consequence, it has no valid endpoints).
:returns: A nested dict representing the service catalog or an
empty dict.
"""
substitutions = dict(
itertools.chain(six.iteritems(CONF),
six.iteritems(CONF.eventlet_server)))
substitutions.update({'tenant_id': tenant_id, 'user_id': user_id})
substitutions.update({'user_id': user_id})
silent_keyerror_failures = []
if tenant_id:
substitutions.update({'tenant_id': tenant_id})
else:
silent_keyerror_failures = ['tenant_id']
session = sql.get_session()
endpoints = (session.query(Endpoint).
@ -285,7 +304,13 @@ class Catalog(catalog.Driver):
if not endpoint.service['enabled']:
continue
try:
url = core.format_url(endpoint['url'], substitutions)
formatted_url = core.format_url(
endpoint['url'], substitutions,
silent_keyerror_failures=silent_keyerror_failures)
if formatted_url is not None:
url = formatted_url
else:
continue
except exception.MalformedEndpoint:
continue # this failure is already logged in format_url()
@ -304,11 +329,27 @@ class Catalog(catalog.Driver):
return catalog
def get_v3_catalog(self, user_id, tenant_id):
"""Retrieve and format the current V3 service catalog.
:param user_id: The id of the user who has been authenticated for
creating service catalog.
:param tenant_id: The id of the project. 'tenant_id' will be None in
the case this being called to create a catalog to go in a domain
scoped token. In this case, any endpoint that requires a
tenant_id as part of their URL will be skipped.
:returns: A list representing the service catalog or an empty list
"""
d = dict(
itertools.chain(six.iteritems(CONF),
six.iteritems(CONF.eventlet_server)))
d.update({'tenant_id': tenant_id,
'user_id': user_id})
d.update({'user_id': user_id})
silent_keyerror_failures = []
if tenant_id:
d.update({'tenant_id': tenant_id})
else:
silent_keyerror_failures = ['tenant_id']
session = sql.get_session()
services = (session.query(Service).filter(Service.enabled == true()).
@ -322,12 +363,20 @@ class Catalog(catalog.Driver):
del endpoint['enabled']
endpoint['region'] = endpoint['region_id']
try:
endpoint['url'] = core.format_url(endpoint['url'], d)
formatted_url = core.format_url(
endpoint['url'], d,
silent_keyerror_failures=silent_keyerror_failures)
if formatted_url:
endpoint['url'] = formatted_url
else:
continue
except exception.MalformedEndpoint:
continue # this failure is already logged in format_url()
yield endpoint
# TODO(davechen): If there is service with no endpoints, we should skip
# the service instead of keeping it in the catalog, see bug #1436704.
def make_v3_service(svc):
eps = list(make_v3_endpoints(svc.endpoints))
service = {'endpoints': eps, 'id': svc.id, 'type': svc.type}

29
keystone/catalog/backends/templated.py
View File

@ -107,19 +107,44 @@ class Catalog(kvs.Catalog):
raise
def get_catalog(self, user_id, tenant_id):
"""Retrieve and format the V2 service catalog.
:param user_id: The id of the user who has been authenticated for
creating service catalog.
:param tenant_id: The id of the project. 'tenant_id' will be None in
the case this being called to create a catalog to go in a domain
scoped token. In this case, any endpoint that requires a tenant_id
as part of their URL will be skipped.
:returns: A nested dict representing the service catalog or an
empty dict.
"""
substitutions = dict(
itertools.chain(six.iteritems(CONF),
six.iteritems(CONF.eventlet_server)))
substitutions.update({'tenant_id': tenant_id, 'user_id': user_id})
substitutions.update({'user_id': user_id})
silent_keyerror_failures = []
if tenant_id:
substitutions.update({'tenant_id': tenant_id})
else:
silent_keyerror_failures = ['tenant_id']
catalog = {}
# TODO(davechen): If there is service with no endpoints, we should
# skip the service instead of keeping it in the catalog.
# see bug #1436704.
for region, region_ref in six.iteritems(self.templates):
catalog[region] = {}
for service, service_ref in six.iteritems(region_ref):
service_data = {}
try:
for k, v in six.iteritems(service_ref):
service_data[k] = core.format_url(v, substitutions)
formatted_value = core.format_url(
v, substitutions,
silent_keyerror_failures=silent_keyerror_failures)
if formatted_value:
service_data[k] = formatted_value
except exception.MalformedEndpoint:
continue # this failure is already logged in format_url()
catalog[region][service] = service_data

17
keystone/catalog/core.py
View File

@ -37,11 +37,13 @@ LOG = log.getLogger(__name__)
MEMOIZE = cache.get_memoization_decorator(section='catalog')
def format_url(url, substitutions):
def format_url(url, substitutions, silent_keyerror_failures=None):
"""Formats a user-defined URL with the given substitutions.
:param string url: the URL to be formatted
:param dict substitutions: the dictionary used for substitution
:param list silent_keyerror_failures: keys for which we should be silent
if there is a KeyError exception on substitution attempt
:returns: a formatted URL
"""
@ -54,6 +56,7 @@ def format_url(url, substitutions):
substitutions = utils.WhiteListedItemFilter(
WHITELISTED_PROPERTIES,
substitutions)
allow_keyerror = silent_keyerror_failures or []
try:
result = url.replace('$(', '%(') % substitutions
except AttributeError:
@ -61,10 +64,14 @@ def format_url(url, substitutions):
{"url": url})
raise exception.MalformedEndpoint(endpoint=url)
except KeyError as e:
LOG.error(_LE("Malformed endpoint %(url)s - unknown key %(keyerror)s"),
{"url": url,
"keyerror": e})
raise exception.MalformedEndpoint(endpoint=url)
if not e.args or e.args[0] not in allow_keyerror:
LOG.error(_LE("Malformed endpoint %(url)s - unknown key "
"%(keyerror)s"),
{"url": url,
"keyerror": e})
raise exception.MalformedEndpoint(endpoint=url)
else:
result = None
except TypeError as e:
LOG.error(_LE("Malformed endpoint '%(url)s'. The following type error "
"occurred during string substitution: %(typeerror)s"),

14
keystone/tests/unit/catalog/test_core.py
View File

@ -72,3 +72,17 @@ class FormatUrlTests(testtools.TestCase):
core.format_url,
url_template,
values)
def test_substitution_with_allowed_keyerror(self):
# No value of 'tenant_id' is passed into url_template.
# mod: format_url will return None instead of raising
# "MalformedEndpoint" exception.
# This is intentional behavior since we don't want to skip
# all the later endpoints once there is an URL of endpoint
# trying to replace 'tenant_id' with None.
url_template = ('http://$(public_bind_host)s:$(admin_port)d/'
'$(tenant_id)s/$(user_id)s')
values = {'public_bind_host': 'server', 'admin_port': 9090,
'user_id': 'B'}
self.assertIsNone(core.format_url(url_template, values,
silent_keyerror_failures=['tenant_id']))

25
keystone/tests/unit/test_backend_templated.py
View File

@ -120,6 +120,31 @@ class TestTemplatedCatalog(tests.TestCase, test_backend.CatalogTests):
'id': '1'}]
self.assert_catalogs_equal(exp_catalog, catalog_ref)
def test_get_catalog_ignores_endpoints_with_invalid_urls(self):
user_id = uuid.uuid4().hex
# If the URL has no 'tenant_id' to substitute, we will skip the
# endpoint which contains this kind of URL.
catalog_ref = self.catalog_api.get_v3_catalog(user_id, tenant_id=None)
exp_catalog = [
{'endpoints': [],
'type': 'compute',
'name': "'Compute Service'",
'id': '2'},
{'endpoints': [
{'interface': 'admin',
'region': 'RegionOne',
'url': 'http://localhost:35357/v2.0'},
{'interface': 'public',
'region': 'RegionOne',
'url': 'http://localhost:5000/v2.0'},
{'interface': 'internal',
'region': 'RegionOne',
'url': 'http://localhost:35357/v2.0'}],
'type': 'identity',
'name': "'Identity Service'",
'id': '1'}]
self.assert_catalogs_equal(exp_catalog, catalog_ref)
def test_list_regions_filtered_by_parent_region_id(self):
self.skipTest('Templated backend does not support hints')

16
keystone/tests/unit/test_v3_catalog.py
View File

@ -15,6 +15,8 @@
import copy
import uuid
from testtools import matchers
from keystone import catalog
from keystone.tests import unit as tests
from keystone.tests.unit.ksfixtures import database
@ -676,6 +678,20 @@ class TestCatalogAPISQL(tests.TestCase):
# all three appear in the backend
self.assertEqual(3, len(self.catalog_api.list_endpoints()))
# create another valid endpoint - tenant_id will be replaced
ref = self.new_endpoint_ref(self.service_id)
ref['url'] = 'http://keystone/%(tenant_id)s'
self.catalog_api.create_endpoint(ref['id'], ref)
# there are two valid endpoints, positive check
catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id)
self.assertThat(catalog[0]['endpoints'], matchers.HasLength(2))
# If the URL has no 'tenant_id' to substitute, we will skip the
# endpoint which contains this kind of URL, negative check.
catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id=None)
self.assertThat(catalog[0]['endpoints'], matchers.HasLength(1))
def test_get_catalog_always_returns_service_name(self):
user_id = uuid.uuid4().hex
tenant_id = uuid.uuid4().hex