From afa410bcf65c8039edb424caa7ae510c515528e3 Mon Sep 17 00:00:00 2001 From: pooja jadhav Date: Wed, 22 Nov 2017 19:31:34 +0530 Subject: [PATCH] V3 jsonschema validation: Volume metadata This patch adds jsonschema validation for below metadata API's * POST /v3/{project_id}/volumes/{volume_id}/metadata * PUT /v3/{project_id}/volumes/{volume_id}/metadata * PUT /v3/{project_id}/volumes/{volume_id}/metadata/{key} The reason behind applying schema for v2(update and update_all) is that in "/cinder/tests/unit/api/v2/test_volume_metadata.py" module from this patch, V2's unit test cases are inherited for V3 and now we are applying schema validation for V3 because of that V2's unit test cases responses are modified. If we remove schema validation from V2 then we will need to add separate unit test cases for V3. Made changes to unit tests to pass body as keyword argument as wsgi calls action method [1] and passes body as keyword argument. [1] https://github.com/openstack/cinder/blob/master/cinder/api/openstack/wsgi.py#L997 Partial-Implements: bp json-schema-validation Change-Id: I2226c8533cbd1ddd274d8dd0b2d77708463896f4 --- cinder/api/schemas/volume_metadata.py | 49 ++++++++++ cinder/api/v2/volume_metadata.py | 8 +- cinder/api/v3/volume_metadata.py | 8 +- .../tests/unit/api/v2/test_volume_metadata.py | 95 ++++++++++--------- .../tests/unit/api/v3/test_volume_metadata.py | 15 +-- 5 files changed, 117 insertions(+), 58 deletions(-) create mode 100644 cinder/api/schemas/volume_metadata.py diff --git a/cinder/api/schemas/volume_metadata.py b/cinder/api/schemas/volume_metadata.py new file mode 100644 index 00000000000..13a36643e19 --- /dev/null +++ b/cinder/api/schemas/volume_metadata.py @@ -0,0 +1,49 @@ +# Copyright (C) 2017 NTT DATA +# 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. + +""" +Schema for V3 Volume metadata API. + +""" + +import copy + +from cinder.api.validation import parameter_types + +metadata_restricted_properties = copy.deepcopy(parameter_types.extra_specs) +metadata_restricted_properties.update({ + 'minProperties': 1, + 'maxProperties': 1 +}) + +create = { + 'type': 'object', + 'properties': { + 'type': 'object', + 'metadata': parameter_types.extra_specs, + }, + 'required': ['metadata'], + 'additionalProperties': False, +} + +update = { + 'type': 'object', + 'properties': { + 'type': 'object', + 'meta': metadata_restricted_properties, + }, + 'required': ['meta'], + 'additionalProperties': False, +} diff --git a/cinder/api/v2/volume_metadata.py b/cinder/api/v2/volume_metadata.py index 5799494f65b..ea5b885c559 100644 --- a/cinder/api/v2/volume_metadata.py +++ b/cinder/api/v2/volume_metadata.py @@ -18,6 +18,8 @@ import webob from cinder.api import common from cinder.api.openstack import wsgi +from cinder.api.schemas import volume_metadata as metadata +from cinder.api import validation from cinder import exception from cinder.i18n import _ from cinder import volume @@ -46,8 +48,8 @@ class Controller(wsgi.Controller): context = req.environ['cinder.context'] return {'metadata': self._get_metadata(context, volume_id)} + @validation.schema(metadata.create) def create(self, req, volume_id, body): - self.assert_valid_body(body, 'metadata') context = req.environ['cinder.context'] metadata = body['metadata'] @@ -56,8 +58,8 @@ class Controller(wsgi.Controller): use_create=True) return {'metadata': new_metadata} + @validation.schema(metadata.update) def update(self, req, volume_id, id, body): - self.assert_valid_body(body, 'meta') meta_item = body['meta'] if id not in meta_item: @@ -76,8 +78,8 @@ class Controller(wsgi.Controller): return {'meta': meta_item} + @validation.schema(metadata.create) def update_all(self, req, volume_id, body): - self.assert_valid_body(body, 'metadata') metadata = body['metadata'] context = req.environ['cinder.context'] diff --git a/cinder/api/v3/volume_metadata.py b/cinder/api/v3/volume_metadata.py index 27c2443d428..810afa72831 100644 --- a/cinder/api/v3/volume_metadata.py +++ b/cinder/api/v3/volume_metadata.py @@ -24,7 +24,9 @@ import webob from cinder.api import microversions as mv from cinder.api.openstack import wsgi +from cinder.api.schemas import volume_metadata as metadata from cinder.api.v2 import volume_metadata as volume_meta_v2 +from cinder.api import validation class Controller(volume_meta_v2.Controller): @@ -55,6 +57,7 @@ class Controller(volume_meta_v2.Controller): return metadata @wsgi.extends + @validation.schema(metadata.update) def update(self, req, volume_id, id, body): req_version = req.api_version_request if req_version.matches(mv.ETAGS): @@ -62,9 +65,10 @@ class Controller(volume_meta_v2.Controller): return webob.Response( status_int=http_client.PRECONDITION_FAILED) return super(Controller, self).update(req, volume_id, - id, body) + id, body=body) @wsgi.extends + @validation.schema(metadata.create) def update_all(self, req, volume_id, body): req_version = req.api_version_request if req_version.matches(mv.ETAGS): @@ -72,7 +76,7 @@ class Controller(volume_meta_v2.Controller): return webob.Response( status_int=http_client.PRECONDITION_FAILED) return super(Controller, self).update_all(req, volume_id, - body) + body=body) def create_resource(): diff --git a/cinder/tests/unit/api/v2/test_volume_metadata.py b/cinder/tests/unit/api/v2/test_volume_metadata.py index 2689022763a..4981c6b6f6b 100644 --- a/cinder/tests/unit/api/v2/test_volume_metadata.py +++ b/cinder/tests/unit/api/v2/test_volume_metadata.py @@ -268,7 +268,7 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.create(req, self.req_id, body) + res_dict = self.controller.create(req, self.req_id, body=body) self.assertEqual(body, res_dict) @mock.patch.object(db, 'volume_metadata_update') @@ -292,7 +292,7 @@ class VolumeMetaDataTest(test.TestCase): get_volume.return_value = fake_volume self.assertRaises(exception.InvalidVolume, self.controller.create, - req, self.req_id, body) + req, self.req_id, body=body) @mock.patch.object(db, 'volume_metadata_update') @mock.patch.object(db, 'volume_metadata_get') @@ -324,7 +324,7 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.create(req, self.req_id, body) + res_dict = self.controller.create(req, self.req_id, body=body) self.assertEqual(expected, res_dict) def test_create_empty_body(self): @@ -334,8 +334,8 @@ class VolumeMetaDataTest(test.TestCase): req.method = 'POST' req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, None) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, body=None) def test_create_metadata_keys_value_none(self): self.mock_object(db, 'volume_metadata_update', @@ -344,8 +344,8 @@ class VolumeMetaDataTest(test.TestCase): req.method = 'POST' req.headers["content-type"] = "application/json" body = {"meta": {"key": None}} - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, body=body) def test_create_item_empty_key(self): self.mock_object(db, 'volume_metadata_update', @@ -356,8 +356,8 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, body=body) def test_create_item_key_too_long(self): self.mock_object(db, 'volume_metadata_update', @@ -368,9 +368,9 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.create, - req, self.req_id, body) + req, self.req_id, body=body) def test_create_nonexistent_volume(self): self.mock_object(volume.api.API, 'get', @@ -386,7 +386,7 @@ class VolumeMetaDataTest(test.TestCase): body = {"metadata": {"key9": "value9"}} req.body = jsonutils.dump_as_bytes(body) self.assertRaises(exception.VolumeNotFound, - self.controller.create, req, self.req_id, body) + self.controller.create, req, self.req_id, body=body) @mock.patch.object(db, 'volume_metadata_update') def test_update_all(self, metadata_update): @@ -409,7 +409,8 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update_all(req, self.req_id, expected) + res_dict = self.controller.update_all(req, self.req_id, + body=expected) self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -436,7 +437,7 @@ class VolumeMetaDataTest(test.TestCase): get_volume.return_value = fake_volume self.assertRaises(exception.InvalidVolume, self.controller.update_all, req, - self.req_id, expected) + self.req_id, body=expected) self.assertFalse(metadata_update.called) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -473,7 +474,7 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update_all(req, self.req_id, body) + res_dict = self.controller.update_all(req, self.req_id, body=body) self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -492,7 +493,8 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update_all(req, self.req_id, expected) + res_dict = self.controller.update_all(req, self.req_id, + body=expected) self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -505,9 +507,9 @@ class VolumeMetaDataTest(test.TestCase): expected = {'meta': {}} req.body = jsonutils.dump_as_bytes(expected) - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update_all, req, self.req_id, - expected) + body=expected) def test_update_all_malformed_data(self): self.mock_object(db, 'volume_metadata_update', @@ -518,9 +520,9 @@ class VolumeMetaDataTest(test.TestCase): expected = {'metadata': ['asdf']} req.body = jsonutils.dump_as_bytes(expected) - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update_all, req, self.req_id, - expected) + body=expected) def test_update_all_nonexistent_volume(self): self.mock_object(db, 'volume_get', return_volume_nonexistent) @@ -531,7 +533,7 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) self.assertRaises(exception.VolumeNotFound, - self.controller.update_all, req, '100', body) + self.controller.update_all, req, '100', body=body) @mock.patch.object(db, 'volume_metadata_update') def test_update_item(self, metadata_update): @@ -548,7 +550,8 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update(req, self.req_id, 'key1', body) + res_dict = self.controller.update(req, self.req_id, 'key1', + body=body) expected = {'meta': {'key1': 'value1'}} self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -562,9 +565,9 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update, - req, self.req_id, 'key1', body) + req, self.req_id, 'key1', body=body) @mock.patch.object(db, 'volume_metadata_update') def test_update_item_volume_maintenance(self, metadata_update): @@ -583,7 +586,7 @@ class VolumeMetaDataTest(test.TestCase): get_volume.return_value = fake_volume self.assertRaises(exception.InvalidVolume, self.controller.update, req, - self.req_id, 'key1', body) + self.req_id, 'key1', body=body) self.assertFalse(metadata_update.called) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -599,7 +602,7 @@ class VolumeMetaDataTest(test.TestCase): self.assertRaises(exception.VolumeNotFound, self.controller.update, req, self.req_id, 'key1', - body) + body=body) def test_update_item_empty_body(self): self.mock_object(db, 'volume_metadata_update', @@ -608,9 +611,9 @@ class VolumeMetaDataTest(test.TestCase): req.method = 'PUT' req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update, req, self.req_id, 'key1', - None) + body=None) @mock.patch.object(db, 'volume_metadata_update') def test_update_item_empty_key(self, metadata_update): @@ -627,11 +630,10 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update, req, self.req_id, - '', body) + '', body=body) self.assertFalse(metadata_update.called) - get_volume.assert_called_once_with(fake_context, self.req_id) @mock.patch.object(db, 'volume_metadata_update') def test_update_item_key_too_long(self, metadata_update): @@ -648,11 +650,10 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, + self.assertRaises(exception.ValidationError, self.controller.update, - req, self.req_id, ("a" * 260), body) + req, self.req_id, ("a" * 260), body=body) self.assertFalse(metadata_update.called) - get_volume.assert_called_once_with(fake_context, self.req_id) @mock.patch.object(db, 'volume_metadata_update') def test_update_item_value_too_long(self, metadata_update): @@ -669,11 +670,10 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, + self.assertRaises(exception.ValidationError, self.controller.update, - req, self.req_id, "key1", body) + req, self.req_id, "key1", body=body) self.assertFalse(metadata_update.called) - get_volume.assert_called_once_with(fake_context, self.req_id) def test_update_item_too_many_keys(self): self.mock_object(db, 'volume_metadata_update', @@ -684,9 +684,9 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update, req, self.req_id, 'key1', - body) + body=body) def test_update_item_body_uri_mismatch(self): self.mock_object(db, 'volume_metadata_update', @@ -699,7 +699,7 @@ class VolumeMetaDataTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, self.req_id, 'bad', - body) + body=body) @mock.patch.object(db, 'volume_metadata_update') def test_invalid_metadata_items_on_create(self, metadata_update): @@ -718,8 +718,9 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, - self.controller.create, req, self.req_id, data) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, + body=data) # test for long value data = {"metadata": {"key": "v" * 260}} @@ -729,8 +730,9 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, - self.controller.create, req, self.req_id, data) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, + body=data) # test for empty key. data = {"metadata": {"": "value1"}} @@ -740,5 +742,6 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, data) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, + body=data) diff --git a/cinder/tests/unit/api/v3/test_volume_metadata.py b/cinder/tests/unit/api/v3/test_volume_metadata.py index 29235610a98..df7e8bb1b52 100644 --- a/cinder/tests/unit/api/v3/test_volume_metadata.py +++ b/cinder/tests/unit/api/v3/test_volume_metadata.py @@ -15,7 +15,6 @@ import uuid import mock from oslo_config import cfg from oslo_serialization import jsonutils -import webob from cinder.api import extensions from cinder.api import microversions as mv @@ -225,7 +224,8 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update_all(req, self.req_id, expected) + res_dict = self.controller.update_all(req, self.req_id, + body=expected) self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -244,7 +244,8 @@ class VolumeMetaDataTest(test.TestCase): with mock.patch.object(self.controller.volume_api, 'get') as get_volume: get_volume.return_value = fake_volume - res_dict = self.controller.update(req, self.req_id, 'key1', body) + res_dict = self.controller.update(req, self.req_id, 'key1', + body=body) expected = {'meta': {'key1': 'value1'}} self.assertEqual(expected, res_dict) get_volume.assert_called_once_with(fake_context, self.req_id) @@ -256,8 +257,8 @@ class VolumeMetaDataTest(test.TestCase): req.method = 'POST' req.headers["content-type"] = "application/json" body = {"meta": {"key": None}} - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, body=body) def test_update_items_value_none(self): self.mock_object(db, 'volume_metadata_update', @@ -268,8 +269,8 @@ class VolumeMetaDataTest(test.TestCase): req.body = jsonutils.dump_as_bytes(body) req.headers["content-type"] = "application/json" - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, self.req_id, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, self.req_id, body=body) class VolumeMetaDataTestNoMicroversion(v2_test.VolumeMetaDataTest):