Merge "Deprecate `endpoint_filter.sql` backend"

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):

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

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
+        # Use the project created by the default fixture since the project
-        catalog = self.catalog_api.get_v3_catalog(user_id, project_id)
+        # 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)

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

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, project['id'])
-
-        catalog = self.catalog_api.get_v3_catalog(user_id, tenant_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)
 

releasenotes/notes/deprecated-as-of-ocata-a5b2f1e3e39f818e.yaml

@@ -0,0 +1,9 @@
+---
+deprecations:
+  - >
+    [`blueprint deprecated-as-of-ocata <https://blueprints.launchpad.net/keystone/+spec/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.