From 7cad745ebac9e054ebc220d464c277f404f261e3 Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Fri, 9 Dec 2016 10:33:35 +0900 Subject: [PATCH] Stop allowing tags as empty string Nova support tagging of instance or other resource like device etc. But those tags are allowed to be as empty string which does not make much sense or any real use case. Updating single tag with empty string is not possible as it will be 404 due to url itself has single tag as id. But updating all tags with few or all of them as empty string does not complain. In those cases, empty tags are being accepted and stored as same. Main issue with those is that empty tag cannot be deleted/show as single tag as it will again 404 with url not found. Only way to delete/show such tag is to delete all together. Empty tag should not be allowed at first. This commit makes empty string request to 400 which are currently 200. Doing for server and device tags and same can be adopted for other resource tagging also if any in future. Fixing this as bug fix hoping no backward incompatibility logically. If such tag could have been deleted alone then we could have fix this with microversion. Change-Id: I18a81f19205b2a40ca470067a9576f2f72ff0f13 Closes-Bug: #1648663 --- api-ref/source/os-server-tags.inc | 2 ++ nova/api/openstack/compute/server_tags.py | 6 +++--- nova/api/validation/parameter_types.py | 2 +- nova/tests/unit/api/openstack/compute/test_server_tags.py | 6 ++++++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/api-ref/source/os-server-tags.inc b/api-ref/source/os-server-tags.inc index 69afbed4eb5f..c672ccedc875 100644 --- a/api-ref/source/os-server-tags.inc +++ b/api-ref/source/os-server-tags.inc @@ -13,6 +13,8 @@ Tags have the following restrictions: - Tag is a Unicode bytestring no longer than 60 characters. +- Tag is a non-empty string. + - Tags are case sensitive. - '/' is not allowed to be in a tag name diff --git a/nova/api/openstack/compute/server_tags.py b/nova/api/openstack/compute/server_tags.py index d21061d3c976..521c2affa83d 100644 --- a/nova/api/openstack/compute/server_tags.py +++ b/nova/api/openstack/compute/server_tags.py @@ -94,9 +94,9 @@ class ServerTagsController(wsgi.Controller): try: jsonschema.validate(id, parameter_types.tag) except jsonschema.ValidationError as e: - msg = (_("Tag '%(tag)s' is invalid. It must be a string without " - "characters '/' and ','. Validation error message: " - "%(err)s") % {'tag': id, 'err': e.message}) + msg = (_("Tag '%(tag)s' is invalid. It must be a non empty string " + "without characters '/' and ','. Validation error " + "message: %(err)s") % {'tag': id, 'err': e.message}) raise webob.exc.HTTPBadRequest(explanation=msg) try: diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index 7340139cfef8..f417965c217a 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -399,6 +399,6 @@ personality = { tag = { "type": "string", - "maxLength": tag.MAX_TAG_LENGTH, + "minLength": 1, "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 06e325c1ef68..28d3fab54cf3 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_tags.py +++ b/nova/tests/unit/api/openstack/compute/test_server_tags.py @@ -121,6 +121,12 @@ class ServerTagsTest(test.TestCase): self.controller.update_all, req, UUID, body={'tags': [1]}) + def test_update_all_tags_with_one_tag_empty_string(self): + req = self._get_request('/v2/fake/servers/%s/tags' % UUID, 'PUT') + self.assertRaises(exception.ValidationError, + self.controller.update_all, + req, UUID, body={'tags': ['tag1', '']}) + 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')