From 64f70797d6d4106f4f7e2e8e13f46a6c683cb9ea Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 25 Nov 2024 16:01:54 +0000 Subject: [PATCH] api: Add response body schemas for keypairs APIs Quite a few test fixes need here in order to ensure our tests generate valid output. Change-Id: I33377c06f40aed70d574cdc8aada27a61128ea61 Signed-off-by: Stephen Finucane --- nova/api/openstack/compute/keypairs.py | 44 +++-- .../api/openstack/compute/schemas/keypairs.py | 152 +++++++++++++++++- nova/api/openstack/compute/views/keypairs.py | 2 - nova/api/validation/parameter_types.py | 2 +- .../api/openstack/compute/test_keypairs.py | 109 +++++++++---- nova/tests/unit/objects/test_keypair.py | 2 +- nova/tests/unit/policies/test_keypairs.py | 26 ++- 7 files changed, 275 insertions(+), 62 deletions(-) diff --git a/nova/api/openstack/compute/keypairs.py b/nova/api/openstack/compute/keypairs.py index 3fed90083d..af5a115532 100644 --- a/nova/api/openstack/compute/keypairs.py +++ b/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): diff --git a/nova/api/openstack/compute/schemas/keypairs.py b/nova/api/openstack/compute/schemas/keypairs.py index 74b992c3e3..bba0f94e8c 100644 --- a/nova/api/openstack/compute/schemas/keypairs.py +++ b/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, +}) diff --git a/nova/api/openstack/compute/views/keypairs.py b/nova/api/openstack/compute/views/keypairs.py index 202be5ac31..6a8d7b2252 100644 --- a/nova/api/openstack/compute/views/keypairs.py +++ b/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) diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index 72ae212099..2bb58696ee 100644 --- a/nova/api/validation/parameter_types.py +++ b/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, { diff --git a/nova/tests/unit/api/openstack/compute/test_keypairs.py b/nova/tests/unit/api/openstack/compute/test_keypairs.py index f0d941b510..673af97cbc 100644 --- a/nova/tests/unit/api/openstack/compute/test_keypairs.py +++ b/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 = { diff --git a/nova/tests/unit/objects/test_keypair.py b/nova/tests/unit/objects/test_keypair.py index b86bbb44de..21107b5846 100644 --- a/nova/tests/unit/objects/test_keypair.py +++ b/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', } diff --git a/nova/tests/unit/policies/test_keypairs.py b/nova/tests/unit/policies/test_keypairs.py index ee39133b7a..42f508d837 100644 --- a/nova/tests/unit/policies/test_keypairs.py +++ b/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,