V2/V3 jsonschema validation: snapshots
This patch adds jsonschema validation for below Snapshots API's
* POST /v3/{project_id}/snapshots
* PUT /v3/{project_id}/snapshots/{snapshot_id}
* POST /v2/{project_id}/snapshots
* PUT /v2/{project_id}/snapshots/{snapshot_id}
Note: For create API 'volume_id' parameter is required, if user
passes 'volume_id' as null, the existing behavior is it returns
"itemNotFound"(404) error response. If we restrict user to accept
volume_id in uuid format only in schema validation then in that case,
if user passes 'volume_id' as null it will raise BadRequest(400)
which will not match existing behavior and also tempest test case[1]
will fail in this case. Also on master if user passes 'metadata' as
null then it is accepted. To maintain consistency we are allowing
'volume_id' and 'metadata' to be null in schema.
Made changes to unit tests to pass body as keyword argument as wsgi
calls action method [2] and passes body as keyword argument.
[1] https://github.com/openstack/tempest/blob/master/tempest/api/volume/test_volumes_snapshots_negative.py#L42
[2] https://github.com/openstack/cinder/blob/master/cinder/api/openstack/wsgi.py#L997
Partial-Implements: bp json-schema-validation
Change-Id: I9869ea08afe3c8f8f1f63bb2ead7f63388580937
This commit is contained in:
Neha Alhat
parent
714139dd5c
commit
94460b64fb
68
cinder/api/schemas/snapshots.py
Normal file
68
cinder/api/schemas/snapshots.py
Normal file
@ -0,0 +1,68 @@
|
||||
# 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 Snapshots API.
|
||||
|
||||
"""
|
||||
|
||||
from cinder.api.validation import parameter_types
|
||||
|
||||
create = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'type': 'object',
|
||||
'snapshot': {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'name': parameter_types.name_allow_zero_min_length,
|
||||
'display_name': parameter_types.name_allow_zero_min_length,
|
||||
'description': parameter_types.description,
|
||||
'volume_id': parameter_types.uuid_allow_null,
|
||||
'force': parameter_types.boolean,
|
||||
'metadata': parameter_types.metadata_allows_null,
|
||||
},
|
||||
'required': ['volume_id'],
|
||||
'additionalProperties': False,
|
||||
},
|
||||
},
|
||||
'required': ['snapshot'],
|
||||
'additionalProperties': False,
|
||||
}
|
||||
|
||||
|
||||
update = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'type': 'object',
|
||||
'snapshot': {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'name': parameter_types.name_allow_zero_min_length,
|
||||
'description': parameter_types.description,
|
||||
'display_name': parameter_types.name_allow_zero_min_length,
|
||||
'display_description': parameter_types.description,
|
||||
},
|
||||
'additionalProperties': False,
|
||||
'anyOf': [
|
||||
{'required': ['name']},
|
||||
{'required': ['description']},
|
||||
{'required': ['display_name']},
|
||||
{'required': ['display_description']}
|
||||
]
|
||||
},
|
||||
},
|
||||
'required': ['snapshot'],
|
||||
'additionalProperties': False,
|
||||
}
|
||||
@ -16,17 +16,15 @@
|
||||
"""The volumes snapshots api."""
|
||||
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import encodeutils
|
||||
from oslo_utils import strutils
|
||||
from six.moves import http_client
|
||||
import webob
|
||||
from webob import exc
|
||||
|
||||
from cinder.api import common
|
||||
from cinder.api.openstack import wsgi
|
||||
from cinder.api.schemas import snapshots as snapshot
|
||||
from cinder.api import validation
|
||||
from cinder.api.views import snapshots as snapshot_views
|
||||
from cinder import exception
|
||||
from cinder.i18n import _
|
||||
from cinder import utils
|
||||
from cinder import volume
|
||||
from cinder.volume import utils as volume_utils
|
||||
@ -110,38 +108,23 @@ class SnapshotsController(wsgi.Controller):
|
||||
return snapshots
|
||||
|
||||
@wsgi.response(http_client.ACCEPTED)
|
||||
@validation.schema(snapshot.create)
|
||||
def create(self, req, body):
|
||||
"""Creates a new snapshot."""
|
||||
kwargs = {}
|
||||
context = req.environ['cinder.context']
|
||||
|
||||
self.assert_valid_body(body, 'snapshot')
|
||||
|
||||
snapshot = body['snapshot']
|
||||
kwargs['metadata'] = snapshot.get('metadata', None)
|
||||
|
||||
try:
|
||||
volume_id = snapshot['volume_id']
|
||||
except KeyError:
|
||||
msg = _("'volume_id' must be specified")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
volume_id = snapshot['volume_id']
|
||||
volume = self.volume_api.get(context, volume_id)
|
||||
force = snapshot.get('force', False)
|
||||
force = strutils.bool_from_string(force, strict=True)
|
||||
LOG.info("Create snapshot from volume %s", volume_id)
|
||||
self.validate_name_and_description(snapshot)
|
||||
|
||||
# NOTE(thingee): v2 API allows name instead of display_name
|
||||
if 'name' in snapshot:
|
||||
snapshot['display_name'] = snapshot.pop('name')
|
||||
|
||||
try:
|
||||
force = strutils.bool_from_string(force, strict=True)
|
||||
except ValueError as error:
|
||||
err_msg = encodeutils.exception_to_unicode(error)
|
||||
msg = _("Invalid value for 'force': '%s'") % err_msg
|
||||
raise exception.InvalidParameterValue(err=msg)
|
||||
|
||||
if force:
|
||||
new_snapshot = self.volume_api.create_snapshot_force(
|
||||
context,
|
||||
@ -160,50 +143,26 @@ class SnapshotsController(wsgi.Controller):
|
||||
|
||||
return self._view_builder.detail(req, new_snapshot)
|
||||
|
||||
@validation.schema(snapshot.update)
|
||||
def update(self, req, id, body):
|
||||
"""Update a snapshot."""
|
||||
context = req.environ['cinder.context']
|
||||
snapshot_body = body['snapshot']
|
||||
|
||||
if not body:
|
||||
msg = _("Missing request body")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
if 'name' in snapshot_body:
|
||||
snapshot_body['display_name'] = snapshot_body.pop('name')
|
||||
|
||||
if 'snapshot' not in body:
|
||||
msg = (_("Missing required element '%s' in request body") %
|
||||
'snapshot')
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
snapshot = body['snapshot']
|
||||
update_dict = {}
|
||||
|
||||
valid_update_keys = (
|
||||
'name',
|
||||
'description',
|
||||
'display_name',
|
||||
'display_description',
|
||||
)
|
||||
self.validate_name_and_description(snapshot)
|
||||
|
||||
# NOTE(thingee): v2 API allows name instead of display_name
|
||||
if 'name' in snapshot:
|
||||
snapshot['display_name'] = snapshot.pop('name')
|
||||
|
||||
# NOTE(thingee): v2 API allows description instead of
|
||||
# display_description
|
||||
if 'description' in snapshot:
|
||||
snapshot['display_description'] = snapshot.pop('description')
|
||||
|
||||
for key in valid_update_keys:
|
||||
if key in snapshot:
|
||||
update_dict[key] = snapshot[key]
|
||||
if 'description' in snapshot_body:
|
||||
snapshot_body['display_description'] = snapshot_body.pop(
|
||||
'description')
|
||||
|
||||
# Not found exception will be handled at the wsgi level
|
||||
snapshot = self.volume_api.get_snapshot(context, id)
|
||||
volume_utils.notify_about_snapshot_usage(context, snapshot,
|
||||
'update.start')
|
||||
self.volume_api.update_snapshot(context, snapshot, update_dict)
|
||||
self.volume_api.update_snapshot(context, snapshot, snapshot_body)
|
||||
|
||||
snapshot.update(update_dict)
|
||||
snapshot.update(snapshot_body)
|
||||
req.cache_db_snapshot(snapshot)
|
||||
volume_utils.notify_about_snapshot_usage(context, snapshot,
|
||||
'update.end')
|
||||
|
||||
@ -161,3 +161,17 @@ group_snapshot_status = {
|
||||
extra_specs_with_null = copy.deepcopy(extra_specs)
|
||||
extra_specs_with_null['patternProperties'][
|
||||
'^[a-zA-Z0-9-_:. ]{1,255}$']['type'] = ['string', 'null']
|
||||
|
||||
|
||||
name_allow_zero_min_length = {
|
||||
'type': 'string', 'minLength': 0, 'maxLength': 255
|
||||
}
|
||||
|
||||
|
||||
uuid_allow_null = {
|
||||
'type': ['string', 'null']
|
||||
}
|
||||
|
||||
|
||||
metadata_allows_null = copy.deepcopy(extra_specs)
|
||||
metadata_allows_null['type'] = ['object', 'null']
|
||||
|
||||
@ -127,16 +127,13 @@ class SnapshotMetaDataTest(test.TestCase):
|
||||
self.url = '/v2/%s/snapshots/%s/metadata' % (
|
||||
fake.PROJECT_ID, self.req_id)
|
||||
|
||||
snap = {"volume_size": 100,
|
||||
"volume_id": fake.VOLUME_ID,
|
||||
snap = {"volume_id": fake.VOLUME_ID,
|
||||
"display_name": "Volume Test Name",
|
||||
"display_description": "Volume Test Desc",
|
||||
"availability_zone": "zone1:host1",
|
||||
"host": "fake-host",
|
||||
"description": "Volume Test Desc",
|
||||
"metadata": {}}
|
||||
body = {"snapshot": snap}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
self.snapshot_controller.create(req, body)
|
||||
self.snapshot_controller.create(req, body=body)
|
||||
|
||||
@mock.patch('cinder.objects.Snapshot.get_by_id')
|
||||
def test_index(self, snapshot_get_by_id):
|
||||
|
||||
@ -89,9 +89,7 @@ class SnapshotApiTest(test.TestCase):
|
||||
self.controller = snapshots.SnapshotsController()
|
||||
self.ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
|
||||
|
||||
@mock.patch(
|
||||
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
|
||||
def test_snapshot_create(self, mock_validate):
|
||||
def test_snapshot_create(self):
|
||||
volume = utils.create_volume(self.ctx)
|
||||
snapshot_name = 'Snapshot Test Name'
|
||||
snapshot_description = 'Snapshot Test Desc'
|
||||
@ -104,17 +102,16 @@ class SnapshotApiTest(test.TestCase):
|
||||
|
||||
body = dict(snapshot=snapshot)
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
resp_dict = self.controller.create(req, body)
|
||||
resp_dict = self.controller.create(req, body=body)
|
||||
|
||||
self.assertIn('snapshot', resp_dict)
|
||||
self.assertEqual(snapshot_name, resp_dict['snapshot']['name'])
|
||||
self.assertEqual(snapshot_description,
|
||||
resp_dict['snapshot']['description'])
|
||||
self.assertTrue(mock_validate.called)
|
||||
self.assertIn('updated_at', resp_dict['snapshot'])
|
||||
db.volume_destroy(self.ctx, volume.id)
|
||||
|
||||
@ddt.data(True, 'y', 'true', 'trUE', 'yes', '1', 'on', 1, "1 ")
|
||||
@ddt.data(True, 'y', 'true', 'yes', '1', 'on')
|
||||
def test_snapshot_create_force(self, force_param):
|
||||
volume = utils.create_volume(self.ctx, status='in-use')
|
||||
snapshot_name = 'Snapshot Test Name'
|
||||
@ -127,7 +124,7 @@ class SnapshotApiTest(test.TestCase):
|
||||
}
|
||||
body = dict(snapshot=snapshot)
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
resp_dict = self.controller.create(req, body)
|
||||
resp_dict = self.controller.create(req, body=body)
|
||||
|
||||
self.assertIn('snapshot', resp_dict)
|
||||
self.assertEqual(snapshot_name,
|
||||
@ -138,7 +135,7 @@ class SnapshotApiTest(test.TestCase):
|
||||
|
||||
db.volume_destroy(self.ctx, volume.id)
|
||||
|
||||
@ddt.data(False, 'n', 'false', 'falSE', 'No', '0', 'off', 0)
|
||||
@ddt.data(False, 'n', 'false', 'No', '0', 'off')
|
||||
def test_snapshot_create_force_failure(self, force_param):
|
||||
volume = utils.create_volume(self.ctx, status='in-use')
|
||||
snapshot_name = 'Snapshot Test Name'
|
||||
@ -154,13 +151,14 @@ class SnapshotApiTest(test.TestCase):
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
self.controller.create,
|
||||
req,
|
||||
body)
|
||||
body=body)
|
||||
|
||||
db.volume_destroy(self.ctx, volume.id)
|
||||
|
||||
@ddt.data("**&&^^%%$$##@@", '-1', 2, '01')
|
||||
@ddt.data("**&&^^%%$$##@@", '-1', 2, '01', 'falSE', 0, 'trUE', 1,
|
||||
"1 ")
|
||||
def test_snapshot_create_invalid_force_param(self, force_param):
|
||||
volume = utils.create_volume(self.ctx, status='in-use')
|
||||
volume = utils.create_volume(self.ctx, status='available')
|
||||
snapshot_name = 'Snapshot Test Name'
|
||||
snapshot_description = 'Snapshot Test Desc'
|
||||
|
||||
@ -172,10 +170,10 @@ class SnapshotApiTest(test.TestCase):
|
||||
}
|
||||
body = dict(snapshot=snapshot)
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
self.assertRaises(exception.InvalidParameterValue,
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.create,
|
||||
req,
|
||||
body)
|
||||
body=body)
|
||||
|
||||
db.volume_destroy(self.ctx, volume.id)
|
||||
|
||||
@ -190,18 +188,16 @@ class SnapshotApiTest(test.TestCase):
|
||||
}
|
||||
}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.create, req, body)
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.create, req, body=body)
|
||||
|
||||
@mock.patch.object(volume.api.API, "update_snapshot",
|
||||
side_effect=v2_fakes.fake_snapshot_update)
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
@mock.patch('cinder.db.volume_get')
|
||||
@mock.patch('cinder.objects.Snapshot.get_by_id')
|
||||
@mock.patch(
|
||||
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
|
||||
def test_snapshot_update(
|
||||
self, mock_validate, snapshot_get_by_id, volume_get,
|
||||
self, snapshot_get_by_id, volume_get,
|
||||
snapshot_metadata_get, update_snapshot):
|
||||
snapshot = {
|
||||
'id': UUID,
|
||||
@ -224,7 +220,7 @@ class SnapshotApiTest(test.TestCase):
|
||||
}
|
||||
body = {"snapshot": updates}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID)
|
||||
res_dict = self.controller.update(req, UUID, body)
|
||||
res_dict = self.controller.update(req, UUID, body=body)
|
||||
expected = {
|
||||
'snapshot': {
|
||||
'id': UUID,
|
||||
@ -240,20 +236,19 @@ class SnapshotApiTest(test.TestCase):
|
||||
}
|
||||
}
|
||||
self.assertEqual(expected, res_dict)
|
||||
self.assertTrue(mock_validate.called)
|
||||
self.assertEqual(2, len(self.notifier.notifications))
|
||||
|
||||
def test_snapshot_update_missing_body(self):
|
||||
body = {}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.update, req, UUID, body)
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.update, req, UUID, body=body)
|
||||
|
||||
def test_snapshot_update_invalid_body(self):
|
||||
body = {'name': 'missing top level snapshot key'}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots/%s' % UUID)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.update, req, UUID, body)
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.update, req, UUID, body=body)
|
||||
|
||||
def test_snapshot_update_not_found(self):
|
||||
self.mock_object(volume.api.API, "get_snapshot", fake_snapshot_get)
|
||||
@ -263,7 +258,7 @@ class SnapshotApiTest(test.TestCase):
|
||||
body = {"snapshot": updates}
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots/not-the-uuid')
|
||||
self.assertRaises(exception.SnapshotNotFound, self.controller.update,
|
||||
req, 'not-the-uuid', body)
|
||||
req, 'not-the-uuid', body=body)
|
||||
|
||||
@mock.patch.object(volume.api.API, "delete_snapshot",
|
||||
side_effect=v2_fakes.fake_snapshot_update)
|
||||
@ -612,8 +607,8 @@ class SnapshotApiTest(test.TestCase):
|
||||
req = fakes.HTTPRequest.blank('/v2/%s/snapshots' % fake.PROJECT_ID)
|
||||
req.method = 'POST'
|
||||
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.create, req, body)
|
||||
self.assertRaises(exception.ValidationError,
|
||||
self.controller.create, req, body=body)
|
||||
|
||||
def test_create_no_body(self):
|
||||
self._create_snapshot_bad_body(body=None)
|
||||
|
||||
@ -118,16 +118,14 @@ class SnapshotApiTest(test.TestCase):
|
||||
def _create_snapshot(self, name=None, metadata=None):
|
||||
"""Creates test snapshopt with provided metadata"""
|
||||
req = fakes.HTTPRequest.blank('/v3/snapshots')
|
||||
snap = {"volume_size": 200,
|
||||
"volume_id": fake.VOLUME_ID,
|
||||
snap = {"volume_id": fake.VOLUME_ID,
|
||||
"display_name": name or "Volume Test Name",
|
||||
"display_description": "Volume Test Desc",
|
||||
"availability_zone": "zone1:host1",
|
||||
"host": "fake-host"}
|
||||
"description": "Volume Test Desc"
|
||||
}
|
||||
if metadata:
|
||||
snap["metadata"] = metadata
|
||||
body = {"snapshot": snap}
|
||||
self.controller.create(req, body)
|
||||
self.controller.create(req, body=body)
|
||||
|
||||
@ddt.data(('host', 'test_host1', True), ('cluster_name', 'cluster1', True),
|
||||
('availability_zone', 'nova1', False))
|
||||
|
||||
Loading…
Reference in New Issue
Block a user