Fix 'openstack keypair list --project <project>'

The --project option of 'openstack keypair list' is supposed to filter
keypairs by a project but has not been working and instead returns
keypairs from all projects.

The reason appears to be because it uses a request for a user list
filtered by project but tenant_id/project_id is not a valid filter for
GET /users.

This fixes the issue by requesting role assignments for the specified
project and then requesting keypairs for users with a role in the
project.

This change depends on a recent openstacksdk bug fix change
Ic552dee83d56278d2b866de0cb365a0c394fe26a which fixed the user_id query
parameter for the compute /os-keypairs APIs. The bug fix was released in
openstacksdk 4.4.0.

Closes-Bug: #2096947

Change-Id: Ibb5757766e3040e58d64388b95678fab9b2b6f23

Author: melwitt

openstackclient/compute/v2/keypair.py

@@ -300,6 +300,7 @@ def get_parser(self, prog_name):
     def take_action(self, parsed_args):
         compute_client = self.app.client_manager.compute
         identity_client = self.app.client_manager.identity
+        identity_sdk_client = self.app.client_manager.sdk_connection.identity
 
         kwargs = {}
 
@@ -345,11 +346,17 @@ def take_action(self, parsed_args):
                 parsed_args.project,
                 parsed_args.project_domain,
             ).id
-            users = identity_client.users.list(tenant_id=project)
+            assignments = identity_sdk_client.role_assignments(
+                scope_project_id=project
+            )
+            user_ids = set()
+            for assignment in assignments:
+                if assignment.user:
+                    user_ids.add(assignment.user['id'])
 
             data = []
-            for user in users:
-                kwargs['user_id'] = user.id
+            for user_id in user_ids:
+                kwargs['user_id'] = user_id
                 data.extend(compute_client.keypairs(**kwargs))
         elif parsed_args.user:
             if not sdk_utils.supports_microversion(compute_client, '2.10'):

openstackclient/tests/functional/compute/v2/test_keypair.py

@@ -21,23 +21,32 @@
 class KeypairBase(base.TestCase):
     """Methods for functional tests."""
 
-    def keypair_create(self, name=data_utils.rand_uuid()):
+    def keypair_create(self, name=data_utils.rand_uuid(), user=None):
         """Create keypair and add cleanup."""
-        raw_output = self.openstack('keypair create ' + name)
-        self.addCleanup(self.keypair_delete, name, True)
+        cmd = 'keypair create ' + name
+        if user is not None:
+            cmd += ' --user ' + user
+        raw_output = self.openstack(cmd)
+        self.addCleanup(
+            self.keypair_delete, name, ignore_exceptions=True, user=user
+        )
         if not raw_output:
             self.fail('Keypair has not been created!')
+        return name
 
     def keypair_list(self, params=''):
         """Return dictionary with list of keypairs."""
         raw_output = self.openstack('keypair list')
         keypairs = self.parse_show_as_object(raw_output)
         return keypairs
 
-    def keypair_delete(self, name, ignore_exceptions=False):
+    def keypair_delete(self, name, ignore_exceptions=False, user=None):
         """Try to delete keypair by name."""
         try:
-            self.openstack('keypair delete ' + name)
+            cmd = 'keypair delete ' + name
+            if user is not None:
+                cmd += ' --user ' + user
+            self.openstack(cmd)
         except exceptions.CommandFailed:
             if not ignore_exceptions:
                 raise
@@ -200,3 +209,30 @@ def test_keypair_show(self):
         items = self.parse_listing(raw_output)
         self.assert_table_structure(items, HEADERS)
         self.assertInOutput(self.KPName, raw_output)
+
+    def test_keypair_list_by_project(self):
+        """Test keypair list by project.
+
+        Test steps:
+        1) Create keypair for admin project in setUp
+        2) Create a new project
+        3) Create a new user
+        4) Associate the new user with the new project
+        5) Create keypair for the new user
+        6) List keypairs by the new project
+        7) Check that only the keypair from step 5 is returned
+        """
+        project_name = data_utils.rand_name('TestProject')
+        self.openstack(f'project create {project_name}')
+        self.addCleanup(self.openstack, f'project delete {project_name}')
+        user_name = data_utils.rand_name('TestUser')
+        self.openstack(f'user create {user_name}')
+        self.addCleanup(self.openstack, f'user delete {user_name}')
+        self.openstack(
+            f'role add --user {user_name} --project {project_name} member'
+        )
+        keypair_name = self.keypair_create(user=user_name)
+        raw_output = self.openstack(f'keypair list --project {project_name}')
+        items = self.parse_listing(raw_output)
+        self.assertEqual(1, len(items))
+        self.assertEqual(keypair_name, items[0]['Name'])

openstackclient/tests/unit/compute/v2/fakes.py

@@ -33,7 +33,7 @@
 from openstack.compute.v2 import volume_attachment as _volume_attachment
 
 from openstackclient.tests.unit import fakes
-from openstackclient.tests.unit.identity.v2_0 import fakes as identity_fakes
+from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
 from openstackclient.tests.unit.image.v2 import fakes as image_fakes
 from openstackclient.tests.unit.network.v2 import fakes as network_fakes
 from openstackclient.tests.unit import utils
@@ -121,10 +121,10 @@ def set_compute_api_version(self, version: str = '2.1'):
 
 
 class TestComputev2(
+    identity_fakes.FakeClientMixin,
     network_fakes.FakeClientMixin,
     image_fakes.FakeClientMixin,
     volume_fakes.FakeClientMixin,
-    identity_fakes.FakeClientMixin,
     FakeClientMixin,
     utils.TestCommand,
 ): ...

openstackclient/tests/unit/compute/v2/test_keypair.py

@@ -18,6 +18,7 @@
 
 from openstack.compute.v2 import keypair as _keypair
 from openstack.identity.v3 import project as _project
+from openstack.identity.v3 import role_assignment as _role_assignment
 from openstack.identity.v3 import user as _user
 from openstack.test import fakes as sdk_fakes
 from osc_lib import exceptions
@@ -529,13 +530,17 @@ def test_keypair_list_with_user_pre_v210(self):
     def test_keypair_list_with_project(self):
         self.set_compute_api_version('2.35')
 
-        projects_mock = self.identity_client.tenants
+        projects_mock = self.identity_client.projects
         projects_mock.reset_mock()
         projects_mock.get.return_value = self._project
 
-        users_mock = self.identity_client.users
-        users_mock.reset_mock()
-        users_mock.list.return_value = [self._user]
+        role_assignments_mock = self.identity_sdk_client.role_assignments
+        role_assignments_mock.reset_mock()
+        assignment = sdk_fakes.generate_fake_resource(
+            _role_assignment.RoleAssignment
+        )
+        assignment.user = self._user
+        role_assignments_mock.return_value = [assignment]
 
         arglist = ['--project', self._project.name]
         verifylist = [('project', self._project.name)]
@@ -544,7 +549,9 @@ def test_keypair_list_with_project(self):
         columns, data = self.cmd.take_action(parsed_args)
 
         projects_mock.get.assert_called_with(self._project.name)
-        users_mock.list.assert_called_with(tenant_id=self._project.id)
+        role_assignments_mock.assert_called_with(
+            scope_project_id=self._project.id
+        )
         self.compute_client.keypairs.assert_called_with(
             user_id=self._user.id,
         )

requirements.txt

@@ -7,7 +7,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0
 cryptography>=2.7 # BSD/Apache-2.0
 cliff>=3.5.0 # Apache-2.0
 iso8601>=0.1.11 # MIT
-openstacksdk>=3.3.0 # Apache-2.0
+openstacksdk>=4.4.0 # Apache-2.0
 osc-lib>=2.3.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 python-keystoneclient>=3.22.0 # Apache-2.0