Identity: Migrate 'group' commands to SDK

Change-Id: I5a477426318d77021c0430efa1d1f9a7b1ee2633

openstackclient/identity/common.py

@@ -214,6 +214,33 @@ def find_group(identity_client, name_or_id, domain_name_or_id=None):
     )
 
 
+def find_group_id_sdk(
+    identity_client,
+    name_or_id,
+    domain_name_or_id=None,
+    *,
+    validate_actor_existence=True,
+):
+    if domain_name_or_id is None:
+        return _find_sdk_id(
+            identity_client.find_group,
+            name_or_id=name_or_id,
+            validate_actor_existence=validate_actor_existence,
+        )
+
+    domain_id = find_domain_id_sdk(
+        identity_client,
+        name_or_id=domain_name_or_id,
+        validate_actor_existence=validate_actor_existence,
+    )
+    return _find_sdk_id(
+        identity_client.find_group,
+        name_or_id=name_or_id,
+        validate_actor_existence=validate_actor_existence,
+        domain_id=domain_id,
+    )
+
+
 def find_project(identity_client, name_or_id, domain_name_or_id=None):
     if domain_name_or_id is None:
         return _find_identity_resource(

openstackclient/identity/v3/group.py

@@ -17,7 +17,7 @@
 
 import logging
 
-from keystoneauth1 import exceptions as ks_exc
+from openstack import exceptions as sdk_exc
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -29,6 +29,25 @@ from openstackclient.identity import common
 LOG = logging.getLogger(__name__)
 
 
+def _format_group(group):
+    columns = (
+        'description',
+        'domain_id',
+        'id',
+        'name',
+    )
+    column_headers = (
+        'description',
+        'domain_id',
+        'id',
+        'name',
+    )
+    return (
+        column_headers,
+        utils.get_item_properties(group, columns),
+    )
+
+
 class AddUserToGroup(command.Command):
     _description = _("Add user to group")
 
@@ -53,19 +72,19 @@ class AddUserToGroup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
-        group_id = common.find_group(
+        group_id = common.find_group_id_sdk(
             identity_client, parsed_args.group, parsed_args.group_domain
-        ).id
+        )
 
         result = 0
         for i in parsed_args.user:
             try:
-                user_id = common.find_user(
+                user_id = common.find_user_id_sdk(
                     identity_client, i, parsed_args.user_domain
-                ).id
+                )
-                identity_client.users.add_to_group(user_id, group_id)
+                identity_client.add_user_to_group(user_id, group_id)
             except Exception as e:
                 result += 1
                 msg = _("%(user)s not added to group %(group)s: %(e)s") % {
@@ -109,32 +128,41 @@ class CheckUserInGroup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
-        user_id = common.find_user(
+        user_id = common.find_user_id_sdk(
-            identity_client, parsed_args.user, parsed_args.user_domain
+            identity_client,
-        ).id
+            parsed_args.user,
-        group_id = common.find_group(
+            parsed_args.user_domain,
-            identity_client, parsed_args.group, parsed_args.group_domain
+            validate_actor_existence=False,
-        ).id
+        )
+        group_id = common.find_group_id_sdk(
+            identity_client,
+            parsed_args.group,
+            parsed_args.group_domain,
+            validate_actor_existence=False,
+        )
 
+        user_in_group = False
         try:
-            identity_client.users.check_in_group(user_id, group_id)
+            user_in_group = identity_client.check_user_in_group(
-        except ks_exc.http.HTTPClientError as e:
+                user_id, group_id
-            if e.http_status == 403 or e.http_status == 404:
+            )
-                msg = _("%(user)s not in group %(group)s\n") % {
+        except sdk_exc.ForbiddenException:
-                    'user': parsed_args.user,
+            # Assume False if forbidden
-                    'group': parsed_args.group,
+            pass
-                }
+        if user_in_group:
-                self.app.stderr.write(msg)
-            else:
-                raise e
-        else:
             msg = _("%(user)s in group %(group)s\n") % {
                 'user': parsed_args.user,
                 'group': parsed_args.group,
             }
             self.app.stdout.write(msg)
+        else:
+            msg = _("%(user)s not in group %(group)s\n") % {
+                'user': parsed_args.user,
+                'group': parsed_args.group,
+            }
+            self.app.stderr.write(msg)
 
 
 class CreateGroup(command.ShowOne):
@@ -165,29 +193,33 @@ class CreateGroup(command.ShowOne):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
-        domain = None
+        kwargs = {}
+        if parsed_args.name:
+            kwargs['name'] = parsed_args.name
+        if parsed_args.description:
+            kwargs['description'] = parsed_args.description
         if parsed_args.domain:
-            domain = common.find_domain(identity_client, parsed_args.domain).id
+            kwargs['domain_id'] = common.find_domain_id_sdk(
+                identity_client, parsed_args.domain
+            )
 
         try:
-            group = identity_client.groups.create(
+            group = identity_client.create_group(**kwargs)
-                name=parsed_args.name,
+        except sdk_exc.ConflictException:
-                domain=domain,
-                description=parsed_args.description,
-            )
-        except ks_exc.Conflict:
             if parsed_args.or_show:
-                group = utils.find_resource(
+                if parsed_args.domain:
-                    identity_client.groups, parsed_args.name, domain_id=domain
+                    group = identity_client.find_group(
-                )
+                        parsed_args.name, domain_id=parsed_args.domain
+                    )
+                else:
+                    group = identity_client.find_group(parsed_args.name)
                 LOG.info(_('Returning existing group %s'), group.name)
             else:
                 raise
 
-        group._info.pop('links')
+        return _format_group(group)
-        return zip(*sorted(group._info.items()))
 
 
 class DeleteGroup(command.Command):
@@ -209,15 +241,15 @@ class DeleteGroup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
         errors = 0
         for group in parsed_args.groups:
             try:
-                group_obj = common.find_group(
+                group_id = common.find_group_id_sdk(
                     identity_client, group, parsed_args.domain
                 )
-                identity_client.groups.delete(group_obj.id)
+                identity_client.delete_group(group_id)
             except Exception as e:
                 errors += 1
                 LOG.error(
@@ -262,29 +294,37 @@ class ListGroup(command.Lister):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
         domain = None
         if parsed_args.domain:
-            domain = common.find_domain(identity_client, parsed_args.domain).id
+            domain = common.find_domain_id_sdk(
+                identity_client, parsed_args.domain
+            )
 
+        data = []
         if parsed_args.user:
-            user = common.find_user(
+            user = common.find_user_id_sdk(
                 identity_client,
                 parsed_args.user,
                 parsed_args.user_domain,
-            ).id
+            )
+            if domain:
+                # NOTE(0weng): The API doesn't actually support filtering additionally by domain_id,
+                # so this doesn't really do anything.
+                data = identity_client.user_groups(user, domain_id=domain)
+            else:
+                data = identity_client.user_groups(user)
         else:
-            user = None
+            if domain:
+                data = identity_client.groups(domain_id=domain)
+            else:
+                data = identity_client.groups()
 
         # List groups
         columns: tuple[str, ...] = ('ID', 'Name')
         if parsed_args.long:
             columns += ('Domain ID', 'Description')
-        data = identity_client.groups.list(
-            domain=domain,
-            user=user,
-        )
 
         return (
             columns,
@@ -323,19 +363,19 @@ class RemoveUserFromGroup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
-        group_id = common.find_group(
+        group_id = common.find_group_id_sdk(
             identity_client, parsed_args.group, parsed_args.group_domain
-        ).id
+        )
 
         result = 0
         for i in parsed_args.user:
             try:
-                user_id = common.find_user(
+                user_id = common.find_user_id_sdk(
                     identity_client, i, parsed_args.user_domain
-                ).id
+                )
-                identity_client.users.remove_from_group(user_id, group_id)
+                identity_client.remove_user_from_group(user_id, group_id)
             except Exception as e:
                 result += 1
                 msg = _("%(user)s not removed from group %(group)s: %(e)s") % {
@@ -387,8 +427,8 @@ class SetGroup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
-        group = common.find_group(
+        group = common.find_group_id_sdk(
             identity_client, parsed_args.group, parsed_args.domain
         )
         kwargs = {}
@@ -397,7 +437,7 @@ class SetGroup(command.Command):
         if parsed_args.description:
             kwargs['description'] = parsed_args.description
 
-        identity_client.groups.update(group.id, **kwargs)
+        identity_client.update_group(group, **kwargs)
 
 
 class ShowGroup(command.ShowOne):
@@ -418,13 +458,18 @@ class ShowGroup(command.ShowOne):
         return parser
 
     def take_action(self, parsed_args):
-        identity_client = self.app.client_manager.identity
+        identity_client = self.app.client_manager.sdk_connection.identity
 
-        group = common.find_group(
+        if parsed_args.domain:
-            identity_client,
+            domain = common.find_domain_id_sdk(
-            parsed_args.group,
+                identity_client, parsed_args.domain
-            domain_name_or_id=parsed_args.domain,
+            )
-        )
+            group = identity_client.find_group(
+                parsed_args.group, domain_id=domain, ignore_missing=False
+            )
+        else:
+            group = identity_client.find_group(
+                parsed_args.group, ignore_missing=False
+            )
 
-        group._info.pop('links')
+        return _format_group(group)
-        return zip(*sorted(group._info.items()))

openstackclient/tests/unit/identity/v3/test_group.py

@@ -14,45 +14,33 @@
 from unittest import mock
 from unittest.mock import call
 
-from keystoneauth1 import exceptions as ks_exc
+from openstack import exceptions as sdk_exc
+from openstack.identity.v3 import domain as _domain
+from openstack.identity.v3 import group as _group
+from openstack.identity.v3 import user as _user
+from openstack.test import fakes as sdk_fakes
 from osc_lib import exceptions
-from osc_lib import utils
 
 from openstackclient.identity.v3 import group
 from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
 
 
-class TestGroup(identity_fakes.TestIdentityv3):
+class TestGroupAddUser(identity_fakes.TestIdentityv3):
     def setUp(self):
         super().setUp()
 
-        # Get a shortcut to the DomainManager Mock
+        self._group = sdk_fakes.generate_fake_resource(_group.Group)
-        self.domains_mock = self.identity_client.domains
+        self.users = tuple(
-        self.domains_mock.reset_mock()
+            sdk_fakes.generate_fake_resources(_user.User, count=2)
+        )
 
-        # Get a shortcut to the GroupManager Mock
+        self.identity_sdk_client.find_group.return_value = self._group
-        self.groups_mock = self.identity_client.groups
+        self.identity_sdk_client.add_user_to_group.return_value = None
-        self.groups_mock.reset_mock()
-
-        # Get a shortcut to the UserManager Mock
-        self.users_mock = self.identity_client.users
-        self.users_mock.reset_mock()
-
-
-class TestGroupAddUser(TestGroup):
-    _group = identity_fakes.FakeGroup.create_one_group()
-    users = identity_fakes.FakeUser.create_users(count=2)
-
-    def setUp(self):
-        super().setUp()
-
-        self.groups_mock.get.return_value = self._group
-        self.users_mock.get = identity_fakes.FakeUser.get_users(self.users)
-        self.users_mock.add_to_group.return_value = None
 
         self.cmd = group.AddUserToGroup(self.app, None)
 
     def test_group_add_user(self):
+        self.identity_sdk_client.find_user.return_value = self.users[0]
         arglist = [
             self._group.name,
             self.users[0].name,
@@ -64,12 +52,16 @@ class TestGroupAddUser(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.users_mock.add_to_group.assert_called_once_with(
+        self.identity_sdk_client.add_user_to_group.assert_called_once_with(
             self.users[0].id, self._group.id
         )
         self.assertIsNone(result)
 
     def test_group_add_multi_users(self):
+        self.identity_sdk_client.find_user.side_effect = [
+            self.users[0],
+            self.users[1],
+        ]
         arglist = [
             self._group.name,
             self.users[0].name,
@@ -86,13 +78,13 @@ class TestGroupAddUser(TestGroup):
             call(self.users[0].id, self._group.id),
             call(self.users[1].id, self._group.id),
         ]
-        self.users_mock.add_to_group.assert_has_calls(calls)
+        self.identity_sdk_client.add_user_to_group.assert_has_calls(calls)
         self.assertIsNone(result)
 
     @mock.patch.object(group.LOG, 'error')
     def test_group_add_user_with_error(self, mock_error):
-        self.users_mock.add_to_group.side_effect = [
+        self.identity_sdk_client.add_user_to_group.side_effect = [
-            exceptions.CommandError(),
+            sdk_exc.ResourceNotFound,
             None,
         ]
         arglist = [
@@ -111,20 +103,20 @@ class TestGroupAddUser(TestGroup):
         except exceptions.CommandError as e:
             msg = f"1 of 2 users not added to group {self._group.name}."
             self.assertEqual(msg, str(e))
-        msg = f"{self.users[0].name} not added to group {self._group.name}: "
+        msg = f"{self.users[0].name} not added to group {self._group.name}: {str(sdk_exc.ResourceNotFound())}"
         mock_error.assert_called_once_with(msg)
 
 
-class TestGroupCheckUser(TestGroup):
+class TestGroupCheckUser(identity_fakes.TestIdentityv3):
-    group = identity_fakes.FakeGroup.create_one_group()
-    user = identity_fakes.FakeUser.create_one_user()
-
     def setUp(self):
         super().setUp()
 
-        self.groups_mock.get.return_value = self.group
+        self.group = sdk_fakes.generate_fake_resource(_group.Group)
-        self.users_mock.get.return_value = self.user
+        self.user = sdk_fakes.generate_fake_resource(_user.User)
-        self.users_mock.check_in_group.return_value = None
+
+        self.identity_sdk_client.find_group.return_value = self.group
+        self.identity_sdk_client.find_user.return_value = self.user
+        self.identity_sdk_client.check_user_in_group.return_value = True
 
         self.cmd = group.CheckUserInGroup(self.app, None)
 
@@ -140,16 +132,15 @@ class TestGroupCheckUser(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.users_mock.check_in_group.assert_called_once_with(
+        self.identity_sdk_client.check_user_in_group.assert_called_once_with(
             self.user.id, self.group.id
         )
         self.assertIsNone(result)
 
     def test_group_check_user_server_error(self):
-        def server_error(*args):
+        self.identity_sdk_client.check_user_in_group.side_effect = (
-            raise ks_exc.http.InternalServerError
+            sdk_exc.SDKException
-
+        )
-        self.users_mock.check_in_group.side_effect = server_error
         arglist = [
             self.group.name,
             self.user.name,
@@ -161,12 +152,12 @@ class TestGroupCheckUser(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.assertRaises(
-            ks_exc.http.InternalServerError, self.cmd.take_action, parsed_args
+            sdk_exc.SDKException, self.cmd.take_action, parsed_args
         )
 
 
-class TestGroupCreate(TestGroup):
+class TestGroupCreate(identity_fakes.TestIdentityv3):
-    domain = identity_fakes.FakeDomain.create_one_domain()
+    domain = sdk_fakes.generate_fake_resource(_domain.Domain)
 
     columns = (
         'description',
@@ -177,23 +168,20 @@ class TestGroupCreate(TestGroup):
 
     def setUp(self):
         super().setUp()
-        self.group = identity_fakes.FakeGroup.create_one_group(
+        self.group = sdk_fakes.generate_fake_resource(
-            attrs={'domain_id': self.domain.id}
+            _group.Group, description=None, domain_id=None
         )
-        self.data = (
+        self.group_with_options = sdk_fakes.generate_fake_resource(
-            self.group.description,
+            _group.Group, domain_id=self.domain.id
-            self.group.domain_id,
-            self.group.id,
-            self.group.name,
         )
 
-        self.groups_mock.create.return_value = self.group
+        self.identity_sdk_client.find_group.return_value = self.group
-        self.groups_mock.get.return_value = self.group
+        self.identity_sdk_client.find_domain.return_value = self.domain
-        self.domains_mock.get.return_value = self.domain
 
         self.cmd = group.CreateGroup(self.app, None)
 
     def test_group_create(self):
+        self.identity_sdk_client.create_group.return_value = self.group
         arglist = [
             self.group.name,
         ]
@@ -203,40 +191,56 @@ class TestGroupCreate(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.groups_mock.create.assert_called_once_with(
+        self.identity_sdk_client.create_group.assert_called_once_with(
             name=self.group.name,
-            domain=None,
-            description=None,
         )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        datalist = (
+            self.group.description,
+            None,
+            self.group.id,
+            self.group.name,
+        )
+        self.assertEqual(datalist, data)
 
     def test_group_create_with_options(self):
+        self.identity_sdk_client.create_group.return_value = (
+            self.group_with_options
+        )
         arglist = [
             '--domain',
             self.domain.name,
             '--description',
-            self.group.description,
+            self.group_with_options.description,
-            self.group.name,
+            self.group_with_options.name,
         ]
         verifylist = [
             ('domain', self.domain.name),
-            ('description', self.group.description),
+            ('description', self.group_with_options.description),
-            ('name', self.group.name),
+            ('name', self.group_with_options.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.groups_mock.create.assert_called_once_with(
+        self.identity_sdk_client.create_group.assert_called_once_with(
-            name=self.group.name,
+            name=self.group_with_options.name,
-            domain=self.domain.id,
+            domain_id=self.domain.id,
-            description=self.group.description,
+            description=self.group_with_options.description,
         )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        datalist = (
+            self.group_with_options.description,
+            self.domain.id,
+            self.group_with_options.id,
+            self.group_with_options.name,
+        )
+        self.assertEqual(datalist, data)
 
     def test_group_create_or_show(self):
-        self.groups_mock.create.side_effect = ks_exc.Conflict()
+        self.identity_sdk_client.find_group.return_value = self.group
+        self.identity_sdk_client.create_group.side_effect = (
+            sdk_exc.ConflictException
+        )
         arglist = [
             '--or-show',
             self.group.name,
@@ -248,46 +252,97 @@ class TestGroupCreate(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_called_once_with(self.group.name)
+        self.identity_sdk_client.find_group.assert_called_once_with(
+            self.group.name
+        )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        datalist = (
+            self.group.description,
+            None,
+            self.group.id,
+            self.group.name,
+        )
+        self.assertEqual(datalist, data)
+
+    def test_group_create_or_show_with_domain(self):
+        self.identity_sdk_client.find_group.return_value = (
+            self.group_with_options
+        )
+        self.identity_sdk_client.create_group.side_effect = (
+            sdk_exc.ConflictException
+        )
+        arglist = [
+            '--or-show',
+            self.group_with_options.name,
+            '--domain',
+            self.domain.id,
+        ]
+        verifylist = [
+            ('or_show', True),
+            ('name', self.group_with_options.name),
+            ('domain', self.domain.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+        self.identity_sdk_client.find_group.assert_called_once_with(
+            self.group_with_options.name, domain_id=self.domain.id
+        )
+        self.assertEqual(self.columns, columns)
+        datalist = (
+            self.group_with_options.description,
+            self.domain.id,
+            self.group_with_options.id,
+            self.group_with_options.name,
+        )
+        self.assertEqual(datalist, data)
 
 
-class TestGroupDelete(TestGroup):
+class TestGroupDelete(identity_fakes.TestIdentityv3):
-    domain = identity_fakes.FakeDomain.create_one_domain()
+    domain = sdk_fakes.generate_fake_resource(_domain.Domain)
-    groups = identity_fakes.FakeGroup.create_groups(
-        attrs={'domain_id': domain.id}, count=2
-    )
 
     def setUp(self):
         super().setUp()
 
-        self.groups_mock.get = identity_fakes.FakeGroup.get_groups(self.groups)
+        self.group = sdk_fakes.generate_fake_resource(
-        self.groups_mock.delete.return_value = None
+            _group.Group,
-        self.domains_mock.get.return_value = self.domain
+            domain_id=None,
+        )
+        self.group_with_domain = sdk_fakes.generate_fake_resource(
+            _group.Group,
+            name=self.group.name,
+            domain_id=self.domain.id,
+        )
+        self.identity_sdk_client.delete_group.return_value = None
+        self.identity_sdk_client.find_domain.return_value = self.domain
 
         self.cmd = group.DeleteGroup(self.app, None)
 
     def test_group_delete(self):
+        self.identity_sdk_client.find_group.return_value = self.group
         arglist = [
-            self.groups[0].id,
+            self.group.id,
         ]
         verifylist = [
-            ('groups', [self.groups[0].id]),
+            ('groups', [self.group.id]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_called_once_with(self.groups[0].id)
+        self.identity_sdk_client.find_group.assert_called_once_with(
-        self.groups_mock.delete.assert_called_once_with(self.groups[0].id)
+            name_or_id=self.group.id, ignore_missing=False
+        )
+        self.identity_sdk_client.delete_group.assert_called_once_with(
+            self.group.id
+        )
         self.assertIsNone(result)
 
     def test_group_multi_delete(self):
-        arglist = []
+        self.identity_sdk_client.find_group.side_effect = [
-        verifylist = []
+            self.group,
-
+            self.group_with_domain,
-        for g in self.groups:
+        ]
-            arglist.append(g.id)
+        arglist = [self.group.id, self.group_with_domain.id]
         verifylist = [
             ('groups', arglist),
         ]
@@ -295,39 +350,50 @@ class TestGroupDelete(TestGroup):
 
         result = self.cmd.take_action(parsed_args)
 
-        calls = []
+        self.identity_sdk_client.delete_group.assert_has_calls(
-        for g in self.groups:
+            [mock.call(self.group.id), mock.call(self.group_with_domain.id)]
-            calls.append(call(g.id))
+        )
-        self.groups_mock.delete.assert_has_calls(calls)
         self.assertIsNone(result)
 
     def test_group_delete_with_domain(self):
-        get_mock_result = [exceptions.CommandError, self.groups[0]]
+        self.identity_sdk_client.find_domain.side_effect = [
-        self.groups_mock.get = mock.Mock(side_effect=get_mock_result)
+            sdk_exc.ForbiddenException
+        ]
+        self.identity_sdk_client.find_group.return_value = (
+            self.group_with_domain
+        )
 
         arglist = [
             '--domain',
-            self.domain.id,
+            self.group_with_domain.domain_id,
-            self.groups[0].id,
+            self.group_with_domain.name,
         ]
         verifylist = [
-            ('domain', self.groups[0].domain_id),
+            ('domain', self.domain.id),
-            ('groups', [self.groups[0].id]),
+            ('groups', [self.group_with_domain.name]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_any_call(
+        self.identity_sdk_client.find_group.assert_called_with(
-            self.groups[0].id, domain_id=self.domain.id
+            name_or_id=self.group_with_domain.name,
+            ignore_missing=False,
+            domain_id=self.domain.id,
+        )
+        self.identity_sdk_client.delete_group.assert_called_once_with(
+            self.group_with_domain.id
         )
-        self.groups_mock.delete.assert_called_once_with(self.groups[0].id)
         self.assertIsNone(result)
 
-    @mock.patch.object(utils, 'find_resource')
+    def test_delete_multi_groups_with_exception(self):
-    def test_delete_multi_groups_with_exception(self, find_mock):
+        self.identity_sdk_client.find_group.side_effect = [
-        find_mock.side_effect = [self.groups[0], exceptions.CommandError]
+            self.group,
+            self.group_with_domain,
+            exceptions.CommandError,
+        ]
         arglist = [
-            self.groups[0].id,
+            self.group.id,
+            self.group_with_domain.id,
             'unexist_group',
         ]
         verifylist = [
@@ -339,45 +405,57 @@ class TestGroupDelete(TestGroup):
             self.cmd.take_action(parsed_args)
             self.fail('CommandError should be raised.')
         except exceptions.CommandError as e:
-            self.assertEqual('1 of 2 groups failed to delete.', str(e))
+            self.assertEqual('1 of 3 groups failed to delete.', str(e))
 
-        find_mock.assert_any_call(self.groups_mock, self.groups[0].id)
+        self.identity_sdk_client.find_group.assert_has_calls(
-        find_mock.assert_any_call(self.groups_mock, 'unexist_group')
+            [
+                mock.call(name_or_id=self.group.id, ignore_missing=False),
+                mock.call(
+                    name_or_id=self.group_with_domain.id, ignore_missing=False
+                ),
+                mock.call(name_or_id='unexist_group', ignore_missing=False),
+            ]
+        )
 
-        self.assertEqual(2, find_mock.call_count)
+        self.assertEqual(3, self.identity_sdk_client.find_group.call_count)
-        self.groups_mock.delete.assert_called_once_with(self.groups[0].id)
+        self.identity_sdk_client.delete_group.assert_has_calls(
+            [
+                mock.call(self.group.id),
+                mock.call(self.group_with_domain.id),
+            ]
+        )
 
 
-class TestGroupList(TestGroup):
+class TestGroupList(identity_fakes.TestIdentityv3):
-    domain = identity_fakes.FakeDomain.create_one_domain()
+    domain = sdk_fakes.generate_fake_resource(_domain.Domain)
-    group = identity_fakes.FakeGroup.create_one_group()
-    user = identity_fakes.FakeUser.create_one_user()
 
     columns = (
         'ID',
         'Name',
     )
-    datalist = (
-        (
-            group.id,
-            group.name,
-        ),
-    )
 
     def setUp(self):
         super().setUp()
 
-        self.groups_mock.get.return_value = self.group
+        self.group = sdk_fakes.generate_fake_resource(
-        self.groups_mock.list.return_value = [self.group]
+            _group.Group, description=None, domain_id=None
+        )
+        self.group_with_domain = sdk_fakes.generate_fake_resource(
+            _group.Group, domain_id=self.domain.id
+        )
+        self.user = sdk_fakes.generate_fake_resource(_user.User)
 
-        self.domains_mock.get.return_value = self.domain
+        self.identity_sdk_client.find_user.return_value = self.user
-
+        self.identity_sdk_client.find_domain.return_value = self.domain
-        self.users_mock.get.return_value = self.user
 
         # Get the command object to test
         self.cmd = group.ListGroup(self.app, None)
 
     def test_group_list_no_options(self):
+        self.identity_sdk_client.groups.return_value = [
+            self.group,
+            self.group_with_domain,
+        ]
         arglist = []
         verifylist = []
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -387,18 +465,23 @@ class TestGroupList(TestGroup):
         # containing the data to be listed.
         columns, data = self.cmd.take_action(parsed_args)
 
-        # Set expected values
+        self.identity_sdk_client.groups.assert_called_with()
-        kwargs = {
-            'domain': None,
-            'user': None,
-        }
-
-        self.groups_mock.list.assert_called_with(**kwargs)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        datalist = (
+            (
+                self.group.id,
+                self.group.name,
+            ),
+            (
+                self.group_with_domain.id,
+                self.group_with_domain.name,
+            ),
+        )
+        self.assertEqual(datalist, tuple(data))
 
     def test_group_list_domain(self):
+        self.identity_sdk_client.groups.return_value = [self.group_with_domain]
         arglist = [
             '--domain',
             self.domain.id,
@@ -415,16 +498,17 @@ class TestGroupList(TestGroup):
 
         # Set expected values
         kwargs = {
-            'domain': self.domain.id,
+            'domain_id': self.domain.id,
-            'user': None,
         }
 
-        self.groups_mock.list.assert_called_with(**kwargs)
+        self.identity_sdk_client.groups.assert_called_with(**kwargs)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        datalist = ((self.group_with_domain.id, self.group_with_domain.name),)
+        self.assertEqual(datalist, tuple(data))
 
     def test_group_list_user(self):
+        self.identity_sdk_client.user_groups.return_value = [self.group]
         arglist = [
             '--user',
             self.user.name,
@@ -439,18 +523,53 @@ class TestGroupList(TestGroup):
         # containing the data to be listed.
         columns, data = self.cmd.take_action(parsed_args)
 
-        # Set expected values
+        self.identity_sdk_client.user_groups.assert_called_with(self.user.id)
-        kwargs = {
-            'domain': None,
-            'user': self.user.id,
-        }
-
-        self.groups_mock.list.assert_called_with(**kwargs)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+
+        datalist = ((self.group.id, self.group.name),)
+        self.assertEqual(datalist, tuple(data))
+
+    def test_group_list_user_domain(self):
+        self.identity_sdk_client.user_groups.return_value = [
+            self.group_with_domain
+        ]
+        arglist = [
+            '--user',
+            self.user.name,
+            '--domain',
+            self.domain.name,
+        ]
+        verifylist = [
+            ('user', self.user.name),
+            ('domain', self.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': self.domain.id,
+        }
+
+        self.identity_sdk_client.user_groups.assert_called_with(
+            self.user.id, **kwargs
+        )
+
+        self.assertEqual(self.columns, columns)
+
+        datalist = ((self.group_with_domain.id, self.group_with_domain.name),)
+        self.assertEqual(datalist, tuple(data))
 
     def test_group_list_long(self):
+        self.identity_sdk_client.groups.return_value = [
+            self.group,
+            self.group_with_domain,
+        ]
         arglist = [
             '--long',
         ]
@@ -464,15 +583,9 @@ class TestGroupList(TestGroup):
         # containing the data to be listed.
         columns, data = self.cmd.take_action(parsed_args)
 
-        # Set expected values
+        self.identity_sdk_client.groups.assert_called_with()
-        kwargs = {
-            'domain': None,
-            'user': None,
-        }
 
-        self.groups_mock.list.assert_called_with(**kwargs)
+        long_columns = self.columns + (
-
-        columns = self.columns + (
             'Domain ID',
             'Description',
         )
@@ -483,25 +596,33 @@ class TestGroupList(TestGroup):
                 self.group.domain_id,
                 self.group.description,
             ),
+            (
+                self.group_with_domain.id,
+                self.group_with_domain.name,
+                self.group_with_domain.domain_id,
+                self.group_with_domain.description,
+            ),
         )
-        self.assertEqual(columns, columns)
+        self.assertEqual(long_columns, columns)
         self.assertEqual(datalist, tuple(data))
 
 
-class TestGroupRemoveUser(TestGroup):
+class TestGroupRemoveUser(identity_fakes.TestIdentityv3):
-    _group = identity_fakes.FakeGroup.create_one_group()
-    users = identity_fakes.FakeUser.create_users(count=2)
-
     def setUp(self):
         super().setUp()
 
-        self.groups_mock.get.return_value = self._group
+        self._group = sdk_fakes.generate_fake_resource(_group.Group)
-        self.users_mock.get = identity_fakes.FakeUser.get_users(self.users)
+        self.users = tuple(
-        self.users_mock.remove_from_group.return_value = None
+            sdk_fakes.generate_fake_resources(_user.User, count=2)
+        )
+
+        self.identity_sdk_client.find_group.return_value = self._group
+        self.identity_sdk_client.remove_user_from_group.return_value = None
 
         self.cmd = group.RemoveUserFromGroup(self.app, None)
 
     def test_group_remove_user(self):
+        self.identity_sdk_client.find_user.return_value = self.users[0]
         arglist = [
             self._group.id,
             self.users[0].id,
@@ -513,12 +634,16 @@ class TestGroupRemoveUser(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.users_mock.remove_from_group.assert_called_once_with(
+        self.identity_sdk_client.remove_user_from_group.assert_called_once_with(
             self.users[0].id, self._group.id
         )
         self.assertIsNone(result)
 
     def test_group_remove_multi_users(self):
+        self.identity_sdk_client.find_user.side_effect = [
+            self.users[0],
+            self.users[1],
+        ]
         arglist = [
             self._group.name,
             self.users[0].name,
@@ -535,13 +660,13 @@ class TestGroupRemoveUser(TestGroup):
             call(self.users[0].id, self._group.id),
             call(self.users[1].id, self._group.id),
         ]
-        self.users_mock.remove_from_group.assert_has_calls(calls)
+        self.identity_sdk_client.remove_user_from_group.assert_has_calls(calls)
         self.assertIsNone(result)
 
     @mock.patch.object(group.LOG, 'error')
     def test_group_remove_user_with_error(self, mock_error):
-        self.users_mock.remove_from_group.side_effect = [
+        self.identity_sdk_client.remove_user_from_group.side_effect = [
-            exceptions.CommandError(),
+            sdk_exc.ResourceNotFound(),
             None,
         ]
         arglist = [
@@ -560,26 +685,29 @@ class TestGroupRemoveUser(TestGroup):
         except exceptions.CommandError as e:
             msg = f"1 of 2 users not removed from group {self._group.id}."
             self.assertEqual(msg, str(e))
-        msg = f"{self.users[0].id} not removed from group {self._group.id}: "
+        msg = f"{self.users[0].id} not removed from group {self._group.id}: {str(sdk_exc.ResourceNotFound())}"
         mock_error.assert_called_once_with(msg)
 
 
-class TestGroupSet(TestGroup):
+class TestGroupSet(identity_fakes.TestIdentityv3):
-    domain = identity_fakes.FakeDomain.create_one_domain()
+    domain = sdk_fakes.generate_fake_resource(_domain.Domain)
-    group = identity_fakes.FakeGroup.create_one_group(
-        attrs={'domain_id': domain.id}
-    )
 
     def setUp(self):
         super().setUp()
+        self.group = sdk_fakes.generate_fake_resource(
+            _group.Group, domain_id=self.domain.id
+        )
+        self.group_with_domain = sdk_fakes.generate_fake_resource(
+            _group.Group, name=self.group.name, domain_id=self.domain.id
+        )
 
-        self.groups_mock.get.return_value = self.group
+        self.identity_sdk_client.find_group.return_value = self.group
-        self.domains_mock.get.return_value = self.domain
+        self.identity_sdk_client.find_domain.return_value = self.domain
-        self.groups_mock.update.return_value = None
 
         self.cmd = group.SetGroup(self.app, None)
 
     def test_group_set_nothing(self):
+        self.identity_sdk_client.update_group.return_value = self.group
         arglist = [
             self.group.id,
         ]
@@ -589,10 +717,13 @@ class TestGroupSet(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.groups_mock.update.assert_called_once_with(self.group.id)
+        self.identity_sdk_client.update_group.assert_called_once_with(
+            self.group.id
+        )
         self.assertIsNone(result)
 
     def test_group_set_name_and_description(self):
+        self.identity_sdk_client.update_group.return_value = self.group
         arglist = [
             '--name',
             'new_name',
@@ -612,36 +743,43 @@ class TestGroupSet(TestGroup):
             'name': 'new_name',
             'description': 'new_description',
         }
-        self.groups_mock.update.assert_called_once_with(
+        self.identity_sdk_client.update_group.assert_called_once_with(
             self.group.id, **kwargs
         )
         self.assertIsNone(result)
 
     def test_group_set_with_domain(self):
-        get_mock_result = [exceptions.CommandError, self.group]
+        self.identity_sdk_client.find_domain.side_effect = [
-        self.groups_mock.get = mock.Mock(side_effect=get_mock_result)
+            sdk_exc.ForbiddenException
-
+        ]
+        self.identity_sdk_client.find_group.return_value = (
+            self.group_with_domain
+        )
         arglist = [
             '--domain',
             self.domain.id,
-            self.group.id,
+            self.group_with_domain.name,
         ]
         verifylist = [
             ('domain', self.domain.id),
-            ('group', self.group.id),
+            ('group', self.group_with_domain.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_any_call(
+        self.identity_sdk_client.find_group.assert_called_once_with(
-            self.group.id, domain_id=self.domain.id
+            name_or_id=self.group_with_domain.name,
+            ignore_missing=False,
+            domain_id=self.domain.id,
+        )
+        self.identity_sdk_client.update_group.assert_called_once_with(
+            self.group_with_domain.id
         )
-        self.groups_mock.update.assert_called_once_with(self.group.id)
         self.assertIsNone(result)
 
 
-class TestGroupShow(TestGroup):
+class TestGroupShow(identity_fakes.TestIdentityv3):
-    domain = identity_fakes.FakeDomain.create_one_domain()
+    domain = sdk_fakes.generate_fake_resource(_domain.Domain)
 
     columns = (
         'description',
@@ -652,22 +790,19 @@ class TestGroupShow(TestGroup):
 
     def setUp(self):
         super().setUp()
-        self.group = identity_fakes.FakeGroup.create_one_group(
+        self.group = sdk_fakes.generate_fake_resource(
-            attrs={'domain_id': self.domain.id}
+            _group.Group, description=None, domain_id=None
         )
-        self.data = (
+        self.group_with_domain = sdk_fakes.generate_fake_resource(
-            self.group.description,
+            _group.Group, name=self.group.name, domain_id=self.domain.id
-            self.group.domain_id,
-            self.group.id,
-            self.group.name,
         )
 
-        self.groups_mock.get.return_value = self.group
+        self.identity_sdk_client.find_domain.return_value = self.domain
-        self.domains_mock.get.return_value = self.domain
 
         self.cmd = group.ShowGroup(self.app, None)
 
     def test_group_show(self):
+        self.identity_sdk_client.find_group.return_value = self.group
         arglist = [
             self.group.id,
         ]
@@ -677,28 +812,44 @@ class TestGroupShow(TestGroup):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_called_once_with(self.group.id)
+        self.identity_sdk_client.find_group.assert_called_once_with(
+            self.group.id, ignore_missing=False
+        )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        datalist = (
+            None,
+            None,
+            self.group.id,
+            self.group.name,
+        )
+        self.assertEqual(datalist, data)
 
     def test_group_show_with_domain(self):
-        get_mock_result = [exceptions.CommandError, self.group]
+        self.identity_sdk_client.find_group.return_value = (
-        self.groups_mock.get = mock.Mock(side_effect=get_mock_result)
+            self.group_with_domain
-
+        )
         arglist = [
             '--domain',
             self.domain.id,
-            self.group.id,
+            self.group_with_domain.name,
         ]
         verifylist = [
             ('domain', self.domain.id),
-            ('group', self.group.id),
+            ('group', self.group_with_domain.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.groups_mock.get.assert_any_call(
+        self.identity_sdk_client.find_group.assert_called_once_with(
-            self.group.id, domain_id=self.domain.id
+            self.group_with_domain.name,
+            domain_id=self.domain.id,
+            ignore_missing=False,
         )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        datalist = (
+            self.group_with_domain.description,
+            self.domain.id,
+            self.group_with_domain.id,
+            self.group_with_domain.name,
+        )
+        self.assertEqual(datalist, data)

releasenotes/notes/migrate-group-to-sdk-59beef31a7c40bbb.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - |
+    Migrate ``group`` commands from keystoneclient to SDK.