From cfe391073a681ba8f1b806fc53428adb36486181 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 31 May 2012 11:17:30 +0200 Subject: [PATCH] Get list properties working again The CloudFormation documentation is very confusing on this point. 'CommaDelimitedList' is one of the data types that are valid for a Parameter (the others are 'String' and 'Number'). A CommaDelimitedList parameter takes the form of a string where the list members are delimted by commas: "item1,item2,item3" However the documentation also uses the phrase "Comma Delimited List" to refer to the type of some Resource Properties that are, in fact, simply lists: [ "item1" , "item2" , "item3" ] ...as if there were *another* way to represent lists. (Note that the items here need not be strings, and in fact are usually objects of some variety.) So we need a different data type to represent the latter. (This patch changes the name from 'TupleList' to just 'List', since the actual Python sequence type is just an implementation detail.) In future, we should probably also verify that only the 3 valid Parameter types are used, and perhaps that list Properties contain only objects of the correct type. Change-Id: I94054f588fc37f7d4ba245f2e92b86ac9c872c37 Signed-off-by: Zane Bitter --- heat/engine/checkeddict.py | 46 +++++++++++++++++++---------------- heat/engine/instance.py | 8 +++--- heat/engine/security_group.py | 4 +-- heat/engine/user.py | 2 +- heat/engine/volume.py | 2 +- 5 files changed, 33 insertions(+), 29 deletions(-) diff --git a/heat/engine/checkeddict.py b/heat/engine/checkeddict.py index 0147aa58..7ecc670a 100644 --- a/heat/engine/checkeddict.py +++ b/heat/engine/checkeddict.py @@ -15,6 +15,9 @@ import collections import re +import logging + +logger = logging.getLogger(__file__) class CheckedDict(collections.MutableMapping): @@ -49,29 +52,30 @@ class CheckedDict(collections.MutableMapping): if 'Type' in self.data[key]: t = self.data[key]['Type'] - if self.data[key]['Type'] == 'String': - if not isinstance(value, (basestring, unicode)): - raise ValueError('%s: %s Value must be a string' % \ + if t == 'String': + if not isinstance(value, basestring): + raise ValueError('%s: %s Value must be a string' % (self.name, key)) if 'MaxLength' in self.data[key]: if len(value) > int(self.data[key]['MaxLength']): - raise ValueError('%s: %s is too long; MaxLength %s' % \ - (self.name, key, - self.data[key]['MaxLength'])) + raise ValueError('%s: %s is too long; MaxLength %s' % + (self.name, key, + self.data[key]['MaxLength'])) if 'MinLength' in self.data[key]: if len(value) < int(self.data[key]['MinLength']): - raise ValueError('%s: %s is too short; MinLength %s' %\ - (self.name, key, - self.data[key]['MinLength'])) + raise ValueError('%s: %s is too short; MinLength %s' % + (self.name, key, + self.data[key]['MinLength'])) if 'AllowedPattern' in self.data[key]: rc = re.match('^%s$' % self.data[key]['AllowedPattern'], value) if rc == None: - raise ValueError('%s: Pattern %s does not match %s' % \ - (self.name, self.data[key]['AllowedPattern'], - key)) + raise ValueError('%s: Pattern %s does not match %s' % + (self.name, + self.data[key]['AllowedPattern'], + key)) - elif self.data[key]['Type'] in ['Integer', 'Number', 'Float']: + elif t in ['Integer', 'Number', 'Float']: # just try convert and see if it will throw a ValueError num = num_converter[t](value) minn = num @@ -84,20 +88,20 @@ class CheckedDict(collections.MutableMapping): raise ValueError('%s: %s is out of range' % (self.name, key)) - elif self.data[key]['Type'] == 'List': - if not isinstance(value, list): - raise ValueError('%s: %s Value must be a list' % \ + elif t == 'List': + if not isinstance(value, (list, tuple)): + raise ValueError('%s: %s Value must be a list' % (self.name, key)) - elif self.data[key]['Type'] == 'CommaDelimitedList': + elif t == 'CommaDelimitedList': sp = value.split(',') - if not isinstance(sp, list): - raise ValueError('%s: %s Value must be a list' % \ - (self.name, key)) + + else: + logger.warn('Unknown value type "%s"' % t) if 'AllowedValues' in self.data[key]: if not value in self.data[key]['AllowedValues']: - raise ValueError('%s: %s Value must be one of %s' % \ + raise ValueError('%s: %s Value must be one of %s' % (self.name, key, str(self.data[key]['AllowedValues']))) diff --git a/heat/engine/instance.py b/heat/engine/instance.py index f0aa58fe..10ba5999 100644 --- a/heat/engine/instance.py +++ b/heat/engine/instance.py @@ -96,21 +96,21 @@ class Instance(Resource): 'Implemented': False}, 'RamDiskId': {'Type': 'String', 'Implemented': False}, - 'SecurityGroups': {'Type': 'TuplesList', + 'SecurityGroups': {'Type': 'List', 'Implemented': False}, - 'SecurityGroupIds': {'Type': 'CommaDelimitedList', + 'SecurityGroupIds': {'Type': 'List', 'Implemented': False}, 'SourceDestCheck': {'Type': 'Boolean', 'Implemented': False}, 'SubnetId': {'Type': 'String', 'Implemented': False}, - 'Tags': {'Type': 'CommaDelimitedList', + 'Tags': {'Type': 'List', 'Implemented': False}, 'Tenancy': {'Type': 'String', 'AllowedValues': ['dedicated', 'default'], 'Implemented': False}, 'UserData': {'Type': 'String'}, - 'Volumes': {'Type': 'CommaDelimitedList', + 'Volumes': {'Type': 'List', 'Implemented': False}} def __init__(self, name, json_snippet, stack): diff --git a/heat/engine/security_group.py b/heat/engine/security_group.py index 82a463ec..34479609 100644 --- a/heat/engine/security_group.py +++ b/heat/engine/security_group.py @@ -29,9 +29,9 @@ class SecurityGroup(Resource): 'Required': True}, 'VpcId': {'Type': 'String', 'Implemented': False}, - 'SecurityGroupIngress': {'Type': 'TuplesList', + 'SecurityGroupIngress': {'Type': 'List', 'Implemented': False}, - 'SecurityGroupEgress': {'Type': 'TuplesList'}} + 'SecurityGroupEgress': {'Type': 'List'}} def __init__(self, name, json_snippet, stack): super(SecurityGroup, self).__init__(name, json_snippet, stack) diff --git a/heat/engine/user.py b/heat/engine/user.py index 4653a3bf..e81e27a2 100644 --- a/heat/engine/user.py +++ b/heat/engine/user.py @@ -30,7 +30,7 @@ class User(Resource): 'Implemented': False}, 'LoginProfile': {'Type': 'String', 'Implemented': False}, - 'Policies': {'Type': 'TuplesList'}} + 'Policies': {'Type': 'List'}} def __init__(self, name, json_snippet, stack): super(User, self).__init__(name, json_snippet, stack) diff --git a/heat/engine/volume.py b/heat/engine/volume.py index df49b20e..0b06c24f 100644 --- a/heat/engine/volume.py +++ b/heat/engine/volume.py @@ -29,7 +29,7 @@ class Volume(Resource): 'Required': True}, 'Size': {'Type': 'Number'}, 'SnapshotId': {'Type': 'String'}, - 'Tags': {'Type': 'TuplesList'}} + 'Tags': {'Type': 'List'}} def __init__(self, name, json_snippet, stack): super(Volume, self).__init__(name, json_snippet, stack) -- 2.45.2