From dbd98a4c857411bf33184332724270c980c99f36 Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Wed, 13 Nov 2013 18:05:22 +0800 Subject: [PATCH] Fix errors for create_endpoint api in version2 1. Create an endpoint without 'service_id' should return a 400 Bad Request with appropriate message instead of 500 Internal Server Error. 2. Empty urls should be discarded. Change-Id: I535467ec35fe2ae738ecae0a4f20d19ff4f001ec Closes-Bug: #1250765 --- keystone/catalog/controllers.py | 14 +++++++++----- keystone/tests/test_catalog.py | 27 +++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/keystone/catalog/controllers.py b/keystone/catalog/controllers.py index 8589483911..ccf4dfd9a5 100644 --- a/keystone/catalog/controllers.py +++ b/keystone/catalog/controllers.py @@ -91,18 +91,22 @@ class Endpoint(controller.V2Controller): # according to the v2 spec publicurl is mandatory self._require_attribute(endpoint, 'publicurl') + # service_id is necessary + self._require_attribute(endpoint, 'service_id') legacy_endpoint_ref = endpoint.copy() 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: + url = '%surl' % i + if endpoint.get(url): # 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) + urls[i] = endpoint.pop(url) + elif url in endpoint: + # null or empty urls can be discarded + endpoint.pop(url) + legacy_endpoint_ref.pop(url) legacy_endpoint_id = uuid.uuid4().hex for interface, url in urls.iteritems(): diff --git a/keystone/tests/test_catalog.py b/keystone/tests/test_catalog.py index f629365910..c7e2f52308 100644 --- a/keystone/tests/test_catalog.py +++ b/keystone/tests/test_catalog.py @@ -91,10 +91,33 @@ class V2CatalogTestCase(rest.RestfulTestCase): self.assertEqual(response.result['endpoint'][field], value) def test_endpoint_create_with_null_adminurl(self): - self._endpoint_create(adminurl=None) + req_body, response = self._endpoint_create(adminurl=None) + self.assertEqual(req_body['endpoint']['adminurl'], None) + self.assertNotIn('adminurl', response.result['endpoint']) + + def test_endpoint_create_with_empty_adminurl(self): + req_body, response = self._endpoint_create(adminurl='') + self.assertEqual(req_body['endpoint']['adminurl'], '') + self.assertNotIn("adminurl", response.result['endpoint']) def test_endpoint_create_with_null_internalurl(self): - self._endpoint_create(internalurl=None) + req_body, response = self._endpoint_create(internalurl=None) + self.assertEqual(req_body['endpoint']['internalurl'], None) + self.assertNotIn('internalurl', response.result['endpoint']) + + def test_endpoint_create_with_empty_internalurl(self): + req_body, response = self._endpoint_create(internalurl='') + self.assertEqual(req_body['endpoint']['internalurl'], '') + self.assertNotIn("internalurl", response.result['endpoint']) def test_endpoint_create_with_null_publicurl(self): self._endpoint_create(expected_status=400, publicurl=None) + + def test_endpoint_create_with_empty_publicurl(self): + self._endpoint_create(expected_status=400, publicurl='') + + def test_endpoint_create_with_null_service_id(self): + self._endpoint_create(expected_status=400, service_id=None) + + def test_endpoint_create_with_empty_service_id(self): + self._endpoint_create(expected_status=400, service_id='')