From 7e4aab33e2d38a87e26f9f4e65b4ba2097564fd2 Mon Sep 17 00:00:00 2001
From: David Stanek <dstanek@dstanek.com>
Date: Tue, 18 Mar 2014 14:05:52 +0000
Subject: [PATCH] Ignore broken endpoints in get_v3_catalog

Change-Id: Ifd858e9f37155e3806329c4688be494d0132a9c7
Partial-bug: #1230279
---
 keystone/catalog/backends/sql.py  | 19 +++++----
 keystone/tests/test_v3_catalog.py | 64 +++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index 160e5c53f..0ffbcd69c 100644
--- a/keystone/catalog/backends/sql.py
+++ b/keystone/catalog/backends/sql.py
@@ -283,17 +283,20 @@ class Catalog(catalog.Driver):
                     options(sql.joinedload(Service.endpoints)).
                     all())
 
-        def make_v3_endpoint(endpoint):
-            endpoint = endpoint.to_dict()
-            del endpoint['service_id']
-            del endpoint['legacy_endpoint_id']
-            del endpoint['enabled']
+        def make_v3_endpoints(endpoints):
+            for endpoint in (ep.to_dict() for ep in endpoints if ep.enabled):
+                del endpoint['service_id']
+                del endpoint['legacy_endpoint_id']
+                del endpoint['enabled']
+                try:
+                    endpoint['url'] = core.format_url(endpoint['url'], d)
+                except exception.MalformedEndpoint:
+                    continue  # this failure is already logged in format_url()
 
-            endpoint['url'] = core.format_url(endpoint['url'], d)
-            return endpoint
+                yield endpoint
 
         def make_v3_service(svc):
-            eps = [make_v3_endpoint(ep) for ep in svc.endpoints if ep.enabled]
+            eps = list(make_v3_endpoints(svc.endpoints))
             service = {'endpoints': eps, 'id': svc.id, 'type': svc.type}
             name = svc.extra.get('name')
             if name:
diff --git a/keystone/tests/test_v3_catalog.py b/keystone/tests/test_v3_catalog.py
index ab64be4eb..c8699f9ad 100644
--- a/keystone/tests/test_v3_catalog.py
+++ b/keystone/tests/test_v3_catalog.py
@@ -15,6 +15,9 @@
 import copy
 import uuid
 
+from keystone import catalog
+from keystone import tests
+from keystone.tests.ksfixtures import database
 from keystone.tests import test_v3
 
 
@@ -414,3 +417,64 @@ class CatalogTestCase(test_v3.RestfulTestCase):
 
         # test for bug 1152635 -- this attribute was being returned by v3
         self.assertNotIn('legacy_endpoint_id', endpoint_v3)
+
+
+class TestCatalogAPISQL(tests.TestCase):
+    """Tests for the catalog Manager against the SQL backend.
+
+    """
+
+    def setUp(self):
+        super(TestCatalogAPISQL, self).setUp()
+        self.useFixture(database.Database())
+        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(TestCatalogAPISQL, 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_v3_catalog(user_id, tenant_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()))
+
+        # create a new, invalid endpoint - malformed type declaration
+        ref = self.new_endpoint_ref(self.service_id)
+        ref['url'] = 'http://keystone/%(tenant_id)'
+        self.catalog_api.create_endpoint(ref['id'], ref)
+
+        # create a new, invalid endpoint - nonexistent key
+        ref = self.new_endpoint_ref(self.service_id)
+        ref['url'] = 'http://keystone/%(you_wont_find_me)s'
+        self.catalog_api.create_endpoint(ref['id'], ref)
+
+        # verify that the invalid endpoints don't appear in the catalog
+        catalog = self.catalog_api.get_v3_catalog(user_id, tenant_id)
+        self.assertEqual(1, len(catalog[0]['endpoints']))
+        # all three appear in the backend
+        self.assertEqual(3, len(self.catalog_api.list_endpoints()))