diff options
| author | Henry Nash <henryn@linux.vnet.ibm.com> | 2016-02-23 11:42:40 +0000 |
|---|---|---|
| committer | Steve Martinelli <s.martinelli@gmail.com> | 2016-08-10 20:32:55 +0000 |
| commit | 5eb7e626b18b033f97f3cf10f2791529f9d75789 (patch) | |
| tree | 1b371a8835abe4dfbf82dbc8003a08621138bb5b /openstackclient | |
| parent | 0b91368164acc596bf97fe4073083e26892f5b1a (diff) | |
| download | python-openstackclient-5eb7e626b18b033f97f3cf10f2791529f9d75789.tar.gz | |
Add support for domain specific roles
A role entity can now be specified as domain specific.
Closes-bug: #1606105
Change-Id: I564cf3da1d61f5bfcf85be591480d2f5c8d694a0
Diffstat (limited to 'openstackclient')
| -rw-r--r-- | openstackclient/identity/common.py | 10 | ||||
| -rw-r--r-- | openstackclient/identity/v3/role.py | 103 | ||||
| -rw-r--r-- | openstackclient/identity/v3/role_assignment.py | 12 | ||||
| -rw-r--r-- | openstackclient/tests/identity/v3/fakes.py | 14 | ||||
| -rw-r--r-- | openstackclient/tests/identity/v3/test_role.py | 272 | ||||
| -rw-r--r-- | openstackclient/tests/identity/v3/test_role_assignment.py | 53 |
6 files changed, 447 insertions, 17 deletions
diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py index 1e40f396..1f645b7d 100644 --- a/openstackclient/identity/common.py +++ b/openstackclient/identity/common.py @@ -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', diff --git a/openstackclient/identity/v3/role.py b/openstackclient/identity/v3/role.py index e8a03ff3..8b911746 100644 --- a/openstackclient/identity/v3/role.py +++ b/openstackclient/identity/v3/role.py @@ -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( @@ -154,6 +161,11 @@ class CreateRole(command.ShowOne): 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', help=_('Return existing role'), @@ -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: - columns = ('ID', 'Name') - data = identity_client.roles.list() + 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( @@ -368,6 +425,11 @@ class SetRole(command.Command): 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>', help=_('Set role 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, - parsed_args.role, - ) + 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, - parsed_args.role, - ) + 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))) diff --git a/openstackclient/identity/v3/role_assignment.py b/openstackclient/identity/v3/role_assignment.py index 6177d1a5..d25cc6ce 100644 --- a/openstackclient/identity/v3/role_assignment.py +++ b/openstackclient/identity/v3/role_assignment.py @@ -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']) diff --git a/openstackclient/tests/identity/v3/fakes.py b/openstackclient/tests/identity/v3/fakes.py index 8c138f7b..c4d24d46 100644 --- a/openstackclient/tests/identity/v3/fakes.py +++ b/openstackclient/tests/identity/v3/fakes.py @@ -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, diff --git a/openstackclient/tests/identity/v3/test_role.py b/openstackclient/tests/identity/v3/test_role.py index d2398e5d..b4e76d96 100644 --- a/openstackclient/tests/identity/v3/test_role.py +++ b/openstackclient/tests/identity/v3/test_role.py @@ -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) diff --git a/openstackclient/tests/identity/v3/test_role_assignment.py b/openstackclient/tests/identity/v3/test_role_assignment.py index 0ae67c72..113cc493 100644 --- a/openstackclient/tests/identity/v3/test_role_assignment.py +++ b/openstackclient/tests/identity/v3/test_role_assignment.py @@ -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)) |
