Remove CONF.domain_id_immutable
We deprecated this functionality during the Mitaka release and staged it for removal in Ocata. Let's remove it! A note for reviewers. Some of the logic for allowing projects to update their domain_id was implemented in the resource Manager(). As a result, there were several tests that tested the manager directly. Now that we're removing the logic for allowing ``domain_id`` to be update, we can rely solely on the ``_require_matching_domain_id()`` method in the keystone.common.controller module. This short-circuit check will return a 4XX response code for all requests attempting to update the ``domain_id`` of an entity. Since this is all completely isolated in the controller layer, we can remove the logic from the resource Manager specifically as well as the tests. This change looks like we're removing a bunch of tests from keystone.tests.unit.resource.test_backends but we're actually just relying on more basic tests in test_v3_resource. Change-Id: Iad0eba66e7ddc9497205af60671451a385d0de58 Closes-Bug: 1653472
This commit is contained in:
Lance Bragstad
parent
f9a5002fcd
commit
663865dfec
|
|
@ -681,7 +681,7 @@ class V3Controller(wsgi.Application):
|
|||
# TODO(henry-nash): It might be safer and more efficient to do this
|
||||
# check in the managers affected, so look to migrate this check to
|
||||
# there in the future.
|
||||
if CONF.domain_id_immutable and 'domain_id' in ref:
|
||||
if 'domain_id' in ref:
|
||||
existing_ref = get_member(ref_id)
|
||||
if ref['domain_id'] != existing_ref['domain_id']:
|
||||
raise exception.ValidationError(_('Cannot change Domain ID'))
|
||||
|
|
|
|||
|
|
@ -16,11 +16,6 @@ from oslo_log import versionutils
|
|||
from keystone.conf import utils
|
||||
|
||||
|
||||
_DEPRECATE_MUTABLE_DOMAIN_IDS = utils.fmt("""
|
||||
The option to set domain_id_immutable to false has been deprecated in the M
|
||||
release and will be removed in the O release.
|
||||
""")
|
||||
|
||||
_DEPRECATE_PROXY_SSL = utils.fmt("""
|
||||
This option has been deprecated in the N release and will be removed in the P
|
||||
release. Use oslo.middleware.http_proxy_to_wsgi configuration instead.
|
||||
|
|
@ -147,21 +142,6 @@ to a reasonable number to prevent operations like listing all users and
|
|||
projects from placing an unnecessary load on the system.
|
||||
"""))
|
||||
|
||||
domain_id_immutable = cfg.BoolOpt(
|
||||
'domain_id_immutable',
|
||||
default=True,
|
||||
deprecated_for_removal=True,
|
||||
deprecated_reason=_DEPRECATE_MUTABLE_DOMAIN_IDS,
|
||||
deprecated_since=versionutils.deprecated.MITAKA,
|
||||
help=utils.fmt("""
|
||||
Set this to false if you want to enable the ability for user, group and project
|
||||
entities to be moved between domains by updating their `domain_id` attribute.
|
||||
Allowing such movement is not recommended if the scope of a domain admin is
|
||||
being restricted by use of an appropriate policy file (see
|
||||
`etc/policy.v3cloudsample.json` as an example). This feature is deprecated and
|
||||
will be removed in a future release, in favor of strictly immutable domain IDs.
|
||||
"""))
|
||||
|
||||
strict_password_check = cfg.BoolOpt(
|
||||
'strict_password_check',
|
||||
default=False,
|
||||
|
|
@ -244,7 +224,6 @@ ALL_OPTS = [
|
|||
member_role_name,
|
||||
crypt_strength,
|
||||
list_limit,
|
||||
domain_id_immutable,
|
||||
strict_password_check,
|
||||
secure_proxy_ssl_header,
|
||||
insecure_debug,
|
||||
|
|
|
|||
|
|
@ -13,7 +13,6 @@
|
|||
"""Main entry point into the Resource service."""
|
||||
|
||||
from oslo_log import log
|
||||
from oslo_log import versionutils
|
||||
import six
|
||||
|
||||
from keystone import assignment
|
||||
|
|
@ -312,38 +311,6 @@ class Manager(manager.Manager):
|
|||
raise exception.ValidationError(
|
||||
message=_('Update of `is_domain` is not allowed.'))
|
||||
|
||||
update_domain = ('domain_id' in project and
|
||||
project['domain_id'] != original_project['domain_id'])
|
||||
|
||||
# NOTE(htruta): Even if we are allowing domain_ids to be
|
||||
# modified (i.e. 'domain_id_immutable' is set False),
|
||||
# a project.domain_id can only be updated for root projects
|
||||
# that have no children. The update of domain_id of a project in
|
||||
# the middle of the hierarchy creates an inconsistent project
|
||||
# hierarchy.
|
||||
if update_domain:
|
||||
if original_project['is_domain']:
|
||||
raise exception.ValidationError(
|
||||
message=_('Update of domain_id of projects acting as '
|
||||
'domains is not allowed.'))
|
||||
parent_project = (
|
||||
self.driver.get_project(original_project['parent_id']))
|
||||
is_root_project = parent_project['is_domain']
|
||||
if not is_root_project:
|
||||
raise exception.ValidationError(
|
||||
message=_('Update of domain_id is only allowed for '
|
||||
'root projects.'))
|
||||
subtree_list = self.list_projects_in_subtree(project_id)
|
||||
if subtree_list:
|
||||
raise exception.ValidationError(
|
||||
message=_('Cannot update domain_id of a project that '
|
||||
'has children.'))
|
||||
versionutils.report_deprecated_feature(
|
||||
LOG,
|
||||
_('update of domain_id is deprecated as of Mitaka '
|
||||
'and will be removed in O.')
|
||||
)
|
||||
|
||||
original_project_enabled = original_project.get('enabled', True)
|
||||
project_enabled = project.get('enabled', True)
|
||||
if not original_project_enabled and project_enabled:
|
||||
|
|
|
|||
|
|
@ -119,112 +119,6 @@ class ResourceTests(object):
|
|||
self.resource_api.create_project(project1['id'], project1)
|
||||
self.resource_api.create_project(project2['id'], project2)
|
||||
|
||||
def test_move_project_between_domains(self):
|
||||
domain1 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain1['id'], domain1)
|
||||
domain2 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain2['id'], domain2)
|
||||
project = unit.new_project_ref(domain_id=domain1['id'])
|
||||
self.resource_api.create_project(project['id'], project)
|
||||
project['domain_id'] = domain2['id']
|
||||
# Update the project asserting that a deprecation warning is emitted
|
||||
with mock.patch(
|
||||
'oslo_log.versionutils.report_deprecated_feature') as mock_dep:
|
||||
self.resource_api.update_project(project['id'], project)
|
||||
self.assertTrue(mock_dep.called)
|
||||
|
||||
updated_project_ref = self.resource_api.get_project(project['id'])
|
||||
self.assertEqual(domain2['id'], updated_project_ref['domain_id'])
|
||||
|
||||
def test_move_project_between_domains_with_clashing_names_fails(self):
|
||||
domain1 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain1['id'], domain1)
|
||||
domain2 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain2['id'], domain2)
|
||||
# First, create a project in domain1
|
||||
project1 = unit.new_project_ref(domain_id=domain1['id'])
|
||||
self.resource_api.create_project(project1['id'], project1)
|
||||
# Now create a project in domain2 with a potentially clashing
|
||||
# name - which should work since we have domain separation
|
||||
project2 = unit.new_project_ref(name=project1['name'],
|
||||
domain_id=domain2['id'])
|
||||
self.resource_api.create_project(project2['id'], project2)
|
||||
# Now try and move project1 into the 2nd domain - which should
|
||||
# fail since the names clash
|
||||
project1['domain_id'] = domain2['id']
|
||||
self.assertRaises(exception.Conflict,
|
||||
self.resource_api.update_project,
|
||||
project1['id'],
|
||||
project1)
|
||||
|
||||
@unit.skip_if_no_multiple_domains_support
|
||||
def test_move_project_with_children_between_domains_fails(self):
|
||||
domain1 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain1['id'], domain1)
|
||||
domain2 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain2['id'], domain2)
|
||||
project = unit.new_project_ref(domain_id=domain1['id'])
|
||||
self.resource_api.create_project(project['id'], project)
|
||||
child_project = unit.new_project_ref(domain_id=domain1['id'],
|
||||
parent_id=project['id'])
|
||||
self.resource_api.create_project(child_project['id'], child_project)
|
||||
project['domain_id'] = domain2['id']
|
||||
|
||||
# Update is not allowed, since updating the whole subtree would be
|
||||
# necessary
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.resource_api.update_project,
|
||||
project['id'],
|
||||
project)
|
||||
|
||||
@unit.skip_if_no_multiple_domains_support
|
||||
def test_move_project_not_root_between_domains_fails(self):
|
||||
domain1 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain1['id'], domain1)
|
||||
domain2 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain2['id'], domain2)
|
||||
project = unit.new_project_ref(domain_id=domain1['id'])
|
||||
self.resource_api.create_project(project['id'], project)
|
||||
child_project = unit.new_project_ref(domain_id=domain1['id'],
|
||||
parent_id=project['id'])
|
||||
self.resource_api.create_project(child_project['id'], child_project)
|
||||
child_project['domain_id'] = domain2['id']
|
||||
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.resource_api.update_project,
|
||||
child_project['id'],
|
||||
child_project)
|
||||
|
||||
@unit.skip_if_no_multiple_domains_support
|
||||
def test_move_root_project_between_domains_succeeds(self):
|
||||
domain1 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain1['id'], domain1)
|
||||
domain2 = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(domain2['id'], domain2)
|
||||
root_project = unit.new_project_ref(domain_id=domain1['id'])
|
||||
root_project = self.resource_api.create_project(root_project['id'],
|
||||
root_project)
|
||||
|
||||
root_project['domain_id'] = domain2['id']
|
||||
self.resource_api.update_project(root_project['id'], root_project)
|
||||
project_from_db = self.resource_api.get_project(root_project['id'])
|
||||
|
||||
self.assertEqual(domain2['id'], project_from_db['domain_id'])
|
||||
|
||||
@unit.skip_if_no_multiple_domains_support
|
||||
def test_update_domain_id_project_is_domain_fails(self):
|
||||
other_domain = unit.new_domain_ref()
|
||||
self.resource_api.create_domain(other_domain['id'], other_domain)
|
||||
project = unit.new_project_ref(is_domain=True)
|
||||
self.resource_api.create_project(project['id'], project)
|
||||
project['domain_id'] = other_domain['id']
|
||||
|
||||
# Update of domain_id of projects acting as domains is not allowed
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.resource_api.update_project,
|
||||
project['id'],
|
||||
project)
|
||||
|
||||
def test_rename_duplicate_project_name_fails(self):
|
||||
project1 = unit.new_project_ref(
|
||||
domain_id=CONF.identity.default_domain_id)
|
||||
|
|
|
|||
|
|
@ -485,20 +485,18 @@ class IdentityTestCase(test_v3.RestfulTestCase):
|
|||
self.v3_create_token(new_password_auth)
|
||||
|
||||
def test_update_user_domain_id(self):
|
||||
"""Call ``PATCH /users/{user_id}`` with domain_id."""
|
||||
"""Call ``PATCH /users/{user_id}`` with domain_id.
|
||||
|
||||
A user's `domain_id` is immutable. Ensure that any attempts to update
|
||||
the `domain_id` of a user fails.
|
||||
"""
|
||||
user = unit.new_user_ref(domain_id=self.domain['id'])
|
||||
user = self.identity_api.create_user(user)
|
||||
user['domain_id'] = CONF.identity.default_domain_id
|
||||
r = self.patch('/users/%(user_id)s' % {
|
||||
self.patch('/users/%(user_id)s' % {
|
||||
'user_id': user['id']},
|
||||
body={'user': user},
|
||||
expected_status=exception.ValidationError.code)
|
||||
self.config_fixture.config(domain_id_immutable=False)
|
||||
user['domain_id'] = self.domain['id']
|
||||
r = self.patch('/users/%(user_id)s' % {
|
||||
'user_id': user['id']},
|
||||
body={'user': user})
|
||||
self.assertValidUserResponse(r, user)
|
||||
|
||||
def test_delete_user(self):
|
||||
"""Call ``DELETE /users/{user_id}``.
|
||||
|
|
@ -592,18 +590,16 @@ class IdentityTestCase(test_v3.RestfulTestCase):
|
|||
self.assertValidGroupResponse(r, group)
|
||||
|
||||
def test_update_group_domain_id(self):
|
||||
"""Call ``PATCH /groups/{group_id}`` with domain_id."""
|
||||
"""Call ``PATCH /groups/{group_id}`` with domain_id.
|
||||
|
||||
A group's `domain_id` is immutable. Ensure that any attempts to update
|
||||
the `domain_id` of a group fails.
|
||||
"""
|
||||
self.group['domain_id'] = CONF.identity.default_domain_id
|
||||
r = self.patch('/groups/%(group_id)s' % {
|
||||
self.patch('/groups/%(group_id)s' % {
|
||||
'group_id': self.group['id']},
|
||||
body={'group': self.group},
|
||||
expected_status=exception.ValidationError.code)
|
||||
self.config_fixture.config(domain_id_immutable=False)
|
||||
self.group['domain_id'] = self.domain['id']
|
||||
r = self.patch('/groups/%(group_id)s' % {
|
||||
'group_id': self.group['id']},
|
||||
body={'group': self.group})
|
||||
self.assertValidGroupResponse(r, self.group)
|
||||
|
||||
def test_delete_group(self):
|
||||
"""Call ``DELETE /groups/{group_id}``."""
|
||||
|
|
|
|||
|
|
@ -1204,20 +1204,18 @@ class ResourceTestCase(test_v3.RestfulTestCase,
|
|||
body={'project': ref})
|
||||
|
||||
def test_update_project_domain_id(self):
|
||||
"""Call ``PATCH /projects/{project_id}`` with domain_id."""
|
||||
"""Call ``PATCH /projects/{project_id}`` with domain_id.
|
||||
|
||||
A projects's `domain_id` is immutable. Ensure that any attempts to
|
||||
update the `domain_id` of a project fails.
|
||||
"""
|
||||
project = unit.new_project_ref(domain_id=self.domain['id'])
|
||||
project = self.resource_api.create_project(project['id'], project)
|
||||
project['domain_id'] = CONF.identity.default_domain_id
|
||||
r = self.patch('/projects/%(project_id)s' % {
|
||||
self.patch('/projects/%(project_id)s' % {
|
||||
'project_id': project['id']},
|
||||
body={'project': project},
|
||||
expected_status=exception.ValidationError.code)
|
||||
self.config_fixture.config(domain_id_immutable=False)
|
||||
project['domain_id'] = self.domain['id']
|
||||
r = self.patch('/projects/%(project_id)s' % {
|
||||
'project_id': project['id']},
|
||||
body={'project': project})
|
||||
self.assertValidProjectResponse(r, project)
|
||||
|
||||
def test_update_project_parent_id(self):
|
||||
"""Call ``PATCH /projects/{project_id}``."""
|
||||
|
|
|
|||
|
|
@ -63,3 +63,10 @@ other:
|
|||
- >
|
||||
Removed the config option ``[os_inherit] enabled`` as the OS-INHERIT
|
||||
extension is now always enabled.
|
||||
- >
|
||||
The ``CONF [DEFAULT] domain_id_immutable`` option has been removed.
|
||||
This removes the ability to change the ``domain_id`` attribute of
|
||||
users, groups, and projects. The behavior was introduced to allow
|
||||
deployers to migrate entities from one domain to another by updating
|
||||
the ``domain_id`` attribute of an entity. This functionality was
|
||||
deprecated in the Mitaka release is now removed.
|
||||
|
|
|
|||
Loading…
Reference in New Issue