openstack/python-openstackclient
Code Issues Proposed changes

Add support for domain specific roles

Browse Source 
A role entity can now be specified as domain specific.

Closes-bug: #1606105
Change-Id: I564cf3da1d61f5bfcf85be591480d2f5c8d694a0
This commit is contained in:
Henry Nash 2016-02-23 11:42:40 +00:00 committed by Steve Martinelli
parent 0b91368164
commit 5eb7e626b1
9 changed files with 505 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/command-objects/role-assignment.rst
View File

@ -14,6 +14,7 @@ List role assignments
os role assignment list
[--role <role>]
[--role-domain <role-domain>]
[--user <user>]
[--user-domain <user-domain>]
[--group <group>]
@ -31,6 +32,13 @@ List role assignments
.. versionadded:: 3
.. option:: --role-domain <role-domain>
Domain the role belongs to (name or ID).
This can be used in case collisions between role names exist.
.. versionadded:: 3
.. option:: --user <user>
User to filter (name or ID)

47
doc/source/command-objects/role.rst
View File

@ -15,6 +15,7 @@ Add role assignment to a user or group in a project or domain
os role add
--domain <domain> | --project <project> [--project-domain <project-domain>]
--user <user> [--user-domain <user-domain>] | --group <group> [--group-domain <group-domain>]
--role-domain <role-domain>
--inherited
<role>
@ -65,6 +66,13 @@ Add role assignment to a user or group in a project or domain
.. versionadded:: 3
.. option:: --role-domain <role-domain>
Domain the role belongs to (name or ID).
This must be specified when the name of a domain specific role is used.
.. versionadded:: 3
.. describe:: <role>
Role to add to <project>:<user> (name or ID)
@ -79,8 +87,15 @@ Create new role
os role create
[--or-show]
[--domain <domain>]
<name>
.. option:: --domain <domain>
Domain the role belongs to (name or ID).
.. versionadded:: 3
.. option:: --or-show
Return existing role
@ -101,11 +116,18 @@ Delete role(s)
os role delete
<role> [<role> ...]
[--domain <domain>]
.. describe:: <role>
Role to delete (name or ID)
.. option:: --domain <domain>
Domain the role belongs to (name or ID).
.. versionadded:: 3
role list
---------
@ -123,7 +145,8 @@ List roles
Filter roles by <domain> (name or ID)
(Deprecated, please use ``role assignment list`` instead)
(Deprecated if being used to list assignments in conjunction with the
``--user <user>``, option, please use ``role assignment list`` instead)
.. option:: --project <project>
@ -189,6 +212,7 @@ Remove role assignment from domain/project : user/group
os role remove
--domain <domain> | --project <project> [--project-domain <project-domain>]
--user <user> [--user-domain <user-domain>] | --group <group> [--group-domain <group-domain>]
--role-domain <role-domain>
--inherited
<role>
@ -239,6 +263,13 @@ Remove role assignment from domain/project : user/group
.. versionadded:: 3
.. option:: --role-domain <role-domain>
Domain the role belongs to (name or ID).
This must be specified when the name of a domain specific role is used.
.. versionadded:: 3
.. describe:: <role>
Role to remove (name or ID)
@ -255,12 +286,19 @@ Set role properties
os role set
[--name <name>]
[--domain <domain>]
<role>
.. option:: --name <name>
Set role name
.. option:: --domain <domain>
Domain the role belongs to (name or ID).
.. versionadded:: 3
.. describe:: <role>
Role to modify (name or ID)
@ -274,8 +312,15 @@ Display role details
.. code:: bash
os role show
[--domain <domain>]
<role>
.. option:: --domain <domain>
Domain the role belongs to (name or ID).
.. versionadded:: 3
.. describe:: <role>
Role to display (name or ID)

10
openstackclient/identity/common.py
View File

@ -201,6 +201,16 @@ def add_project_domain_option_to_parser(parser):
)
def add_role_domain_option_to_parser(parser):
parser.add_argument(
'--role-domain',
metavar='<role-domain>',
help=_('Domain the role belongs to (name or ID). '
'This must be specified when the name of a domain specific '
'role is used.'),
)
def add_inherited_option_to_parser(parser):
parser.add_argument(
'--inherited',

95
openstackclient/identity/v3/role.py
View File

@ -109,7 +109,7 @@ def _process_identity_and_resource_options(parsed_args,
class AddRole(command.Command):
"""Adds a role to a user or group on a domain or project"""
"""Adds a role assignment to a user or group on a domain or project"""
def get_parser(self, prog_name):
parser = super(AddRole, self).get_parser(prog_name)
@ -119,6 +119,7 @@ class AddRole(command.Command):
help=_('Role to add to <user> (name or ID)'),
)
_add_identity_and_resource_options_to_parser(parser)
common.add_role_domain_option_to_parser(parser)
return parser
def take_action(self, parsed_args):
@ -127,9 +128,15 @@ class AddRole(command.Command):
if (not parsed_args.user and not parsed_args.domain
and not parsed_args.group and not parsed_args.project):
return
domain_id = None
if parsed_args.role_domain:
domain_id = common.find_domain(identity_client,
parsed_args.role_domain).id
role = utils.find_resource(
identity_client.roles,
parsed_args.role,
domain_id=domain_id
)
kwargs = _process_identity_and_resource_options(
@ -153,6 +160,11 @@ class CreateRole(command.ShowOne):
metavar='<role-name>',
help=_('New role name'),
)
parser.add_argument(
'--domain',
metavar='<domain>',
help=_('Domain the role belongs to (name or ID)'),
)
parser.add_argument(
'--or-show',
action='store_true',
@ -163,12 +175,20 @@ class CreateRole(command.ShowOne):
def take_action(self, parsed_args):
identity_client = self.app.client_manager.identity
domain_id = None
if parsed_args.domain:
domain_id = common.find_domain(identity_client,
parsed_args.domain).id
try:
role = identity_client.roles.create(name=parsed_args.name)
role = identity_client.roles.create(
name=parsed_args.name, domain=domain_id)
except ks_exc.Conflict:
if parsed_args.or_show:
role = utils.find_resource(identity_client.roles,
parsed_args.name)
parsed_args.name,
domain_id=domain_id)
LOG.info(_('Returning existing role %s'), role.name)
else:
raise
@ -188,15 +208,26 @@ class DeleteRole(command.Command):
nargs="+",
help=_('Role(s) to delete (name or ID)'),
)
parser.add_argument(
'--domain',
metavar='<domain>',
help=_('Domain the role belongs to (name or ID)'),
)
return parser
def take_action(self, parsed_args):
identity_client = self.app.client_manager.identity
domain_id = None
if parsed_args.domain:
domain_id = common.find_domain(identity_client,
parsed_args.domain).id
for role in parsed_args.roles:
role_obj = utils.find_resource(
identity_client.roles,
role,
domain_id=domain_id
)
identity_client.roles.delete(role_obj.id)
@ -206,6 +237,18 @@ class ListRole(command.Lister):
def get_parser(self, prog_name):
parser = super(ListRole, self).get_parser(prog_name)
# TODO(henry-nash): The use of the List Role command to list
# assignments (as well as roles) has been deprecated. In order
# to support domain specific roles, we are overriding the domain
# option to allow specification of the domain for the role. This does
# not conflict with any existing commands, since for the deprecated
# assignments listing you were never allowed to only specify a domain
# (you also needed to specify a user).
#
# Once we have removed the deprecated options entirely, we must
# replace the call to _add_identity_and_resource_options_to_parser()
# below with just adding the domain option into the parser.
_add_identity_and_resource_options_to_parser(parser)
return parser
@ -239,8 +282,14 @@ class ListRole(command.Lister):
# no user or group specified, list all roles in the system
if not parsed_args.user and not parsed_args.group:
if not parsed_args.domain:
columns = ('ID', 'Name')
data = identity_client.roles.list()
else:
columns = ('ID', 'Name', 'Domain')
data = identity_client.roles.list(domain_id=domain.id)
for role in data:
role.domain = domain.name
elif parsed_args.user and parsed_args.domain:
columns = ('ID', 'Name', 'Domain', 'User')
data = identity_client.roles.list(
@ -322,7 +371,7 @@ class ListRole(command.Lister):
class RemoveRole(command.Command):
"""Remove role from domain/project : user/group"""
"""Removes a role assignment from domain/project : user/group"""
def get_parser(self, prog_name):
parser = super(RemoveRole, self).get_parser(prog_name)
@ -332,6 +381,8 @@ class RemoveRole(command.Command):
help=_('Role to remove (name or ID)'),
)
_add_identity_and_resource_options_to_parser(parser)
common.add_role_domain_option_to_parser(parser)
return parser
def take_action(self, parsed_args):
@ -342,9 +393,15 @@ class RemoveRole(command.Command):
sys.stderr.write(_("Incorrect set of arguments provided. "
"See openstack --help for more details\n"))
return
domain_id = None
if parsed_args.role_domain:
domain_id = common.find_domain(identity_client,
parsed_args.role_domain).id
role = utils.find_resource(
identity_client.roles,
parsed_args.role,
domain_id=domain_id
)
kwargs = _process_identity_and_resource_options(
@ -367,6 +424,11 @@ class SetRole(command.Command):
metavar='<role>',
help=_('Role to modify (name or ID)'),
)
parser.add_argument(
'--domain',
metavar='<domain>',
help=_('Domain the role belongs to (name or ID)'),
)
parser.add_argument(
'--name',
metavar='<name>',
@ -377,10 +439,14 @@ class SetRole(command.Command):
def take_action(self, parsed_args):
identity_client = self.app.client_manager.identity
role = utils.find_resource(
identity_client.roles,
domain_id = None
if parsed_args.domain:
domain_id = common.find_domain(identity_client,
parsed_args.domain).id
role = utils.find_resource(identity_client.roles,
parsed_args.role,
)
domain_id=domain_id)
identity_client.roles.update(role.id, name=parsed_args.name)
@ -395,15 +461,24 @@ class ShowRole(command.ShowOne):
metavar='<role>',
help=_('Role to display (name or ID)'),
)
parser.add_argument(
'--domain',
metavar='<domain>',
help=_('Domain the role belongs to (name or ID)'),
)
return parser
def take_action(self, parsed_args):
identity_client = self.app.client_manager.identity
role = utils.find_resource(
identity_client.roles,
domain_id = None
if parsed_args.domain:
domain_id = common.find_domain(identity_client,
parsed_args.domain).id
role = utils.find_resource(identity_client.roles,
parsed_args.role,
)
domain_id=domain_id)
role._info.pop('links')
return zip(*sorted(six.iteritems(role._info)))

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

@ -36,6 +36,7 @@ class ListRoleAssignment(command.Lister):
metavar='<role>',
help=_('Role to filter (name or ID)'),
)
common.add_role_domain_option_to_parser(parser)
parser.add_argument(
'--names',
action="store_true",
@ -91,10 +92,15 @@ class ListRoleAssignment(command.Lister):
auth_ref = self.app.client_manager.auth_ref
role = None
role_domain_id = None
if parsed_args.role_domain:
role_domain_id = common.find_domain(identity_client,
parsed_args.role_domain).id
if parsed_args.role:
role = utils.find_resource(
identity_client.roles,
parsed_args.role,
domain_id=role_domain_id
)
user = None
@ -205,6 +211,12 @@ class ListRoleAssignment(command.Lister):
if hasattr(assignment, 'role'):
if include_names:
# TODO(henry-nash): If this is a domain specific role it
# would be good show this as role@domain, although this
# domain info is not yet included in the response from the
# server. Although we could get it by re-reading the role
# from the ID, let's wait until the server does the right
# thing.
setattr(assignment, 'role', assignment.role['name'])
else:
setattr(assignment, 'role', assignment.role['id'])

14
openstackclient/tests/identity/v3/fakes.py
View File

@ -173,9 +173,17 @@ role_name = 'roller'
ROLE = {
'id': role_id,
'name': role_name,
'domain': None,
'links': base_url + 'roles/' + role_id,
}
ROLE_2 = {
'id': 'r2',
'name': 'Rolls Royce',
'domain': domain_id,
'links': base_url + 'roles/' + 'r2',
}
service_id = 's-123'
service_name = 'Texaco'
service_type = 'gas'
@ -358,6 +366,12 @@ ASSIGNMENT_WITH_DOMAIN_ID_AND_USER_ID = {
'role': {'id': role_id},
}
ASSIGNMENT_WITH_DOMAIN_ROLE = {
'scope': {'domain': {'id': domain_id}},
'user': {'id': user_id},
'role': {'id': ROLE_2['id']},
}
ASSIGNMENT_WITH_DOMAIN_ID_AND_USER_ID_INCLUDE_NAMES = {
'scope': {
'domain': {'id': domain_id,

272
openstackclient/tests/identity/v3/test_role.py
View File

@ -230,6 +230,45 @@ class TestRoleAdd(TestRole):
)
self.assertIsNone(result)
def test_role_add_domain_role_on_user_project(self):
self.roles_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
arglist = [
'--user', identity_fakes.user_name,
'--project', identity_fakes.project_name,
'--role-domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
if self._is_inheritance_testcase():
arglist.append('--inherited')
verifylist = [
('user', identity_fakes.user_name),
('group', None),
('domain', None),
('project', identity_fakes.project_name),
('role', identity_fakes.ROLE_2['name']),
('inherited', self._is_inheritance_testcase()),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
# Set expected values
kwargs = {
'user': identity_fakes.user_id,
'project': identity_fakes.project_id,
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
}
# RoleManager.grant(role, user=, group=, domain=, project=)
self.roles_mock.grant.assert_called_with(
identity_fakes.ROLE_2['id'],
**kwargs
)
self.assertIsNone(result)
class TestRoleAddInherited(TestRoleAdd, TestRoleInherited):
pass
@ -240,6 +279,12 @@ class TestRoleCreate(TestRole):
def setUp(self):
super(TestRoleCreate, self).setUp()
self.domains_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.DOMAIN),
loaded=True,
)
self.roles_mock.create.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE),
@ -265,22 +310,67 @@ class TestRoleCreate(TestRole):
# Set expected values
kwargs = {
'domain': None,
'name': identity_fakes.role_name,
}
# RoleManager.create(name=)
# RoleManager.create(name=, domain=)
self.roles_mock.create.assert_called_with(
**kwargs
)
collist = ('id', 'name')
collist = ('domain', 'id', 'name')
self.assertEqual(collist, columns)
datalist = (
None,
identity_fakes.role_id,
identity_fakes.role_name,
)
self.assertEqual(datalist, data)
def test_role_create_with_domain(self):
self.roles_mock.create.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
arglist = [
'--domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
verifylist = [
('domain', identity_fakes.domain_name),
('name', identity_fakes.ROLE_2['name']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class ShowOne in cliff, abstract method take_action()
# returns a two-part tuple with a tuple of column names and a tuple of
# data to be shown.
columns, data = self.cmd.take_action(parsed_args)
# Set expected values
kwargs = {
'domain': identity_fakes.domain_id,
'name': identity_fakes.ROLE_2['name'],
}
# RoleManager.create(name=, domain=)
self.roles_mock.create.assert_called_with(
**kwargs
)
collist = ('domain', 'id', 'name')
self.assertEqual(collist, columns)
datalist = (
identity_fakes.domain_id,
identity_fakes.ROLE_2['id'],
identity_fakes.ROLE_2['name'],
)
self.assertEqual(datalist, data)
class TestRoleDelete(TestRole):
@ -313,6 +403,31 @@ class TestRoleDelete(TestRole):
)
self.assertIsNone(result)
def test_role_delete_with_domain(self):
self.roles_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
self.roles_mock.delete.return_value = None
arglist = [
'--domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
verifylist = [
('roles', [identity_fakes.ROLE_2['name']]),
('domain', identity_fakes.domain_name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.roles_mock.delete.assert_called_with(
identity_fakes.ROLE_2['id'],
)
self.assertIsNone(result)
class TestRoleList(TestRole):
@ -583,6 +698,45 @@ class TestRoleList(TestRole):
), )
self.assertEqual(datalist, tuple(data))
def test_role_list_domain_role(self):
self.roles_mock.list.return_value = [
fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
),
]
arglist = [
'--domain', identity_fakes.domain_name,
]
verifylist = [
('domain', identity_fakes.domain_name),
]
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)
# Set expected values
kwargs = {
'domain_id': identity_fakes.domain_id
}
# RoleManager.list(user=, group=, domain=, project=, **kwargs)
self.roles_mock.list.assert_called_with(
**kwargs
)
collist = ('ID', 'Name', 'Domain')
self.assertEqual(collist, columns)
datalist = ((
identity_fakes.ROLE_2['id'],
identity_fakes.ROLE_2['name'],
identity_fakes.domain_name,
), )
self.assertEqual(datalist, tuple(data))
class TestRoleRemove(TestRole):
@ -756,6 +910,44 @@ class TestRoleRemove(TestRole):
)
self.assertIsNone(result)
def test_role_remove_domain_role_on_group_domain(self):
self.roles_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
arglist = [
'--group', identity_fakes.group_name,
'--domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
if self._is_inheritance_testcase():
arglist.append('--inherited')
verifylist = [
('user', None),
('group', identity_fakes.group_name),
('domain', identity_fakes.domain_name),
('project', None),
('role', identity_fakes.ROLE_2['name']),
('inherited', self._is_inheritance_testcase()),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
# Set expected values
kwargs = {
'group': identity_fakes.group_id,
'domain': identity_fakes.domain_id,
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
}
# RoleManager.revoke(role, user=, group=, domain=, project=)
self.roles_mock.revoke.assert_called_with(
identity_fakes.ROLE_2['id'],
**kwargs
)
self.assertIsNone(result)
class TestRoleSet(TestRole):
@ -796,6 +988,37 @@ class TestRoleSet(TestRole):
)
self.assertIsNone(result)
def test_role_set_domain_role(self):
self.roles_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
arglist = [
'--name', 'over',
'--domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
verifylist = [
('name', 'over'),
('domain', identity_fakes.domain_name),
('role', identity_fakes.ROLE_2['name']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
# Set expected values
kwargs = {
'name': 'over',
}
# RoleManager.update(role, name=)
self.roles_mock.update.assert_called_with(
identity_fakes.ROLE_2['id'],
**kwargs
)
self.assertIsNone(result)
class TestRoleShow(TestRole):
@ -830,10 +1053,53 @@ class TestRoleShow(TestRole):
identity_fakes.role_name,
)
collist = ('id', 'name')
collist = ('domain', 'id', 'name')
self.assertEqual(collist, columns)
datalist = (
None,
identity_fakes.role_id,
identity_fakes.role_name,
)
self.assertEqual(datalist, data)
def test_role_show_domain_role(self):
self.roles_mock.get.return_value = fakes.FakeResource(
None,
copy.deepcopy(identity_fakes.ROLE_2),
loaded=True,
)
arglist = [
'--domain', identity_fakes.domain_name,
identity_fakes.ROLE_2['name'],
]
verifylist = [
('domain', identity_fakes.domain_name),
('role', identity_fakes.ROLE_2['name']),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
# In base command class ShowOne in cliff, abstract method take_action()
# returns a two-part tuple with a tuple of column names and a tuple of
# data to be shown.
columns, data = self.cmd.take_action(parsed_args)
# RoleManager.get(role). This is called from utils.find_resource().
# In fact, the current implementation calls the get(role) first with
# just the name, then with the name+domain_id. So technically we should
# mock this out with a call list, with the first call returning None
# and the second returning the object. However, if we did that we are
# then just testing the current sequencing within the utils method, and
# would become brittle to changes within that method. Hence we just
# check for the first call which is always lookup by name.
self.roles_mock.get.assert_called_with(
identity_fakes.ROLE_2['name'],
)
collist = ('domain', 'id', 'name')
self.assertEqual(collist, columns)
datalist = (
identity_fakes.domain_id,
identity_fakes.ROLE_2['id'],
identity_fakes.ROLE_2['name'],
)
self.assertEqual(datalist, data)

53
openstackclient/tests/identity/v3/test_role_assignment.py
View File

@ -628,3 +628,56 @@ class TestRoleAssignmentList(TestRoleAssignment):
False
),)
self.assertEqual(tuple(data), datalist1)
def test_role_assignment_list_domain_role(self):
self.role_assignments_mock.list.return_value = [
fakes.FakeResource(
None,
copy.deepcopy(
identity_fakes.ASSIGNMENT_WITH_DOMAIN_ROLE),
loaded=True,
),
]
arglist = [
'--role', identity_fakes.ROLE_2['name'],
'--role-domain', identity_fakes.domain_name
]
verifylist = [
('user', None),
('group', None),
('domain', None),
('project', None),
('role', identity_fakes.ROLE_2['name']),
('effective', False),
('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.role_assignments_mock.list.assert_called_with(
domain=None,
user=None,
group=None,
project=None,
role=self.roles_mock.get(),
effective=False,
os_inherit_extension_inherited_to=None,
include_names=False)
self.assertEqual(self.columns, columns)
datalist = ((
identity_fakes.ROLE_2['id'],
identity_fakes.user_id,
'',
'',
identity_fakes.domain_id,
False
),)
self.assertEqual(datalist, tuple(data))

4
releasenotes/notes/bug-1606105-ca06b230e22ab5c6.yaml Normal file
View File

@ -0,0 +1,4 @@
---
features:
- Add support for domain specific roles in ``role`` and``role assignment`` commands.
[Bug `1606105 <https://bugs.launchpad.net/python-openstackclient/+bug/1606105>`_]