From 658cd3a7f121e2ef2ca669732b150fca5f268876 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Wed, 7 Aug 2019 10:45:51 -0400 Subject: [PATCH] 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 | 29 ++++++++++++++-- openstack/config/cloud_region.py | 33 +++++++++++++++---- openstack/tests/unit/cloud/test_shade.py | 15 +++++++++ .../tests/unit/config/test_cloud_config.py | 17 ++++++++++ openstack/tests/unit/fixtures/catalog-v3.json | 6 ++++ 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/openstack/cloud/openstackcloud.py b/openstack/cloud/openstackcloud.py index 235ddc624..dfc8af553 100755 --- a/openstack/cloud/openstackcloud.py +++ b/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): diff --git a/openstack/config/cloud_region.py b/openstack/config/cloud_region.py index 6844c5fe4..c9c626426 100644 --- a/openstack/config/cloud_region.py +++ b/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, diff --git a/openstack/tests/unit/cloud/test_shade.py b/openstack/tests/unit/cloud/test_shade.py index 4bd0e4e9a..5a9dfbbc9 100644 --- a/openstack/tests/unit/cloud/test_shade.py +++ b/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 diff --git a/openstack/tests/unit/config/test_cloud_config.py b/openstack/tests/unit/config/test_cloud_config.py index 718450217..5a0f03212 100644 --- a/openstack/tests/unit/config/test_cloud_config.py +++ b/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')) diff --git a/openstack/tests/unit/fixtures/catalog-v3.json b/openstack/tests/unit/fixtures/catalog-v3.json index 1df1dc196..40bc7bb39 100644 --- a/openstack/tests/unit/fixtures/catalog-v3.json +++ b/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",