Handle oslo.config exceptions in from_conf

In the following cases, we used to happily allow the Connection to the
service, using default Adapter settings:

- If the conf section for a given service is missing, or present but
  without ksa adapter opts registered.
- If the conf section for a given service has bogus values.

Now for these scenarios, we disable that service with a helpful reason.

The service disabling is supported at a broader level: If someone sets
has_{service_type} to false in their config, we remove the adapter for
that service and replace it with something that throws errors when
people try to use it. They can optionally set
{service_type}_disabled_reason to make the message more informative.

Co-Authored-By: Monty Taylor <mordred@inaugust.com>

Change-Id: I3aa1f1633790e6e958bbc510ac5e5a11c0c27a9f

lower-constraints.txt

@@ -21,7 +21,7 @@ mox3==0.20.0
 munch==2.1.0
 netifaces==0.10.4
 os-client-config==1.28.0
-os-service-types==1.2.0
+os-service-types==1.6.0
 oslo.config==6.1.0
 oslotest==3.2.0
 pbr==2.0.0

openstack/cloud/openstackcloud.py

@@ -670,7 +670,7 @@ class _OpenStackCloudMixin(object):
         return endpoint
 
     def has_service(self, service_key):
-        if not self.config.config.get('has_%s' % service_key, True):
+        if not self.config.has_service(service_key):
             # TODO(mordred) add a stamp here so that we only report this once
             if not (service_key in self._disable_warnings
                     and self._disable_warnings[service_key]):

openstack/config/cloud_region.py

@@ -39,6 +39,9 @@ from openstack.config import defaults as config_defaults
 from openstack import exceptions
 from openstack import proxy
 
+
+_logger = _log.setup_logging('openstack')
+
 SCOPE_KEYS = {
     'domain_id', 'domain_name',
     'project_id', 'project_name',
@@ -58,6 +61,15 @@ def _make_key(key, service_type):
         return "_".join([service_type, key])
 
 
+def _disable_service(config, service_type, reason=None):
+    service_type = service_type.lower().replace('-', '_')
+    key = 'has_{service_type}'.format(service_type=service_type)
+    config[key] = False
+    if reason:
+        d_key = _make_key('disabled_reason', service_type)
+        config[d_key] = reason
+
+
 def _get_implied_microversion(version):
     if not version:
         return
@@ -130,18 +142,38 @@ def from_conf(conf, session=None, **kwargs):
     for st in stm.all_types_by_service_type:
         project_name = stm.get_project_name(st)
         if project_name not in conf:
+            _disable_service(
+                config_dict, st,
+                reason="No section for project '{project}' (service type "
+                       "'{service_type}') was present in the config.".format(
+                           project=project_name, service_type=st))
             continue
         opt_dict = {}
         # Populate opt_dict with (appropriately processed) Adapter conf opts
         try:
             ks_load_adap.process_conf_options(conf[project_name], opt_dict)
-        except Exception:
-            # NOTE(efried): This is for oslo_config.cfg.NoSuchOptError, but we
-            # don't want to drag in oslo.config just for that.
+        except Exception as e:
+            # NOTE(efried): This is for (at least) a couple of scenarios:
+            # (1) oslo_config.cfg.NoSuchOptError when ksa adapter opts are not
+            #     registered in this section.
+            # (2) TypeError, when opts are registered but bogus (e.g.
+            #     'interface' and 'valid_interfaces' are both present).
+            # We may want to consider (providing a kwarg giving the caller the
+            # option of) blowing up right away for (2) rather than letting them
+            # get all the way to the point of trying the service and having
+            # *that* blow up.
+            reason = ("Encountered an exception attempting to process config "
+                      "for project '{project}' (service type "
+                      "'{service_type}'): {exception}".format(
+                          project=project_name, service_type=st, exception=e))
+            _logger.warn("Disabling service '{service_type}'.".format(
+                service_type=st))
+            _logger.warn(reason)
+            _disable_service(config_dict, st, reason=reason)
             continue
         # Load them into config_dict under keys prefixed by ${service_type}_
         for raw_name, opt_val in opt_dict.items():
-            config_name = '_'.join([st, raw_name])
+            config_name = _make_key(raw_name, st)
             config_dict[config_name] = opt_val
     return CloudRegion(
         session=session, config=config_dict, **kwargs)
@@ -929,3 +961,22 @@ class CloudRegion(object):
             )
             registry._openstacksdk_counter = counter
         return counter
+
+    def has_service(self, service_type):
+        service_type = service_type.lower().replace('-', '_')
+        key = 'has_{service_type}'.format(service_type=service_type)
+        return self.config.get(
+            key, self._service_type_manager.is_official(service_type))
+
+    def disable_service(self, service_type, reason=None):
+        _disable_service(self.config, service_type, reason=reason)
+
+    def enable_service(self, service_type):
+        service_type = service_type.lower().replace('-', '_')
+        key = 'has_{service_type}'.format(service_type=service_type)
+        self.config[key] = True
+
+    def get_disabled_reason(self, service_type):
+        service_type = service_type.lower().replace('-', '_')
+        d_key = _make_key('disabled_reason', service_type)
+        return self.config.get(d_key)

openstack/connection.py

@@ -446,6 +446,7 @@ class Connection(six.with_metaclass(_meta.ConnectionMeta,
                 attr_name.replace('-', '_'),
                 property(fget=getter)
             )
+            self.config.enable_service(attr_name)
 
     def authorize(self):
         """Authorize this Connection

openstack/exceptions.py

@@ -253,3 +253,7 @@ class ValidationException(SDKException):
 
 class TaskManagerStopped(SDKException):
     """Operations were attempted on a stopped TaskManager."""
+
+
+class ServiceDisabledException(ConfigException):
+    """This service is disabled for reasons."""

openstack/service_description.py

@@ -26,6 +26,18 @@ _logger = _log.setup_logging('openstack')
 _service_type_manager = os_service_types.ServiceTypes()
 
 
+class _ServiceDisabledProxyShim(object):
+    def __init__(self, service_type, reason):
+        self.service_type = service_type
+        self.reason = reason
+
+    def __getattr__(self, item):
+        raise exceptions.ServiceDisabledException(
+            "Service '{service_type}' is disabled because its configuration "
+            "could not be loaded. {reason}".format(
+                service_type=self.service_type, reason=self.reason or ''))
+
+
 class ServiceDescription(object):
 
     #: Dictionary of supported versions and proxy classes for that version
@@ -83,6 +95,11 @@ class ServiceDescription(object):
         """
         config = instance.config
 
+        if not config.has_service(self.service_type):
+            return _ServiceDisabledProxyShim(
+                self.service_type,
+                config.get_disabled_reason(self.service_type))
+
         # First, check to see if we've got config that matches what we
         # understand in the SDK.
         version_string = config.get_api_version(self.service_type)

openstack/tests/unit/config/test_from_conf.py

@@ -30,8 +30,8 @@ class TestFromConf(base.TestCase):
         self.oslo_config_dict = {
             # All defaults for nova
             'nova': {},
-            # monasca not in the service catalog
-            'monasca': {},
+            # monasca-api not in the service catalog
+            'monasca-api': {},
             # Overrides for heat
             'heat': {
                 'region_name': 'SpecialRegion',
@@ -43,6 +43,7 @@ class TestFromConf(base.TestCase):
     def _load_ks_cfg_opts(self):
         conf = cfg.ConfigOpts()
         for group, opts in self.oslo_config_dict.items():
+            conf.register_group(cfg.OptGroup(group))
             if opts is not None:
                 ks_loading.register_adapter_conf_options(conf, group)
                 for name, val in opts.items():
@@ -122,27 +123,42 @@ class TestFromConf(base.TestCase):
         self.assertEqual(s.name, server_name)
         self.assert_calls()
 
-    def test_no_adapter_opts(self):
-        """Adapter opts for service type not registered."""
-        del self.oslo_config_dict['heat']
+    def _test_missing_invalid_permutations(self, expected_reason):
+        # Do special things to self.oslo_config_dict['heat'] before calling
+        # this method.
         conn = self._get_conn()
 
-        # TODO(efried): This works, even though adapter opts are not
-        # registered. Should it?
         adap = conn.orchestration
-        self.assertIsNone(adap.region_name)
-        self.assertEqual('orchestration', adap.service_type)
-        self.assertEqual('public', adap.interface)
-        self.assertIsNone(adap.endpoint_override)
+        ex = self.assertRaises(
+            exceptions.ServiceDisabledException, getattr, adap, 'get')
+        self.assertIn("Service 'orchestration' is disabled because its "
+                      "configuration could not be loaded.", ex.message)
+        self.assertIn(expected_reason, ex.message)
 
-        self.register_uris([
-            dict(method='GET',
-                 uri=self.get_mock_url(
-                     'orchestration', append=['foo']),
-                 json={'foo': {}})
-        ])
-        adap.get('/foo')
-        self.assert_calls()
+    def test_no_such_conf_section(self):
+        """No conf section (therefore no adapter opts) for service type."""
+        del self.oslo_config_dict['heat']
+        self._test_missing_invalid_permutations(
+            "No section for project 'heat' (service type 'orchestration') was "
+            "present in the config.")
+
+    def test_no_adapter_opts(self):
+        """Conf section present, but opts for service type not registered."""
+        self.oslo_config_dict['heat'] = None
+        self._test_missing_invalid_permutations(
+            "Encountered an exception attempting to process config for "
+            "project 'heat' (service type 'orchestration'): no such option")
+
+    def test_invalid_adapter_opts(self):
+        """Adapter opts are bogus, in exception-raising ways."""
+        self.oslo_config_dict['heat'] = {
+            'interface': 'public',
+            'valid_interfaces': 'private',
+        }
+        self._test_missing_invalid_permutations(
+            "Encountered an exception attempting to process config for "
+            "project 'heat' (service type 'orchestration'): interface and "
+            "valid_interfaces are mutually exclusive.")
 
     def test_no_session(self):
         # TODO(efried): Currently calling without a Session is not implemented.

requirements.txt

@@ -7,7 +7,7 @@ appdirs>=1.3.0 # MIT License
 requestsexceptions>=1.2.0 # Apache-2.0
 jsonpatch!=1.20,>=1.16 # BSD
 six>=1.10.0 # MIT
-os-service-types>=1.2.0 # Apache-2.0
+os-service-types>=1.6.0 # Apache-2.0
 keystoneauth1>=3.14.0 # Apache-2.0
 
 munch>=2.1.0 # MIT