From fc8579dfa8f2d897ce4ea9f6acfeea3ca170d6f0 Mon Sep 17 00:00:00 2001 From: Sergio Cazzolato 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" %