From 5ae0d72247f02c56c59a7fcb5b15e48539740d60 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 15 Aug 2019 17:59:44 -0500 Subject: [PATCH] Add strict_proxies option for Connection Add a strict_proxies kwarg to Connection, defaulting to False. When True, ServiceDescription will perform an extra check when creating a proxy. If it doesn't look like a valid connection is available, raise a new SDKException called ServiceDiscoveryException. Change-Id: I0b404d5744a4465d365780a4273aa8dc1cebeb14 Co-Authored-By: Monty Taylor --- openstack/connection.py | 12 ++ openstack/exceptions.py | 4 + openstack/service_description.py | 37 +++++- openstack/tests/unit/base.py | 3 +- openstack/tests/unit/config/test_from_conf.py | 105 ++++++++++++++++-- .../strict-proxies-4a315f68f387ee89.yaml | 8 ++ 6 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/strict-proxies-4a315f68f387ee89.yaml diff --git a/openstack/connection.py b/openstack/connection.py index 5916b1868..17a9ef158 100644 --- a/openstack/connection.py +++ b/openstack/connection.py @@ -273,6 +273,7 @@ class Connection(six.with_metaclass(_meta.ConnectionMeta, oslo_conf=None, service_types=None, global_request_id=None, + strict_proxies=False, **kwargs): """Create a connection to a cloud. @@ -330,12 +331,23 @@ class Connection(six.with_metaclass(_meta.ConnectionMeta, **Currently only supported in conjunction with the ``oslo_conf`` kwarg.** :param global_request_id: A Request-id to send with all interactions. + :param strict_proxies: + If True, check proxies on creation and raise + ServiceDiscoveryException if the service is unavailable. + :type strict_proxies: bool + Throw an ``openstack.exceptions.ServiceDiscoveryException`` if the + endpoint for a given service doesn't work. This is useful for + OpenStack services using sdk to talk to other OpenStack services + where it can be expected that the deployer config is correct and + errors should be reported immediately. + Default false. :param kwargs: If a config is not provided, the rest of the parameters provided are assumed to be arguments to be passed to the CloudRegion constructor. """ self.config = config self._extra_services = {} + self._strict_proxies = strict_proxies if extra_services: for service in extra_services: self._extra_services[service.service_type] = service diff --git a/openstack/exceptions.py b/openstack/exceptions.py index a267c2c9f..56fba3b5e 100644 --- a/openstack/exceptions.py +++ b/openstack/exceptions.py @@ -257,3 +257,7 @@ class TaskManagerStopped(SDKException): class ServiceDisabledException(ConfigException): """This service is disabled for reasons.""" + + +class ServiceDiscoveryException(SDKException): + """The service cannot be discovered.""" diff --git a/openstack/service_description.py b/openstack/service_description.py index 643184eff..c6cd11e3f 100644 --- a/openstack/service_description.py +++ b/openstack/service_description.py @@ -17,6 +17,7 @@ import os_service_types from openstack import _log from openstack import exceptions +from openstack import proxy as proxy_mod __all__ = [ 'ServiceDescription', @@ -83,10 +84,34 @@ class ServiceDescription(object): if instance is None: return self if self.service_type not in instance._proxies: - instance._proxies[self.service_type] = self._make_proxy(instance) - instance._proxies[self.service_type]._connection = instance + proxy = self._make_proxy(instance) + if not isinstance(proxy, _ServiceDisabledProxyShim): + # The keystone proxy has a method called get_endpoint + # that is about managing keystone endpoints. This is + # unfortunate. + endpoint = proxy_mod.Proxy.get_endpoint(proxy) + if instance._strict_proxies: + self._validate_proxy(proxy, endpoint) + proxy._connection = instance + instance._proxies[self.service_type] = proxy return instance._proxies[self.service_type] + def _validate_proxy(self, proxy, endpoint): + exc = None + service_url = getattr(proxy, 'skip_discovery', None) + try: + # Don't go too wild for e.g. swift + if service_url is None: + service_url = proxy.get_endpoint_data().service_url + except Exception as e: + exc = e + if exc or not endpoint or not service_url: + raise exceptions.ServiceDiscoveryException( + "Failed to create a working proxy for service {service_type}: " + "{message}".format( + service_type=self.service_type, + message=exc or "No valid endpoint was discoverable.")) + def _make_proxy(self, instance): """Create a Proxy for the service in question. @@ -108,8 +133,6 @@ class ServiceDescription(object): self.service_type, allow_version_hack=True, ) - # trigger EndpointNotFound exception if this is bogus - temp_client.get_endpoint() return temp_client # Check to see if we've got config that matches what we @@ -173,6 +196,12 @@ class ServiceDescription(object): return proxy_obj data = proxy_obj.get_endpoint_data() + if not data and instance._strict_proxies: + raise exceptions.ServiceDiscoveryException( + "Failed to create a working proxy for service " + "{service_type}: No endpoint data found.".format( + service_type=self.service_type)) + # If we've gotten here with a proxy object it means we have # an endpoint_override in place. If the catalog_url and # service_url don't match, which can happen if there is a diff --git a/openstack/tests/unit/base.py b/openstack/tests/unit/base.py index 9a439d451..238d13b00 100644 --- a/openstack/tests/unit/base.py +++ b/openstack/tests/unit/base.py @@ -673,7 +673,8 @@ class TestCase(base.TestCase): if 'content-type' not in headers: headers[u'content-type'] = 'application/json' - to_mock['headers'] = headers + if 'exc' not in to_mock: + to_mock['headers'] = headers self.calls += [ dict( diff --git a/openstack/tests/unit/config/test_from_conf.py b/openstack/tests/unit/config/test_from_conf.py index 49b8e4a68..c9ab677cc 100644 --- a/openstack/tests/unit/config/test_from_conf.py +++ b/openstack/tests/unit/config/test_from_conf.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import requests.exceptions import uuid from keystoneauth1 import exceptions as ks_exc @@ -31,7 +32,7 @@ class TestFromConf(base.TestCase): **from_conf_kwargs) self.assertEqual('from_conf.example.com', config.name) - return connection.Connection(config=config) + return connection.Connection(config=config, strict_proxies=True) def test_adapter_opts_set(self): """Adapter opts specified in the conf.""" @@ -75,16 +76,10 @@ class TestFromConf(base.TestCase): """Adapter opts are registered, but all defaulting in conf.""" conn = self._get_conn() - # Nova has empty adapter config, so these default - adap = conn.compute - self.assertIsNone(adap.region_name) - self.assertEqual('compute', adap.service_type) - self.assertEqual('public', adap.interface) - self.assertIsNone(adap.endpoint_override) - server_id = str(uuid.uuid4()) server_name = self.getUniqueString('name') fake_server = fakes.make_fake_server(server_id, server_name) + self.register_uris([ self.get_nova_discovery_mock_dict(), dict(method='GET', @@ -92,6 +87,49 @@ class TestFromConf(base.TestCase): 'compute', 'public', append=['servers', 'detail']), json={'servers': [fake_server]}), ]) + + # Nova has empty adapter config, so these default + adap = conn.compute + self.assertIsNone(adap.region_name) + self.assertEqual('compute', adap.service_type) + self.assertEqual('public', adap.interface) + self.assertIsNone(adap.endpoint_override) + + s = next(adap.servers()) + self.assertEqual(s.id, server_id) + self.assertEqual(s.name, server_name) + self.assert_calls() + + def test_service_not_ready_catalog(self): + """Adapter opts are registered, but all defaulting in conf.""" + conn = self._get_conn() + + server_id = str(uuid.uuid4()) + server_name = self.getUniqueString('name') + fake_server = fakes.make_fake_server(server_id, server_name) + + self.register_uris([ + dict(method='GET', + uri='https://compute.example.com/v2.1/', + exc=requests.exceptions.ConnectionError), + self.get_nova_discovery_mock_dict(), + dict(method='GET', + uri=self.get_mock_url( + 'compute', 'public', append=['servers', 'detail']), + json={'servers': [fake_server]}), + ]) + + self.assertRaises( + exceptions.ServiceDiscoveryException, + getattr, conn, 'compute') + + # Nova has empty adapter config, so these default + adap = conn.compute + self.assertIsNone(adap.region_name) + self.assertEqual('compute', adap.service_type) + self.assertEqual('public', adap.interface) + self.assertIsNone(adap.endpoint_override) + s = next(adap.servers()) self.assertEqual(s.id, server_id) self.assertEqual(s.name, server_name) @@ -119,6 +157,11 @@ class TestFromConf(base.TestCase): dict(method='GET', uri='https://example.org:5050', json=discovery), + # strict-proxies means we're going to fetch the discovery + # doc from the versioned endpoint to verify it works. + dict(method='GET', + uri='https://example.org:5050/v1', + json=discovery), dict(method='GET', uri='https://example.org:5050/v1/introspection/abcd', json=status), @@ -131,6 +174,52 @@ class TestFromConf(base.TestCase): self.assertTrue(adap.get_introspection('abcd').is_finished) + def test_service_not_ready_endpoint_override(self): + conn = self._get_conn() + + discovery = { + "versions": { + "values": [ + {"status": "stable", + "id": "v1", + "links": [{ + "href": "https://example.org:5050/v1", + "rel": "self"}] + }] + } + } + status = { + 'finished': True, + 'error': None + } + self.register_uris([ + dict(method='GET', + uri='https://example.org:5050', + exc=requests.exceptions.ConnectTimeout), + dict(method='GET', + uri='https://example.org:5050', + json=discovery), + # strict-proxies means we're going to fetch the discovery + # doc from the versioned endpoint to verify it works. + dict(method='GET', + uri='https://example.org:5050/v1', + json=discovery), + dict(method='GET', + uri='https://example.org:5050/v1/introspection/abcd', + json=status), + ]) + + self.assertRaises( + exceptions.ServiceDiscoveryException, + getattr, conn, 'baremetal_introspection') + + adap = conn.baremetal_introspection + self.assertEqual('baremetal-introspection', adap.service_type) + self.assertEqual('public', adap.interface) + self.assertEqual('https://example.org:5050/v1', adap.endpoint_override) + + self.assertTrue(adap.get_introspection('abcd').is_finished) + def assert_service_disabled(self, service_type, expected_reason, **from_conf_kwargs): conn = self._get_conn(**from_conf_kwargs) diff --git a/releasenotes/notes/strict-proxies-4a315f68f387ee89.yaml b/releasenotes/notes/strict-proxies-4a315f68f387ee89.yaml new file mode 100644 index 000000000..ae2e11bc1 --- /dev/null +++ b/releasenotes/notes/strict-proxies-4a315f68f387ee89.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Added new option for Connection, ``strict_proxies``. When set to ``True``, + Connection will throw a ``ServiceDiscoveryException`` if the endpoint for + a given service doesn't work. This is useful for OpenStack services using + sdk to talk to other OpenStack services where it can be expected that the + deployer config is correct and errors should be reported immediately.