openstack/cinder
Code Issues Proposed changes

V3 jsonschema validation: Volume metadata

Browse Source 
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
This commit is contained in:
pooja jadhav 2017-11-22 19:31:34 +05:30
parent 4f649809a4
commit afa410bcf6
5 changed files with 117 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
cinder/api/schemas/volume_metadata.py Normal file
View File

@ -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,
}

8
cinder/api/v2/volume_metadata.py
View File

@ -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']

8
cinder/api/v3/volume_metadata.py
View File

@ -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():

95
cinder/tests/unit/api/v2/test_volume_metadata.py
View File

@ -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)

15
cinder/tests/unit/api/v3/test_volume_metadata.py
View File

@ -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):