From fd3e6276a3fb90506ef7e3cb08efe7cb3f94c1ca Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Sun, 25 Sep 2016 11:50:28 +0800 Subject: [PATCH] Deprecate `endpoint_filter.sql` backend `endpoint_filter.sql` backend is the only left-over from endpoint filter extension, all others has been moved into keystone catalog dir. This patch deprecate `endpoint_filter.sql` backend and consolidate the backend with SQL backend. This patch also update some related testcases to make sure project id exists instead of some random uuids since original logic from endpoint filter extension has the constraint and this is make sense to inherent into SQL backend as well. Partially implements: bp deprecated-as-of-ocata Change-Id: I28b37fc98cf63da11c0dd200b5f657507c0bca6a --- keystone/catalog/backends/sql.py | 49 +++++++++++++- .../endpoint_filter/backends/catalog_sql.py | 62 +++--------------- keystone/tests/unit/catalog/test_backends.py | 7 +- keystone/tests/unit/test_backend_sql.py | 64 +++++++++++++++++++ keystone/tests/unit/test_v3_catalog.py | 51 +++++++++++---- ...precated-as-of-ocata-a5b2f1e3e39f818e.yaml | 9 +++ 6 files changed, 172 insertions(+), 70 deletions(-) create mode 100644 releasenotes/notes/deprecated-as-of-ocata-a5b2f1e3e39f818e.yaml 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 d7bbda20d9..5878a29e6d 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.