From 0a81b69ef696264654c37213f4954f222fc78700 Mon Sep 17 00:00:00 2001
From: Dolph Mathews <dolph.mathews@gmail.com>
Date: Wed, 13 Mar 2013 21:59:38 -0500
Subject: [PATCH] Discard null endpoints (bug 1152632)

If a v2 client passed {..., "adminurl": null, ...} in a endpoint-create
request, then the null value was being persisted on an endpoint with a
different interface value (i.e. a publicly facing endpoint would have an
"adminurl": null value inexplicably attached to it.)

This change simply pops null urls from the endpoint and discards them.

Change-Id: Idd0964b6ec34fbc8b979253d32f655ea9797f259
---
 keystone/catalog/controllers.py | 12 ++++++--
 tests/test_v3.py                |  1 +
 tests/test_v3_catalog.py        | 49 +++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/keystone/catalog/controllers.py b/keystone/catalog/controllers.py
index 0269a169c4..3e9b7309c0 100644
--- a/keystone/catalog/controllers.py
+++ b/keystone/catalog/controllers.py
@@ -86,9 +86,15 @@ class Endpoint(controller.V2Controller):
 
         legacy_endpoint_ref = endpoint.copy()
 
-        # pop all urls off the endpoint so we don't persist them more than once
-        urls = dict((i, endpoint.pop('%surl' % i)) for i in INTERFACES
-                    if endpoint.get('%surl' % i) is not None)
+        urls = {}
+        for i in INTERFACES:
+            # remove all urls so they aren't persisted them more than once
+            if endpoint.get('%surl' % i) is not None:
+                # valid urls need to be persisted
+                urls[i] = endpoint.pop('%surl' % i)
+            elif '%surl' % i in endpoint:
+                # null urls can be discarded
+                endpoint.pop('%surl' % i)
 
         legacy_endpoint_id = uuid.uuid4().hex
         for interface, url in urls.iteritems():
diff --git a/tests/test_v3.py b/tests/test_v3.py
index 2facfe370c..f75a865afc 100644
--- a/tests/test_v3.py
+++ b/tests/test_v3.py
@@ -98,6 +98,7 @@ class RestfulTestCase(test_content_types.RestfulTestCase):
         ref['interface'] = uuid.uuid4().hex[:8]
         ref['service_id'] = service_id
         ref['url'] = uuid.uuid4().hex
+        ref['region'] = uuid.uuid4().hex
         return ref
 
     def new_domain_ref(self):
diff --git a/tests/test_v3_catalog.py b/tests/test_v3_catalog.py
index 67cbd34025..e81308fc21 100644
--- a/tests/test_v3_catalog.py
+++ b/tests/test_v3_catalog.py
@@ -114,3 +114,52 @@ class CatalogTestCase(test_v3.RestfulTestCase):
         self.delete(
             '/endpoints/%(endpoint_id)s' % {
                 'endpoint_id': self.endpoint_id})
+
+    def test_create_endpoint_on_v2(self):
+        # clear the v3 endpoint so we only have endpoints created on v2
+        self.delete(
+            '/endpoints/%(endpoint_id)s' % {
+                'endpoint_id': self.endpoint_id})
+
+        # create a v3 endpoint ref, and then tweak it back to a v2-style ref
+        ref = self.new_endpoint_ref(service_id=self.service['id'])
+        del ref['id']
+        del ref['interface']
+        ref['publicurl'] = ref.pop('url')
+        ref['internalurl'] = None
+        # don't set adminurl to ensure it's absence is handled like internalurl
+
+        # create the endpoint on v2 (using a v3 token)
+        r = self.admin_request(
+            method='POST',
+            path='/v2.0/endpoints',
+            token=self.get_scoped_token(),
+            body={'endpoint': ref})
+        endpoint_v2 = r.body['endpoint']
+
+        # test the endpoint on v3
+        r = self.get('/endpoints')
+        endpoints = self.assertValidEndpointListResponse(r)
+        self.assertEqual(len(endpoints), 1)
+        endpoint_v3 = endpoints.pop()
+
+        # these attributes are identical between both API's
+        self.assertEqual(endpoint_v3['region'], ref['region'])
+        self.assertEqual(endpoint_v3['service_id'], ref['service_id'])
+        self.assertEqual(endpoint_v3['description'], ref['description'])
+
+        # a v2 endpoint is not quite the same concept as a v3 endpoint, so they
+        # receive different identifiers
+        self.assertNotEqual(endpoint_v2['id'], endpoint_v3['id'])
+
+        # v2 has a publicurl; v3 has a url + interface type
+        self.assertEqual(endpoint_v3['url'], ref['publicurl'])
+        self.assertEqual(endpoint_v3['interface'], 'public')
+
+        # tests for bug 1152632 -- these attributes were being returned by v3
+        self.assertNotIn('publicurl', endpoint_v3)
+        self.assertNotIn('adminurl', endpoint_v3)
+        self.assertNotIn('internalurl', endpoint_v3)
+
+        # test for bug 1152635 -- this attribute was being returned by v3
+        self.assertNotIn('legacy_endpoint_id', endpoint_v3)