Merge "Flavor ExtraSpecs containing '/' cannot be deleted"

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" %