Rationalize endpoint_for and get_endpoint_from_catalog

We have two methods in two different places which do almost the
same thing. Make one use the other and then document what they do.
Finally, make sure endpoint_for respects endpoint_override, since
that would be the rational expectation at that point in the code.

Add some tests.

Change-Id: I4ff51373496c558342cfb54820eab4dddd195625

openstack/cloud/openstackcloud.py

@@ -499,9 +499,32 @@ class _OpenStackCloudMixin(object):
     def service_catalog(self):
         return self._keystone_catalog.catalog
 
-    def endpoint_for(self, service_type, interface='public'):
-        return self._keystone_catalog.url_for(
-            service_type=service_type, interface=interface)
+    def endpoint_for(self, service_type, interface=None, region_name=None):
+        """Return the endpoint for a given service.
+
+        Respects config values for Connection, including
+        ``*_endpoint_override``. For direct values from the catalog
+        regardless of overrides, see
+        :meth:`~openstack.config.cloud_region.CloudRegion.get_endpoint_from_catalog`
+
+        :param service_type: Service Type of the endpoint to search for.
+        :param interface:
+            Interface of the endpoint to search for. Optional, defaults to
+            the configured value for interface for this Connection.
+        :param region_name:
+            Region Name of the endpoint to search for. Optional, defaults to
+            the configured value for region_name for this Connection.
+
+        :returns: The endpoint of the service, or None if not found.
+        """
+
+        endpoint_override = self.config.get_endpoint(service_type)
+        if endpoint_override:
+            return endpoint_override
+        return self.config.get_endpoint_from_catalog(
+            service_type=service_type,
+            interface=interface,
+            region_name=region_name)
 
     @property
     def auth_token(self):

openstack/config/cloud_region.py

@@ -466,13 +466,34 @@ class CloudRegion(object):
             value = auth.get('auth_url')
         return value
 
-    def get_endpoint_from_catalog(self, service_type):
+    def get_endpoint_from_catalog(
+            self, service_type, interface=None, region_name=None):
+        """Return the endpoint for a given service as found in the catalog.
+
+        For values respecting endpoint overrides, see
+        :meth:`~openstack.connection.Connection.endpoint_for`
+
+        :param service_type: Service Type of the endpoint to search for.
+        :param interface:
+            Interface of the endpoint to search for. Optional, defaults to
+            the configured value for interface for this Connection.
+        :param region_name:
+            Region Name of the endpoint to search for. Optional, defaults to
+            the configured value for region_name for this Connection.
+
+        :returns: The endpoint of the service, or None if not found.
+        """
+        interface = interface or self.get_interface(service_type)
+        region_name = region_name or self.get_region_name(service_type)
         session = self.get_session()
-        return session.auth.get_endpoint(
-            session,
-            service_type=service_type,
-            region_name=self.get_region_name(service_type),
-            interface=self.get_interface(service_type))
+        catalog = session.auth.get_access(session).service_catalog
+        try:
+            return catalog.url_for(
+                service_type=service_type,
+                interface=interface,
+                region_name=region_name)
+        except keystoneauth1.exceptions.catalog.EndpointNotFound:
+            return None
 
     def get_connect_retries(self, service_type):
         return self._get_config('connect_retries', service_type,

openstack/tests/unit/cloud/test_shade.py

@@ -54,6 +54,21 @@ class TestShade(base.TestCase):
     def test_openstack_cloud(self):
         self.assertIsInstance(self.cloud, connection.Connection)
 
+    def test_endpoint_for(self):
+        dns_override = 'https://override.dns.example.com'
+        self.cloud.config.config['dns_endpoint_override'] = dns_override
+        self.assertEqual(
+            'https://compute.example.com/v2.1/',
+            self.cloud.endpoint_for('compute'))
+        self.assertEqual(
+            'https://internal.compute.example.com/v2.1/',
+            self.cloud.endpoint_for('compute', interface='internal'))
+        self.assertIsNone(
+            self.cloud.endpoint_for('compute', region_name='unknown-region'))
+        self.assertEqual(
+            dns_override,
+            self.cloud.endpoint_for('dns'))
+
     def test_connect_as(self):
         # Do initial auth/catalog steps
         # This should authenticate a second time, but should not

openstack/tests/unit/config/test_cloud_config.py

@@ -365,3 +365,20 @@ class TestCloudRegion(base.TestCase):
         cc = cloud_region.CloudRegion(
             "test1", "region-al", {}, auth_plugin=mock.Mock())
         self.assertIsNone(cc.get_session_endpoint('notfound'))
+
+    def test_get_endpoint_from_catalog(self):
+        dns_override = 'https://override.dns.example.com'
+        self.cloud.config.config['dns_endpoint_override'] = dns_override
+        self.assertEqual(
+            'https://compute.example.com/v2.1/',
+            self.cloud.config.get_endpoint_from_catalog('compute'))
+        self.assertEqual(
+            'https://internal.compute.example.com/v2.1/',
+            self.cloud.config.get_endpoint_from_catalog(
+                'compute', interface='internal'))
+        self.assertIsNone(
+            self.cloud.config.get_endpoint_from_catalog(
+                'compute', region_name='unknown-region'))
+        self.assertEqual(
+            'https://dns.example.com',
+            self.cloud.config.get_endpoint_from_catalog('dns'))

openstack/tests/unit/fixtures/catalog-v3.json

@@ -11,6 +11,12 @@
             "interface": "public",
             "region": "RegionOne",
             "url": "https://compute.example.com/v2.1/"
+          },
+          {
+            "id": "32466f357f3545248c47471ca51b0d3b",
+            "interface": "internal",
+            "region": "RegionOne",
+            "url": "https://internal.compute.example.com/v2.1/"
           }
         ],
         "name": "nova",