openstack/python-openstackclient
Code Issues Proposed changes

Fix ignored --user-domain in role assignment list

Browse Source 
Fix the wrong value assignment which made the --user-domain option
ignored. Unit tests are updated to verify usage of domain options to
avoid further regressions.

Also drop the redundant look up of domain id to avoid unnecessary API
call.

Closes-Bug: #2085604
Change-Id: I5112b8e831fb26eb6544615277f0d3fe4f15dc5a
This commit is contained in:
Takashi Kajinami 2024-10-25 18:27:48 +09:00
parent 7c6b47b451
commit 2e491191e5
3 changed files with 284 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
openstackclient/identity/common.py
View File

@ -184,13 +184,6 @@ def _get_token_resource(client, resource, parsed_name, parsed_domain=None):
return parsed_name
def _get_domain_id_if_requested(identity_client, domain_name_or_id):
if not domain_name_or_id:
return None
domain = find_domain(identity_client, domain_name_or_id)
return domain.id
def find_domain(identity_client, name_or_id):
return _find_identity_resource(
identity_client.domains, name_or_id, domains.Domain
@ -198,48 +191,43 @@ def find_domain(identity_client, name_or_id):
def find_group(identity_client, name_or_id, domain_name_or_id=None):
domain_id = _get_domain_id_if_requested(identity_client, domain_name_or_id)
if not domain_id:
if domain_name_or_id is None:
return _find_identity_resource(
identity_client.groups, name_or_id, groups.Group
)
else:
domain_id = find_domain(identity_client, domain_id).id
return _find_identity_resource(
identity_client.groups,
name_or_id,
groups.Group,
domain_id=domain_id,
)
domain_id = find_domain(identity_client, domain_name_or_id).id
return _find_identity_resource(
identity_client.groups,
name_or_id,
groups.Group,
domain_id=domain_id,
)
def find_project(identity_client, name_or_id, domain_name_or_id=None):
domain_id = _get_domain_id_if_requested(identity_client, domain_name_or_id)
if not domain_id:
if domain_name_or_id is None:
return _find_identity_resource(
identity_client.projects, name_or_id, projects.Project
)
else:
domain_id = find_domain(identity_client, domain_id).id
return _find_identity_resource(
identity_client.projects,
name_or_id,
projects.Project,
domain_id=domain_id,
)
domain_id = find_domain(identity_client, domain_name_or_id).id
return _find_identity_resource(
identity_client.projects,
name_or_id,
projects.Project,
domain_id=domain_id,
)
def find_user(identity_client, name_or_id, domain_name_or_id=None):
domain_id = _get_domain_id_if_requested(identity_client, domain_name_or_id)
if not domain_id:
if domain_name_or_id is None:
return _find_identity_resource(
identity_client.users, name_or_id, users.User
)
else:
domain_id = find_domain(identity_client, domain_id).id
return _find_identity_resource(
identity_client.users, name_or_id, users.User, domain_id=domain_id
)
domain_id = find_domain(identity_client, domain_name_or_id).id
return _find_identity_resource(
identity_client.users, name_or_id, users.User, domain_id=domain_id
)
def _find_identity_resource(

2
openstackclient/identity/v3/role_assignment.py
View File

@ -148,7 +148,7 @@ class ListRoleAssignment(command.Lister):
user_domain_id = None
if parsed_args.user_domain:
project_domain_id = _find_sdk_id(
user_domain_id = _find_sdk_id(
identity_client.find_domain,
name_or_id=parsed_args.user_domain,
)

261
openstackclient/tests/unit/identity/v3/test_role_assignment.py
View File

@ -165,7 +165,75 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--user', self.user.name]
verifylist = [
('user', self.user.name),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
('names', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class Lister in cliff, abstract method take_action()
# returns a tuple containing the column names and an iterable
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_user.assert_called_with(
name_or_id=self.user.name, ignore_missing=False, domain_id=None
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=self.user.id,
group_id=None,
scope_project_id=None,
scope_domain_id=None,
scope_system=None,
effective=None,
include_names=None,
inherited_to=None,
)
self.assertEqual(self.columns, columns)
datalist = (
(
self.role.id,
self.user.id,
'',
'',
self.domain.id,
'',
False,
),
(
self.role.id,
self.user.id,
'',
self.project.id,
'',
'',
False,
),
)
self.assertEqual(datalist, tuple(data))
def test_role_assignment_list_user_with_domain(self):
self.identity_sdk_client.role_assignments.return_value = [
self.assignment_with_domain_id_and_user_id,
self.assignment_with_project_id_and_user_id,
]
arglist = ['--user', self.user.name, '--user-domain', self.domain.name]
verifylist = [
('user', self.user.name),
('user_domain', self.domain.name),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
@ -181,6 +249,14 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_domain.assert_called_with(
name_or_id=self.domain.name, ignore_missing=False
)
self.identity_sdk_client.find_user.assert_called_with(
name_or_id=self.user.name,
ignore_missing=False,
domain_id=self.domain.id,
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=self.user.id,
@ -230,10 +306,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--user', self.user.id]
verifylist = [
('user', self.user.id),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -290,10 +369,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--group', self.group.name]
verifylist = [
('user', None),
('user_domain', None),
('group', self.group.name),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -306,6 +388,85 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_group.assert_called_with(
name_or_id=self.group.name, ignore_missing=False, domain_id=None
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=None,
group_id=self.group.id,
scope_project_id=None,
scope_domain_id=None,
scope_system=None,
effective=None,
include_names=None,
inherited_to=None,
)
self.assertEqual(self.columns, columns)
datalist = (
(
self.role.id,
'',
self.group.id,
'',
self.domain.id,
'',
False,
),
(
self.role.id,
'',
self.group.id,
self.project.id,
'',
'',
False,
),
)
self.assertEqual(datalist, tuple(data))
def test_role_assignment_list_group_with_domain(self):
self.identity_sdk_client.role_assignments.return_value = [
self.assignment_with_domain_id_and_group_id,
self.assignment_with_project_id_and_group_id,
]
arglist = [
'--group',
self.group.name,
'--group-domain',
self.domain.name,
]
verifylist = [
('user', None),
('user_domain', None),
('group', self.group.name),
('group_domain', self.domain.name),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
('names', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class Lister in cliff, abstract method take_action()
# returns a tuple containing the column names and an iterable
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_domain.assert_called_with(
name_or_id=self.domain.name, ignore_missing=False
)
self.identity_sdk_client.find_group.assert_called_with(
name_or_id=self.group.name,
ignore_missing=False,
domain_id=self.domain.id,
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=None,
@ -350,10 +511,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--domain', self.domain.name]
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', self.domain.name),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -410,10 +574,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--project', self.project.name]
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', self.project.name),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -426,6 +593,85 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_project.assert_called_with(
name_or_id=self.project.name, ignore_missing=False, domain_id=None
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=None,
group_id=None,
scope_project_id=self.project.id,
scope_domain_id=None,
scope_system=None,
effective=None,
include_names=None,
inherited_to=None,
)
self.assertEqual(self.columns, columns)
datalist = (
(
self.role.id,
self.user.id,
'',
self.project.id,
'',
'',
False,
),
(
self.role.id,
'',
self.group.id,
self.project.id,
'',
'',
False,
),
)
self.assertEqual(datalist, tuple(data))
def test_role_assignment_list_project_with_domain(self):
self.identity_sdk_client.role_assignments.return_value = [
self.assignment_with_project_id_and_user_id,
self.assignment_with_project_id_and_group_id,
]
arglist = [
'--project',
self.project.name,
'--project-domain',
self.domain.name,
]
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', self.project.name),
('project_domain', self.domain.name),
('role', None),
('effective', None),
('inherited', False),
('names', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class Lister in cliff, abstract method take_action()
# returns a tuple containing the column names and an iterable
# containing the data to be listed.
columns, data = self.cmd.take_action(parsed_args)
self.identity_sdk_client.find_domain.assert_called_with(
name_or_id=self.domain.name, ignore_missing=False
)
self.identity_sdk_client.find_project.assert_called_with(
name_or_id=self.project.name,
ignore_missing=False,
domain_id=self.domain.id,
)
self.identity_sdk_client.role_assignments.assert_called_with(
role_id=None,
user_id=None,
@ -476,10 +722,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
]
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -529,10 +778,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--effective']
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', True),
('inherited', False),
@ -611,10 +863,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--inherited']
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', True),
@ -707,10 +962,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
arglist = ['--names']
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', None),
('effective', None),
('inherited', False),
@ -799,10 +1057,13 @@ class TestRoleAssignmentList(identity_fakes.TestIdentityv3):
]
verifylist = [
('user', None),
('user_domain', None),
('group', None),
('group_domain', None),
('system', None),
('domain', None),
('project', None),
('project_domain', None),
('role', role_2.name),
('effective', None),
('inherited', False),