Extra_spec containing '/' can't be deleted

This patch validates the keys of the extra specs
before setting them.
This will make possible to remove those keys.
It allows alphanumeric characters, underscores,
periods, colons and hyphens.

Change-Id: Ic2e5d7ce42c8ecb7ee45cd168bd008035fd4bbdf
Closes-Bug: #1259711

cinder/api/common.py

@@ -48,6 +48,22 @@ LOG = logging.getLogger(__name__)
 XML_NS_V1 = 'http://docs.openstack.org/volume/api/v1'
 
 
+# Regex that matches alphanumeric characters, periods, hypens,
+# colons and underscores:
+# ^ assert position at start of the string
+# [\w\.\-\:\_] match expression
+# $ assert position at end of the string
+VALID_KEY_NAME_REGEX = re.compile(r"^[\w\.\-\:\_]+$", re.UNICODE)
+
+
+def validate_key_names(key_names_list):
+    """Validate each item of the list to match key name regex."""
+    for key_name in key_names_list:
+        if not VALID_KEY_NAME_REGEX.match(key_name):
+            return False
+    return True
+
+
 def get_pagination_params(request):
     """Return marker, limit tuple from request.
 

cinder/api/contrib/types_extra_specs.py

@@ -17,6 +17,7 @@
 
 import webob
 
+from cinder.api import common
 from cinder.api import extensions
 from cinder.api.openstack import wsgi
 from cinder.api import xmlutil
@@ -81,8 +82,8 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
             raise webob.exc.HTTPBadRequest()
 
         self._check_type(context, type_id)
-
         specs = body['extra_specs']
+        self._check_key_names(specs.keys())
         db.volume_type_extra_specs_update_or_create(context,
                                                     type_id,
                                                     specs)
@@ -144,6 +145,13 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
                             notifier_api.INFO, notifier_info)
         return webob.Response(status_int=202)
 
+    def _check_key_names(self, keys):
+        if not common.validate_key_names(keys):
+            expl = _('Key names can only contain alphanumeric characters, '
+                     'underscores, periods, colons and hyphens.')
+
+            raise webob.exc.HTTPBadRequest(explanation=expl)
+
 
 class Types_extra_specs(extensions.ExtensionDescriptor):
     """Type extra specs support."""

cinder/tests/api/contrib/test_types_extra_specs.py

@@ -18,6 +18,8 @@
 from lxml import etree
 import webob
 
+import mock
+
 from cinder.api.contrib import types_extra_specs
 from cinder import exception
 from cinder.openstack.common.notifier import api as notifier_api
@@ -143,6 +145,54 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual('value1', res_dict['extra_specs']['key1'])
 
+    @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create')
+    def test_create_key_allowed_chars(
+            self, volume_type_extra_specs_update_or_create):
+        mock_return_value = {"key1": "value1",
+                             "key2": "value2",
+                             "key3": "value3",
+                             "key4": "value4",
+                             "key5": "value5"}
+        volume_type_extra_specs_update_or_create.\
+            return_value = mock_return_value
+
+        body = {"extra_specs": {"other_alphanum.-_:": "value1"}}
+
+        self.assertEqual(len(test_notifier.NOTIFICATIONS), 0)
+
+        req = fakes.HTTPRequest.blank(self.api_path)
+        res_dict = self.controller.create(req, 1, body)
+        self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
+        self.assertEqual('value1',
+                         res_dict['extra_specs']['other_alphanum.-_:'])
+
+    @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create')
+    def test_create_too_many_keys_allowed_chars(
+            self, volume_type_extra_specs_update_or_create):
+        mock_return_value = {"key1": "value1",
+                             "key2": "value2",
+                             "key3": "value3",
+                             "key4": "value4",
+                             "key5": "value5"}
+        volume_type_extra_specs_update_or_create.\
+            return_value = mock_return_value
+
+        body = {"extra_specs": {"other_alphanum.-_:": "value1",
+                                "other2_alphanum.-_:": "value2",
+                                "other3_alphanum.-_:": "value3"}}
+
+        self.assertEqual(len(test_notifier.NOTIFICATIONS), 0)
+
+        req = fakes.HTTPRequest.blank(self.api_path)
+        res_dict = self.controller.create(req, 1, body)
+        self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
+        self.assertEqual('value1',
+                         res_dict['extra_specs']['other_alphanum.-_:'])
+        self.assertEqual('value2',
+                         res_dict['extra_specs']['other2_alphanum.-_:'])
+        self.assertEqual('value3',
+                         res_dict['extra_specs']['other3_alphanum.-_:'])
+
     def test_update_item(self):
         self.stubs.Set(cinder.db,
                        'volume_type_extra_specs_update_or_create',
@@ -192,6 +242,7 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
     def _extra_specs_create_bad_body(self, body):
         req = fakes.HTTPRequest.blank('/v2/fake/types/1/extra_specs')
         req.method = 'POST'
+
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.create, req, '1', body)
 
@@ -206,6 +257,14 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
         body = {'extra_specs': 'string'}
         self._extra_specs_create_bad_body(body=body)
 
+    def test_create_invalid_key(self):
+        body = {"extra_specs": {"ke/y1": "value1"}}
+        self._extra_specs_create_bad_body(body=body)
+
+    def test_create_invalid_too_many_key(self):
+        body = {"key1": "value1", "ke/y2": "value2", "key3": "value3"}
+        self._extra_specs_create_bad_body(body=body)
+
 
 class VolumeTypeExtraSpecsSerializerTest(test.TestCase):
     def test_index_create_serializer(self):