Allow project domain_id to be nullable at the manager level

Since the domain_id column is part of a uniqueness constraint and
it might now contain a nullable value, we actually store a special
value (as opposed to None) in the sql attribute so as to still use
the sql uniqueness constraint to ensure we do not have race
conditions in multi-process keystone configurations. So as to
continue to support a FK on the project.domain_id column, we create
a hidden domain ("the root of all domains"). In preparation for the
move to projects acting as domains, we also create a matching project
for this root. These special rows, and indeed the special domain_id
value stored, are entirely hidden within the sql driver and are not
exposed to the manager layer and above. The ability to store a project
domain_id of None is also added to the wrapper for legacy drivers.

In addition, one incorrectly written test relating to upcoming support
of projects as a domain had to be modified, since it started
passing prematurely, and the (now unused) driver base class method
for setting a domain_id is moved to the legacy V8 class.

Follow-on patches will build on this enable projects acting as domains,
which will have a null domain_id.

Co-Authored-By: Henry Nash <henryn@linux.vnet.ibm.com>
Partially-Implements: bp reseller

Change-Id: I4e5b9f3978482082c23f729d29bbdc701bc7c473
This commit is contained in:
Henrique Truta 2016-01-06 20:52:09 -03:00 committed by Henry Nash
parent a320eaa903
commit 24b5dd021d
7 changed files with 398 additions and 37 deletions

View File

@ -0,0 +1,76 @@
# 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
_PROJECT_TABLE_NAME = 'project'
_DOMAIN_TABLE_NAME = 'domain'
NULL_DOMAIN_ID = '<<keystone.domain.root>>'
def upgrade(migrate_engine):
def _generate_root_domain_project():
# Generate a project that will act as a root for all domains, in order
# for use to be able to use a FK constraint on domain_id. Projects
# acting as a domain will not reference this as their parent_id, just
# as domain_id.
#
# This special project is filtered out by the driver, so is never
# visible to the manager or API.
project_ref = {
'id': NULL_DOMAIN_ID,
'name': NULL_DOMAIN_ID,
'enabled': False,
'description': '',
'domain_id': NULL_DOMAIN_ID,
'is_domain': True,
'parent_id': None,
'extra': '{}'
}
return project_ref
def _generate_root_domain():
# Generate a similar root for the domain table, this is an interim
# step so as to allow continuation of current project domain_id FK.
#
# This special domain is filtered out by the driver, so is never
# visible to the manager or API.
domain_ref = {
'id': NULL_DOMAIN_ID,
'name': NULL_DOMAIN_ID,
'enabled': False,
'extra': '{}'
}
return domain_ref
meta = sql.MetaData()
meta.bind = migrate_engine
session = sql.orm.sessionmaker(bind=migrate_engine)()
project_table = sql.Table(_PROJECT_TABLE_NAME, meta, autoload=True)
domain_table = sql.Table(_DOMAIN_TABLE_NAME, meta, autoload=True)
root_domain = _generate_root_domain()
new_entry = domain_table.insert().values(**root_domain)
session.execute(new_entry)
session.commit()
root_domain_project = _generate_root_domain_project()
new_entry = project_table.insert().values(**root_domain_project)
session.execute(new_entry)
session.commit()
session.close()

View File

@ -30,9 +30,12 @@ class Resource(keystone_resource.ResourceDriverV9):
def default_assignment_driver(self):
return 'sql'
def _is_hidden_ref(self, ref):
return ref.id == keystone_resource.NULL_DOMAIN_ID
def _get_project(self, session, project_id):
project_ref = session.query(Project).get(project_id)
if project_ref is None:
if project_ref is None or self._is_hidden_ref(project_ref):
raise exception.ProjectNotFound(project_id=project_id)
return project_ref
@ -44,19 +47,36 @@ class Resource(keystone_resource.ResourceDriverV9):
with sql.transaction() as session:
query = session.query(Project)
query = query.filter_by(name=project_name)
query = query.filter_by(domain_id=domain_id)
if domain_id is None:
query = query.filter_by(
domain_id=keystone_resource.NULL_DOMAIN_ID)
else:
query = query.filter_by(domain_id=domain_id)
try:
project_ref = query.one()
except sql.NotFound:
raise exception.ProjectNotFound(project_id=project_name)
if self._is_hidden_ref(project_ref):
raise exception.ProjectNotFound(project_id=project_name)
return project_ref.to_dict()
@driver_hints.truncated
def list_projects(self, hints):
# If there is a filter on domain_id and the value is None, then to
# ensure that the sql filtering works correctly, we need to patch
# the value to be NULL_DOMAIN_ID. This is safe to do here since we
# know we are able to satisfy any filter of this type in the call to
# filter_limit_query() below, which will remove the filter from the
# hints (hence ensuring our substitution is not exposed to the caller).
for f in hints.filters:
if (f['name'] == 'domain_id' and f['value'] is None):
f['value'] = keystone_resource.NULL_DOMAIN_ID
with sql.transaction() as session:
query = session.query(Project)
project_refs = sql.filter_limit_query(Project, query, hints)
return [project_ref.to_dict() for project_ref in project_refs]
return [project_ref.to_dict() for project_ref in project_refs
if not self._is_hidden_ref(project_ref)]
def list_projects_from_ids(self, ids):
if not ids:
@ -65,7 +85,8 @@ class Resource(keystone_resource.ResourceDriverV9):
with sql.transaction() as session:
query = session.query(Project)
query = query.filter(Project.id.in_(ids))
return [project_ref.to_dict() for project_ref in query.all()]
return [project_ref.to_dict() for project_ref in query.all()
if not self._is_hidden_ref(project_ref)]
def list_project_ids_from_domain_ids(self, domain_ids):
if not domain_ids:
@ -75,7 +96,8 @@ class Resource(keystone_resource.ResourceDriverV9):
query = session.query(Project.id)
query = (
query.filter(Project.domain_id.in_(domain_ids)))
return [x.id for x in query.all()]
return [x.id for x in query.all()
if not self._is_hidden_ref(x)]
def list_projects_in_domain(self, domain_id):
with sql.transaction() as session:
@ -140,8 +162,9 @@ class Resource(keystone_resource.ResourceDriverV9):
@sql.handle_conflicts(conflict_type='project')
def create_project(self, project_id, project):
project['name'] = clean.project_name(project['name'])
new_project = self._encode_domain_id(project)
with sql.transaction() as session:
project_ref = Project.from_dict(project)
project_ref = Project.from_dict(new_project)
session.add(project_ref)
return project_ref.to_dict()
@ -150,11 +173,15 @@ class Resource(keystone_resource.ResourceDriverV9):
if 'name' in project:
project['name'] = clean.project_name(project['name'])
update_project = self._encode_domain_id(project)
with sql.transaction() as session:
project_ref = self._get_project(session, project_id)
old_project_dict = project_ref.to_dict()
for k in project:
old_project_dict[k] = project[k]
for k in update_project:
old_project_dict[k] = update_project[k]
# When we read the old_project_dict, any "null" domain_id will have
# been decoded, so we need to re-encode it
old_project_dict = self._encode_domain_id(old_project_dict)
new_project = Project.from_dict(old_project_dict)
for attr in Project.attributes:
if attr != 'id':
@ -177,7 +204,8 @@ class Resource(keystone_resource.ResourceDriverV9):
project_ids))
project_ids_from_bd = [p['id'] for p in query.all()]
for project_id in project_ids:
if project_id not in project_ids_from_bd:
if (project_id not in project_ids_from_bd or
project_id == keystone_resource.NULL_DOMAIN_ID):
LOG.warning(_LW('Project %s does not exist and was not '
'deleted.') % project_id)
query.delete(synchronize_session=False)
@ -196,7 +224,8 @@ class Resource(keystone_resource.ResourceDriverV9):
with sql.transaction() as session:
query = session.query(Domain)
refs = sql.filter_limit_query(Domain, query, hints)
return [ref.to_dict() for ref in refs]
return [ref.to_dict() for ref in refs
if not self._is_hidden_ref(ref)]
def list_domains_from_ids(self, ids):
if not ids:
@ -206,11 +235,12 @@ class Resource(keystone_resource.ResourceDriverV9):
query = session.query(Domain)
query = query.filter(Domain.id.in_(ids))
domain_refs = query.all()
return [domain_ref.to_dict() for domain_ref in domain_refs]
return [domain_ref.to_dict() for domain_ref in domain_refs
if not self._is_hidden_ref(domain_ref)]
def _get_domain(self, session, domain_id):
ref = session.query(Domain).get(domain_id)
if ref is None:
if ref is None or self._is_hidden_ref(ref):
raise exception.DomainNotFound(domain_id=domain_id)
return ref
@ -225,6 +255,9 @@ class Resource(keystone_resource.ResourceDriverV9):
filter_by(name=domain_name).one())
except sql.NotFound:
raise exception.DomainNotFound(domain_id=domain_name)
if self._is_hidden_ref(ref):
raise exception.DomainNotFound(domain_id=domain_name)
return ref.to_dict()
@sql.handle_conflicts(conflict_type='domain')
@ -258,6 +291,20 @@ class Domain(sql.ModelBase, sql.DictBase):
class Project(sql.ModelBase, sql.DictBase):
# NOTE(henry-nash): From the manager and above perspective, the domain_id
# is nullable. However, to ensure uniqueness in multi-process
# configurations, it is better to still use the sql uniqueness constraint.
# Since the support for a nullable component of a uniqueness constraint
# across different sql databases is mixed, we instead store a special value
# to represent null, as defined in NULL_DOMAIN_ID above.
def to_dict(self, include_extra_dict=False):
d = super(Project, self).to_dict(
include_extra_dict=include_extra_dict)
if d['domain_id'] == keystone_resource.NULL_DOMAIN_ID:
d['domain_id'] = None
return d
__tablename__ = 'project'
attributes = ['id', 'name', 'domain_id', 'description', 'enabled',
'parent_id', 'is_domain']

View File

@ -119,15 +119,15 @@ class Manager(manager.Manager):
"""Enforces regular project hierarchy constraints
Called when is_domain is false, and the project must contain a valid
parent_id or domain_id (this is checked in the controller). If the
parent is a regular project, this project must be in the same domain
of its parent.
domain_id and may have a parent. If the parent is a regular project,
this project must be in the same domain as its parent.
:raises keystone.exception.ValidationError: If one of the constraints
was not satisfied.
:raises keystone.exception.DomainNotFound: In case the domain is not
found.
"""
# Ensure domain_id is valid, and by inference will not be None.
domain = self.get_domain(project_ref['domain_id'])
if parent_ref:
@ -720,12 +720,30 @@ class Manager(manager.Manager):
# class to the V8 class. Do not modify any of the method signatures in the Base
# class - changes should only be made in the V8 and subsequent classes.
# Starting with V9, some drivers support a null domain_id by storing a special
# value in its place.
NULL_DOMAIN_ID = '<<keystone.domain.root>>'
@six.add_metaclass(abc.ABCMeta)
class ResourceDriverBase(object):
def _get_list_limit(self):
return CONF.resource.list_limit or CONF.list_limit
def _encode_domain_id(self, ref):
if 'domain_id' in ref and ref['domain_id'] is None:
new_ref = ref.copy()
new_ref['domain_id'] = NULL_DOMAIN_ID
return new_ref
else:
return ref
def _decode_domain_id(self, ref):
if ref['domain_id'] == NULL_DOMAIN_ID:
ref['domain_id'] = None
return ref
# domain crud
@abc.abstractmethod
def create_domain(self, domain_id, domain):
@ -940,21 +958,6 @@ class ResourceDriverBase(object):
"""
raise exception.NotImplemented()
# Domain management functions for backends that only allow a single
# domain. Currently, this is only LDAP, but might be used by other
# backends in the future.
def _set_default_domain(self, ref):
"""If the domain ID has not been set, set it to the default."""
if isinstance(ref, dict):
if 'domain_id' not in ref:
ref = ref.copy()
ref['domain_id'] = CONF.identity.default_domain_id
return ref
elif isinstance(ref, list):
return [self._set_default_domain(x) for x in ref]
else:
raise ValueError(_('Expected dict or list: %s') % type(ref))
def _validate_default_domain(self, ref):
"""Validate that either the default domain or nothing is specified.
@ -994,6 +997,21 @@ class ResourceDriverV8(ResourceDriverBase):
"""
raise exception.NotImplemented() # pragma: no cover
# Domain management functions for backends that only allow a single
# domain. Although we no longer use this, a custom legacy driver might
# have made use of it, so keep it here in case.
def _set_default_domain(self, ref):
"""If the domain ID has not been set, set it to the default."""
if isinstance(ref, dict):
if 'domain_id' not in ref:
ref = ref.copy()
ref['domain_id'] = CONF.identity.default_domain_id
return ref
elif isinstance(ref, list):
return [self._set_default_domain(x) for x in ref]
else:
raise ValueError(_('Expected dict or list: %s') % type(ref))
class ResourceDriverV9(ResourceDriverBase):
"""New or redefined methods from V8.
@ -1047,6 +1065,18 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9):
since we do not guarantee to support new
functionality with legacy drivers.
This wrapper contains the following support for newer manager code:
- The current manager code expects to be able to store a project domain_id
with a value of None, something that earlier drivers may not support.
Hence we pre and post process the domain_id value to substitute it for
a special value (defined by NULL_DOMAIN_ID). In fact the V9 and later
drivers use this same algorithm internally (to ensure uniqueness
constraints including domain_id can still work), so, although not a
specific goal of our legacy driver support, using this same value may
ease any customers who want to migrate from a legacy customer driver to
the standard community driver.
"""
@versionutils.deprecated(
@ -1058,7 +1088,12 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9):
self.driver = wrapped_driver
def get_project_by_name(self, project_name, domain_id):
return self.driver.get_project_by_name(project_name, domain_id)
if domain_id is None:
actual_domain_id = NULL_DOMAIN_ID
else:
actual_domain_id = domain_id
return self._decode_domain_id(
self.driver.get_project_by_name(project_name, actual_domain_id))
def create_domain(self, domain_id, domain):
return self.driver.create_domain(domain_id, domain)
@ -1082,25 +1117,44 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9):
self.driver.delete_domain(domain_id)
def create_project(self, project_id, project):
return self.driver.create_project(project_id, project)
new_project = self._encode_domain_id(project)
return self._decode_domain_id(
self.driver.create_project(project_id, new_project))
def list_projects(self, hints):
return self.driver.list_projects(hints)
for f in hints.filters:
if (f['name'] == 'domain_id' and f['value'] is None):
f['value'] = NULL_DOMAIN_ID
refs = self.driver.list_projects(hints)
# We can't assume the driver was able to satisfy any given filter,
# so check that the domain_id filter isn't still in there. If it is,
# see if we need to re-substitute the None value. This will allow
# an unsatisfied filter to be processed by the controller wrapper.
for f in hints.filters:
if (f['name'] == 'domain_id' and f['value'] == NULL_DOMAIN_ID):
f['value'] = None
return [self._decode_domain_id(p) for p in refs]
def list_projects_from_ids(self, project_ids):
return self.driver.list_projects_from_ids(project_ids)
return [self._decode_domain_id(p)
for p in self.driver.list_projects_from_ids(project_ids)]
def list_project_ids_from_domain_ids(self, domain_ids):
return self.driver.list_project_ids_from_domain_ids(domain_ids)
def list_projects_in_domain(self, domain_id):
# Projects within a domain never have a null domain_id, so no need to
# post-process the output
return self.driver.list_projects_in_domain(domain_id)
def get_project(self, project_id):
return self.driver.get_project(project_id)
return self._decode_domain_id(
self.driver.get_project(project_id))
def update_project(self, project_id, project):
return self.driver.update_project(project_id, project)
update_project = self._encode_domain_id(project)
return self._decode_domain_id(
self.driver.update_project(project_id, update_project))
def delete_project(self, project_id):
self.driver.delete_project(project_id)
@ -1109,7 +1163,8 @@ class V9ResourceWrapperForV8Driver(ResourceDriverV9):
raise exception.NotImplemented() # pragma: no cover
def list_project_parents(self, project_id):
return self.driver.list_project_parents(project_id)
return [self._decode_domain_id(p)
for p in self.driver.list_project_parents(project_id)]
def list_projects_in_subtree(self, project_id):
return self.driver.list_projects_in_subtree(project_id)

View File

@ -40,3 +40,6 @@ class SqlIdentityV8(test_backend_sql.SqlIdentity):
def test_delete_large_project_cascade(self):
self.skipTest('Operation not supported in v8 and earlier drivers')
def test_hidden_project_domain_root_is_really_hidden(self):
self.skipTest('Operation not supported in v8 and earlier drivers')

View File

@ -2593,6 +2593,9 @@ class IdentityTests(AssignmentTestHelperMixin):
is_domain=True)
self.resource_api.create_project(project['id'], project)
# Check that a corresponding domain was created
self.resource_api.get_domain(project['id'])
# Try to delete is_domain project that is enabled
self.assertRaises(exception.ValidationError,
self.resource_api.delete_project,
@ -2605,6 +2608,14 @@ class IdentityTests(AssignmentTestHelperMixin):
# Successfully delete the project
self.resource_api.delete_project(project['id'])
self.assertRaises(exception.ProjectNotFound,
self.resource_api.get_project,
project['id'])
self.assertRaises(exception.DomainNotFound,
self.resource_api.get_domain,
project['id'])
@unit.skip_if_no_multiple_domains_support
def test_create_subproject_acting_as_domain_fails(self):
root_project = {'id': uuid.uuid4().hex,

View File

@ -29,6 +29,7 @@ from keystone.common import driver_hints
from keystone.common import sql
from keystone import exception
from keystone.identity.backends import sql as identity_sql
from keystone import resource
from keystone.tests import unit
from keystone.tests.unit import default_fixtures
from keystone.tests.unit.ksfixtures import database
@ -411,6 +412,155 @@ class SqlIdentity(SqlTests, test_backend.IdentityTests):
# roles assignments.
self.assertThat(user_domains, matchers.HasLength(0))
def test_storing_null_domain_id_in_project_ref(self):
"""Test the special storage of domain_id=None in sql resource driver.
The resource driver uses a special value in place of None for domain_id
in the project record. This shouldn't escape the driver. Hence we test
the interface to ensure that you can store a domain_id of None, and
that any special value used inside the driver does not escape through
the interface.
"""
spoiler_project = unit.new_project_ref(domain_id=DEFAULT_DOMAIN_ID)
self.resource_api.create_project(spoiler_project['id'],
spoiler_project)
# First let's create a project with a None domain_id and make sure we
# can read it back.
project = unit.new_project_ref(domain_id=None, is_domain=True)
project = self.resource_api.create_project(project['id'], project)
ref = self.resource_api.get_project(project['id'])
self.assertDictEqual(project, ref)
# Can we get it by name?
ref = self.resource_api.get_project_by_name(project['name'], None)
self.assertDictEqual(project, ref)
# Can we filter for them - create a second domain to ensure we are
# testing the receipt of more than one.
project2 = unit.new_project_ref(domain_id=None, is_domain=True)
project2 = self.resource_api.create_project(project2['id'], project2)
hints = driver_hints.Hints()
hints.add_filter('domain_id', None)
refs = self.resource_api.list_projects(hints)
self.assertThat(refs, matchers.HasLength(2))
self.assertIn(project, refs)
self.assertIn(project2, refs)
# Can we update it?
project['name'] = uuid.uuid4().hex
self.resource_api.update_project(project['id'], project)
ref = self.resource_api.get_project(project['id'])
self.assertDictEqual(project, ref)
# Finally, make sure we can delete it
project['enabled'] = False
self.resource_api.update_project(project['id'], project)
self.resource_api.delete_project(project['id'])
self.assertRaises(exception.ProjectNotFound,
self.resource_api.get_project,
project['id'])
def test_hidden_project_domain_root_is_really_hidden(self):
"""Ensure we cannot access the hidden root of all project domains.
Calling any of the driver methods should result in the same as
would be returned if we passed a project that does not exist. We don't
test create_project, since we do not allow a caller of our API to
specify their own ID for a new entity.
"""
def _exercise_project_api(ref_id):
driver = self.resource_api.driver
self.assertRaises(exception.ProjectNotFound,
driver.get_project,
ref_id)
self.assertRaises(exception.ProjectNotFound,
driver.get_project_by_name,
resource.NULL_DOMAIN_ID,
ref_id)
project_ids = [x['id'] for x in
driver.list_projects(driver_hints.Hints())]
self.assertNotIn(ref_id, project_ids)
projects = driver.list_projects_from_ids([ref_id])
self.assertThat(projects, matchers.HasLength(0))
project_ids = [x for x in
driver.list_project_ids_from_domain_ids([ref_id])]
self.assertNotIn(ref_id, project_ids)
self.assertRaises(exception.DomainNotFound,
driver.list_projects_in_domain,
ref_id)
projects = driver.list_projects_in_subtree(ref_id)
self.assertThat(projects, matchers.HasLength(0))
self.assertRaises(exception.ProjectNotFound,
driver.list_project_parents,
ref_id)
# A non-existing project just returns True from the driver
self.assertTrue(driver.is_leaf_project(ref_id))
self.assertRaises(exception.ProjectNotFound,
driver.update_project,
ref_id,
{})
self.assertRaises(exception.ProjectNotFound,
driver.delete_project,
ref_id)
# Deleting list of projects that includes a non-existing project
# should be silent
driver.delete_projects_from_ids([ref_id])
_exercise_project_api(uuid.uuid4().hex)
_exercise_project_api(resource.NULL_DOMAIN_ID)
def test_hidden_domain_root_is_really_hidden(self):
"""Ensure we cannot access the hidden root of all domains.
Calling any of the driver methods should result in the same as
would be returned if we passed a domain that does not exist. We don't
test create_domain, since we do not allow a caller of our API to
specify their own ID for a new entity.
"""
def _exercise_domain_api(ref_id):
driver = self.resource_api.driver
self.assertRaises(exception.DomainNotFound,
driver.get_domain,
ref_id)
self.assertRaises(exception.DomainNotFound,
driver.get_domain_by_name,
resource.NULL_DOMAIN_ID)
domain_ids = [x['id'] for x in
driver.list_domains(driver_hints.Hints())]
self.assertNotIn(ref_id, domain_ids)
domains = driver.list_domains_from_ids([ref_id])
self.assertThat(domains, matchers.HasLength(0))
self.assertRaises(exception.DomainNotFound,
driver.update_domain,
ref_id,
{})
self.assertRaises(exception.DomainNotFound,
driver.delete_domain,
ref_id)
_exercise_domain_api(uuid.uuid4().hex)
_exercise_domain_api(resource.NULL_DOMAIN_ID)
class SqlTrust(SqlTests, test_backend.TrustTests):
pass

View File

@ -681,6 +681,25 @@ class SqlUpgradeTests(SqlMigrateBase):
role_entry = session.execute(statement).fetchone()
self.assertEqual(NULL_DOMAIN_ID, role_entry[0])
def test_add_root_of_all_domains(self):
NULL_DOMAIN_ID = '<<keystone.domain.root>>'
self.upgrade(89)
session = self.Session()
domain_table = sqlalchemy.Table(
'domain', self.metadata, autoload=True)
query = session.query(domain_table).filter_by(id=NULL_DOMAIN_ID)
domain_from_db = query.one()
self.assertIn(NULL_DOMAIN_ID, domain_from_db)
project_table = sqlalchemy.Table(
'project', self.metadata, autoload=True)
query = session.query(project_table).filter_by(id=NULL_DOMAIN_ID)
project_from_db = query.one()
self.assertIn(NULL_DOMAIN_ID, project_from_db)
session.close()
def populate_user_table(self, with_pass_enab=False,
with_pass_enab_domain=False):
# Populate the appropriate fields in the user