Merge "Add service name to catalog"

4 files changed, 126 insertions, 20 deletions

diff --git a/keystoneclient/service_catalog.py b/keystoneclient/service_catalog.py
index 917c598..3018e34 100644
--- a/keystoneclient/service_catalog.py
+++ b/keystoneclient/service_catalog.py
@@ -84,11 +84,15 @@ class ServiceCatalog(object):
         """
 
     def get_endpoints(self, service_type=None, endpoint_type=None,
-                      region_name=None):
+                      region_name=None, service_name=None):
         """Fetch and filter endpoints for the specified service(s).
 
         Returns endpoints for the specified service (or all) containing
-        the specified type (or all) and region (or all).
+        the specified type (or all) and region (or all) and service name.
+
+        If there is no name in the service catalog the service_name check will
+        be skipped.  This allows compatibility with services that existed
+        before the name was available in the catalog.
         """
         endpoint_type = self._normalize_endpoint_type(endpoint_type)
         region_name = region_name or self._region_name
@@ -104,6 +108,21 @@ class ServiceCatalog(object):
             if service_type and service_type != st:
                 continue
 
+            # NOTE(jamielennox): service_name is different. It is not available
+            # in API < v3.3. If it is in the catalog then we enforce it, if it
+            # is not then we don't because the name could be correct we just
+            # don't have that information to check against.
+            if service_name:
+                try:
+                    sn = service['name']
+                except KeyError:
+                    # assume that we're in v3.0-v3.2 and don't have the name in
+                    # the catalog. Skip the check.
+                    pass
+                else:
+                    if service_name != sn:
+                        continue
+
             sc[st] = []
 
             for endpoint in service.get('endpoints', []):
@@ -117,13 +136,14 @@ class ServiceCatalog(object):
         return sc
 
     def _get_service_endpoints(self, attr, filter_value, service_type,
-                               endpoint_type, region_name):
+                               endpoint_type, region_name, service_name):
         """Fetch the endpoints of a particular service_type and handle
         the filtering.
         """
         sc_endpoints = self.get_endpoints(service_type=service_type,
                                           endpoint_type=endpoint_type,
-                                          region_name=region_name)
+                                          region_name=region_name,
+                                          service_name=service_name)
 
         try:
             endpoints = sc_endpoints[service_type]
@@ -144,7 +164,7 @@ class ServiceCatalog(object):
     @utils.positional(enforcement=utils.positional.WARN)
     def get_urls(self, attr=None, filter_value=None,
                  service_type='identity', endpoint_type='publicURL',
-                 region_name=None):
+                 region_name=None, service_name=None):
         """Fetch endpoint urls from the service catalog.
 
         Fetch the endpoints from the service catalog for a particular
@@ -159,6 +179,7 @@ class ServiceCatalog(object):
                                      internal or internalURL, admin or
                                      adminURL
         :param string region_name: Region of the endpoint.
+        :param string service_name: The assigned name of the service.
 
         :returns: tuple of urls or None (if no match found)
         """
@@ -167,7 +188,7 @@ class ServiceCatalog(object):
     @utils.positional(3, enforcement=utils.positional.WARN)
     def url_for(self, attr=None, filter_value=None,
                 service_type='identity', endpoint_type='publicURL',
-                region_name=None):
+                region_name=None, service_name=None):
         """Fetch an endpoint from the service catalog.
 
         Fetch the specified endpoint from the service catalog for
@@ -177,6 +198,14 @@ class ServiceCatalog(object):
         Valid endpoint types: `public` or `publicURL`,
                               `internal` or `internalURL`,
                               `admin` or 'adminURL`
+
+        :param string attr: Endpoint attribute name.
+        :param string filter_value: Endpoint attribute value.
+        :param string service_type: Service type of the endpoint.
+        :param string endpoint_type: Type of endpoint.
+        :param string region_name: Region of the endpoint.
+        :param string service_name: The assigned name of the service.
+        :
         """
         if not self.get_data():
             raise exceptions.EmptyCatalog('The service catalog is empty.')
@@ -185,19 +214,20 @@ class ServiceCatalog(object):
                              filter_value=filter_value,
                              service_type=service_type,
                              endpoint_type=endpoint_type,
-                             region_name=region_name)
+                             region_name=region_name,
+                             service_name=service_name)
 
         try:
             return urls[0]
         except Exception:
             pass
 
-        msg = '%(endpoint)s endpoint for %(service)s%(region)s not found'
-        region = ' in %s region' % region_name if region_name else ''
-        msg = msg % {'endpoint': endpoint_type,
-                     'service': service_type,
-                     'region': region}
-
+        msg = '%s endpoint for %s service' % (endpoint_type, service_type)
+        if service_name:
+            msg += ' named %s' % service_name
+        if region_name:
+            msg += ' in %s region' % region_name
+        msg += ' not found'
         raise exceptions.EndpointNotFound(msg)
 
     @abc.abstractmethod
@@ -254,13 +284,14 @@ class ServiceCatalogV2(ServiceCatalog):
     @utils.positional(enforcement=utils.positional.WARN)
     def get_urls(self, attr=None, filter_value=None,
                  service_type='identity', endpoint_type='publicURL',
-                 region_name=None):
+                 region_name=None, service_name=None):
         endpoint_type = self._normalize_endpoint_type(endpoint_type)
         endpoints = self._get_service_endpoints(attr=attr,
                                                 filter_value=filter_value,
                                                 service_type=service_type,
                                                 endpoint_type=endpoint_type,
-                                                region_name=region_name)
+                                                region_name=region_name,
+                                                service_name=service_name)
 
         if endpoints:
             return tuple([endpoint[endpoint_type] for endpoint in endpoints])
@@ -319,12 +350,13 @@ class ServiceCatalogV3(ServiceCatalog):
     @utils.positional(enforcement=utils.positional.WARN)
     def get_urls(self, attr=None, filter_value=None,
                  service_type='identity', endpoint_type='public',
-                 region_name=None):
+                 region_name=None, service_name=None):
         endpoints = self._get_service_endpoints(attr=attr,
                                                 filter_value=filter_value,
                                                 service_type=service_type,
                                                 endpoint_type=endpoint_type,
-                                                region_name=region_name)
+                                                region_name=region_name,
+                                                service_name=service_name)
 
         if endpoints:
             return tuple([endpoint['url'] for endpoint in endpoints])
diff --git a/keystoneclient/tests/v2_0/test_service_catalog.py b/keystoneclient/tests/v2_0/test_service_catalog.py
index 60fd450..bc219fd 100644
--- a/keystoneclient/tests/v2_0/test_service_catalog.py
+++ b/keystoneclient/tests/v2_0/test_service_catalog.py
@@ -150,3 +150,26 @@ class ServiceCatalogTest(utils.TestCase):
         self.assertEqual(len(endpoints['image']), 1)
         self.assertEqual(endpoints['image'][0]['publicURL'],
                          'https://image.south.host/v1/')
+
+    def test_service_catalog_service_name(self):
+        auth_ref = access.AccessInfo.factory(resp=None,
+                                             body=self.AUTH_RESPONSE_BODY)
+        sc = auth_ref.service_catalog
+
+        url = sc.url_for(service_name='Image Servers', endpoint_type='public',
+                         service_type='image', region_name='North')
+        self.assertEqual('https://image.north.host/v1/', url)
+
+        self.assertRaises(exceptions.EndpointNotFound, sc.url_for,
+                          service_name='Image Servers', service_type='compute')
+
+        urls = sc.get_urls(service_type='image', service_name='Image Servers',
+                           endpoint_type='public')
+
+        self.assertIn('https://image.north.host/v1/', urls)
+        self.assertIn('https://image.south.host/v1/', urls)
+
+        urls = sc.get_urls(service_type='image', service_name='Servers',
+                           endpoint_type='public')
+
+        self.assertIsNone(urls)
diff --git a/keystoneclient/tests/v3/client_fixtures.py b/keystoneclient/tests/v3/client_fixtures.py
index 32958b4..87361c8 100644
--- a/keystoneclient/tests/v3/client_fixtures.py
+++ b/keystoneclient/tests/v3/client_fixtures.py
@@ -332,7 +332,8 @@ AUTH_RESPONSE_BODY = {
                 'region': 'North',
                 'interface': 'admin'
             }],
-            'type': 'compute'
+            'type': 'compute',
+            'name': 'nova',
         }, {
             'endpoints': [{
                 'url': 'http://swift.north.host/swiftapi/public',
@@ -347,7 +348,8 @@ AUTH_RESPONSE_BODY = {
                 'region': 'South',
                 'interface': 'admin'
             }],
-            'type': 'object-store'
+            'type': 'object-store',
+            'name': 'swift',
         }, {
             'endpoints': [{
                 'url': 'http://glance.north.host/glanceapi/public',
@@ -374,7 +376,8 @@ AUTH_RESPONSE_BODY = {
                 'region': 'South',
                 'interface': 'admin'
             }],
-            'type': 'image'
+            'type': 'image',
+            'name': 'glance',
         }]
     }
 }
diff --git a/keystoneclient/tests/v3/test_service_catalog.py b/keystoneclient/tests/v3/test_service_catalog.py
index 504cbba..5ad303c 100644
--- a/keystoneclient/tests/v3/test_service_catalog.py
+++ b/keystoneclient/tests/v3/test_service_catalog.py
@@ -171,3 +171,51 @@ class ServiceCatalogTest(utils.TestCase):
         for endpoint in endpoints['image']:
             self.assertEqual(endpoint['url'],
                              self.south_endpoints[endpoint['interface']])
+
+    def test_service_catalog_service_name(self):
+        auth_ref = access.AccessInfo.factory(resp=None,
+                                             body=self.AUTH_RESPONSE_BODY)
+        sc = auth_ref.service_catalog
+
+        url = sc.url_for(service_name='glance', endpoint_type='public',
+                         service_type='image', region_name='North')
+        self.assertEqual('http://glance.north.host/glanceapi/public', url)
+
+        url = sc.url_for(service_name='glance', endpoint_type='public',
+                         service_type='image', region_name='South')
+        self.assertEqual('http://glance.south.host/glanceapi/public', url)
+
+        self.assertRaises(exceptions.EndpointNotFound, sc.url_for,
+                          service_name='glance', service_type='compute')
+
+        urls = sc.get_urls(service_type='image', service_name='glance',
+                           endpoint_type='public')
+
+        self.assertIn('http://glance.north.host/glanceapi/public', urls)
+        self.assertIn('http://glance.south.host/glanceapi/public', urls)
+
+        urls = sc.get_urls(service_type='image', service_name='Servers',
+                           endpoint_type='public')
+
+        self.assertIsNone(urls)
+
+    def test_service_catalog_without_name(self):
+        pr_auth_ref = access.AccessInfo.factory(
+            resp=None,
+            body=client_fixtures.PROJECT_SCOPED_TOKEN)
+        pr_sc = pr_auth_ref.service_catalog
+
+        # this will work because there are no service names on that token
+        url_ref = 'http://public.com:8774/v2/225da22d3ce34b15877ea70b2a575f58'
+        url = pr_sc.url_for(service_type='compute', service_name='NotExist',
+                            endpoint_type='public')
+        self.assertEqual(url_ref, url)
+
+        ab_auth_ref = access.AccessInfo.factory(resp=None,
+                                                body=self.AUTH_RESPONSE_BODY)
+        ab_sc = ab_auth_ref.service_catalog
+
+        # this won't work because there is a name and it's not this one
+        self.assertRaises(exceptions.EndpointNotFound, ab_sc.url_for,
+                          service_type='compute', service_name='NotExist',
+                          endpoint_type='public')