Ignore broken endpoints in get_catalog

If an endpoint is created with a malformed URL then get_catalog would
throw a HTTP 500 error. With this change we ignore all malformed
endpoints.

Change-Id: Ibe9a8fa49f410f2a76a0df732247bd6813fc734b
Partial-bug: #1347862
(cherry picked from commit 8737b43d2c)

keystone/catalog/backends/sql.py

@@ -261,6 +261,10 @@ class Catalog(catalog.Driver):
         for endpoint in endpoints:
             if not endpoint.service['enabled']:
                 continue
+            try:
+                url = core.format_url(endpoint['url'], d)
+            except exception.MalformedEndpoint:
+                continue  # this failure is already logged in format_url()
 
             region = endpoint['region']
             service_type = endpoint.service['type']
@@ -271,7 +275,6 @@ class Catalog(catalog.Driver):
             }
             catalog.setdefault(region, {})
             catalog[region].setdefault(service_type, default_service)
-            url = core.format_url(endpoint['url'], d)
             interface_url = '%sURL' % endpoint['interface']
             catalog[region][service_type][interface_url] = url
 

keystone/catalog/backends/templated.py

@@ -115,9 +115,13 @@ class Catalog(kvs.Catalog):
         for region, region_ref in six.iteritems(self.templates):
             o[region] = {}
             for service, service_ref in six.iteritems(region_ref):
-                o[region][service] = {}
-                for k, v in six.iteritems(service_ref):
-                    o[region][service][k] = core.format_url(v, d)
+                service_data = {}
+                try:
+                    for k, v in six.iteritems(service_ref):
+                        service_data[k] = core.format_url(v, d)
+                except exception.MalformedEndpoint:
+                    continue  # this failure is already logged in format_url()
+                o[region][service] = service_data
 
         return o
 

keystone/tests/test_backend_sql.py

@@ -382,7 +382,7 @@ class SqlToken(SqlTests, test_backend.TokenTests):
 
 
 class SqlCatalog(SqlTests, test_backend.CatalogTests):
-    def test_malformed_catalog_throws_error(self):
+    def test_catalog_ignored_malformed_urls(self):
         service = {
             'id': uuid.uuid4().hex,
             'type': uuid.uuid4().hex,
@@ -401,10 +401,9 @@ class SqlCatalog(SqlTests, test_backend.CatalogTests):
         }
         self.catalog_api.create_endpoint(endpoint['id'], endpoint.copy())
 
-        self.assertRaises(exception.MalformedEndpoint,
-                          self.catalog_api.get_catalog,
-                          'fake-user',
-                          'fake-tenant')
+        # NOTE(dstanek): there are no valid URLs, so nothing is in the catalog
+        catalog = self.catalog_api.get_catalog('fake-user', 'fake-tenant')
+        self.assertEqual({}, catalog)
 
     def test_get_catalog_with_empty_public_url(self):
         service = {

keystone/tests/test_backend_templated.py

@@ -60,14 +60,18 @@ class TestTemplatedCatalog(tests.TestCase, test_backend.CatalogTests):
         catalog_ref = self.catalog_api.get_catalog('foo', 'bar')
         self.assertDictEqual(catalog_ref, self.DEFAULT_FIXTURE)
 
-    def test_malformed_catalog_throws_error(self):
+    def test_catalog_ignored_malformed_urls(self):
+        # both endpoints are in the catalog
+        catalog_ref = self.catalog_api.get_catalog('foo', 'bar')
+        self.assertEqual(2, len(catalog_ref['RegionOne']))
+
         (self.catalog_api.driver.templates
          ['RegionOne']['compute']['adminURL']) = \
             'http://localhost:$(compute_port)s/v1.1/$(tenant)s'
-        self.assertRaises(exception.MalformedEndpoint,
-                          self.catalog_api.get_catalog,
-                          'fake-user',
-                          'fake-tenant')
+
+        # the malformed one has been removed
+        catalog_ref = self.catalog_api.get_catalog('foo', 'bar')
+        self.assertEqual(1, len(catalog_ref['RegionOne']))
 
     def test_get_catalog_endpoint_disabled(self):
         self.skipTest("Templated backend doesn't have disabled endpoints")

keystone/tests/test_catalog.py

@@ -16,6 +16,9 @@ import uuid
 
 import six
 
+from keystone import catalog
+from keystone import tests
+from keystone.tests import default_fixtures
 from keystone.tests import rest
 
 
@@ -118,3 +121,63 @@ class V2CatalogTestCase(rest.RestfulTestCase):
 
     def test_endpoint_create_with_empty_service_id(self):
         self._endpoint_create(expected_status=400, service_id='')
+
+
+class TestV2CatalogAPISQL(tests.TestCase):
+
+    def setUp(self):
+        super(TestV2CatalogAPISQL, self).setUp()
+        self.load_backends()
+        self.load_fixtures(default_fixtures)
+
+        self.catalog_api = catalog.Manager()
+
+        self.service_id = uuid.uuid4().hex
+        service = {'id': self.service_id, 'name': uuid.uuid4().hex}
+        self.catalog_api.create_service(self.service_id, service)
+
+        endpoint = self.new_endpoint_ref(service_id=self.service_id)
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+    def config_overrides(self):
+        super(TestV2CatalogAPISQL, self).config_overrides()
+        self.config_fixture.config(
+            group='catalog',
+            driver='keystone.catalog.backends.sql.Catalog')
+
+    def new_endpoint_ref(self, service_id):
+        return {
+            'id': uuid.uuid4().hex,
+            'name': uuid.uuid4().hex,
+            'description': uuid.uuid4().hex,
+            'interface': uuid.uuid4().hex[:8],
+            'service_id': service_id,
+            'url': uuid.uuid4().hex,
+            'region': uuid.uuid4().hex,
+        }
+
+    def test_get_catalog_ignores_endpoints_with_invalid_urls(self):
+        user_id = uuid.uuid4().hex
+        tenant_id = uuid.uuid4().hex
+
+        # the only endpoint in the catalog is the one created in setUp
+        catalog = self.catalog_api.get_catalog(user_id, tenant_id)
+        self.assertEqual(1, len(catalog))
+        # it's also the only endpoint in the backend
+        self.assertEqual(1, len(self.catalog_api.list_endpoints()))
+
+        # create a new, invalid endpoint - malformed type declaration
+        endpoint = self.new_endpoint_ref(self.service_id)
+        endpoint['url'] = 'http://keystone/%(tenant_id)'
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        # create a new, invalid endpoint - nonexistent key
+        endpoint = self.new_endpoint_ref(self.service_id)
+        endpoint['url'] = 'http://keystone/%(you_wont_find_me)s'
+        self.catalog_api.create_endpoint(endpoint['id'], endpoint)
+
+        # verify that the invalid endpoints don't appear in the catalog
+        catalog = self.catalog_api.get_catalog(user_id, tenant_id)
+        self.assertEqual(1, len(catalog))
+        # all three endpoints appear in the backend
+        self.assertEqual(3, len(self.catalog_api.list_endpoints()))