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 <zbitter@redhat.com>

diff --git a/heat/engine/checkeddict.py b/heat/engine/checkeddict.py
index 0147aa58c12b6b09888374013971850f4e0cf23a..7ecc670a5ab31c528d833a5c57cf01a721539e35 100644 (file)
--- 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 f0aa58fe8ae0783a9845c92edcf9d5e8107368b4..10ba59990f3637b21ff7934b38622bc9bae17bfb 100644 (file)
--- 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 82a463ecdbb7dca27a2e5b2339181540b21893ed..344796097bd5963dc6ef0e936f8f11ed3f3069b3 100644 (file)
--- 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 4653a3bf3021e9a39e38ccb6f2130982a8253668..e81e27a25c5ee030b424902f377d68effe848d15 100644 (file)
--- 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 df49b20edd95a629f14774ee4175d164d7c2292d..0b06c24f5f52a04af56415891c6b7ffea80804ae 100644 (file)
--- 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)