diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py index ad1f36b5a1..e6a6edf6f4 100644 --- a/keystone/catalog/backends/sql.py +++ b/keystone/catalog/backends/sql.py @@ -19,6 +19,7 @@ import sqlalchemy from sqlalchemy.sql import true from keystone.catalog.backends import base +from keystone.common import dependency from keystone.common import driver_hints from keystone.common import sql from keystone.common import utils @@ -81,6 +82,7 @@ class Endpoint(sql.ModelBase, sql.DictBase): extra = sql.Column(sql.JsonBlob()) +@dependency.requires('catalog_api') class Catalog(base.CatalogDriverBase): # Regions def list_regions(self, hints): @@ -373,7 +375,52 @@ class Catalog(base.CatalogDriverBase): service['name'] = svc.extra.get('name', '') return service - return [make_v3_service(svc) for svc in services] + # Build the unfiltered catalog, this is the catalog that is + # returned if endpoint filtering is not performed and the + # option of `return_all_endpoints_if_no_filter` is set to true. + catalog_ref = [make_v3_service(svc) for svc in services] + + # Filter the `catalog_ref` above by any project-endpoint + # association configured by endpoint filter. + filtered_endpoints = {} + if tenant_id: + filtered_endpoints = ( + self.catalog_api.list_endpoints_for_project(tenant_id)) + # endpoint filter is enabled, only return the filtered endpoints. + if filtered_endpoints: + filtered_ids = list(filtered_endpoints.keys()) + # This is actually working on the copy of `catalog_ref` since + # the index will be shifted if remove/add any entry for the + # original one. + for service in catalog_ref[:]: + endpoints = service['endpoints'] + for endpoint in endpoints[:]: + endpoint_id = endpoint['id'] + # remove the endpoint that is not associated with + # the project. + if endpoint_id not in filtered_ids: + service['endpoints'].remove(endpoint) + continue + # remove the disabled endpoint from the list. + if not filtered_endpoints[endpoint_id]['enabled']: + service['endpoints'].remove(endpoint) + # NOTE(davechen): The service will not be included in the + # catalog if the service doesn't have any endpoint when + # endpoint filter is enabled, this is inconsistent with + # full catalog that is returned when endpoint filter is + # disabled. + if not service.get('endpoints'): + catalog_ref.remove(service) + # When it arrives here it means it's domain scoped token ( + # `tenant_id` is not set) or it's a project scoped token + # but the endpoint filtering is not performed. + # Both of them tell us the endpoint filtering is not enabled, so + # check the option of `return_all_endpoints_if_no_filter`, it will + # judge whether a full unfiltered catalog or a empty service + # catalog will be returned. + elif not CONF.endpoint_filter.return_all_endpoints_if_no_filter: + return [] + return catalog_ref @sql.handle_conflicts(conflict_type='project_endpoint') def add_endpoint_to_project(self, endpoint_id, project_id): diff --git a/keystone/contrib/endpoint_filter/backends/catalog_sql.py b/keystone/contrib/endpoint_filter/backends/catalog_sql.py index b3c2ac5a3b..fb4b3555e7 100644 --- a/keystone/contrib/endpoint_filter/backends/catalog_sql.py +++ b/keystone/contrib/endpoint_filter/backends/catalog_sql.py @@ -12,66 +12,22 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_log import versionutils from keystone.catalog.backends import sql -from keystone.common import dependency -from keystone.common import utils import keystone.conf CONF = keystone.conf.CONF -@dependency.requires('catalog_api') +@versionutils.deprecated( + what=('keystone.contrib.endpoint_filter.' + 'backends.catalog_sql.EndPointFilterCatalog'), + as_of=versionutils.deprecated.OCATA, + remove_in=+1, + in_favor_of='keystone.catalog.backends.sql.Catalog') class EndpointFilterCatalog(sql.Catalog): def get_v3_catalog(self, user_id, project_id): - substitutions = dict(CONF.items()) - substitutions.update({ - 'tenant_id': project_id, - 'project_id': project_id, - 'user_id': user_id, - }) - - services = {} - - dict_of_endpoint_refs = (self.catalog_api. - list_endpoints_for_project(project_id)) - - if (not dict_of_endpoint_refs and - CONF.endpoint_filter.return_all_endpoints_if_no_filter): - return super(EndpointFilterCatalog, self).get_v3_catalog( - user_id, project_id) - - for endpoint_id, endpoint in dict_of_endpoint_refs.items(): - if not endpoint['enabled']: - # Skip disabled endpoints. - continue - service_id = endpoint['service_id'] - services.setdefault( - service_id, - self.get_service(service_id)) - service = services[service_id] - del endpoint['service_id'] - del endpoint['enabled'] - del endpoint['legacy_endpoint_id'] - # Include deprecated region for backwards compatibility - endpoint['region'] = endpoint['region_id'] - endpoint['url'] = utils.format_url( - endpoint['url'], substitutions) - # populate filtered endpoints - if 'endpoints' in services[service_id]: - service['endpoints'].append(endpoint) - else: - service['endpoints'] = [endpoint] - - # format catalog - catalog = [] - for service_id, service in services.items(): - formatted_service = {} - formatted_service['id'] = service['id'] - formatted_service['type'] = service['type'] - formatted_service['name'] = service['name'] - formatted_service['endpoints'] = service['endpoints'] - catalog.append(formatted_service) - - return catalog + return super(EndpointFilterCatalog, self).get_v3_catalog( + user_id, project_id) diff --git a/keystone/tests/unit/catalog/test_backends.py b/keystone/tests/unit/catalog/test_backends.py index 33b8cffb19..1afb89e56b 100644 --- a/keystone/tests/unit/catalog/test_backends.py +++ b/keystone/tests/unit/catalog/test_backends.py @@ -560,8 +560,11 @@ class CatalogTests(object): enabled_endpoint_ref = self._create_endpoints()[1] user_id = uuid.uuid4().hex - project_id = uuid.uuid4().hex - catalog = self.catalog_api.get_v3_catalog(user_id, project_id) + # Use the project created by the default fixture since the project + # should exist if we want to filter the catalog by the project or + # replace the url with a valid project id. + catalog = self.catalog_api.get_v3_catalog(user_id, + self.tenant_bar['id']) endpoint_ids = [x['id'] for x in catalog[0]['endpoints']] self.assertEqual([enabled_endpoint_ref['id']], endpoint_ids) diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index c6b73c72f8..898198481f 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -827,6 +827,70 @@ class SqlCatalog(SqlTests, catalog_tests.CatalogTests): self.catalog_api.delete_region, region['id']) + def test_v3_catalog_domain_scoped_token(self): + # test the case that tenant_id is None. + srv_1 = unit.new_service_ref() + self.catalog_api.create_service(srv_1['id'], srv_1) + endpoint_1 = unit.new_endpoint_ref(service_id=srv_1['id'], + region_id=None) + self.catalog_api.create_endpoint(endpoint_1['id'], endpoint_1) + + srv_2 = unit.new_service_ref() + self.catalog_api.create_service(srv_2['id'], srv_2) + endpoint_2 = unit.new_endpoint_ref(service_id=srv_2['id'], + region_id=None) + self.catalog_api.create_endpoint(endpoint_2['id'], endpoint_2) + + self.config_fixture.config(group='endpoint_filter', + return_all_endpoints_if_no_filter=True) + catalog_ref = self.catalog_api.get_v3_catalog(uuid.uuid4().hex, None) + self.assertThat(catalog_ref, matchers.HasLength(2)) + self.config_fixture.config(group='endpoint_filter', + return_all_endpoints_if_no_filter=False) + catalog_ref = self.catalog_api.get_v3_catalog(uuid.uuid4().hex, None) + self.assertThat(catalog_ref, matchers.HasLength(0)) + + def test_v3_catalog_endpoint_filter_enabled(self): + srv_1 = unit.new_service_ref() + self.catalog_api.create_service(srv_1['id'], srv_1) + endpoint_1 = unit.new_endpoint_ref(service_id=srv_1['id'], + region_id=None) + self.catalog_api.create_endpoint(endpoint_1['id'], endpoint_1) + endpoint_2 = unit.new_endpoint_ref(service_id=srv_1['id'], + region_id=None) + self.catalog_api.create_endpoint(endpoint_2['id'], endpoint_2) + # create endpoint-project association. + self.catalog_api.add_endpoint_to_project( + endpoint_1['id'], + self.tenant_bar['id']) + + catalog_ref = self.catalog_api.get_v3_catalog(uuid.uuid4().hex, + self.tenant_bar['id']) + self.assertThat(catalog_ref, matchers.HasLength(1)) + self.assertThat(catalog_ref[0]['endpoints'], matchers.HasLength(1)) + # the endpoint is that defined in the endpoint-project association. + self.assertEqual(endpoint_1['id'], + catalog_ref[0]['endpoints'][0]['id']) + + def test_v3_catalog_endpoint_filter_disabled(self): + # there is no endpoint-project association defined. + self.config_fixture.config(group='endpoint_filter', + return_all_endpoints_if_no_filter=True) + srv_1 = unit.new_service_ref() + self.catalog_api.create_service(srv_1['id'], srv_1) + endpoint_1 = unit.new_endpoint_ref(service_id=srv_1['id'], + region_id=None) + self.catalog_api.create_endpoint(endpoint_1['id'], endpoint_1) + + srv_2 = unit.new_service_ref() + self.catalog_api.create_service(srv_2['id'], srv_2) + + catalog_ref = self.catalog_api.get_v3_catalog(uuid.uuid4().hex, + self.tenant_bar['id']) + self.assertThat(catalog_ref, matchers.HasLength(2)) + srv_id_list = [catalog_ref[0]['id'], catalog_ref[1]['id']] + self.assertItemsEqual([srv_1['id'], srv_2['id']], srv_id_list) + class SqlPolicy(SqlTests, policy_tests.PolicyTests): pass diff --git a/keystone/tests/unit/test_v3_catalog.py b/keystone/tests/unit/test_v3_catalog.py index e33e4e4548..01ba75438c 100644 --- a/keystone/tests/unit/test_v3_catalog.py +++ b/keystone/tests/unit/test_v3_catalog.py @@ -18,7 +18,6 @@ import uuid from six.moves import http_client from testtools import matchers -from keystone import catalog from keystone.tests import unit from keystone.tests.unit.ksfixtures import database from keystone.tests.unit import test_v3 @@ -803,7 +802,7 @@ class TestCatalogAPISQL(unit.TestCase): def setUp(self): super(TestCatalogAPISQL, self).setUp() self.useFixture(database.Database()) - self.catalog_api = catalog.Manager() + self.load_backends() service = unit.new_service_ref() self.service_id = service['id'] @@ -824,10 +823,17 @@ class TestCatalogAPISQL(unit.TestCase): def test_get_catalog_ignores_endpoints_with_invalid_urls(self): user_id = uuid.uuid4().hex - tenant_id = uuid.uuid4().hex + + # create a project since the project should exist if we want to + # filter the catalog by the project or replace the url with a + # valid project id. + domain = unit.new_domain_ref() + self.resource_api.create_domain(domain['id'], domain) + project = unit.new_project_ref(domain_id=domain['id']) + self.resource_api.create_project(project['id'], project) # the only endpoint in the catalog is the one created in setUp - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) self.assertEqual(1, len(catalog[0]['endpoints'])) # it's also the only endpoint in the backend self.assertEqual(1, len(self.catalog_api.list_endpoints())) @@ -841,7 +847,7 @@ class TestCatalogAPISQL(unit.TestCase): url='http://keystone/%(you_wont_find_me)s') # verify that the invalid endpoints don't appear in the catalog - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) self.assertEqual(1, len(catalog[0]['endpoints'])) # all three appear in the backend self.assertEqual(3, len(self.catalog_api.list_endpoints())) @@ -851,7 +857,7 @@ class TestCatalogAPISQL(unit.TestCase): url='http://keystone/%(tenant_id)s') # there are two valid endpoints, positive check - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) self.assertThat(catalog[0]['endpoints'], matchers.HasLength(2)) # If the URL has no 'tenant_id' to substitute, we will skip the @@ -862,7 +868,13 @@ class TestCatalogAPISQL(unit.TestCase): def test_get_catalog_always_returns_service_name(self): user_id = uuid.uuid4().hex - tenant_id = uuid.uuid4().hex + # create a project since the project should exist if we want to + # filter the catalog by the project or replace the url with a + # valid project id. + domain = unit.new_domain_ref() + self.resource_api.create_domain(domain['id'], domain) + project = unit.new_project_ref(domain_id=domain['id']) + self.resource_api.create_project(project['id'], project) # create a service, with a name named_svc = unit.new_service_ref() @@ -875,7 +887,7 @@ class TestCatalogAPISQL(unit.TestCase): self.catalog_api.create_service(unnamed_svc['id'], unnamed_svc) self.create_endpoint(service_id=unnamed_svc['id']) - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) named_endpoint = [ep for ep in catalog if ep['type'] == named_svc['type']][0] @@ -894,7 +906,7 @@ class TestCatalogAPISQLRegions(unit.TestCase): def setUp(self): super(TestCatalogAPISQLRegions, self).setUp() self.useFixture(database.Database()) - self.catalog_api = catalog.Manager() + self.load_backends() def config_overrides(self): super(TestCatalogAPISQLRegions, self).config_overrides() @@ -910,10 +922,15 @@ class TestCatalogAPISQLRegions(unit.TestCase): del endpoint['region_id'] self.catalog_api.create_endpoint(endpoint['id'], endpoint) + # create a project since the project should exist if we want to + # filter the catalog by the project or replace the url with a + # valid project id. + domain = unit.new_domain_ref() + self.resource_api.create_domain(domain['id'], domain) + project = unit.new_project_ref(domain_id=domain['id']) + self.resource_api.create_project(project['id'], project) user_id = uuid.uuid4().hex - tenant_id = uuid.uuid4().hex - - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) self.assertValidCatalogEndpoint( catalog[0]['endpoints'][0], ref=endpoint) @@ -929,9 +946,15 @@ class TestCatalogAPISQLRegions(unit.TestCase): endpoint = self.catalog_api.get_endpoint(endpoint['id']) user_id = uuid.uuid4().hex - tenant_id = uuid.uuid4().hex + # create a project since the project should exist if we want to + # filter the catalog by the project or replace the url with a + # valid project id. + domain = unit.new_domain_ref() + self.resource_api.create_domain(domain['id'], domain) + project = unit.new_project_ref(domain_id=domain['id']) + self.resource_api.create_project(project['id'], project) - catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id) + catalog = self.catalog_api.get_v3_catalog(user_id, project['id']) self.assertValidCatalogEndpoint( catalog[0]['endpoints'][0], ref=endpoint) diff --git a/releasenotes/notes/deprecated-as-of-ocata-a5b2f1e3e39f818e.yaml b/releasenotes/notes/deprecated-as-of-ocata-a5b2f1e3e39f818e.yaml new file mode 100644 index 0000000000..52f7522102 --- /dev/null +++ b/releasenotes/notes/deprecated-as-of-ocata-a5b2f1e3e39f818e.yaml @@ -0,0 +1,9 @@ +--- +deprecations: + - > + [`blueprint deprecated-as-of-ocata `_] + The catalog backend ``endpoint_filter.sql``has been deprecated in the + `Ocata` release, it has been consolidated with the ``sql`` backend. + It's recommended to replace the ``endpoint_filter.sql`` catalog backend + with the ``sql`` backend. The ``endpoint_filter.sql`` backend will be + removed in the `Pike` release.