Relax newly imposed sql driver restriction for domain config

The first implementation of the domain config API introduced a
new restriction in terms of which identity drivers could be SQL
backend (to avoid potential multi-process race conditions). This
restriction specified that only the default domain could be SQL
backend.

This patch resolves these issues and hence ensures the restrictions
in terms of SQL identity drivers are the same as the original config
file based approach (i.e. only one driver can be SQL backend, but
it can be for any given domain).

Although this patch changes the V8 driver interface, this is for
the domain_config backend which is still marked as experimental.

Closes-Bug: #1450344
Change-Id: Id1b4415cb5f263dc7afd9e25bd0da2b667f1c90a
This commit is contained in:
Henry Nash 2015-06-15 21:57:47 +01:00
parent fb9fbbafcd
commit 1696b875dd
8 changed files with 357 additions and 17 deletions

View File

@ -0,0 +1,29 @@
# 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.
import sqlalchemy as sql
REGISTRATION_TABLE = 'config_register'
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
registration_table = sql.Table(
REGISTRATION_TABLE,
meta,
sql.Column('type', sql.String(64), primary_key=True),
sql.Column('domain_id', sql.String(64), nullable=False),
mysql_engine='InnoDB',
mysql_charset='utf8')
registration_table.create(migrate_engine, checkfirst=True)

View File

@ -357,6 +357,13 @@ class DomainConfigNotFound(NotFound):
'configuration for domain %(domain_id)s')
class ConfigRegistrationNotFound(Exception):
# This is used internally between the domain config backend and the
# manager, so should not escape to the client. If it did, it is a coding
# error on our part, and would end up, appropriately, as a 500 error.
pass
class Conflict(Error):
message_format = _("Conflict occurred attempting to store %(type)s -"
" %(details)s")

View File

@ -44,6 +44,14 @@ MEMOIZE = cache.get_memoization_decorator(section='identity')
DOMAIN_CONF_FHEAD = 'keystone.'
DOMAIN_CONF_FTAIL = '.conf'
# The number of times we will attempt to register a domain to use the SQL
# driver, if we find that another process is in the middle of registering or
# releasing at the same time as us.
REGISTRATION_ATTEMPTS = 10
# Config Registration Types
SQL_DRIVER = 'SQL'
def filter_user(user_ref):
"""Filter out private items in a user dict.
@ -67,7 +75,7 @@ def filter_user(user_ref):
return user_ref
@dependency.requires('domain_config_api')
@dependency.requires('domain_config_api', 'resource_api')
class DomainConfigs(dict):
"""Discover, store and provide access to domain specific configs.
@ -95,7 +103,7 @@ class DomainConfigs(dict):
def _assert_no_more_than_one_sql_driver(self, domain_id, new_config,
config_file=None):
"""Ensure there is more than one sql driver.
"""Ensure there is no more than one sql driver.
Check to see if the addition of the driver in this new config
would cause there to now be more than one sql driver.
@ -108,6 +116,13 @@ class DomainConfigs(dict):
(self.driver.is_sql or self._any_sql)):
# The addition of this driver would cause us to have more than
# one sql driver, so raise an exception.
# TODO(henry-nash): This method is only used in the file-based
# case, so has no need to worry about the database/API case. The
# code that overrides config_file below is therefore never used
# and should be removed, and this method perhaps moved inside
# _load_config_from_file(). This is raised as bug #1466772.
if not config_file:
config_file = _('Database at /domains/%s/config') % domain_id
raise exception.MultipleSQLDriversInConfig(source=config_file)
@ -177,19 +192,93 @@ class DomainConfigs(dict):
def _load_config_from_database(self, domain_id, specific_config):
def _assert_not_sql_driver(domain_id, new_config):
"""Ensure this is not an sql driver.
def _assert_no_more_than_one_sql_driver(domain_id, new_config):
"""Ensure adding driver doesn't push us over the limit of 1
Due to multi-threading safety concerns, we do not currently support
the setting of a specific identity driver to sql via the Identity
API.
The checks we make in this method need to take into account that
we may be in a multiple process configuration and ensure that
any race conditions are avoided.
"""
if new_config['driver'].is_sql:
reason = _('Domain specific sql drivers are not supported via '
'the Identity API. One is specified in '
'/domains/%s/config') % domain_id
raise exception.InvalidDomainConfig(reason=reason)
if not new_config['driver'].is_sql:
self.domain_config_api.release_registration(domain_id)
return
# To ensure the current domain is the only SQL driver, we attempt
# to register our use of SQL. If we get it we know we are good,
# if we fail to register it then we should:
#
# - First check if another process has registered for SQL for our
# domain, in which case we are fine
# - If a different domain has it, we should check that this domain
# is still valid, in case, for example, domain deletion somehow
# failed to remove its registration (i.e. we self heal for these
# kinds of issues).
domain_registered = 'Unknown'
for attempt in range(REGISTRATION_ATTEMPTS):
if self.domain_config_api.obtain_registration(
domain_id, SQL_DRIVER):
LOG.debug('Domain %s successfully registered to use the '
'SQL driver.', domain_id)
return
# We failed to register our use, let's find out who is using it
try:
domain_registered = (
self.domain_config_api.read_registration(
SQL_DRIVER))
except exception.ConfigRegistrationNotFound:
msg = ('While attempting to register domain %(domain)s to '
'use the SQL driver, another process released it, '
'retrying (attempt %(attempt)s).')
LOG.debug(msg, {'domain': domain_id,
'attempt': attempt + 1})
continue
if domain_registered == domain_id:
# Another process already registered it for us, so we are
# fine. In the race condition when another process is
# in the middle of deleting this domain, we know the domain
# is already disabled and hence telling the caller that we
# are registered is benign.
LOG.debug('While attempting to register domain %s to use '
'the SQL driver, found that another process had '
'already registered this domain. This is normal '
'in multi-process configurations.', domain_id)
return
# So we don't have it, but someone else does...let's check that
# this domain is still valid
try:
self.resource_api.get_domain(domain_registered)
except exception.DomainNotFound:
msg = ('While attempting to register domain %(domain)s to '
'use the SQL driver, found that it was already '
'registered to a domain that no longer exists '
'(%(old_domain)s). Removing this stale '
'registration and retrying (attempt %(attempt)s).')
LOG.debug(msg, {'domain': domain_id,
'old_domain': domain_registered,
'attempt': attempt + 1})
self.domain_config_api.release_registration(
domain_registered, type=SQL_DRIVER)
continue
# The domain is valid, so we really do have an attempt at more
# than one SQL driver.
details = (
_('Config API entity at /domains/%s/config') % domain_id)
raise exception.MultipleSQLDriversInConfig(source=details)
# We fell out of the loop without either registering our domain or
# being able to find who has it...either we were very very very
# unlucky or something is awry.
msg = _('Exceeded attempts to register domain %(domain)s to use '
'the SQL driver, the last domain that appears to have '
'had it is %(last_domain)s, giving up') % {
'domain': domain_id, 'last_domain': domain_registered}
raise exception.UnexpectedError(msg)
domain_config = {}
domain_config['cfg'] = cfg.ConfigOpts()
@ -207,7 +296,7 @@ class DomainConfigs(dict):
domain_config['cfg_overrides'] = specific_config
domain_config['driver'] = self._load_driver(domain_config)
_assert_not_sql_driver(domain_id, domain_config)
_assert_no_more_than_one_sql_driver(domain_id, domain_config)
self[domain_id] = domain_config
def _setup_domain_drivers_from_database(self, standard_driver,

View File

@ -42,6 +42,12 @@ class SensitiveConfig(sql.ModelBase, sql.ModelDictMixin):
return d
class ConfigRegister(sql.ModelBase, sql.ModelDictMixin):
__tablename__ = 'config_register'
type = sql.Column(sql.String(64), primary_key=True)
domain_id = sql.Column(sql.String(64), nullable=False)
class DomainConfig(resource.DomainConfigDriverV8):
def choose_table(self, sensitive):
@ -117,3 +123,30 @@ class DomainConfig(resource.DomainConfigDriverV8):
if option:
query = query.filter_by(option=option)
query.delete(False)
def obtain_registration(self, domain_id, type):
try:
with sql.transaction() as session:
ref = ConfigRegister(type=type, domain_id=domain_id)
session.add(ref)
return True
except sql.DBDuplicateEntry:
pass
return False
def read_registration(self, type):
with sql.transaction() as session:
ref = session.query(ConfigRegister).get(type)
if not ref:
raise exception.ConfigRegistrationNotFound()
return ref.domain_id
def release_registration(self, domain_id, type=None):
"""Silently delete anything registered for the domain specified."""
with sql.transaction() as session:
query = session.query(ConfigRegister)
if type:
query = query.filter_by(type=type)
query = query.filter_by(domain_id=domain_id)
query.delete(False)

View File

@ -460,6 +460,7 @@ class Manager(manager.Manager):
# Delete any database stored domain config
self.domain_config_api.delete_config_options(domain_id)
self.domain_config_api.delete_config_options(domain_id, sensitive=True)
self.domain_config_api.release_registration(domain_id)
# TODO(henry-nash): Although the controller will ensure deletion of
# all users & groups within the domain (which will cause all
# assignments for those users/groups to also be deleted), there
@ -1373,5 +1374,44 @@ class DomainConfigDriverV8(object):
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def obtain_registration(self, domain_id, type):
"""Try and register this domain to use the type specified.
:param domain_id: the domain required
:param type: type of registration
:returns: True if the domain was registered, False otherwise. Failing
to register means that someone already has it (which could
even be the domain being requested).
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def read_registration(self, type):
"""Get the domain ID of who is registered to use this type.
:param type: type of registration
:returns: domain_id of who is registered.
:raises: keystone.exception.ConfigRegistrationNotFound: nobody is
registered.
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def release_registration(self, domain_id, type=None):
"""Release registration if it is held by the domain specified.
If the specified domain is registered for this domain then free it,
if it is not then do nothing - no exception is raised.
:param domain_id: the domain in question
:param type: type of registration, if None then all registrations
for this domain will be freed
"""
raise exception.NotImplemented() # pragma: no cover
DomainConfigDriver = manager.create_legacy_driver(DomainConfigDriverV8)

View File

@ -549,3 +549,53 @@ class DomainConfigTests(object):
{},
self.domain_config_api.get_config_with_sensitive_info(
self.domain['id']))
def test_config_registration(self):
type = uuid.uuid4().hex
self.domain_config_api.obtain_registration(
self.domain['id'], type)
self.domain_config_api.release_registration(
self.domain['id'], type=type)
# Make sure that once someone has it, nobody else can get it.
# This includes the domain who already has it.
self.domain_config_api.obtain_registration(
self.domain['id'], type)
self.assertFalse(
self.domain_config_api.obtain_registration(
self.domain['id'], type))
# Make sure we can read who does have it
self.assertEqual(
self.domain['id'],
self.domain_config_api.read_registration(type))
# Make sure releasing it is silent if the domain specified doesn't
# have the registration
domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
self.resource_api.create_domain(domain2['id'], domain2)
self.domain_config_api.release_registration(
domain2['id'], type=type)
# If nobody has the type registered, then trying to read it should
# raise ConfigRegistrationNotFound
self.domain_config_api.release_registration(
self.domain['id'], type=type)
self.assertRaises(exception.ConfigRegistrationNotFound,
self.domain_config_api.read_registration,
type)
# Finally check multiple registrations are cleared if you free the
# registration without specifying the type
type2 = uuid.uuid4().hex
self.domain_config_api.obtain_registration(
self.domain['id'], type)
self.domain_config_api.obtain_registration(
self.domain['id'], type2)
self.domain_config_api.release_registration(self.domain['id'])
self.assertRaises(exception.ConfigRegistrationNotFound,
self.domain_config_api.read_registration,
type)
self.assertRaises(exception.ConfigRegistrationNotFound,
self.domain_config_api.read_registration,
type2)

View File

@ -2943,15 +2943,100 @@ class MultiLDAPandSQLIdentityDomainConfigsInSQL(MultiLDAPandSQLIdentity):
domain_cfgs.get_domain_conf(CONF.identity.default_domain_id))
self.assertEqual(CONF.ldap.url, default_config.ldap.url)
def test_setting_sql_driver_raises_exception(self):
"""Ensure setting of domain specific sql driver is prevented."""
def test_setting_multiple_sql_driver_raises_exception(self):
"""Ensure setting multiple domain specific sql drivers is prevented."""
new_config = {'identity': {'driver': 'sql'}}
self.domain_config_api.create_config(
CONF.identity.default_domain_id, new_config)
self.assertRaises(exception.InvalidDomainConfig,
self.identity_api.domain_configs.get_domain_conf(
CONF.identity.default_domain_id)
self.domain_config_api.create_config(self.domains['domain1']['id'],
new_config)
self.assertRaises(exception.MultipleSQLDriversInConfig,
self.identity_api.domain_configs.get_domain_conf,
CONF.identity.default_domain_id)
self.domains['domain1']['id'])
def test_same_domain_gets_sql_driver(self):
"""Ensure we can set an SQL driver if we have had it before."""
new_config = {'identity': {'driver': 'sql'}}
self.domain_config_api.create_config(
CONF.identity.default_domain_id, new_config)
self.identity_api.domain_configs.get_domain_conf(
CONF.identity.default_domain_id)
# By using a slightly different config, we cause the driver to be
# reloaded...and hence check if we can reuse the sql driver
new_config = {'identity': {'driver': 'sql'},
'ldap': {'url': 'fake://memory1'}}
self.domain_config_api.create_config(
CONF.identity.default_domain_id, new_config)
self.identity_api.domain_configs.get_domain_conf(
CONF.identity.default_domain_id)
def test_delete_domain_clears_sql_registration(self):
"""Ensure registration is deleted when a domain is deleted."""
domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
domain = self.resource_api.create_domain(domain['id'], domain)
new_config = {'identity': {'driver': 'sql'}}
self.domain_config_api.create_config(domain['id'], new_config)
self.identity_api.domain_configs.get_domain_conf(domain['id'])
# First show that trying to set SQL for another driver fails
self.domain_config_api.create_config(self.domains['domain1']['id'],
new_config)
self.assertRaises(exception.MultipleSQLDriversInConfig,
self.identity_api.domain_configs.get_domain_conf,
self.domains['domain1']['id'])
self.domain_config_api.delete_config(self.domains['domain1']['id'])
# Now we delete the domain
domain['enabled'] = False
self.resource_api.update_domain(domain['id'], domain)
self.resource_api.delete_domain(domain['id'])
# The registration should now be available
self.domain_config_api.create_config(self.domains['domain1']['id'],
new_config)
self.identity_api.domain_configs.get_domain_conf(
self.domains['domain1']['id'])
def test_orphaned_registration_does_not_prevent_getting_sql_driver(self):
"""Ensure we self heal an orphaned sql registration."""
domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
domain = self.resource_api.create_domain(domain['id'], domain)
new_config = {'identity': {'driver': 'sql'}}
self.domain_config_api.create_config(domain['id'], new_config)
self.identity_api.domain_configs.get_domain_conf(domain['id'])
# First show that trying to set SQL for another driver fails
self.domain_config_api.create_config(self.domains['domain1']['id'],
new_config)
self.assertRaises(exception.MultipleSQLDriversInConfig,
self.identity_api.domain_configs.get_domain_conf,
self.domains['domain1']['id'])
# Now we delete the domain by using the backend driver directly,
# which causes the domain to be deleted without any of the cleanup
# that is in the manager (this is simulating a server process crashing
# in the middle of a delete domain operation, and somehow leaving the
# domain config settings in place, but the domain is deleted). We
# should still be able to set another domain to SQL, since we should
# self heal this issue.
self.resource_api.driver.delete_domain(domain['id'])
# Invalidate cache (so we will see the domain has gone)
self.resource_api.get_domain.invalidate(
self.resource_api, domain['id'])
# The registration should now be available
self.domain_config_api.create_config(self.domains['domain1']['id'],
new_config)
self.identity_api.domain_configs.get_domain_conf(
self.domains['domain1']['id'])
class DomainSpecificLDAPandSQLIdentity(

View File

@ -636,6 +636,13 @@ class SqlUpgradeTests(SqlMigrateBase):
'enabled', 'domain_id', 'parent_id',
'is_domain'])
def test_add_config_registration(self):
config_registration = 'config_register'
self.upgrade(74)
self.assertTableDoesNotExist(config_registration)
self.upgrade(75)
self.assertTableColumns(config_registration, ['type', 'domain_id'])
def populate_user_table(self, with_pass_enab=False,
with_pass_enab_domain=False):
# Populate the appropriate fields in the user