Merge "Improve configuration of out-of-tree identity drivers"

doc/source/contributor/developing-drivers.rst

@@ -74,6 +74,62 @@ The TLDR; steps (and too long didn't write yet):
 5. Configure your new driver in ``keystone.conf``
 6. Sit back and enjoy!
 
+Identity Driver Configuration
+-----------------------------
+
+As described in the :ref:`domain_specific_configuration` there are 2 ways of
+configuring domain specific drivers: using files and using database.
+Configuration with files is straight forward but is having a major disadvantage
+of requiring restart of Keystone for the refresh of configuration or even for
+Keystone to start using chosen driver after adding a new domain.
+
+Configuring drivers using database is a flexible alternative that allows
+dynamic reconfiguration and even changes using the API (requires admin
+privileges by default). There are 2 independent parts for this to work properly:
+
+Defining configuration options
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Driver class (as pointed by EntryPoints) may have a static method
+`register_opts` accepting `conf` argument. This method, if present, is being
+invoked during loading the driver and registered options are then
+available when the driver is being instantiated.
+
+.. code-block:: python
+
+   class CustomDriver(base.IdentityDriverBase):
+
+       @classmethod
+       def register_opts(cls, conf):
+           grp = cfg.OptGroup("foo")
+           opts = [cfg.StrOpt("opt1")]
+           conf.register_group(grp)
+           conf.register_opts(opts, group=grp)
+
+       def __init__(self, conf=None):
+           # conf contains options registered above and domain specific values
+           # being set.
+           pass
+
+       ...
+
+Allowing domain configuration per API
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+A safety measure of the Keystone domain configuration API is that options
+allowed for the change need to be explicitly whitelisted. This is done
+in the `domain_config` section of the main Keystone configuration file.
+
+.. code-block:: cfg
+
+   [domain_config]
+   additional_whitelisted_options=<GROUP_NAME>:[opt1,opt2,opt3]
+   additional_sensitive_options=<GROUP_NAME>:[password]
+
+The `<GROUP_NAME>` is the name of the configuration group as defined by the
+driver. Sensitive options are not included in the GET api call and are stored
+in a separate database table.
+
 Driver Interface Changes
 ------------------------
 

keystone/conf/domain_config.py

@@ -42,12 +42,32 @@ Time-to-live (TTL, in seconds) to cache domain-specific configuration data.
 This has no effect unless `[domain_config] caching` is enabled.
 """))
 
+additional_whitelisted_options = cfg.Opt(
+    'additional_whitelisted_options',
+    type=cfg.types.Dict(value_type=cfg.types.List(bounds=True)),
+    help=utils.fmt("""
+Additional whitelisted domain-specific options for out-of-tree drivers.
+This is a dictonary of lists with the key being the group name and value a list
+of group options.""")
+)
+
+additional_sensitive_options = cfg.Opt(
+    'additional_sensitive_options',
+    type=cfg.types.Dict(value_type=cfg.types.List(bounds=True)),
+    help=utils.fmt("""
+Additional sensitive domain-specific options for out-of-tree drivers.
+This is a dictonary of lists with the key being the group name and value a list
+of group options.""")
+)
+
 
 GROUP_NAME = __name__.split('.')[-1]
 ALL_OPTS = [
     driver,
     caching,
     cache_time,
+    additional_whitelisted_options,
+    additional_sensitive_options
 ]
 
 

keystone/identity/backends/base.py

@@ -134,6 +134,16 @@ class IdentityDriverBase(object, metaclass=abc.ABCMeta):
 
     """
 
+    # @classmethod
+    # def register_opts(cls, conf):
+    #     """Register driver specific configuration options.
+
+    #     For domain configuration being stored in the database it is necessary
+    #     for the driver to register configuration options. This method is
+    #     optional and if it is not present no options are registered.
+    #     """
+    #     pass
+
     def _get_conf(self):
         try:
             return self.conf or CONF

keystone/identity/core.py

@@ -25,6 +25,7 @@ import uuid
 from oslo_config import cfg
 from oslo_log import log
 from pycadf import reason
+import stevedore
 
 from keystone import assignment  # TODO(lbragstad): Decouple this dependency
 from keystone.common import cache
@@ -64,6 +65,23 @@ REGISTRATION_ATTEMPTS = 10
 SQL_DRIVER = 'SQL'
 
 
+def get_driver(namespace, driver_name, *args):
+    """Get identity driver without initializing.
+
+    The method is invoked to be able to introspect domain specific driver
+    looking for additional configuration options required by the driver.
+    """
+    try:
+        driver_manager = stevedore.DriverManager(namespace,
+                                                 driver_name,
+                                                 invoke_on_load=False,
+                                                 invoke_args=args)
+        return driver_manager.driver
+    except stevedore.exception.NoMatches:
+        msg = (_('Unable to find %(name)r driver in %(namespace)r.'))
+        raise ImportError(msg % {'name': driver_name, 'namespace': namespace})
+
+
 class DomainConfigs(provider_api.ProviderAPIMixin, dict):
     """Discover, store and provide access to domain specific configs.
 
@@ -262,12 +280,51 @@ class DomainConfigs(provider_api.ProviderAPIMixin, dict):
                              default_config_files=[],
                              default_config_dirs=[])
 
+        # Try to identify the required driver for the domain to let it register
+        # supported configuration options. In difference to the FS based
+        # configuration this is being set through `oslo_cfg.set_override` and
+        # thus require special treatment.
+        try:
+            driver_name = specific_config.get("identity", {}).get(
+                "driver", domain_config["cfg"].identity.driver)
+            # For the non in-tree drivers ...
+            if driver_name not in ["sql", "ldap"]:
+                # Locate the driver without invoking ...
+                driver = get_driver(
+                    Manager.driver_namespace, driver_name)
+                # Check whether it wants to register additional config options
+                # ...
+                if hasattr(driver, "register_opts"):
+                    # And register them for the domain_config (not the global
+                    # Keystone config)
+                    driver.register_opts(domain_config["cfg"])
+        except Exception as ex:
+            # If we failed for some reason - something wrong with the driver,
+            # so let's just skip registering config options. This matches older
+            # behavior of Keystone where out-of-tree drivers were not able to
+            # register config options with the DB configuration loading branch.
+            LOG.debug(
+                f"Exception during attempt to load domain specific "
+                f"configuration options: {ex}")
+
         # Override any options that have been passed in as specified in the
         # database.
         for group in specific_config:
             for option in specific_config[group]:
-                domain_config['cfg'].set_override(
+                # NOTE(gtema): Very first time default driver is being ordered
-                    option, specific_config[group][option], group)
+                # to process the domain. This will change once initialization
+                # completes. Until the driver specific configuration is being
+                # registered `set_override` will fail for options not known by
+                # the core Keystone. Make this loading not failing letting code
+                # to complete the process properly.
+                try:
+                    domain_config['cfg'].set_override(
+                        option, specific_config[group][option], group)
+                except (cfg.NoSuchOptError, cfg.NoSuchGroupError):
+                    # Error to register config overrides for wrong driver. This
+                    # is not worth of logging since it is a normal case during
+                    # Keystone initialization.
+                    pass
 
         domain_config['cfg_overrides'] = specific_config
         domain_config['driver'] = self._load_driver(domain_config)

keystone/resource/core.py

@@ -1124,6 +1124,13 @@ class DomainConfigManager(manager.Manager):
                         'option': option}
             raise exception.UnexpectedError(exception=msg)
 
+        if CONF.domain_config.additional_whitelisted_options:
+            self.whitelisted_options.update(
+                **CONF.domain_config.additional_whitelisted_options)
+        if CONF.domain_config.additional_sensitive_options:
+            self.sensitive_options.update(
+                **CONF.domain_config.additional_sensitive_options)
+
         if (group and group not in self.whitelisted_options and
                 group not in self.sensitive_options):
             msg = _('Group %(group)s is not supported '
@@ -1131,15 +1138,15 @@ class DomainConfigManager(manager.Manager):
             raise exception.InvalidDomainConfig(reason=msg)
 
         if option:
-            if (option not in self.whitelisted_options[group] and option not in
+            if (option not in self.whitelisted_options.get(group, {})
-                    self.sensitive_options[group]):
+                    and option not in self.sensitive_options.get(group, {})):
                 msg = _('Option %(option)s in group %(group)s is not '
                         'supported for domain specific configurations') % {
                             'group': group, 'option': option}
                 raise exception.InvalidDomainConfig(reason=msg)
 
     def _is_sensitive(self, group, option):
-        return option in self.sensitive_options[group]
+        return option in self.sensitive_options.get(group, {})
 
     def _config_to_list(self, config):
         """Build list of options for use by backend drivers."""

keystone/tests/unit/identity/backends/fake_driver.py

@@ -0,0 +1,94 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""Fake driver to test out-of-tree drivers handling."""
+
+from oslo_config import cfg
+
+from keystone import exception
+from keystone.identity.backends import base
+
+
+class FooDriver(base.IdentityDriverBase):
+    """Fake out-of-tree driver.
+
+    It does not make much sense to inherit from BaseClass, but in certain
+    places across the code methods are invoked
+
+    """
+
+    @classmethod
+    def register_opts(cls, conf):
+        grp = cfg.OptGroup("foo")
+        opts = [cfg.StrOpt("opt1")]
+        conf.register_group(grp)
+        conf.register_opts(opts, group=grp)
+
+    def authenticate(self, user_id, password):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def create_user(self, user_id, user):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def list_users(self, hints):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def unset_default_project_id(self, project_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def list_users_in_group(self, group_id, hints):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def get_user(self, user_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def update_user(self, user_id, user):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def change_password(self, user_id, new_password):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def add_user_to_group(self, user_id, group_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def check_user_in_group(self, user_id, group_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def remove_user_from_group(self, user_id, group_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def delete_user(self, user_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def get_user_by_name(self, user_name, domain_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def create_group(self, group_id, group):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def list_groups(self, hints):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def list_groups_for_user(self, user_id, hints):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def get_group(self, group_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def get_group_by_name(self, group_name, domain_id):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def update_group(self, group_id, group):
+        raise exception.NotImplemented()  # pragma: no cover
+
+    def delete_group(self, group_id):
+        raise exception.NotImplemented()  # pragma: no cover

keystone/tests/unit/identity/test_core.py

@@ -12,12 +12,14 @@
 
 """Unit tests for core identity behavior."""
 
+import fixtures
 import itertools
 import os
 from unittest import mock
 import uuid
 
 from oslo_config import fixture as config_fixture
+import stevedore
 
 from keystone.common import provider_api
 import keystone.conf
@@ -25,6 +27,7 @@ from keystone import exception
 from keystone import identity
 from keystone.tests import unit
 from keystone.tests.unit import default_fixtures
+from keystone.tests.unit.identity.backends import fake_driver
 from keystone.tests.unit.ksfixtures import database
 
 
@@ -181,3 +184,50 @@ class TestDatabaseDomainConfigs(unit.TestCase):
         self.assertEqual(CONF.ldap.suffix, res.ldap.suffix)
         self.assertEqual(CONF.ldap.use_tls, res.ldap.use_tls)
         self.assertEqual(CONF.ldap.query_scope, res.ldap.query_scope)
+
+    def test_loading_config_from_database_out_of_tree(self):
+        # Test domain config loading for out-of-tree driver supporting own
+        # config options
+
+        # Prepare fake driver
+        extension = stevedore.extension.Extension(
+            name="foo", entry_point=None,
+            obj=fake_driver.FooDriver(), plugin=None
+        )
+        fake_driver_manager = stevedore.DriverManager.make_test_instance(
+            extension, namespace="keystone.identity"
+        )
+        # replace DriverManager with a patched test instance
+        self.useFixture(
+            fixtures.MockPatchObject(
+                stevedore, "DriverManager", return_value=fake_driver_manager
+            )
+        ).mock
+
+        self.config_fixture.config(
+            domain_configurations_from_database=True, group="identity"
+        )
+        self.config_fixture.config(
+            additional_whitelisted_options={"foo": ["opt1"]},
+            group="domain_config"
+        )
+        domain = unit.new_domain_ref()
+        PROVIDERS.resource_api.create_domain(domain["id"], domain)
+        # Override two config options for our domain
+        conf = {
+            "foo": {"opt1": uuid.uuid4().hex},
+            "identity": {"driver": "foo"}}
+        PROVIDERS.domain_config_api.create_config(domain["id"], conf)
+        domain_config = identity.DomainConfigs()
+        domain_config.setup_domain_drivers("foo", PROVIDERS.resource_api)
+        # Make sure our two overrides are in place, and others are not affected
+        res = domain_config.get_domain_conf(domain["id"])
+
+        self.assertEqual(conf["foo"]["opt1"], res.foo.opt1)
+
+        # Reset whitelisted options in the provider directly. Due to the fact
+        # that there are too many singletons used around the code basis there
+        # is a chance of clash when other API domain_config tests are being
+        # executed by the same process. It is NOT ENOUGH just to invoke reset
+        # on fixtures.
+        PROVIDERS.domain_config_api.whitelisted_options.pop("foo", None)

releasenotes/notes/improve-driver-donfiguration-ecedaf6ad0c3f9d2.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Improve configuration management for the out-of-tree identity drivers. When
+    driver implements a special method it is being invoked before instantiating
+    the driver when reading configuration from the database. Also 2 new
+    `domain_config` section configuration options are added to allow such
+    driver specific parameters to be managed using the API.