From 3efb8070da6becedcaa9af58b38bf9385baa2e7f Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Tue, 8 Apr 2014 14:07:20 +0900 Subject: [PATCH] Add API schema for v2.1/v3 cells API By defining the API schema, it is possible to separate the validation code from the API method. The API method can be more simple. In addition, a response of API validation error can be consistent for the whole Nova API. Partially implements blueprint v3-api-schema Change-Id: Ic400a31fe2c05e07785a2dc6c4fd864e684f4965 --- .../api/openstack/compute/plugins/v3/cells.py | 50 ++-------- .../api/openstack/compute/schemas/v3/cells.py | 99 +++++++++++++++++++ nova/api/validation/validators.py | 11 +++ .../compute/plugins/v3/test_cells.py | 65 +++++------- nova/tests/test_api_validation.py | 45 +++++++++ 5 files changed, 187 insertions(+), 83 deletions(-) create mode 100644 nova/api/openstack/compute/schemas/v3/cells.py diff --git a/nova/api/openstack/compute/plugins/v3/cells.py b/nova/api/openstack/compute/plugins/v3/cells.py index 9825f79e171b..ee35daee4312 100644 --- a/nova/api/openstack/compute/plugins/v3/cells.py +++ b/nova/api/openstack/compute/plugins/v3/cells.py @@ -22,14 +22,15 @@ import six from webob import exc from nova.api.openstack import common +from nova.api.openstack.compute.schemas.v3 import cells from nova.api.openstack import extensions from nova.api.openstack import wsgi +from nova.api import validation from nova.cells import rpcapi as cells_rpcapi from nova.compute import api as compute from nova import exception from nova.i18n import _ from nova.openstack.common import strutils -from nova.openstack.common import timeutils from nova import rpc @@ -187,21 +188,6 @@ class CellsController(object): raise exc.HTTPNotFound( explanation=_("Cell %s doesn't exist.") % id) - def _validate_cell_name(self, cell_name): - """Validate cell name is not empty and doesn't contain '!' or '.'.""" - if not cell_name: - msg = _("Cell name cannot be empty") - raise exc.HTTPBadRequest(explanation=msg) - if '!' in cell_name or '.' in cell_name: - msg = _("Cell name cannot contain '!' or '.'") - raise exc.HTTPBadRequest(explanation=msg) - - def _validate_cell_type(self, cell_type): - """Validate cell_type is 'parent' or 'child'.""" - if cell_type not in ['parent', 'child']: - msg = _("Cell type must be 'parent' or 'child'") - raise exc.HTTPBadRequest(explanation=msg) - def _normalize_cell(self, cell, existing=None): """Normalize input cell data. Normalizations include: @@ -211,7 +197,6 @@ class CellsController(object): # Start with the cell type conversion if 'type' in cell: - self._validate_cell_type(cell['type']) cell['is_parent'] = cell['type'] == 'parent' del cell['type'] # Avoid cell type being overwritten to 'child' @@ -249,6 +234,7 @@ class CellsController(object): @extensions.expected_errors((400, 403, 501)) @common.check_cells_enabled @wsgi.response(201) + @validation.schema(cells.create) def create(self, req, body): """Create a child cell entry.""" context = req.environ['nova.context'] @@ -256,14 +242,7 @@ class CellsController(object): authorize(context) authorize(context, action="create") - if 'cell' not in body: - msg = _("No cell information in request") - raise exc.HTTPBadRequest(explanation=msg) cell = body['cell'] - if 'name' not in cell: - msg = _("No cell name in request") - raise exc.HTTPBadRequest(explanation=msg) - self._validate_cell_name(cell['name']) self._normalize_cell(cell) try: cell = self.cells_rpcapi.cell_create(context, cell) @@ -273,6 +252,7 @@ class CellsController(object): @extensions.expected_errors((400, 403, 404, 501)) @common.check_cells_enabled + @validation.schema(cells.update) def update(self, req, id, body): """Update a child cell entry. 'id' is the cell name to update.""" context = req.environ['nova.context'] @@ -280,13 +260,9 @@ class CellsController(object): authorize(context) authorize(context, action="update") - if 'cell' not in body: - msg = _("No cell information in request") - raise exc.HTTPBadRequest(explanation=msg) cell = body['cell'] cell.pop('id', None) - if 'name' in cell: - self._validate_cell_name(cell['name']) + try: # NOTE(Vek): There is a race condition here if multiple # callers are trying to update the cell @@ -309,6 +285,7 @@ class CellsController(object): @extensions.expected_errors((400, 501)) @common.check_cells_enabled @wsgi.response(204) + @validation.schema(cells.sync_instances) def sync_instances(self, req, body): """Tell all cells to sync instance info.""" context = req.environ['nova.context'] @@ -319,21 +296,8 @@ class CellsController(object): project_id = body.pop('project_id', None) deleted = body.pop('deleted', False) updated_since = body.pop('updated_since', None) - if body: - msg = _("Only 'updated_since', 'project_id' and 'deleted' are " - "understood.") - raise exc.HTTPBadRequest(explanation=msg) if isinstance(deleted, six.string_types): - try: - deleted = strutils.bool_from_string(deleted, strict=True) - except ValueError as err: - raise exc.HTTPBadRequest(explanation=str(err)) - if updated_since: - try: - timeutils.parse_isotime(updated_since) - except ValueError: - msg = _('Invalid changes-since value') - raise exc.HTTPBadRequest(explanation=msg) + deleted = strutils.bool_from_string(deleted, strict=True) self.cells_rpcapi.sync_instances(context, project_id=project_id, updated_since=updated_since, deleted=deleted) diff --git a/nova/api/openstack/compute/schemas/v3/cells.py b/nova/api/openstack/compute/schemas/v3/cells.py new file mode 100644 index 000000000000..37a9ed5cc0de --- /dev/null +++ b/nova/api/openstack/compute/schemas/v3/cells.py @@ -0,0 +1,99 @@ +# Copyright 2014 NEC Corporation. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova.api.validation import parameter_types + + +create = { + 'type': 'object', + 'properties': { + 'cell': { + 'type': 'object', + 'properties': { + 'name': parameter_types.name, + 'type': { + 'type': 'string', + 'enum': ['parent', 'child'], + }, + + # NOTE: In unparse_transport_url(), a url consists of the + # following parameters: + # "qpid://:@:/" + # or + # "rabiit://:@:/" + # Then the url is stored into transport_url of cells table + # which is defined with String(255). + 'username': { + 'type': 'string', 'maxLength': 255, + 'pattern': '^[a-zA-Z0-9-_]*$' + }, + 'password': { + # Allow to specify any string for strong password. + 'type': 'string', 'maxLength': 255, + }, + 'rpc_host': parameter_types.hostname_or_ip_address, + 'rpc_port': parameter_types.tcp_udp_port, + 'rpc_virtual_host': parameter_types.hostname_or_ip_address, + }, + 'required': ['name'], + 'additionalProperties': False, + }, + }, + 'required': ['cell'], + 'additionalProperties': False, +} + + +update = { + 'type': 'object', + 'properties': { + 'cell': { + 'type': 'object', + 'properties': { + 'name': parameter_types.name, + 'type': { + 'type': 'string', + 'enum': ['parent', 'child'], + }, + 'username': { + 'type': 'string', 'maxLength': 255, + 'pattern': '^[a-zA-Z0-9-_]*$' + }, + 'password': { + 'type': 'string', 'maxLength': 255, + }, + 'rpc_host': parameter_types.hostname_or_ip_address, + 'rpc_port': parameter_types.tcp_udp_port, + 'rpc_virtual_host': parameter_types.hostname_or_ip_address, + }, + 'additionalProperties': False, + }, + }, + 'required': ['cell'], + 'additionalProperties': False, +} + + +sync_instances = { + 'type': 'object', + 'properties': { + 'project_id': parameter_types.project_id, + 'deleted': parameter_types.boolean, + 'updated_since': { + 'type': 'string', + 'format': 'date-time', + }, + }, + 'additionalProperties': False, +} diff --git a/nova/api/validation/validators.py b/nova/api/validation/validators.py index a5090e18c81f..ce749237569a 100644 --- a/nova/api/validation/validators.py +++ b/nova/api/validation/validators.py @@ -21,9 +21,20 @@ import six from nova import exception from nova.i18n import _ +from nova.openstack.common import timeutils from nova.openstack.common import uuidutils +@jsonschema.FormatChecker.cls_checks('date-time') +def _validate_datetime_format(instance): + try: + timeutils.parse_isotime(instance) + except ValueError: + return False + else: + return True + + @jsonschema.FormatChecker.cls_checks('uuid') def _validate_uuid_format(instance): return uuidutils.is_uuid_like(instance) diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_cells.py b/nova/tests/api/openstack/compute/plugins/v3/test_cells.py index 63cf2d96bd56..404525ebb05a 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_cells.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_cells.py @@ -180,13 +180,11 @@ class CellsTest(BaseCellsTest): 'username': 'fred', 'password': 'fubar', 'rpc_host': 'r3.example.org', - 'type': 'parent', - # Also test this is ignored/stripped - 'is_parent': False}} + 'type': 'parent'}} req = self._get_request("cells") req.environ['nova.context'] = self.context - res_dict = self.controller.create(req, body) + res_dict = self.controller.create(req, body=body) cell = res_dict['cell'] self.assertEqual(self.controller.create.wsgi_code, 201) self.assertEqual(cell['name'], 'meow') @@ -194,7 +192,6 @@ class CellsTest(BaseCellsTest): self.assertEqual(cell['rpc_host'], 'r3.example.org') self.assertEqual(cell['type'], 'parent') self.assertNotIn('password', cell) - self.assertNotIn('is_parent', cell) def test_cell_create_parent(self): # Test create with just cells policy @@ -215,7 +212,7 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells") req.environ['nova.context'] = self.context - res_dict = self.controller.create(req, body) + res_dict = self.controller.create(req, body=body) cell = res_dict['cell'] self.assertEqual(self.controller.create.wsgi_code, 201) self.assertEqual(cell['name'], 'meow') @@ -243,8 +240,8 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, body=body) def test_cell_create_name_empty_string_raises(self): body = {'cell': {'name': '', @@ -255,8 +252,8 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, body=body) def test_cell_create_name_with_bang_raises(self): body = {'cell': {'name': 'moo!cow', @@ -267,20 +264,8 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.create, req, body) - - def test_cell_create_name_with_dot_raises(self): - body = {'cell': {'name': 'moo.cow', - 'username': 'fred', - 'password': 'secret', - 'rpc_host': 'r3.example.org', - 'type': 'parent'}} - - req = self._get_request("cells") - req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, body=body) def test_cell_create_name_with_invalid_type_raises(self): body = {'cell': {'name': 'moocow', @@ -291,8 +276,8 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, body=body) def test_cell_create_fails_for_invalid_policy(self): body = {'cell': {'name': 'fake'}} @@ -300,7 +285,7 @@ class CellsTest(BaseCellsTest): req.environ['nova.context'] = self.context req.environ['nova.context'].is_admin = False self.assertRaises(exception.PolicyNotAuthorized, - self.controller.create, req, body) + self.controller.create, req, body=body) def _cell_update(self): body = {'cell': {'username': 'zeb', @@ -308,7 +293,7 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells/cell1") req.environ['nova.context'] = self.context - res_dict = self.controller.update(req, 'cell1', body) + res_dict = self.controller.update(req, 'cell1', body=body) cell = res_dict['cell'] self.assertEqual(cell['name'], 'cell1') @@ -332,7 +317,7 @@ class CellsTest(BaseCellsTest): req.environ['nova.context'] = self.context req.environ['nova.context'].is_admin = False self.assertRaises(exception.PolicyNotAuthorized, - self.controller.create, req, body) + self.controller.create, req, body=body) def test_cell_update_empty_name_raises(self): body = {'cell': {'name': '', @@ -341,8 +326,8 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells/cell1") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.update, req, 'cell1', body) + self.assertRaises(exception.ValidationError, + self.controller.update, req, 'cell1', body=body) def test_cell_update_invalid_type_raises(self): body = {'cell': {'username': 'zeb', @@ -351,15 +336,15 @@ class CellsTest(BaseCellsTest): req = self._get_request("cells/cell1") req.environ['nova.context'] = self.context - self.assertRaises(exc.HTTPBadRequest, - self.controller.update, req, 'cell1', body) + self.assertRaises(exception.ValidationError, + self.controller.update, req, 'cell1', body=body) def test_cell_update_without_type_specified(self): body = {'cell': {'username': 'wingwj'}} req = self._get_request("cells/cell1") req.environ['nova.context'] = self.context - res_dict = self.controller.update(req, 'cell1', body) + res_dict = self.controller.update(req, 'cell1', body=body) cell = res_dict['cell'] self.assertEqual(cell['name'], 'cell1') @@ -373,12 +358,12 @@ class CellsTest(BaseCellsTest): req1 = self._get_request("cells/cell1") req1.environ['nova.context'] = self.context - res_dict1 = self.controller.update(req1, 'cell1', body1) + res_dict1 = self.controller.update(req1, 'cell1', body=body1) cell1 = res_dict1['cell'] req2 = self._get_request("cells/cell2") req2.environ['nova.context'] = self.context - res_dict2 = self.controller.update(req2, 'cell2', body2) + res_dict2 = self.controller.update(req2, 'cell2', body=body2) cell2 = res_dict2['cell'] self.assertEqual(cell1['name'], 'cell1') @@ -500,7 +485,7 @@ class CellsTest(BaseCellsTest): self.assertEqual(call_info['updated_since'], expected) body = {'updated_since': 'skjdfkjsdkf'} - self.assertRaises(exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.sync_instances, req, body=body) body = {'deleted': False} @@ -522,11 +507,11 @@ class CellsTest(BaseCellsTest): self.assertEqual(call_info['deleted'], True) body = {'deleted': 'foo'} - self.assertRaises(exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.sync_instances, req, body=body) body = {'foo': 'meow'} - self.assertRaises(exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.sync_instances, req, body=body) def test_sync_instances_fails_for_invalid_policy(self): @@ -541,7 +526,7 @@ class CellsTest(BaseCellsTest): body = {} self.assertRaises(exception.PolicyNotAuthorized, - self.controller.sync_instances, req, body) + self.controller.sync_instances, req, body=body) def test_cells_disabled(self): self.flags(enable=False, group='cells') diff --git a/nova/tests/test_api_validation.py b/nova/tests/test_api_validation.py index 7544c8d7b300..9a9348839b36 100644 --- a/nova/tests/test_api_validation.py +++ b/nova/tests/test_api_validation.py @@ -602,6 +602,51 @@ class TcpUdpPortTestCase(APIValidationTestCase): expected_detail=detail) +class DatetimeTestCase(APIValidationTestCase): + + def setUp(self): + super(DatetimeTestCase, self).setUp() + schema = { + 'type': 'object', + 'properties': { + 'foo': { + 'type': 'string', + 'format': 'date-time', + }, + }, + } + + @validation.schema(schema) + def post(body): + return 'Validation succeeded.' + + self.post = post + + def test_validate_datetime(self): + self.assertEqual('Validation succeeded.', + self.post( + body={'foo': '2014-01-14T01:00:00Z'} + )) + + def test_validate_datetime_fails(self): + detail = ("Invalid input for field/attribute foo." + " Value: 2014-13-14T01:00:00Z." + " '2014-13-14T01:00:00Z' is not a 'date-time'") + self.check_validation_error(self.post, + body={'foo': '2014-13-14T01:00:00Z'}, + expected_detail=detail) + + detail = ("Invalid input for field/attribute foo." + " Value: bar. 'bar' is not a 'date-time'") + self.check_validation_error(self.post, body={'foo': 'bar'}, + expected_detail=detail) + + detail = ("Invalid input for field/attribute foo. Value: 1." + " '1' is not a 'date-time'") + self.check_validation_error(self.post, body={'foo': '1'}, + expected_detail=detail) + + class UuidTestCase(APIValidationTestCase): def setUp(self):