diff --git a/cinder/api/contrib/snapshot_manage.py b/cinder/api/contrib/snapshot_manage.py index f374655ee00..858b4704aca 100644 --- a/cinder/api/contrib/snapshot_manage.py +++ b/cinder/api/contrib/snapshot_manage.py @@ -14,14 +14,14 @@ from oslo_log import log as logging from six.moves import http_client -from webob import exc from cinder.api.contrib import resource_common_manage from cinder.api import extensions from cinder.api.openstack import wsgi +from cinder.api.schemas import snapshot_manage +from cinder.api import validation from cinder.api.views import manageable_snapshots as list_manageable_view from cinder.api.views import snapshots as snapshot_views -from cinder.i18n import _ from cinder.policies import manageable_snapshots as policy from cinder import volume as cinder_volume @@ -39,6 +39,7 @@ class SnapshotManageController(wsgi.Controller): self._list_manageable_view = list_manageable_view.ViewBuilder() @wsgi.response(http_client.ACCEPTED) + @validation.schema(snapshot_manage.create) def create(self, req, body): """Instruct Cinder to manage a storage snapshot object. @@ -85,20 +86,7 @@ class SnapshotManageController(wsgi.Controller): context = req.environ['cinder.context'] context.authorize(policy.MANAGE_POLICY) - self.assert_valid_body(body, 'snapshot') - snapshot = body['snapshot'] - - # Check that the required keys are present, return an error if they - # are not. - required_keys = ('ref', 'volume_id') - missing_keys = set(required_keys) - set(snapshot.keys()) - - if missing_keys: - msg = _("The following elements are required: " - "%s") % ', '.join(missing_keys) - raise exc.HTTPBadRequest(explanation=msg) - # Check whether volume exists volume_id = snapshot['volume_id'] # Not found exception will be handled at the wsgi level diff --git a/cinder/api/schemas/snapshot_manage.py b/cinder/api/schemas/snapshot_manage.py new file mode 100644 index 00000000000..562e6bcbcd1 --- /dev/null +++ b/cinder/api/schemas/snapshot_manage.py @@ -0,0 +1,42 @@ +# 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 snapshot_manage API. + +""" + +from cinder.api.validation import parameter_types + + +create = { + 'type': 'object', + 'properties': { + 'type': 'object', + 'snapshot': { + 'type': 'object', + 'properties': { + "description": parameter_types.description, + "metadata": parameter_types.extra_specs, + "name": parameter_types.name_allow_zero_min_length, + "volume_id": parameter_types.uuid, + "ref": {'type': ['object', 'null']}, + }, + 'required': ['ref', 'volume_id'], + 'additionalProperties': False, + }, + }, + 'required': ['snapshot'], + 'additionalProperties': False, +} diff --git a/cinder/api/v3/snapshot_manage.py b/cinder/api/v3/snapshot_manage.py index baeef54bd2a..a4aca611588 100644 --- a/cinder/api/v3/snapshot_manage.py +++ b/cinder/api/v3/snapshot_manage.py @@ -29,7 +29,7 @@ class SnapshotManageController(common.ManageResource, @wsgi.response(http_client.ACCEPTED) def create(self, req, body): self._ensure_min_version(req, mv.MANAGE_EXISTING_LIST) - return super(SnapshotManageController, self).create(req, body) + return super(SnapshotManageController, self).create(req, body=body) def create_resource(): diff --git a/cinder/tests/unit/api/contrib/test_snapshot_manage.py b/cinder/tests/unit/api/contrib/test_snapshot_manage.py index 2d35163c0cd..fda02e333cd 100644 --- a/cinder/tests/unit/api/contrib/test_snapshot_manage.py +++ b/cinder/tests/unit/api/contrib/test_snapshot_manage.py @@ -122,7 +122,10 @@ class SnapshotManageTest(test.TestCase): mock_db.return_value = fake_service.fake_service_obj( self._admin_ctxt, binary=constants.VOLUME_BINARY) - body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}} + + body = {'snapshot': {'volume_id': fake.VOLUME_ID, + 'ref': {'fake_key': 'fake_ref'}}} + res = self._get_resp_post(body) self.assertEqual(http_client.ACCEPTED, res.status_int, res) @@ -142,7 +145,7 @@ class SnapshotManageTest(test.TestCase): # correct arguments. self.assertEqual(1, mock_rpcapi.call_count) args = mock_rpcapi.call_args[0] - self.assertEqual('fake_ref', args[2]) + self.assertEqual({u'fake_key': u'fake_ref'}, args[2]) @mock.patch('cinder.objects.service.Service.is_up', return_value=True, @@ -155,7 +158,8 @@ class SnapshotManageTest(test.TestCase): """Test manage snapshot failure due to disabled service.""" mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt, disabled=True) - body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}} + body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': { + 'fake_key': 'fake_ref'}}} res = self._get_resp_post(body) self.assertEqual(http_client.BAD_REQUEST, res.status_int, res) self.assertEqual(exception.ServiceUnavailable.message, @@ -173,7 +177,8 @@ class SnapshotManageTest(test.TestCase): mock_rpcapi, mock_is_up): """Test manage snapshot failure due to down service.""" mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt) - body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}} + body = {'snapshot': {'volume_id': fake.VOLUME_ID, + 'ref': {'fake_key': 'fake_ref'}}} res = self._get_resp_post(body) self.assertEqual(http_client.BAD_REQUEST, res.status_int, res) self.assertEqual(exception.ServiceUnavailable.message, @@ -202,10 +207,9 @@ class SnapshotManageTest(test.TestCase): def test_manage_snapshot_error_volume_id(self): """Test correct failure when volume can't be found.""" - body = {'snapshot': {'volume_id': 'error_volume_id', - 'ref': 'fake_ref'}} + body = {'snapshot': {'volume_id': 'error_volume_id', 'ref': {}}} res = self._get_resp_post(body) - self.assertEqual(http_client.NOT_FOUND, res.status_int) + self.assertEqual(http_client.BAD_REQUEST, res.status_int) def _get_resp_get(self, host, detailed, paging, admin=True): """Helper to execute a GET os-snapshot-manage API call.""" diff --git a/cinder/tests/unit/api/v3/test_snapshot_manage.py b/cinder/tests/unit/api/v3/test_snapshot_manage.py index 1e012a085de..81f8fe1d735 100644 --- a/cinder/tests/unit/api/v3/test_snapshot_manage.py +++ b/cinder/tests/unit/api/v3/test_snapshot_manage.py @@ -80,7 +80,8 @@ class SnapshotManageTest(test.TestCase): self._admin_ctxt, binary=constants.VOLUME_BINARY) - body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}} + body = {'snapshot': {'volume_id': fake.VOLUME_ID, + 'ref': {'fake_ref': "fake_val"}}} res = self._get_resp_post(body) self.assertEqual(http_client.ACCEPTED, res.status_int, res)