Flavor ExtraSpecs containing '/' cannot be deleted

This change applies a regular expression in order to filter
flavor extraspects keys with invalid characters.
The characters allowed are: letters, numbers, underscores,
periods, colons, spaces and hyphens.
A new test flavor has been created which doesn't check the
keys in the post body. This flavor has been created in the
third place (instead of in the last) in order to keep
working existent test cases which depend on the last flavor
received in the get method.

Change-Id: Ifd86bed23a05a5946ae8b9ba6f6c9bf4b24b1d4c
Partial-Bug: #1256119

novaclient/tests/test_utils.py

@@ -286,3 +286,19 @@ class FlattenTestCase(test_utils.TestCase):
              "k3": "v3"}
         r = utils.pretty_choice_dict(d)
         self.assertEqual(r, "'k1=v1', 'k2=v2', 'k3=v3'")
+
+
+class ValidationsTestCase(test_utils.TestCase):
+    def test_validate_flavor_metadata_keys_with_valid_keys(self):
+        valid_keys = ['key1', 'month.price', 'I-Am:AK-ey.01-', 'spaces and _']
+        for key in valid_keys:
+            utils.validate_flavor_metadata_keys(valid_keys)
+
+    def test_validate_flavor_metadata_keys_with_invalid_keys(self):
+        invalid_keys = ['/1', '?1', '%1', '<', '>', '\1']
+        for key in invalid_keys:
+            try:
+                utils.validate_flavor_metadata_keys([key])
+                self.assertFail()
+            except exceptions.CommandError as ce:
+                self.assertTrue(key in str(ce))

novaclient/tests/v1_1/fakes.py

@@ -667,6 +667,10 @@ class FakeHTTPClient(base_client.HTTPClient):
              'OS-FLV-EXT-DATA:ephemeral': 20,
              'os-flavor-access:is_public': False,
              'links': {}},
+            {'id': 4, 'name': '1024 MB Server', 'ram': 1024, 'disk': 10,
+             'OS-FLV-EXT-DATA:ephemeral': 10,
+             'os-flavor-access:is_public': True,
+             'links': {}},
             {'id': 'aa1', 'name': '128 MB Server', 'ram': 128, 'disk': 0,
              'OS-FLV-EXT-DATA:ephemeral': 0,
              'os-flavor-access:is_public': True,
@@ -730,6 +734,14 @@ class FakeHTTPClient(base_client.HTTPClient):
 
     def get_flavors_aa1(self, **kw):
         # Aplhanumeric flavor id are allowed.
+        return (
+            200,
+            {},
+            {'flavor':
+                self.get_flavors_detail(is_public='None')[2]['flavors'][3]}
+        )
+
+    def get_flavors_4(self, **kw):
         return (
             200,
             {},
@@ -765,6 +777,11 @@ class FakeHTTPClient(base_client.HTTPClient):
         return (200, {},
             {'extra_specs': {"k3": "v3"}})
 
+    def get_flavors_4_os_extra_specs(self, **kw):
+        return (200,
+            {},
+            {'extra_specs': {"k4": "v4"}})
+
     def post_flavors_1_os_extra_specs(self, body, **kw):
         assert list(body) == ['extra_specs']
         fakes.assert_has_keys(body['extra_specs'],
@@ -773,6 +790,13 @@ class FakeHTTPClient(base_client.HTTPClient):
             {},
             {'extra_specs': {"k1": "v1"}})
 
+    def post_flavors_4_os_extra_specs(self, body, **kw):
+        assert list(body) == ['extra_specs']
+
+        return (200,
+            {},
+            body)
+
     def delete_flavors_1_os_extra_specs_k1(self, **kw):
         return (204, {}, None)
 

novaclient/tests/v1_1/test_flavors.py

@@ -189,6 +189,23 @@ class FlavorsTest(utils.TestCase):
         self.cs.assert_called('POST', '/flavors/1/os-extra_specs',
                          {"extra_specs": {'k1': 'v1'}})
 
+    def test_set_with_valid_keys(self):
+        valid_keys = ['key4', 'month.price', 'I-Am:AK-ey.44-',
+                      'key with spaces and _']
+
+        f = self.cs.flavors.get(4)
+        for key in valid_keys:
+            f.set_keys({key: 'v4'})
+            self.cs.assert_called('POST', '/flavors/4/os-extra_specs',
+                                  {"extra_specs": {key: 'v4'}})
+
+    def test_set_with_invalid_keys(self):
+        invalid_keys = ['/1', '?1', '%1', '<', '>']
+
+        f = self.cs.flavors.get(1)
+        for key in invalid_keys:
+            self.assertRaises(exceptions.CommandError, f.set_keys, {key: 'v1'})
+
     def test_unset_keys(self):
         f = self.cs.flavors.get(1)
         f.unset_keys(['k1'])

novaclient/tests/v3/fakes.py

@@ -66,6 +66,9 @@ class FakeHTTPClient(fakes_v1_1.FakeHTTPClient):
     post_flavors_1_flavor_extra_specs = (
         fakes_v1_1.FakeHTTPClient.post_flavors_1_os_extra_specs)
 
+    post_flavors_4_flavor_extra_specs = (
+        fakes_v1_1.FakeHTTPClient.post_flavors_4_os_extra_specs)
+
     delete_flavors_1_flavor_extra_specs_k1 = (
         fakes_v1_1.FakeHTTPClient.delete_flavors_1_os_extra_specs_k1)
 
@@ -79,6 +82,10 @@ class FakeHTTPClient(fakes_v1_1.FakeHTTPClient):
              'ephemeral': 20,
              'flavor-access:is_public': False,
              'links': {}},
+            {'id': 4, 'name': '1024 MB Server', 'ram': 1024, 'disk': 10,
+             'ephemeral': 10,
+             'flavor-access:is_public': True,
+             'links': {}},
             {'id': 'aa1', 'name': '128 MB Server', 'ram': 128, 'disk': 0,
              'ephemeral': 0,
              'flavor-access:is_public': True,

novaclient/tests/v3/test_flavors.py

@@ -47,6 +47,16 @@ class FlavorsTest(test_flavors.FlavorsTest):
         self.cs.assert_called('POST', '/flavors/1/flavor-extra-specs',
                          {"extra_specs": {'k1': 'v1'}})
 
+    def test_set_with_valid_keys(self):
+        valid_keys = ['key4', 'month.price', 'I-Am:AK-ey.44-',
+                      'key with spaces and _']
+
+        f = self.cs.flavors.get(4)
+        for key in valid_keys:
+            f.set_keys({key: 'v4'})
+            self.cs.assert_called('POST', '/flavors/4/flavor-extra-specs',
+                                  {"extra_specs": {key: 'v4'}})
+
     def test_unset_keys(self):
         f = self.cs.flavors.get(1)
         f.unset_keys(['k1'])

novaclient/utils.py

@@ -13,6 +13,7 @@
 
 import json
 import pkg_resources
+import re
 import sys
 import textwrap
 import uuid
@@ -22,6 +23,7 @@ import six
 
 from novaclient import exceptions
 from novaclient.openstack.common import cliutils
+from novaclient.openstack.common.gettextutils import _
 from novaclient.openstack.common import jsonutils
 from novaclient.openstack.common import strutils
 
@@ -29,6 +31,8 @@ from novaclient.openstack.common import strutils
 arg = cliutils.arg
 env = cliutils.env
 
+VALID_KEY_REGEX = re.compile(r"[\w\.\- :]+$", re.UNICODE)
+
 
 def add_resource_manager_extra_kwargs_hook(f, hook):
     """Add hook to bind CLI arguments to ResourceManager calls.
@@ -349,3 +353,13 @@ def is_integer_like(val):
         return True
     except (TypeError, ValueError, AttributeError):
         return False
+
+
+def validate_flavor_metadata_keys(keys):
+    for key in keys:
+        valid_name = VALID_KEY_REGEX.match(key)
+        if not valid_name:
+            msg = _('Invalid key: "%s". Keys may only contain letters, '
+                    'numbers, spaces, underscores, periods, colons and '
+                    'hyphens.')
+            raise exceptions.CommandError(msg % key)

novaclient/v1_1/flavors.py

@@ -15,10 +15,12 @@
 """
 Flavor interface.
 """
+
 from novaclient import base
 from novaclient import exceptions
 from novaclient.openstack.common.py3kcompat import urlutils
 from novaclient.openstack.common import strutils
+from novaclient import utils
 
 
 class Flavor(base.Resource):
@@ -62,6 +64,8 @@ class Flavor(base.Resource):
         :param flavor: The :class:`Flavor` to set extra spec on
         :param metadata: A dict of key/value pairs to be set
         """
+        utils.validate_flavor_metadata_keys(metadata.keys())
+
         body = {'extra_specs': metadata}
         return self.manager._create(
                             "/flavors/%s/os-extra_specs" % base.getid(self),

novaclient/v3/flavors.py

@@ -16,7 +16,9 @@
 """
 Flavor interface.
 """
+
 from novaclient import base
+from novaclient import utils
 from novaclient.v1_1 import flavors
 
 
@@ -54,6 +56,8 @@ class Flavor(base.Resource):
         :param flavor: The :class:`Flavor` to set extra spec on
         :param metadata: A dict of key/value pairs to be set
         """
+        utils.validate_flavor_metadata_keys(metadata.keys())
+
         body = {'extra_specs': metadata}
         return self.manager._create(
                             "/flavors/%s/flavor-extra-specs" %