diff --git a/doc/source/contributor/developing-drivers.rst b/doc/source/contributor/developing-drivers.rst index 7f3f02fa7d..e17e0c441d 100644 --- a/doc/source/contributor/developing-drivers.rst +++ b/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=:[opt1,opt2,opt3] + additional_sensitive_options=:[password] + +The `` 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 ------------------------ diff --git a/keystone/conf/domain_config.py b/keystone/conf/domain_config.py index 1fa4b66120..93814851f3 100644 --- a/keystone/conf/domain_config.py +++ b/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 ] diff --git a/keystone/identity/backends/base.py b/keystone/identity/backends/base.py index 45c454a4ad..666540cc8b 100644 --- a/keystone/identity/backends/base.py +++ b/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 diff --git a/keystone/identity/core.py b/keystone/identity/core.py index d7e5344865..f440bfc31f 100644 --- a/keystone/identity/core.py +++ b/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( - option, specific_config[group][option], group) + # NOTE(gtema): Very first time default driver is being ordered + # 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) diff --git a/keystone/resource/core.py b/keystone/resource/core.py index 5676a44953..659540af05 100644 --- a/keystone/resource/core.py +++ b/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 - self.sensitive_options[group]): + if (option not in self.whitelisted_options.get(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.""" diff --git a/keystone/tests/unit/identity/backends/fake_driver.py b/keystone/tests/unit/identity/backends/fake_driver.py new file mode 100644 index 0000000000..be7b2bd67e --- /dev/null +++ b/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 diff --git a/keystone/tests/unit/identity/test_core.py b/keystone/tests/unit/identity/test_core.py index 45ff9e97d7..ee077daad6 100644 --- a/keystone/tests/unit/identity/test_core.py +++ b/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) diff --git a/releasenotes/notes/improve-driver-donfiguration-ecedaf6ad0c3f9d2.yaml b/releasenotes/notes/improve-driver-donfiguration-ecedaf6ad0c3f9d2.yaml new file mode 100644 index 0000000000..76d483180b --- /dev/null +++ b/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.