diff --git a/cinder/api/schemas/snapshots.py b/cinder/api/schemas/snapshots.py new file mode 100644 index 00000000000..bcf379d6ea1 --- /dev/null +++ b/cinder/api/schemas/snapshots.py @@ -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, +} diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index 903096a8478..084cc728658 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -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') diff --git a/cinder/api/validation/parameter_types.py b/cinder/api/validation/parameter_types.py index b05e8ae32db..befb107a9f0 100644 --- a/cinder/api/validation/parameter_types.py +++ b/cinder/api/validation/parameter_types.py @@ -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'] diff --git a/cinder/tests/unit/api/v2/test_snapshot_metadata.py b/cinder/tests/unit/api/v2/test_snapshot_metadata.py index 7b097f0c462..efaacdaf145 100644 --- a/cinder/tests/unit/api/v2/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v2/test_snapshot_metadata.py @@ -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): diff --git a/cinder/tests/unit/api/v2/test_snapshots.py b/cinder/tests/unit/api/v2/test_snapshots.py index 539ec664714..cb7f345b65b 100644 --- a/cinder/tests/unit/api/v2/test_snapshots.py +++ b/cinder/tests/unit/api/v2/test_snapshots.py @@ -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) diff --git a/cinder/tests/unit/api/v3/test_snapshots.py b/cinder/tests/unit/api/v3/test_snapshots.py index 948a13f19bc..7c799bc0ae3 100644 --- a/cinder/tests/unit/api/v3/test_snapshots.py +++ b/cinder/tests/unit/api/v3/test_snapshots.py @@ -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))