From 74ef703c34c3f19010fb1d6e9b42dd41c5cca7dd Mon Sep 17 00:00:00 2001 From: pooja jadhav Date: Thu, 12 Oct 2017 15:56:31 +0530 Subject: [PATCH] V3 jsonschema validation: qos-specs This patch adds jsonschema validation for below qos-specs API's * PUT /v3/{project_id}/qos-specs/{qos_id}/delete_keys * PUT /v3/{project_id}/qos-specs/{qos_id} * POST /v3/{project_id}/qos-specs Change-Id: I95756e549cc1b3bfbad16d029b7021527278b0d5 Partial-Implements: bp json-schema-validation --- cinder/api/contrib/qos_specs_manage.py | 19 ++---- cinder/api/schemas/qos_specs.py | 63 +++++++++++++++++ cinder/api/validation/validators.py | 16 +++++ .../unit/api/contrib/test_qos_specs_manage.py | 67 +++++++++++-------- 4 files changed, 125 insertions(+), 40 deletions(-) create mode 100644 cinder/api/schemas/qos_specs.py diff --git a/cinder/api/contrib/qos_specs_manage.py b/cinder/api/contrib/qos_specs_manage.py index 828112292dd..fddf0981b26 100644 --- a/cinder/api/contrib/qos_specs_manage.py +++ b/cinder/api/contrib/qos_specs_manage.py @@ -24,6 +24,8 @@ import webob from cinder.api import common from cinder.api import extensions from cinder.api.openstack import wsgi +from cinder.api.schemas import qos_specs as qos_specs_schema +from cinder.api import validation from cinder.api.views import qos_specs as view_qos_specs from cinder import exception from cinder.i18n import _ @@ -73,20 +75,13 @@ class QoSSpecsController(wsgi.Controller): sort_dirs=sort_dirs) return self._view_builder.summary_list(req, specs) + @validation.schema(qos_specs_schema.create) def create(self, req, body=None): context = req.environ['cinder.context'] context.authorize(policy.CREATE_POLICY) - self.assert_valid_body(body, 'qos_specs') - specs = body['qos_specs'] name = specs.pop('name', None) - if name is None: - msg = _("Please specify a name for QoS specs.") - raise webob.exc.HTTPBadRequest(explanation=msg) - - self.validate_string_length(name, 'name', min_length=1, - max_length=255, remove_whitespaces=True) name = name.strip() try: @@ -119,10 +114,11 @@ class QoSSpecsController(wsgi.Controller): return self._view_builder.detail(req, spec) + @validation.schema(qos_specs_schema.set) def update(self, req, id, body=None): context = req.environ['cinder.context'] context.authorize(policy.UPDATE_POLICY) - self.assert_valid_body(body, 'qos_specs') + specs = body['qos_specs'] try: spec = qos_specs.get_qos_specs(context, id) @@ -201,15 +197,12 @@ class QoSSpecsController(wsgi.Controller): return webob.Response(status_int=http_client.ACCEPTED) + @validation.schema(qos_specs_schema.unset) def delete_keys(self, req, id, body): """Deletes specified keys in qos specs.""" context = req.environ['cinder.context'] context.authorize(policy.DELETE_POLICY) - if not (body and 'keys' in body - and isinstance(body.get('keys'), list)): - raise webob.exc.HTTPBadRequest() - keys = body['keys'] LOG.debug("Delete_key spec: %(id)s, keys: %(keys)s", {'id': id, 'keys': keys}) diff --git a/cinder/api/schemas/qos_specs.py b/cinder/api/schemas/qos_specs.py new file mode 100644 index 00000000000..18123f4da81 --- /dev/null +++ b/cinder/api/schemas/qos_specs.py @@ -0,0 +1,63 @@ +# Copyright 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. + +from cinder.api.validation import parameter_types + + +create = { + 'type': 'object', + 'properties': { + 'type': 'object', + 'qos_specs': { + 'type': 'object', + 'properties': { + 'name': { + 'type': 'string', + 'format': 'name_skip_leading_trailing_spaces' + }, + }, + 'required': ['name'], + 'additionalProperties': True, + }, + }, + 'required': ['qos_specs'], + 'additionalProperties': False, +} + + +set = { + 'type': 'object', + 'properties': { + 'qos_specs': parameter_types.extra_specs_with_null + }, + 'required': ['qos_specs'], + 'additionalProperties': False, +} + + +unset = { + 'type': 'object', + 'properties': { + 'keys': { + 'type': 'array', + 'items': { + 'type': 'string', 'minLength': 1, 'maxLength': 255 + }, + 'uniqueItems': True + }, + }, + 'required': ['keys'], + 'additionalProperties': False, +} diff --git a/cinder/api/validation/validators.py b/cinder/api/validation/validators.py index 92873289548..a430f0b83d7 100644 --- a/cinder/api/validation/validators.py +++ b/cinder/api/validation/validators.py @@ -106,6 +106,22 @@ def _validate_name(param_value): return True +@jsonschema.FormatChecker.cls_checks('name_skip_leading_trailing_spaces', + exception.InvalidName) +def _validate_name_skip_leading_trailing_spaces(param_value): + if not param_value: + msg = _("The 'name' can not be None.") + raise exception.InvalidName(reason=msg) + param_value = param_value.strip() + if len(param_value) == 0: + msg = _("The 'name' can not be empty.") + raise exception.InvalidName(reason=msg) + elif len(param_value) > 255: + msg = _("The 'name' can not be greater than 255 characters.") + raise exception.InvalidInput(reason=msg) + return True + + @jsonschema.FormatChecker.cls_checks('uuid') def _validate_uuid_format(instance): return uuidutils.is_uuid_like(instance) diff --git a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py index 116473bfc8d..d12fd41bc8e 100644 --- a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py +++ b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py @@ -345,7 +345,7 @@ class QoSSpecManageApiTest(test.TestCase): (fake.PROJECT_ID, fake.IN_USE_ID), use_admin_context=True) - self.controller.delete_keys(req, fake.IN_USE_ID, body) + self.controller.delete_keys(req, fake.IN_USE_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.delete_keys', @@ -359,9 +359,20 @@ class QoSSpecManageApiTest(test.TestCase): self.assertRaises(exception.QoSSpecsNotFound, self.controller.delete_keys, - req, fake.WILL_NOT_BE_FOUND_ID, body) + req, fake.WILL_NOT_BE_FOUND_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) + def test_qos_specs_delete_keys_invalid_key(self): + body = {"keys": ['', None]} + req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s/delete_keys' % + (fake.PROJECT_ID, + fake.IN_USE_ID), + use_admin_context=True) + + self.assertRaises(exception.ValidationError, + self.controller.delete_keys, + req, fake.IN_USE_ID, body=body) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.delete_keys', @@ -375,7 +386,7 @@ class QoSSpecManageApiTest(test.TestCase): self.assertRaises(exception.QoSSpecsKeyNotFound, self.controller.delete_keys, - req, fake.IN_USE_ID, body) + req, fake.IN_USE_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.delete_keys', @@ -389,7 +400,7 @@ class QoSSpecManageApiTest(test.TestCase): (fake.PROJECT_ID, fake.IN_USE_ID), use_admin_context=True) - self.controller.delete_keys(req, fake.IN_USE_ID, body) + self.controller.delete_keys(req, fake.IN_USE_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.create', @@ -402,7 +413,7 @@ class QoSSpecManageApiTest(test.TestCase): fake.PROJECT_ID, use_admin_context=True) - res_dict = self.controller.create(req, body) + res_dict = self.controller.create(req, body=body) self.assertEqual(1, self.notifier.get_notification_count()) self.assertEqual('qos_specs_%s' % fake.QOS_SPEC_ID, @@ -417,7 +428,7 @@ class QoSSpecManageApiTest(test.TestCase): use_admin_context=True) self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, body) + self.controller.create, req, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.create', @@ -429,7 +440,7 @@ class QoSSpecManageApiTest(test.TestCase): use_admin_context=True) self.assertRaises(webob.exc.HTTPConflict, - self.controller.create, req, body) + self.controller.create, req, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.create', @@ -441,7 +452,7 @@ class QoSSpecManageApiTest(test.TestCase): use_admin_context=True) self.assertRaises(webob.exc.HTTPInternalServerError, - self.controller.create, req, body) + self.controller.create, req, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @ddt.data({'foo': {'a': 'b'}}, @@ -452,8 +463,8 @@ class QoSSpecManageApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs' % fake.PROJECT_ID, use_admin_context=True) req.method = 'POST' - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception.ValidationError, + self.controller.create, req, body=body) @ddt.data({'name': 'fake_name', 'a' * 256: 'a'}, {'name': 'fake_name', 'a': 'a' * 256}, @@ -464,19 +475,20 @@ class QoSSpecManageApiTest(test.TestCase): use_admin_context=True) req.method = 'POST' self.assertRaises(exception.InvalidInput, - self.controller.create, req, body) + self.controller.create, req, body=body) - @ddt.data({'name': None}, - {'name': 'n' * 256}, - {'name': ''}, - {'name': ' '}) - def test_create_qos_with_invalid_spec_name(self, value): + @ddt.data(({'name': None}, exception.ValidationError), + ({'name': ''}, exception.ValidationError), + ({'name': ' '}, exception.ValidationError), + ({'name': 'n' * 256}, exception.InvalidInput)) + @ddt.unpack + def test_create_qos_with_invalid_spec_name(self, value, exception_class): body = {'qos_specs': value} req = fakes.HTTPRequest.blank('/v2/%s/qos-specs' % fake.PROJECT_ID, use_admin_context=True) req.method = 'POST' - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, body) + self.assertRaises(exception_class, + self.controller.create, req, body=body) @mock.patch('cinder.volume.qos_specs.update', side_effect=return_qos_specs_update) @@ -488,7 +500,7 @@ class QoSSpecManageApiTest(test.TestCase): use_admin_context=True) body = {'qos_specs': {'key1': 'value1', 'key2': 'value2'}} - res = self.controller.update(req, fake.QOS_SPEC_ID, body) + res = self.controller.update(req, fake.QOS_SPEC_ID, body=body) self.assertDictEqual(body, res) self.assertEqual(1, self.notifier.get_notification_count()) @@ -505,7 +517,7 @@ class QoSSpecManageApiTest(test.TestCase): 'key2': 'value2'}} self.assertRaises(exception.QoSSpecsNotFound, self.controller.update, - req, fake.WILL_NOT_BE_FOUND_ID, body) + req, fake.WILL_NOT_BE_FOUND_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.get_qos_specs', @@ -520,7 +532,7 @@ class QoSSpecManageApiTest(test.TestCase): 'key2': 'value2'}} self.assertRaises(exception.InvalidQoSSpecs, self.controller.update, - req, fake.INVALID_ID, body) + req, fake.INVALID_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.get_qos_specs', @@ -532,10 +544,10 @@ class QoSSpecManageApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.UUID1), use_admin_context=True) - self.assertRaises(exception.InvalidQoSSpecs, + self.assertRaises(exception.ValidationError, self.controller.update, - req, fake.UUID1, body) - self.assertEqual(1, self.notifier.get_notification_count()) + req, fake.UUID1, body=body) + self.assertEqual(0, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.get_qos_specs', side_effect=return_qos_specs_get_qos_specs) @@ -550,7 +562,7 @@ class QoSSpecManageApiTest(test.TestCase): 'key2': 'value2'}} self.assertRaises(webob.exc.HTTPInternalServerError, self.controller.update, - req, fake.UPDATE_FAILED_ID, body) + req, fake.UPDATE_FAILED_ID, body=body) self.assertEqual(1, self.notifier.get_notification_count()) @mock.patch('cinder.volume.qos_specs.get_qos_specs', @@ -769,7 +781,7 @@ class QoSSpecManageApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs' % fake.PROJECT_ID, use_admin_context=False) self.assertRaises(exception.PolicyNotAuthorized, - self.controller.create, req, body) + self.controller.create, req, body=body) def test_update_no_admin_user(self): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % @@ -778,7 +790,8 @@ class QoSSpecManageApiTest(test.TestCase): body = {'qos_specs': {'key1': 'value1', 'key2': 'value2'}} self.assertRaises(exception.PolicyNotAuthorized, - self.controller.update, req, fake.QOS_SPEC_ID, body) + self.controller.update, req, fake.QOS_SPEC_ID, + body=body) def test_qos_specs_delete_no_admin_user(self): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (