From 8f618682693775bd55448c11815e22472230cc3f Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Thu, 8 Dec 2016 15:18:43 +0900 Subject: [PATCH] Move tags validation code to json schema Currently we have lot of validation code inside server tag controller class which can be moved to json schema. Same can be reused on other place also. For example, microversion 2.40 adds server tag while boot. Same schema can be used there. Current python code validation shows conceret error message about each tag where using json schema error msg will be like below- For requesting tags more than max limit: Invalid input for field/attribute tags. Value: ['0', '1', '2', '3', '4', '5', '6', ...] is too long For requesting tags more than char limit: Invalid input for field/attribute 0. Value: aaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa. 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' is too long I think those error message are clear enough to understand the wrong tags. Change-Id: I457cf630aeae9034c0b175d016148a1d50a0b469 --- .../openstack/compute/schemas/server_tags.py | 13 +++----- nova/api/openstack/compute/server_tags.py | 33 ------------------- nova/api/validation/parameter_types.py | 2 ++ .../api/openstack/compute/test_server_tags.py | 8 +++-- 4 files changed, 12 insertions(+), 44 deletions(-) diff --git a/nova/api/openstack/compute/schemas/server_tags.py b/nova/api/openstack/compute/schemas/server_tags.py index c60484d6878..bc616acee98 100644 --- a/nova/api/openstack/compute/schemas/server_tags.py +++ b/nova/api/openstack/compute/schemas/server_tags.py @@ -10,20 +10,17 @@ # License for the specific language governing permissions and limitations # under the License. +from nova.api.validation import parameter_types +from nova.objects import instance + update_all = { - "definitions": { - "tag": { - "type": "string" - } - }, "title": "Server tags", "type": "object", "properties": { "tags": { "type": "array", - "items": { - "$ref": "#/definitions/tag" - } + "items": parameter_types.tag, + "maxItems": instance.MAX_TAG_COUNT } }, 'required': ['tags'], diff --git a/nova/api/openstack/compute/server_tags.py b/nova/api/openstack/compute/server_tags.py index de0bb844d46..d21061d3c97 100644 --- a/nova/api/openstack/compute/server_tags.py +++ b/nova/api/openstack/compute/server_tags.py @@ -109,12 +109,6 @@ def update(self, req, server_id, id, body): % objects.instance.MAX_TAG_COUNT) raise webob.exc.HTTPBadRequest(explanation=msg) - if len(id) > objects.tag.MAX_TAG_LENGTH: - msg = (_("Tag '%(tag)s' is too long. Maximum length of a tag " - "is %(length)d") % {'tag': id, - 'length': objects.tag.MAX_TAG_LENGTH}) - raise webob.exc.HTTPBadRequest(explanation=msg) - if id in _get_tags_names(tags): # NOTE(snikitin): server already has specified tag return webob.Response(status_int=204) @@ -139,33 +133,6 @@ def update_all(self, req, server_id, body): context.can(st_policies.POLICY_ROOT % 'update_all') self._check_instance_in_valid_state(context, server_id, 'update tags') - invalid_tags = [] - for tag in body['tags']: - try: - jsonschema.validate(tag, parameter_types.tag) - except jsonschema.ValidationError: - invalid_tags.append(tag) - if invalid_tags: - msg = (_("Tags '%s' are invalid. Each tag must be a string " - "without characters '/' and ','.") % invalid_tags) - raise webob.exc.HTTPBadRequest(explanation=msg) - - tag_count = len(body['tags']) - if tag_count > objects.instance.MAX_TAG_COUNT: - msg = (_("The number of tags exceeded the per-server limit " - "%(max)d. The number of tags in request is %(count)d.") - % {'max': objects.instance.MAX_TAG_COUNT, - 'count': tag_count}) - raise webob.exc.HTTPBadRequest(explanation=msg) - - long_tags = [ - t for t in body['tags'] if len(t) > objects.tag.MAX_TAG_LENGTH] - if long_tags: - msg = (_("Tags %(tags)s are too long. Maximum length of a tag " - "is %(length)d") % {'tags': long_tags, - 'length': objects.tag.MAX_TAG_LENGTH}) - raise webob.exc.HTTPBadRequest(explanation=msg) - try: tags = objects.TagList.create(context, server_id, body['tags']) except exception.InstanceNotFound as e: diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index 352b956c08f..7340139cfef 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -23,6 +23,7 @@ from nova import db from nova.i18n import _ +from nova.objects import tag class ValidationRegex(object): @@ -398,5 +399,6 @@ def valid_char(char): tag = { "type": "string", + "maxLength": tag.MAX_TAG_LENGTH, "pattern": "^[^,/]*$" } diff --git a/nova/tests/unit/api/openstack/compute/test_server_tags.py b/nova/tests/unit/api/openstack/compute/test_server_tags.py index a5269b7bad0..06e325c1ef6 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_tags.py +++ b/nova/tests/unit/api/openstack/compute/test_server_tags.py @@ -103,14 +103,15 @@ def test_update_all_too_many_tags(self): req = self._get_request( '/v2/fake/servers/%s/tags' % UUID, 'PUT') - self.assertRaises(exc.HTTPBadRequest, self.controller.update_all, + self.assertRaises(exception.ValidationError, + self.controller.update_all, req, UUID, body=fake_tags) def test_update_all_forbidden_characters(self): self.stub_out('nova.api.openstack.common.get_instance', return_server) req = self._get_request('/v2/fake/servers/%s/tags' % UUID, 'PUT') for tag in ['tag,1', 'tag/1']: - self.assertRaises(exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller.update_all, req, UUID, body={'tags': [tag, 'tag2']}) @@ -124,7 +125,8 @@ def test_update_all_too_long_tag(self): self.stub_out('nova.api.openstack.common.get_instance', return_server) req = self._get_request('/v2/fake/servers/%s/tags' % UUID, 'PUT') tag = "a" * (tag_obj.MAX_TAG_LENGTH + 1) - self.assertRaises(exc.HTTPBadRequest, self.controller.update_all, + self.assertRaises(exception.ValidationError, + self.controller.update_all, req, UUID, body={'tags': [tag]}) def test_update_all_invalid_tag_list_type(self):