Merge "api: Add response body schemas for keypairs APIs"

nova/api/openstack/compute/keypairs.py

@@ -20,7 +20,7 @@ import webob.exc
 
 from nova.api.openstack import api_version_request
 from nova.api.openstack import common
-from nova.api.openstack.compute.schemas import keypairs
+from nova.api.openstack.compute.schemas import keypairs as schema
 from nova.api.openstack.compute.views import keypairs as keypairs_view
 from nova.api.openstack import wsgi
 from nova.api import validation
@@ -30,6 +30,7 @@ from nova.objects import keypair as keypair_obj
 from nova.policies import keypairs as kp_policies
 
 
+@validation.validated
 class KeypairController(wsgi.Controller):
 
     """Keypair API controller for the OpenStack API."""
@@ -43,11 +44,14 @@ class KeypairController(wsgi.Controller):
     @wsgi.response(200, "2.0", "2.1")
     @wsgi.response(201, "2.2")
     @wsgi.expected_errors((400, 403, 409))
-    @validation.schema(keypairs.create_v20, "2.0", "2.0")
-    @validation.schema(keypairs.create, "2.1", "2.1")
-    @validation.schema(keypairs.create_v22, "2.2", "2.9")
-    @validation.schema(keypairs.create_v210, "2.10", "2.91")
-    @validation.schema(keypairs.create_v292, "2.92")
+    @validation.schema(schema.create_v20, "2.0", "2.0")
+    @validation.schema(schema.create, "2.1", "2.1")
+    @validation.schema(schema.create_v22, "2.2", "2.9")
+    @validation.schema(schema.create_v210, "2.10", "2.91")
+    @validation.schema(schema.create_v292, "2.92")
+    @validation.response_body_schema(schema.create_response, "2.0", "2.1")
+    @validation.response_body_schema(schema.create_response_v22, "2.2", "2.91")
+    @validation.response_body_schema(schema.create_response_v292, "2.92")
     def create(self, req, body):
         """Create or import keypair.
 
@@ -83,12 +87,12 @@ class KeypairController(wsgi.Controller):
         context.can(kp_policies.POLICY_ROOT % 'create',
                     target={'user_id': user_id})
 
-        return_priv_key = False
         try:
             if 'public_key' in params:
                 keypair = self.api.import_key_pair(
                     context, user_id, name, params['public_key'],
                     key_type_value)
+                return_priv_key = False
             else:
                 # public_key is a required field starting with 2.92 so this
                 # generation should only happen with older versions.
@@ -114,9 +118,10 @@ class KeypairController(wsgi.Controller):
 
     @wsgi.response(202, '2.0', '2.1')
     @wsgi.response(204, '2.2')
-    @validation.query_schema(keypairs.delete_query_schema_v20, '2.1', '2.9')
-    @validation.query_schema(keypairs.delete_query_schema_v210, '2.10', '2.74')
-    @validation.query_schema(keypairs.delete_query_schema_v275, '2.75')
+    @validation.query_schema(schema.delete_query_schema_v20, '2.0', '2.9')
+    @validation.query_schema(schema.delete_query_schema_v210, '2.10', '2.74')
+    @validation.query_schema(schema.delete_query_schema_v275, '2.75')
+    @validation.response_body_schema(schema.delete_response)
     @wsgi.expected_errors(404)
     def delete(self, req, id):
         user_id = None
@@ -138,9 +143,11 @@ class KeypairController(wsgi.Controller):
         except exception.KeypairNotFound as exc:
             raise webob.exc.HTTPNotFound(explanation=exc.format_message())
 
-    @validation.query_schema(keypairs.show_query_schema_v20, '2.0', '2.9')
-    @validation.query_schema(keypairs.show_query_schema_v210, '2.10', '2.74')
-    @validation.query_schema(keypairs.show_query_schema_v275, '2.75')
+    @validation.query_schema(schema.show_query_schema_v20, '2.0', '2.9')
+    @validation.query_schema(schema.show_query_schema_v210, '2.10', '2.74')
+    @validation.query_schema(schema.show_query_schema_v275, '2.75')
+    @validation.response_body_schema(schema.show_response, '2.0', '2.1')
+    @validation.response_body_schema(schema.show_response_v22, '2.2')
     @wsgi.expected_errors(404)
     def show(self, req, id):
         key_type = False
@@ -167,10 +174,13 @@ class KeypairController(wsgi.Controller):
             raise webob.exc.HTTPNotFound(explanation=exc.format_message())
         return self._view_builder.show(keypair, key_type=key_type)
 
-    @validation.query_schema(keypairs.index_query_schema_v20, '2.0', '2.9')
-    @validation.query_schema(keypairs.index_query_schema_v210, '2.10', '2.34')
-    @validation.query_schema(keypairs.index_query_schema_v235, '2.35', '2.74')
-    @validation.query_schema(keypairs.index_query_schema_v275, '2.75')
+    @validation.query_schema(schema.index_query_schema_v20, '2.0', '2.9')
+    @validation.query_schema(schema.index_query_schema_v210, '2.10', '2.34')
+    @validation.query_schema(schema.index_query_schema_v235, '2.35', '2.74')
+    @validation.query_schema(schema.index_query_schema_v275, '2.75')
+    @validation.response_body_schema(schema.index_response, '2.0', '2.1')
+    @validation.response_body_schema(schema.index_response_v22, '2.2', '2.34')
+    @validation.response_body_schema(schema.index_response_v235, '2.35')
     @wsgi.expected_errors((), '2.0', '2.9')
     @wsgi.expected_errors(400, '2.10')
     def index(self, req):

nova/api/openstack/compute/schemas/keypairs.py

@@ -15,6 +15,7 @@
 import copy
 
 from nova.api.validation import parameter_types
+from nova.api.validation import response_types
 
 
 create = {
@@ -36,8 +37,8 @@ create = {
 
 
 create_v20 = copy.deepcopy(create)
-create_v20['properties']['keypair']['properties']['name'] = (parameter_types.
-    name_with_leading_trailing_spaces)
+create_v20['properties']['keypair']['properties']['name'] = (
+    parameter_types.name_with_leading_trailing_spaces)
 
 
 create_v22 = {
@@ -84,8 +85,8 @@ create_v210 = {
 }
 
 create_v292 = copy.deepcopy(create_v210)
-create_v292['properties']['keypair']['properties']['name'] = (parameter_types.
-    keypair_name_special_chars_292)
+create_v292['properties']['keypair']['properties']['name'] = (
+    parameter_types.keypair_name_special_chars_v292)
 create_v292['properties']['keypair']['required'] = ['name', 'public_key']
 
 index_query_schema_v20 = {
@@ -117,3 +118,146 @@ show_query_schema_v275 = copy.deepcopy(show_query_schema_v210)
 show_query_schema_v275['additionalProperties'] = False
 delete_query_schema_v275 = copy.deepcopy(delete_query_schema_v210)
 delete_query_schema_v275['additionalProperties'] = False
+
+create_response = {
+    'type': 'object',
+    'properties': {
+        'keypair': {
+            'type': 'object',
+            'properties': {
+                'fingerprint': {'type': 'string'},
+                'name': parameter_types.keypair_name_special_chars,
+                'private_key': {'type': 'string'},
+                'public_key': {'type': 'string'},
+                'user_id': parameter_types.user_id,
+            },
+            'required': ['fingerprint', 'name', 'public_key', 'user_id'],
+            'additionalProperties': False,
+        }
+    },
+    'required': ['keypair'],
+    'additionalProperties': False,
+}
+
+create_response_v22 = copy.deepcopy(create_response)
+create_response_v22['properties']['keypair']['properties'].update({
+    'type': {
+        'type': 'string',
+        'enum': ['ssh', 'x509']
+    },
+})
+create_response_v22['properties']['keypair']['required'].append('type')
+
+create_response_v292 = copy.deepcopy(create_response_v22)
+del create_response_v292['properties']['keypair']['properties']['private_key']
+create_response_v292['properties']['keypair']['properties']['name'] = (
+    parameter_types.keypair_name_special_chars_v292
+)
+
+delete_response = {
+    'type': 'null',
+}
+
+show_response = {
+    'type': 'object',
+    'properties': {
+        'keypair': {
+            'type': 'object',
+            'properties': {
+                'created_at': {'type': 'string', 'format': 'date-time'},
+                'deleted': {'type': 'boolean'},
+                'deleted_at': {
+                    'oneOf': [
+                        {'type': 'string', 'format': 'date-time'},
+                        {'type': 'null'},
+                    ],
+                },
+                'fingerprint': {'type': 'string'},
+                'id': {'type': 'integer'},
+                'name': parameter_types.keypair_name_special_chars,
+                'public_key': {'type': 'string'},
+                'updated_at': {
+                    'oneOf': [
+                        {'type': 'string', 'format': 'date-time'},
+                        {'type': 'null'},
+                    ],
+                },
+                'user_id': parameter_types.user_id,
+            },
+            'required': [
+                'created_at',
+                'deleted',
+                'deleted_at',
+                'fingerprint',
+                'id',
+                'name',
+                'public_key',
+                'updated_at',
+                'user_id'
+            ],
+            'additionalProperties': False,
+        }
+    },
+    'required': ['keypair'],
+    'additionalProperties': False,
+}
+
+show_response_v22 = copy.deepcopy(show_response)
+show_response_v22['properties']['keypair']['properties'].update({
+    'type': {
+        'type': 'string',
+        'enum': ['ssh', 'x509']
+    },
+})
+show_response_v22['properties']['keypair']['required'].append('type')
+
+index_response = {
+    'type': 'object',
+    'properties': {
+        'keypairs': {
+            'type': 'array',
+            'items': {
+                'type': 'object',
+                'properties': {
+                    'keypair': {
+                        'type': 'object',
+                        'items': {
+                            'type': 'object',
+                            'properties': {
+                                'fingerprint': {'type': 'string'},
+                                'name': parameter_types.keypair_name_special_chars,  # noqa: E501
+                                'public_key': {'type': 'string'},
+                            },
+                            'required': ['fingerprint', 'name', 'public_key'],
+                            'additionalProperties': False,
+                        },
+                    },
+                },
+                'required': ['keypair'],
+                'additionalProperties': False,
+            },
+        },
+    },
+    'required': ['keypairs'],
+    'additionalProperties': False,
+}
+
+index_response_v22 = copy.deepcopy(index_response)
+index_response_v22['properties']['keypairs']['items']['properties'][
+    'keypair'
+]['items']['properties'].update({
+    'type': {
+        'type': 'string',
+        'enum': ['ssh', 'x509']
+    },
+})
+index_response_v22['properties']['keypairs']['items']['properties'][
+    'keypair'
+]['items']['required'].append(
+    'type'
+)
+
+index_response_v235 = copy.deepcopy(index_response_v22)
+index_response_v235['properties'].update({
+    'keypairs_links': response_types.collection_links,
+})

nova/api/openstack/compute/views/keypairs.py

@@ -46,8 +46,6 @@ class ViewBuilder(common.ViewBuilder):
         params = []
         if private_key:
             params.append('private_key')
-        # TODO(takashin): After v2 and v2.1 is no longer supported,
-        # 'type' can always be included in the response.
         if key_type:
             params.append('type')
         params.extend(self._create_params)

nova/api/validation/parameter_types.py

@@ -308,7 +308,7 @@ keypair_name_special_chars = {
     ]
 }
 
-keypair_name_special_chars_292 = {
+keypair_name_special_chars_v292 = {
     'allOf': [
         name,
         {

nova/tests/unit/api/openstack/compute/test_keypairs.py

@@ -15,6 +15,7 @@
 
 from unittest import mock
 
+from oslo_utils.fixture import uuidsentinel as uuids
 import webob
 
 from nova.api.openstack.compute import keypairs as keypairs_v21
@@ -98,7 +99,9 @@ class KeypairsTestV21(test.TestCase):
 
         self._setup_app_and_controller()
 
-        self.req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version)
+        self.req = fakes.HTTPRequest.blank(
+            '', version=self.wsgi_api_version, user_id=uuids.user_id,
+        )
 
     def test_keypair_list(self):
         res_dict = self.controller.index(self.req)
@@ -317,35 +320,50 @@ class KeypairsTestV210(KeypairsTestV22):
         pass
 
     def test_keypair_list_other_user(self):
-        req = fakes.HTTPRequest.blank(self.base_url +
-                                      '/os-keypairs?user_id=foo',
-                                      version=self.wsgi_api_version,
-                                      use_admin_context=True)
-        with mock.patch.object(self.controller.api, 'get_key_pairs') as mock_g:
+        req = fakes.HTTPRequest.blank(
+            self.base_url + f'/os-keypairs?user_id={uuids.other_user_id}',
+            version=self.wsgi_api_version,
+            use_admin_context=True)
+        kps = objects.KeyPairList(
+            objects=[
+                objects.KeyPair(
+                    nova_context.get_admin_context(), **fake_keypair('FAKE')
+                ),
+            ],
+        )
+        with mock.patch.object(
+            self.controller.api, 'get_key_pairs', return_value=kps,
+        ) as mock_g:
             self.controller.index(req)
             userid = mock_g.call_args_list[0][0][1]
-            self.assertEqual('foo', userid)
+            self.assertEqual(uuids.other_user_id, userid)
 
     def test_keypair_show_other_user(self):
-        req = fakes.HTTPRequest.blank(self.base_url +
-                                      '/os-keypairs/FAKE?user_id=foo',
-                                      version=self.wsgi_api_version,
-                                      use_admin_context=True)
-        with mock.patch.object(self.controller.api, 'get_key_pair') as mock_g:
+        req = fakes.HTTPRequest.blank(
+            self.base_url + f'/os-keypairs/FAKE?user_id={uuids.other_user_id}',
+            version=self.wsgi_api_version,
+            use_admin_context=True)
+        kp = objects.KeyPair(
+            nova_context.get_admin_context(), **fake_keypair('FAKE')
+        )
+        with mock.patch.object(
+            self.controller.api, 'get_key_pair', return_value=kp,
+        ) as mock_g:
             self.controller.show(req, 'FAKE')
             userid = mock_g.call_args_list[0][0][1]
-            self.assertEqual('foo', userid)
+            self.assertEqual(uuids.other_user_id, userid)
 
     def test_keypair_delete_other_user(self):
-        req = fakes.HTTPRequest.blank(self.base_url +
-                                      '/os-keypairs/FAKE?user_id=foo',
-                                      version=self.wsgi_api_version,
-                                      use_admin_context=True)
-        with mock.patch.object(self.controller.api,
-                               'delete_key_pair') as mock_g:
+        req = fakes.HTTPRequest.blank(
+            self.base_url + f'/os-keypairs/FAKE?user_id={uuids.other_user_id}',
+            version=self.wsgi_api_version,
+            use_admin_context=True)
+        with mock.patch.object(
+            self.controller.api, 'delete_key_pair', return_value=None,
+        ) as mock_g:
             self.controller.delete(req, 'FAKE')
             userid = mock_g.call_args_list[0][0][1]
-            self.assertEqual('foo', userid)
+            self.assertEqual(uuids.other_user_id, userid)
 
     def test_keypair_create_other_user(self):
         req = fakes.HTTPRequest.blank(self.base_url +
@@ -354,9 +372,12 @@ class KeypairsTestV210(KeypairsTestV22):
                                       use_admin_context=True)
         body = {'keypair': {'name': 'create_test',
                             'user_id': '8861f37f-034e-4ca8-8abe-6d13c074574a'}}
-        with mock.patch.object(self.controller.api,
-                               'create_key_pair',
-                               return_value=(mock.MagicMock(), 1)) as mock_g:
+        kp = objects.KeyPair(
+            nova_context.get_admin_context(), **fake_keypair('FAKE')
+        )
+        with mock.patch.object(
+            self.controller.api, 'create_key_pair', return_value=(kp, 'key')
+        ) as mock_g:
             res = self.controller.create(req, body=body)
             userid = mock_g.call_args_list[0][0][1]
             self.assertEqual('8861f37f-034e-4ca8-8abe-6d13c074574a', userid)
@@ -370,18 +391,22 @@ class KeypairsTestV210(KeypairsTestV22):
         body = {'keypair': {'name': 'create_test',
                             'user_id': '8861f37f-034e-4ca8-8abe-6d13c074574a',
                             'public_key': 'public_key'}}
-        with mock.patch.object(self.controller.api,
-                               'import_key_pair') as mock_g:
+        kp = objects.KeyPair(
+            nova_context.get_admin_context(), **fake_keypair('FAKE')
+        )
+        with mock.patch.object(
+            self.controller.api, 'import_key_pair', return_value=kp
+        ) as mock_g:
             res = self.controller.create(req, body=body)
             userid = mock_g.call_args_list[0][0][1]
             self.assertEqual('8861f37f-034e-4ca8-8abe-6d13c074574a', userid)
         self.assertIn('keypair', res)
 
     def test_keypair_list_other_user_invalid_in_old_microversion(self):
-        req = fakes.HTTPRequest.blank(self.base_url +
-                                      '/os-keypairs?user_id=foo',
-                                      version="2.9",
-                                      use_admin_context=True)
+        req = fakes.HTTPRequest.blank(
+            self.base_url + f'/os-keypairs?user_id={uuids.other_user_id}',
+            version="2.9",
+            use_admin_context=True)
         with mock.patch.object(self.controller.api, 'get_key_pairs') as mock_g:
             self.controller.index(req)
             userid = mock_g.call_args_list[0][0][1]
@@ -457,13 +482,24 @@ class KeypairsTestV275(test.TestCase):
         super(KeypairsTestV275, self).setUp()
         self.controller = keypairs_v21.KeypairController()
 
-    @mock.patch('nova.objects.KeyPair.get_by_name')
-    def test_keypair_list_additional_param_old_version(self, mock_get_by_name):
+    def test_keypair_list_additional_param_old_version(self):
         req = fakes.HTTPRequest.blank(
             '/os-keypairs?unknown=3',
             version='2.74', use_admin_context=True)
-        self.controller.index(req)
-        self.controller.show(req, 1)
+
+        kp = objects.KeyPair(
+            nova_context.get_admin_context(), **fake_keypair('FAKE')
+        )
+        kps = objects.KeyPairList(objects=[kp])
+        with mock.patch.object(
+            self.controller.api, 'get_key_pairs', return_value=kps,
+        ):
+            self.controller.index(req)
+
+        with mock.patch.object(
+            self.controller.api, 'get_key_pair', return_value=kp,
+        ):
+            self.controller.show(req, 1)
         with mock.patch.object(self.controller.api, 'delete_key_pair'):
             self.controller.delete(req, 1)
 
@@ -496,9 +532,12 @@ class KeypairsTestV292(test.TestCase):
     def setUp(self):
         super(KeypairsTestV292, self).setUp()
         self.controller = keypairs_v21.KeypairController()
-        self.req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version)
+        self.req = fakes.HTTPRequest.blank(
+            '', version=self.wsgi_api_version, user_id=uuids.user_id,
+        )
         self.old_req = fakes.HTTPRequest.blank(
-            '', version=self.wsgi_old_api_version)
+            '', version=self.wsgi_old_api_version, user_id=uuids.user_id,
+        )
 
     def test_keypair_create_no_longer_supported(self):
         body = {

nova/tests/unit/objects/test_keypair.py

@@ -29,7 +29,7 @@ fake_keypair = {
     'id': 123,
     'name': 'foo-keypair',
     'type': 'ssh',
-    'user_id': 'fake-user',
+    'user_id': '6aa30431-e604-43f3-b75e-10a4276b3247',
     'fingerprint': 'fake-fingerprint',
     'public_key': 'fake\npublic\nkey',
 }

nova/tests/unit/policies/test_keypairs.py

@@ -10,16 +10,34 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
 from unittest import mock
 
-from nova.policies import keypairs as policies
+from oslo_utils.fixture import uuidsentinel as uuids
 
 from nova.api.openstack.compute import keypairs
+from nova import objects
+from nova.policies import keypairs as policies
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit.objects import test_keypair
 from nova.tests.unit.policies import base
 
 
+FAKE_KEYPAIR = objects.KeyPair(
+    created_at=datetime.datetime(2024, 10, 29, 13, 42, 2),
+    deleted=False,
+    deleted_at=None,
+    fingerprint='foo',
+    id=123,
+    name='foo',
+    private_key='ssh-rsa foo',
+    public_key='ssh-rsa foo',
+    type='ssh',
+    updated_at=datetime.datetime(2024, 10, 29, 13, 42, 2),
+    user_id=uuids.user_alt_id,
+)
+
+
 class KeypairsPolicyTest(base.BasePolicyTest):
     """Test Keypairs APIs policies with all possible context.
     This class defines the set of context with different roles
@@ -53,6 +71,7 @@ class KeypairsPolicyTest(base.BasePolicyTest):
 
     @mock.patch('nova.compute.api.KeypairAPI.get_key_pairs')
     def test_index_keypairs_policy(self, mock_get):
+        mock_get.return_value = objects.KeyPairList(objects=[])
         rule_name = policies.POLICY_ROOT % 'index'
         self.common_policy_auth(self.everyone_authorized_contexts,
                                 rule_name,
@@ -61,6 +80,7 @@ class KeypairsPolicyTest(base.BasePolicyTest):
 
     @mock.patch('nova.compute.api.KeypairAPI.get_key_pairs')
     def test_index_others_keypairs_policy(self, mock_get):
+        mock_get.return_value = objects.KeyPairList(objects=[])
         req = fakes.HTTPRequest.blank('?user_id=user2', version='2.10')
         rule_name = policies.POLICY_ROOT % 'index'
         self.common_policy_auth(self.admin_authorized_contexts,
@@ -70,6 +90,7 @@ class KeypairsPolicyTest(base.BasePolicyTest):
 
     @mock.patch('nova.compute.api.KeypairAPI.get_key_pair')
     def test_show_keypairs_policy(self, mock_get):
+        mock_get.return_value = FAKE_KEYPAIR
         rule_name = policies.POLICY_ROOT % 'show'
         self.common_policy_auth(self.everyone_authorized_contexts,
                                 rule_name,
@@ -78,6 +99,7 @@ class KeypairsPolicyTest(base.BasePolicyTest):
 
     @mock.patch('nova.compute.api.KeypairAPI.get_key_pair')
     def test_show_others_keypairs_policy(self, mock_get):
+        mock_get.return_value = FAKE_KEYPAIR
         # Change the user_id in request context.
         req = fakes.HTTPRequest.blank('?user_id=user2', version='2.10')
         rule_name = policies.POLICY_ROOT % 'show'
@@ -88,8 +110,8 @@ class KeypairsPolicyTest(base.BasePolicyTest):
 
     @mock.patch('nova.compute.api.KeypairAPI.create_key_pair')
     def test_create_keypairs_policy(self, mock_create):
-        rule_name = policies.POLICY_ROOT % 'create'
         mock_create.return_value = (test_keypair.fake_keypair, 'FAKE_KEY')
+        rule_name = policies.POLICY_ROOT % 'create'
         self.common_policy_auth(self.everyone_authorized_contexts,
                                 rule_name,
                                 self.controller.create,