From eec975c0639e0fbc3c6dfdfeacc6edcccb0bc536 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 22 Mar 2024 18:50:52 +0000 Subject: [PATCH] api: Add response body schemas for host aggregate actions API Change-Id: Id6644a943c7ac735e3abf072bdc78674645945a4 Signed-off-by: Stephen Finucane --- nova/api/openstack/compute/aggregates.py | 26 ++++-- .../openstack/compute/schemas/aggregates.py | 91 ++++++++++++++++--- .../api/openstack/compute/test_aggregates.py | 28 ++++-- nova/tests/unit/policies/test_aggregates.py | 26 +++++- 4 files changed, 138 insertions(+), 33 deletions(-) diff --git a/nova/api/openstack/compute/aggregates.py b/nova/api/openstack/compute/aggregates.py index bf2ea7f0bf..70d685eee1 100644 --- a/nova/api/openstack/compute/aggregates.py +++ b/nova/api/openstack/compute/aggregates.py @@ -23,7 +23,7 @@ from webob import exc from nova.api.openstack import api_version_request from nova.api.openstack import common from nova.api.openstack.compute.schemas import aggregate_images -from nova.api.openstack.compute.schemas import aggregates +from nova.api.openstack.compute.schemas import aggregates as schema from nova.api.openstack import wsgi from nova.api import validation from nova.compute import api as compute @@ -49,7 +49,7 @@ class AggregateController(wsgi.Controller): self.conductor_tasks = conductor.ComputeTaskAPI() @wsgi.expected_errors(()) - @validation.query_schema(aggregates.index_query) + @validation.query_schema(schema.index_query) def index(self, req): """Returns a list a host aggregate's id, name, availability_zone.""" context = _get_context(req) @@ -61,8 +61,8 @@ class AggregateController(wsgi.Controller): # NOTE(gmann): Returns 200 for backwards compatibility but should be 201 # as this operation complete the creation of aggregates resource. @wsgi.expected_errors((400, 409)) - @validation.schema(aggregates.create_v20, '2.0', '2.0') - @validation.schema(aggregates.create, '2.1') + @validation.schema(schema.create_v20, '2.0', '2.0') + @validation.schema(schema.create, '2.1') def create(self, req, body): """Creates an aggregate, given its name and optional availability zone. @@ -95,7 +95,7 @@ class AggregateController(wsgi.Controller): return agg @wsgi.expected_errors((400, 404)) - @validation.query_schema(aggregates.show_query) + @validation.query_schema(schema.show_query) def show(self, req, id): """Shows the details of an aggregate, hosts and metadata included.""" context = _get_context(req) @@ -113,8 +113,8 @@ class AggregateController(wsgi.Controller): return self._marshall_aggregate(req, aggregate) @wsgi.expected_errors((400, 404, 409)) - @validation.schema(aggregates.update_v20, '2.0', '2.0') - @validation.schema(aggregates.update, '2.1') + @validation.schema(schema.update_v20, '2.0', '2.0') + @validation.schema(schema.update, '2.1') def update(self, req, id, body): """Updates the name and/or availability_zone of given aggregate.""" context = _get_context(req) @@ -165,7 +165,9 @@ class AggregateController(wsgi.Controller): # request hypervisor driver to complete the same in async mode. @wsgi.expected_errors((400, 404, 409)) @wsgi.action('add_host') - @validation.schema(aggregates.add_host) + @validation.schema(schema.add_host) + @validation.response_body_schema(schema.add_host_response, '2.1', '2.40') # noqa: E501 + @validation.response_body_schema(schema.add_host_response_v241, '2.41') def _add_host(self, req, id, body): """Adds a host to the specified aggregate.""" host = body['add_host']['host'] @@ -194,7 +196,9 @@ class AggregateController(wsgi.Controller): # request hypervisor driver to complete the same in async mode. @wsgi.expected_errors((400, 404, 409)) @wsgi.action('remove_host') - @validation.schema(aggregates.remove_host) + @validation.schema(schema.remove_host) + @validation.response_body_schema(schema.remove_host_response, '2.1', '2.40') # noqa: E501 + @validation.response_body_schema(schema.remove_host_response_v241, '2.41') def _remove_host(self, req, id, body): """Removes a host from the specified aggregate.""" host = body['remove_host']['host'] @@ -226,7 +230,9 @@ class AggregateController(wsgi.Controller): @wsgi.expected_errors((400, 404)) @wsgi.action('set_metadata') - @validation.schema(aggregates.set_metadata) + @validation.schema(schema.set_metadata) + @validation.response_body_schema(schema.set_metadata_response, '2.1', '2.40') # noqa: E501 + @validation.response_body_schema(schema.set_metadata_response_v241, '2.41') def _set_metadata(self, req, id, body): """Replaces the aggregate's existing metadata with new metadata.""" context = _get_context(req) diff --git a/nova/api/openstack/compute/schemas/aggregates.py b/nova/api/openstack/compute/schemas/aggregates.py index ff44cfd873..db0e36faf7 100644 --- a/nova/api/openstack/compute/schemas/aggregates.py +++ b/nova/api/openstack/compute/schemas/aggregates.py @@ -16,13 +16,12 @@ import copy from nova.api.validation import parameter_types -availability_zone = {'oneOf': [parameter_types.az_name, {'type': 'null'}]} -availability_zone_with_leading_trailing_spaces = { +_availability_zone = {'oneOf': [parameter_types.az_name, {'type': 'null'}]} +_availability_zone_with_leading_trailing_spaces = { 'oneOf': [parameter_types.az_name_with_leading_trailing_spaces, {'type': 'null'}] } - create = { 'type': 'object', 'properties': { @@ -30,7 +29,7 @@ create = { 'type': 'object', 'properties': { 'name': parameter_types.name, - 'availability_zone': availability_zone, + 'availability_zone': _availability_zone, }, 'required': ['name'], 'additionalProperties': False, @@ -42,10 +41,10 @@ create = { create_v20 = copy.deepcopy(create) -create_v20['properties']['aggregate']['properties']['name'] = (parameter_types. - name_with_leading_trailing_spaces) +create_v20['properties']['aggregate']['properties']['name'] = ( + parameter_types.name_with_leading_trailing_spaces) create_v20['properties']['aggregate']['properties']['availability_zone'] = ( - availability_zone_with_leading_trailing_spaces) + _availability_zone_with_leading_trailing_spaces) update = { @@ -55,7 +54,7 @@ update = { 'type': 'object', 'properties': { 'name': parameter_types.name_with_leading_trailing_spaces, - 'availability_zone': availability_zone + 'availability_zone': _availability_zone }, 'additionalProperties': False, 'anyOf': [ @@ -70,10 +69,10 @@ update = { update_v20 = copy.deepcopy(update) -update_v20['properties']['aggregate']['properties']['name'] = (parameter_types. - name_with_leading_trailing_spaces) +update_v20['properties']['aggregate']['properties']['name'] = ( + parameter_types.name_with_leading_trailing_spaces) update_v20['properties']['aggregate']['properties']['availability_zone'] = ( - availability_zone_with_leading_trailing_spaces) + _availability_zone_with_leading_trailing_spaces) add_host = { @@ -108,7 +107,6 @@ remove_host = { 'additionalProperties': False, } - set_metadata = { 'type': 'object', 'properties': { @@ -138,3 +136,72 @@ show_query = { 'properties': {}, 'additionalProperties': True, } + +_aggregate_response = { + 'type': 'object', + 'properties': { + 'aggregate': { + 'type': 'object', + 'properties': { + 'availability_zone': {'type': ['null', 'string']}, + 'created_at': {'type': 'string', 'format': 'date-time'}, + 'deleted': {'type': 'boolean'}, + 'deleted_at': { + 'oneOf': [ + {'type': 'null'}, + {'type': 'string', 'format': 'date-time'}, + ], + }, + 'hosts': { + 'type': ['array', 'null'], + 'items': { + 'type': 'string', + }, + }, + 'id': {'type': 'integer'}, + # TODO(stephenfin): This should be stricter + 'metadata': { + 'type': ['null', 'object'], + 'properties': {}, + 'additionalProperties': True, + }, + 'name': {'type': 'string'}, + 'updated_at': { + 'oneOf': [ + {'type': 'null'}, + {'type': 'string', 'format': 'date-time'}, + ], + }, + }, + 'required': [ + 'availability_zone', + 'created_at', + 'deleted', + 'deleted_at', + 'hosts', + 'id', + 'metadata', + 'name', + 'updated_at', + ], + 'additionalProperties': False, + }, + }, + 'required': ['aggregate'], + 'additionalProperties': False, +} + +_aggregate_response_v241 = copy.deepcopy(_aggregate_response) +_aggregate_response_v241['properties']['aggregate']['properties'].update( + {'uuid': {'type': 'string', 'format': 'uuid'}}, +) +_aggregate_response_v241['properties']['aggregate']['required'].append('uuid') + +add_host_response = copy.deepcopy(_aggregate_response) +add_host_response_v241 = copy.deepcopy(_aggregate_response_v241) + +remove_host_response = copy.deepcopy(_aggregate_response) +remove_host_response_v241 = copy.deepcopy(_aggregate_response_v241) + +set_metadata_response = copy.deepcopy(_aggregate_response) +set_metadata_response_v241 = copy.deepcopy(_aggregate_response_v241) diff --git a/nova/tests/unit/api/openstack/compute/test_aggregates.py b/nova/tests/unit/api/openstack/compute/test_aggregates.py index 891388590d..b9c7ac34b3 100644 --- a/nova/tests/unit/api/openstack/compute/test_aggregates.py +++ b/nova/tests/unit/api/openstack/compute/test_aggregates.py @@ -15,6 +15,7 @@ """Tests for the aggregates admin api.""" +import datetime from unittest import mock from oslo_utils.fixture import uuidsentinel @@ -32,7 +33,19 @@ from nova.tests.unit.api.openstack import fakes def _make_agg_obj(agg_dict): - return objects.Aggregate(**agg_dict) + _aggregate = { + "name": "aggregate1", + "id": "1", + "uuid": uuidsentinel.aggregate, + "metadata": {"availability_zone": "nova1"}, + "hosts": ["host1", "host2"], + "created_at": datetime.datetime(2012, 11, 14, 9, 53, 25, 0), + "deleted": False, + "deleted_at": None, + "updated_at": None, + } + _aggregate.update(**agg_dict) + return objects.Aggregate(**_aggregate) def _make_agg_list(agg_list): @@ -68,11 +81,12 @@ AGGREGATE_LIST = [ "metadata": {"availability_zone": "nova1"}}] AGGREGATE_LIST = _make_agg_list(AGGREGATE_LIST) -AGGREGATE = {"name": "aggregate1", - "id": "1", - "metadata": {"foo": "bar", - "availability_zone": "nova1"}, - "hosts": ["host1", "host2"]} +AGGREGATE = { + "name": "aggregate1", + "id": "1", + "metadata": {"foo": "bar", "availability_zone": "nova1"}, + "hosts": ["host1", "host2"], +} AGGREGATE = _make_agg_obj(AGGREGATE) FORMATTED_AGGREGATE = {"name": "aggregate1", @@ -706,7 +720,7 @@ class AggregateTestCaseV21(test.NoDBTestCase): 'id': 1, 'uuid': uuidsentinel.aggregate, 'metadata': {'foo': 'bar', 'availability_zone': 'nova'}, 'hosts': ['host1', 'host2']} - agg_obj = _make_agg_obj(agg) + agg_obj = objects.Aggregate(**agg) # _marshall_aggregate() puts all fields and obj_extra_fields in the # top-level dict, so we need to put availability_zone at the top also diff --git a/nova/tests/unit/policies/test_aggregates.py b/nova/tests/unit/policies/test_aggregates.py index 6ac7b6e010..f5e27e8d6d 100644 --- a/nova/tests/unit/policies/test_aggregates.py +++ b/nova/tests/unit/policies/test_aggregates.py @@ -13,6 +13,7 @@ from unittest import mock from oslo_utils.fixture import uuidsentinel as uuids +from oslo_utils import timeutils from nova.api.openstack.compute import aggregates from nova import objects @@ -39,6 +40,20 @@ class AggregatesPolicyTest(base.BasePolicyTest): self.legacy_admin_context, self.system_admin_context, self.project_admin_context] + now = timeutils.utcnow() + self.fake_aggregate = objects.Aggregate( + **{ + "created_at": now, + "name": "aggregate1", + "id": "1", + "metadata": {'availability_zone': 'nova1'}, + "hosts": ["host1", "host2"], + "deleted": False, + "deleted_at": None, + "updated_at": now, + } + ) + @mock.patch('nova.compute.api.AggregateAPI.get_aggregate_list') def test_list_aggregate_policy(self, mock_list): rule_name = "os_compute_api:os-aggregates:index" @@ -49,10 +64,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.create_aggregate') def test_create_aggregate_policy(self, mock_create): rule_name = "os_compute_api:os-aggregates:create" - mock_create.return_value = objects.Aggregate(**{"name": "aggregate1", - "id": "1", - "metadata": {'availability_zone': 'nova1'}, - "hosts": ["host1", "host2"]}) + mock_create.return_value = self.fake_aggregate body = {"aggregate": {"name": "test", "availability_zone": "nova1"}} self.common_policy_auth(self.project_admin_authorized_contexts, @@ -63,6 +75,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.update_aggregate') def test_update_aggregate_policy(self, mock_update): rule_name = "os_compute_api:os-aggregates:update" + mock_update.return_value = self.fake_aggregate self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller.update, self.req, 1, @@ -71,6 +84,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.delete_aggregate') def test_delete_aggregate_policy(self, mock_delete): rule_name = "os_compute_api:os-aggregates:delete" + mock_delete.return_value = None self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller.delete, @@ -79,6 +93,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.get_aggregate') def test_show_aggregate_policy(self, mock_show): rule_name = "os_compute_api:os-aggregates:show" + mock_show.return_value = self.fake_aggregate self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller.show, self.req, 1) @@ -86,6 +101,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.update_aggregate_metadata') def test_set_metadata_aggregate_policy(self, mock_metadata): rule_name = "os_compute_api:os-aggregates:set_metadata" + mock_metadata.return_value = self.fake_aggregate body = {"set_metadata": {"metadata": {"foo": "bar"}}} self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, @@ -95,6 +111,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.add_host_to_aggregate') def test_add_host_aggregate_policy(self, mock_add): rule_name = "os_compute_api:os-aggregates:add_host" + mock_add.return_value = self.fake_aggregate self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller._add_host, self.req, 1, @@ -103,6 +120,7 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.remove_host_from_aggregate') def test_remove_host_aggregate_policy(self, mock_remove): rule_name = "os_compute_api:os-aggregates:remove_host" + mock_remove.return_value = self.fake_aggregate self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller._remove_host,