From fc8579dfa8f2d897ce4ea9f6acfeea3ca170d6f0 Mon Sep 17 00:00:00 2001
From: Sergio Cazzolato <sergio.j.cazzolato@intel.com>
Date: Tue, 30 Nov 1999 22:24:27 -0500
Subject: [PATCH] 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        | 16 ++++++++++++++++
 novaclient/tests/v1_1/fakes.py        | 24 ++++++++++++++++++++++++
 novaclient/tests/v1_1/test_flavors.py | 17 +++++++++++++++++
 novaclient/tests/v3/fakes.py          |  7 +++++++
 novaclient/tests/v3/test_flavors.py   | 10 ++++++++++
 novaclient/utils.py                   | 14 ++++++++++++++
 novaclient/v1_1/flavors.py            |  4 ++++
 novaclient/v3/flavors.py              |  4 ++++
 8 files changed, 96 insertions(+)

diff --git a/novaclient/tests/test_utils.py b/novaclient/tests/test_utils.py
index a7660b8b5..cce2db029 100644
--- a/novaclient/tests/test_utils.py
+++ b/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))
diff --git a/novaclient/tests/v1_1/fakes.py b/novaclient/tests/v1_1/fakes.py
index 8590ce4f1..d00dd0785 100644
--- a/novaclient/tests/v1_1/fakes.py
+++ b/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)
 
diff --git a/novaclient/tests/v1_1/test_flavors.py b/novaclient/tests/v1_1/test_flavors.py
index d1762949e..c18305401 100644
--- a/novaclient/tests/v1_1/test_flavors.py
+++ b/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'])
diff --git a/novaclient/tests/v3/fakes.py b/novaclient/tests/v3/fakes.py
index ffc7e6056..60063f3e9 100644
--- a/novaclient/tests/v3/fakes.py
+++ b/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,
diff --git a/novaclient/tests/v3/test_flavors.py b/novaclient/tests/v3/test_flavors.py
index 34714aef0..ea76988a0 100644
--- a/novaclient/tests/v3/test_flavors.py
+++ b/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'])
diff --git a/novaclient/utils.py b/novaclient/utils.py
index 812cbfa15..f4c8b8dd0 100644
--- a/novaclient/utils.py
+++ b/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)
diff --git a/novaclient/v1_1/flavors.py b/novaclient/v1_1/flavors.py
index cfb06d1ed..91c879bf5 100644
--- a/novaclient/v1_1/flavors.py
+++ b/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),
diff --git a/novaclient/v3/flavors.py b/novaclient/v3/flavors.py
index 120574aef..38f288504 100644
--- a/novaclient/v3/flavors.py
+++ b/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" %