diff --git a/keystone/catalog/core.py b/keystone/catalog/core.py index 5c005fa2a8..ce4307f1a5 100644 --- a/keystone/catalog/core.py +++ b/keystone/catalog/core.py @@ -18,6 +18,7 @@ import abc import itertools +from oslo_cache import core as oslo_cache from oslo_config import cfg from oslo_log import log import six @@ -35,12 +36,23 @@ from keystone import notifications CONF = cfg.CONF LOG = log.getLogger(__name__) -MEMOIZE = cache.get_memoization_decorator(group='catalog') WHITELISTED_PROPERTIES = [ 'tenant_id', 'user_id', 'public_bind_host', 'admin_bind_host', 'compute_host', 'admin_port', 'public_port', 'public_endpoint', 'admin_endpoint', ] +# This is a general cache region for catalog administration (CRUD operations). +MEMOIZE = cache.get_memoization_decorator(group='catalog') + +# This builds a discrete cache region dedicated to complete service catalogs +# computed for a given user + project pair. Any write operation to create, +# modify or delete elements of the service catalog should invalidate this +# entire cache region. +COMPUTED_CATALOG_REGION = oslo_cache.create_region() +MEMOIZE_COMPUTED_CATALOG = cache.get_memoization_decorator( + group='catalog', + region=COMPUTED_CATALOG_REGION) + def format_url(url, substitutions, silent_keyerror_failures=None): """Formats a user-defined URL with the given substitutions. @@ -148,6 +160,7 @@ class Manager(manager.Manager): raise exception.RegionNotFound(region_id=parent_region_id) notifications.Audit.created(self._REGION, ret['id'], initiator) + COMPUTED_CATALOG_REGION.invalidate() return ret @MEMOIZE @@ -166,6 +179,7 @@ class Manager(manager.Manager): ref = self.driver.update_region(region_id, region_ref) notifications.Audit.updated(self._REGION, region_id, initiator) self.get_region.invalidate(self, region_id) + COMPUTED_CATALOG_REGION.invalidate() return ref def delete_region(self, region_id, initiator=None): @@ -173,6 +187,7 @@ class Manager(manager.Manager): ret = self.driver.delete_region(region_id) notifications.Audit.deleted(self._REGION, region_id, initiator) self.get_region.invalidate(self, region_id) + COMPUTED_CATALOG_REGION.invalidate() return ret except exception.NotFound: raise exception.RegionNotFound(region_id=region_id) @@ -186,6 +201,7 @@ class Manager(manager.Manager): service_ref.setdefault('name', '') ref = self.driver.create_service(service_id, service_ref) notifications.Audit.created(self._SERVICE, service_id, initiator) + COMPUTED_CATALOG_REGION.invalidate() return ref @MEMOIZE @@ -199,6 +215,7 @@ class Manager(manager.Manager): ref = self.driver.update_service(service_id, service_ref) notifications.Audit.updated(self._SERVICE, service_id, initiator) self.get_service.invalidate(self, service_id) + COMPUTED_CATALOG_REGION.invalidate() return ref def delete_service(self, service_id, initiator=None): @@ -210,6 +227,7 @@ class Manager(manager.Manager): for endpoint in endpoints: if endpoint['service_id'] == service_id: self.get_endpoint.invalidate(self, endpoint['id']) + COMPUTED_CATALOG_REGION.invalidate() return ret except exception.NotFound: raise exception.ServiceNotFound(service_id=service_id) @@ -240,6 +258,7 @@ class Manager(manager.Manager): ref = self.driver.create_endpoint(endpoint_id, endpoint_ref) notifications.Audit.created(self._ENDPOINT, endpoint_id, initiator) + COMPUTED_CATALOG_REGION.invalidate() return ref def update_endpoint(self, endpoint_id, endpoint_ref, initiator=None): @@ -248,6 +267,7 @@ class Manager(manager.Manager): ref = self.driver.update_endpoint(endpoint_id, endpoint_ref) notifications.Audit.updated(self._ENDPOINT, endpoint_id, initiator) self.get_endpoint.invalidate(self, endpoint_id) + COMPUTED_CATALOG_REGION.invalidate() return ref def delete_endpoint(self, endpoint_id, initiator=None): @@ -255,6 +275,7 @@ class Manager(manager.Manager): ret = self.driver.delete_endpoint(endpoint_id) notifications.Audit.deleted(self._ENDPOINT, endpoint_id, initiator) self.get_endpoint.invalidate(self, endpoint_id) + COMPUTED_CATALOG_REGION.invalidate() return ret except exception.NotFound: raise exception.EndpointNotFound(endpoint_id=endpoint_id) @@ -270,12 +291,17 @@ class Manager(manager.Manager): def list_endpoints(self, hints=None): return self.driver.list_endpoints(hints or driver_hints.Hints()) + @MEMOIZE_COMPUTED_CATALOG def get_catalog(self, user_id, tenant_id): try: return self.driver.get_catalog(user_id, tenant_id) except exception.NotFound: raise exception.NotFound('Catalog not found for user and tenant') + @MEMOIZE_COMPUTED_CATALOG + def get_v3_catalog(self, user_id, tenant_id): + return self.driver.get_v3_catalog(user_id, tenant_id) + @six.add_metaclass(abc.ABCMeta) class CatalogDriverV8(object): diff --git a/keystone/common/cache/core.py b/keystone/common/cache/core.py index e8db82d5c2..10c467c962 100644 --- a/keystone/common/cache/core.py +++ b/keystone/common/cache/core.py @@ -23,12 +23,16 @@ CONF = cfg.CONF CACHE_REGION = cache.create_region() -def configure_cache(): - cache.configure_cache_region(CONF, CACHE_REGION) +def configure_cache(region=None): + if region is None: + region = CACHE_REGION + cache.configure_cache_region(CONF, region) -def get_memoization_decorator(group, expiration_group=None): - return cache.get_memoization_decorator(CONF, CACHE_REGION, group, +def get_memoization_decorator(group, expiration_group=None, region=None): + if region is None: + region = CACHE_REGION + return cache.get_memoization_decorator(CONF, region, group, expiration_group=expiration_group) diff --git a/keystone/contrib/endpoint_filter/core.py b/keystone/contrib/endpoint_filter/core.py index b66465ea33..a425b8443c 100644 --- a/keystone/contrib/endpoint_filter/core.py +++ b/keystone/contrib/endpoint_filter/core.py @@ -20,6 +20,7 @@ from oslo_config import cfg from oslo_log import log import six +from keystone import catalog from keystone.common import dependency from keystone.common import extension from keystone.common import manager @@ -63,6 +64,25 @@ class Manager(manager.Manager): def __init__(self): super(Manager, self).__init__(CONF.endpoint_filter.driver) + def add_endpoint_to_project(self, endpoint_id, project_id): + self.driver.add_endpoint_to_project(endpoint_id, project_id) + catalog.COMPUTED_CATALOG_REGION.invalidate() + + def remove_endpoint_to_project(self, endpoint_id, project_id): + self.driver.remove_endpoint_to_project(endpoint_id, project_id) + catalog.COMPUTED_CATALOG_REGION.invalidate() + + def add_endpoint_group_to_project(self, endpoint_group_id, project_id): + self.driver.add_endpoint_group_to_project( + endpoint_group_id, project_id) + catalog.COMPUTED_CATALOG_REGION.invalidate() + + def remove_endpoint_group_from_project(self, endpoint_group_id, + project_id): + self.driver.remove_endpoint_group_from_project( + endpoint_group_id, project_id) + catalog.COMPUTED_CATALOG_REGION.invalidate() + @six.add_metaclass(abc.ABCMeta) class EndpointFilterDriverV8(object): diff --git a/keystone/server/backends.py b/keystone/server/backends.py index dfc6b6565f..511b805739 100644 --- a/keystone/server/backends.py +++ b/keystone/server/backends.py @@ -31,6 +31,7 @@ def load_backends(): # Configure and build the cache cache.configure_cache() + cache.configure_cache(region=catalog.COMPUTED_CATALOG_REGION) # Ensure that the identity driver is created before the assignment manager # and that the assignment driver is created before the resource manager. diff --git a/keystone/tests/unit/core.py b/keystone/tests/unit/core.py index 794244d0fb..6727c38afc 100644 --- a/keystone/tests/unit/core.py +++ b/keystone/tests/unit/core.py @@ -208,6 +208,22 @@ def skip_if_cache_disabled(*sections): return wrapper +def skip_if_cache_is_enabled(*sections): + def wrapper(f): + @functools.wraps(f) + def inner(*args, **kwargs): + if CONF.cache.enabled: + for s in sections: + conf_sec = getattr(CONF, s, None) + if conf_sec is not None: + if getattr(conf_sec, 'caching', True): + raise testcase.TestSkipped('%s caching enabled.' % + s) + return f(*args, **kwargs) + return inner + return wrapper + + def skip_if_no_multiple_domains_support(f): """Decorator to skip tests for identity drivers limited to one domain.""" @functools.wraps(f) diff --git a/keystone/tests/unit/ksfixtures/cache.py b/keystone/tests/unit/ksfixtures/cache.py index 1160a1f4a9..cf315dcb05 100644 --- a/keystone/tests/unit/ksfixtures/cache.py +++ b/keystone/tests/unit/ksfixtures/cache.py @@ -13,9 +13,13 @@ import fixtures +from keystone import catalog from keystone.common import cache +CACHE_REGIONS = (cache.CACHE_REGION, catalog.COMPUTED_CATALOG_REGION) + + class Cache(fixtures.Fixture): """A fixture for setting up and tearing down the cache between test cases. """ @@ -29,8 +33,9 @@ class Cache(fixtures.Fixture): # NOTE(morganfainberg): The only way to reconfigure the CacheRegion # object on each setUp() call is to remove the .backend property. - if cache.CACHE_REGION.is_configured: - del cache.CACHE_REGION.backend + for region in CACHE_REGIONS: + if region.is_configured: + del region.backend - # ensure the cache region instance is setup - cache.configure_cache() + # ensure the cache region instance is setup + cache.configure_cache(region=region) diff --git a/keystone/tests/unit/test_backend_templated.py b/keystone/tests/unit/test_backend_templated.py index 393ef40f6f..f65c6d472d 100644 --- a/keystone/tests/unit/test_backend_templated.py +++ b/keystone/tests/unit/test_backend_templated.py @@ -66,6 +66,9 @@ class TestTemplatedCatalog(unit.TestCase, test_backend.CatalogTests): catalog_ref = self.catalog_api.get_catalog('foo', 'bar') self.assertDictEqual(self.DEFAULT_FIXTURE, catalog_ref) + # NOTE(lbragstad): This test is skipped because the catalog is being + # modified within the test and not through the API. + @unit.skip_if_cache_is_enabled('catalog') def test_catalog_ignored_malformed_urls(self): # both endpoints are in the catalog catalog_ref = self.catalog_api.get_catalog('foo', 'bar') @@ -126,9 +129,10 @@ class TestTemplatedCatalog(unit.TestCase, test_backend.CatalogTests): def test_get_catalog_ignores_endpoints_with_invalid_urls(self): user_id = uuid.uuid4().hex + tenant_id = None # 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) + catalog_ref = self.catalog_api.get_v3_catalog(user_id, tenant_id) exp_catalog = [ {'endpoints': [], 'type': 'compute', diff --git a/keystone/tests/unit/test_v3_catalog.py b/keystone/tests/unit/test_v3_catalog.py index 5f4c36871b..305f33b953 100644 --- a/keystone/tests/unit/test_v3_catalog.py +++ b/keystone/tests/unit/test_v3_catalog.py @@ -853,7 +853,8 @@ class TestCatalogAPISQL(unit.TestCase): # 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) + tenant_id = None + catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) self.assertThat(catalog[0]['endpoints'], matchers.HasLength(1)) def test_get_catalog_always_returns_service_name(self):