From 1201515997002c3472f0cc74a0a92736414f3633 Mon Sep 17 00:00:00 2001 From: Neha Alhat Date: Wed, 22 Nov 2017 10:14:15 +0530 Subject: [PATCH] V3 json schema validation: volume manage This patch adds jsonschema validation for below volume_manage API * POST /v3/{project_id}/manageable_volumes Change-Id: I516fc5ef4b174bda54da306a2135aa4eb30e5ea1 Partial-Implements: bp json-schema-validation --- cinder/api/contrib/volume_manage.py | 26 +++++----- cinder/api/schemas/volume_manage.py | 51 +++++++++++++++++++ cinder/api/v3/volume_manage.py | 2 +- .../unit/api/contrib/test_volume_manage.py | 37 +++++--------- 4 files changed, 76 insertions(+), 40 deletions(-) create mode 100644 cinder/api/schemas/volume_manage.py diff --git a/cinder/api/contrib/volume_manage.py b/cinder/api/contrib/volume_manage.py index dfc2f915a31..52367a25be2 100644 --- a/cinder/api/contrib/volume_manage.py +++ b/cinder/api/contrib/volume_manage.py @@ -13,6 +13,7 @@ # under the License. from oslo_log import log as logging +from oslo_utils import strutils from six.moves import http_client from cinder.api import common @@ -20,7 +21,9 @@ from cinder.api.contrib import resource_common_manage from cinder.api import extensions from cinder.api import microversions as mv from cinder.api.openstack import wsgi +from cinder.api.schemas import volume_manage from cinder.api.v2.views import volumes as volume_views +from cinder.api import validation from cinder.api.views import manageable_volumes as list_manageable_view from cinder import exception from cinder.i18n import _ @@ -43,6 +46,8 @@ class VolumeManageController(wsgi.Controller): self._list_manageable_view = list_manageable_view.ViewBuilder() @wsgi.response(http_client.ACCEPTED) + @validation.schema(volume_manage.volume_manage_create, '2.0', '3.15') + @validation.schema(volume_manage.volume_manage_create_v316, '3.16') def create(self, req, body): """Instruct Cinder to manage a storage object. @@ -98,16 +103,7 @@ class VolumeManageController(wsgi.Controller): """ context = req.environ['cinder.context'] context.authorize(policy.MANAGE_POLICY) - - self.assert_valid_body(body, 'volume') - volume = body['volume'] - self.validate_name_and_description(volume) - - # Check that the required keys are present, return an error if they - # are not. - if 'ref' not in volume: - raise exception.MissingRequired(element='ref') cluster_name, host = common.get_cluster_host( req, volume, mv.VOLUME_MIGRATE_CLUSTER) @@ -127,13 +123,15 @@ class VolumeManageController(wsgi.Controller): else: kwargs['volume_type'] = {} - kwargs['name'] = volume.get('name', None) - kwargs['description'] = volume.get('description', None) + if volume.get('name'): + kwargs['name'] = volume.get('name').strip() + if volume.get('description'): + kwargs['description'] = volume.get('description').strip() + kwargs['metadata'] = volume.get('metadata', None) kwargs['availability_zone'] = volume.get('availability_zone', None) - kwargs['bootable'] = utils.get_bool_param('bootable', volume) - - utils.check_metadata_properties(kwargs['metadata']) + bootable = volume.get('bootable', False) + kwargs['bootable'] = strutils.bool_from_string(bootable, strict=True) try: new_volume = self.volume_api.manage_existing(context, diff --git a/cinder/api/schemas/volume_manage.py b/cinder/api/schemas/volume_manage.py new file mode 100644 index 00000000000..544214c4944 --- /dev/null +++ b/cinder/api/schemas/volume_manage.py @@ -0,0 +1,51 @@ +# Copyright (C) 2018 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 manage API. + +""" +import copy + + +from cinder.api.validation import parameter_types + +volume_manage_create = { + 'type': 'object', + 'properties': { + 'volume': { + 'type': 'object', + 'properties': { + "description": parameter_types.description, + "availability_zone": parameter_types. + name_allow_zero_min_length, + "bootable": parameter_types.boolean, + "volume_type": parameter_types.name_allow_zero_min_length, + "name": parameter_types.name_allow_zero_min_length, + "host": parameter_types.hostname, + "ref": {'type': ['object', 'string']}, + "metadata": parameter_types.metadata_allows_null, + }, + 'required': ['ref'], + 'additionalProperties': False, + }, + }, + 'required': ['volume'], + 'additionalProperties': False, +} + +volume_manage_create_v316 = copy.deepcopy(volume_manage_create) +volume_manage_create_v316['properties']['volume']['properties'][ + 'cluster'] = {'type': ['string', 'null'], + 'minLength': 0, 'maxLength': 255} diff --git a/cinder/api/v3/volume_manage.py b/cinder/api/v3/volume_manage.py index a09ddd5dd34..aa07a47f843 100644 --- a/cinder/api/v3/volume_manage.py +++ b/cinder/api/v3/volume_manage.py @@ -29,7 +29,7 @@ class VolumeManageController(common.ManageResource, @wsgi.response(http_client.ACCEPTED) def create(self, req, body): self._ensure_min_version(req, mv.MANAGE_EXISTING_LIST) - return super(VolumeManageController, self).create(req, body) + return super(VolumeManageController, self).create(req, body=body) def create_resource(): diff --git a/cinder/tests/unit/api/contrib/test_volume_manage.py b/cinder/tests/unit/api/contrib/test_volume_manage.py index 8588766e0e0..76f515d300e 100644 --- a/cinder/tests/unit/api/contrib/test_volume_manage.py +++ b/cinder/tests/unit/api/contrib/test_volume_manage.py @@ -201,11 +201,8 @@ class VolumeManageTest(test.TestCase): res = req.get_response(app_v3()) return res - @ddt.data(False, True) @mock.patch('cinder.volume.api.API.manage_existing', wraps=api_manage) - @mock.patch( - 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') - def test_manage_volume_ok(self, cluster, mock_validate, mock_api_manage): + def test_manage_volume_ok(self, mock_api_manage): """Test successful manage volume execution. Tests for correct operation when valid arguments are passed in the @@ -215,9 +212,7 @@ class VolumeManageTest(test.TestCase): """ body = {'volume': {'host': 'host_ok', 'ref': 'fake_ref'}} - # This will be ignored - if cluster: - body['volume']['cluster'] = 'cluster' + res = self._get_resp_post(body) self.assertEqual(http_client.ACCEPTED, res.status_int) @@ -225,9 +220,7 @@ class VolumeManageTest(test.TestCase): self.assertEqual(1, mock_api_manage.call_count) args = mock_api_manage.call_args[0] self.assertEqual(body['volume']['host'], args[1]) - self.assertIsNone(args[2]) # Cluster argument self.assertEqual(body['volume']['ref'], args[3]) - self.assertTrue(mock_validate.called) def _get_resp_create(self, body, version=mv.BASE_VERSION): url = '/v3/%s/os-volume-manage' % fake.PROJECT_ID @@ -238,13 +231,11 @@ class VolumeManageTest(test.TestCase): req.environ['cinder.context'] = self._admin_ctxt req.body = jsonutils.dump_as_bytes(body) req.api_version_request = mv.get_api_version(version) - res = self.controller.create(req, body) + res = self.controller.create(req, body=body) return res @mock.patch('cinder.volume.api.API.manage_existing', wraps=api_manage) - @mock.patch( - 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') - def test_manage_volume_ok_cluster(self, mock_validate, mock_api_manage): + def test_manage_volume_ok_cluster(self, mock_api_manage): body = {'volume': {'cluster': 'cluster', 'ref': 'fake_ref'}} res = self._get_resp_create(body, mv.VOLUME_MIGRATE_CLUSTER) @@ -256,11 +247,8 @@ class VolumeManageTest(test.TestCase): self.assertIsNone(args[1]) self.assertEqual(body['volume']['cluster'], args[2]) self.assertEqual(body['volume']['ref'], args[3]) - self.assertTrue(mock_validate.called) - @mock.patch( - 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') - def test_manage_volume_fail_host_cluster(self, mock_validate): + def test_manage_volume_fail_host_cluster(self): body = {'volume': {'host': 'host_ok', 'cluster': 'cluster', 'ref': 'fake_ref'}} @@ -322,9 +310,7 @@ class VolumeManageTest(test.TestCase): self.assertTrue(mock_is_up.called) @mock.patch('cinder.volume.api.API.manage_existing', api_manage) - @mock.patch( - 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') - def test_manage_volume_volume_type_by_uuid(self, mock_validate): + def test_manage_volume_volume_type_by_uuid(self): """Tests for correct operation when a volume type is specified by ID. We wrap cinder.volume.api.API.manage_existing so that managing is not @@ -336,12 +322,9 @@ class VolumeManageTest(test.TestCase): 'bootable': True}} res = self._get_resp_post(body) self.assertEqual(http_client.ACCEPTED, res.status_int) - self.assertTrue(mock_validate.called) @mock.patch('cinder.volume.api.API.manage_existing', api_manage) - @mock.patch( - 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') - def test_manage_volume_volume_type_by_name(self, mock_validate): + def test_manage_volume_volume_type_by_name(self): """Tests for correct operation when a volume type is specified by name. We wrap cinder.volume.api.API.manage_existing so that managing is not @@ -352,7 +335,6 @@ class VolumeManageTest(test.TestCase): 'volume_type': 'good_fakevt'}} res = self._get_resp_post(body) self.assertEqual(http_client.ACCEPTED, res.status_int) - self.assertTrue(mock_validate.called) def test_manage_volume_bad_volume_type_by_uuid(self): """Test failure on nonexistent volume type specified by ID.""" @@ -506,3 +488,8 @@ class VolumeManageTest(test.TestCase): self.assertEqual(1, mock_api_manage.call_count) self.assertEqual('creating', jsonutils.loads(res.body)['volume']['status']) + + @ddt.data({'volume': {}}, None) + def test_manage_volume_with_invalid_body(self, body): + res = self._get_resp_post(body) + self.assertEqual(http_client.BAD_REQUEST, res.status_int)