Merge pull request #16 from kddejong/Fix/GettingUnexpectedList

Fix prop and req checks when they get a list instead of dict

src/cfnlint/__main__.py

@@ -80,9 +80,9 @@ def main():
                 sys.exit(1)
         except (ParserError, ScannerError) as err:
             if vars(args[0])['ignore_bad_template']:
-                LOGGER.info('Template %s is maflormed: %s', filename, err)
+                LOGGER.info('Template %s is malformed: %s', filename, err)
             else:
-                LOGGER.error('Template %s is maflormed: %s', filename, err)
+                LOGGER.error('Template %s is malformed: %s', filename, err)
                 sys.exit(1)
 
     parser.add_argument(

src/cfnlint/rules/resources/ectwo/Ebs.py

@@ -74,11 +74,6 @@ def _checkEbs(self, cfn, ebs, path):
                                     RuleMatch(
                                         pathmessage,
                                         message.format(volume_type, '/'.join(map(str, pathmessage)))))
-        else:
-            pathmessage = path[:]
-            message = "Ebs should be a type dict for {0}"
-            matches.append(
-                RuleMatch(pathmessage, message.format('/'.join(map(str, pathmessage)))))
 
         return matches
 

src/cfnlint/rules/resources/properties/Properties.py

@@ -33,54 +33,55 @@ def __init__(self):
         self.propertytypes = {}
         self.parameternames = {}
 
-    def primitivetypecheck(self, text, prop, primtype, proppath):
+    def primitivetypecheck(self, value, primtype, proppath):
         """Check primitive types"""
         matches = list()
-        if isinstance(text[prop], dict) and primtype != 'Json':
-            if len(text[prop]) == 1:
-                for sub_key, sub_value in text[prop].items():
+        if isinstance(value, dict) and primtype == 'Json':
+            return matches
+        elif isinstance(value, dict):
+            if len(value) == 1:
+                for sub_key, sub_value in value.items():
                     if sub_key in cfnlint.helpers.CONDITION_FUNCTIONS:
                         cond_values = self.cfn.get_condition_values(sub_value)
                         for cond_value in cond_values:
-                            sub_dict = {}
-                            sub_dict[prop] = cond_value['Value']
                             matches.extend(self.primitivetypecheck(
-                                sub_dict, prop, primtype, proppath + cond_value['Path']))
+                                cond_value['Value'], primtype, proppath + cond_value['Path']))
                     elif sub_key not in ['Fn::Base64', 'Fn::GetAtt', 'Fn::GetAZs', 'Fn::ImportValue',
                                          'Fn::Join', 'Fn::Split', 'Fn::FindInMap', 'Fn::Select', 'Ref',
                                          'Fn::If', 'Fn::Contains', 'Fn::Sub', 'Fn::Cidr']:
                         message = "Property %s has an illegal function %s" % ('/'.join(map(str, proppath)), sub_key)
                         matches.append(RuleMatch(proppath, message))
-
-            # LOGGER.info("Probably a ref or getatt")
-        elif isinstance(text[prop], str):
+        elif isinstance(value, str):
             if primtype in ['Integer', 'Double']:
                 try:
-                    int(text[prop])
+                    int(value)
                 except ValueError:
                     # LOGGER.error("Tried to convert %s to int. %s",
                     #             text[prop], Exception)
                     message = "Property %s should be of type Int/Boolean" % ('/'.join(map(str, proppath)))
                     matches.append(RuleMatch(proppath, message))
             elif primtype == "Boolean":
-                if text[prop] not in ['true', 'false', 'True', 'False']:
+                if value not in ['true', 'false', 'True', 'False']:
                     message = "Property %s should be of type Boolean" % ('/'.join(map(str, proppath)))
                     matches.append(RuleMatch(proppath, message))
             elif primtype != "String":
                 message = "Property %s should be of type String" % ('/'.join(map(str, proppath)))
                 matches.append(RuleMatch(proppath, message))
-        elif isinstance(text[prop], bool):
+        elif isinstance(value, bool):
             if primtype != "Boolean" and primtype != "String":
                 message = "Property %s should be of type %s" % ('/'.join(map(str, proppath)), primtype)
                 matches.append(RuleMatch(proppath, message))
-        elif isinstance(text[prop], int):
+        elif isinstance(value, int):
             if primtype in ["String", "Double"]:
                 pass
                 # LOGGER.info("%s is an int but should be %s for resource %s",
                 #            text[prop], primtype, resourcename)
             elif primtype != "Integer":
                 message = "Property %s should be of type Integer" % ('/'.join(map(str, proppath)))
                 matches.append(RuleMatch(proppath, message))
+        elif isinstance(value, list):
+            message = "Property should be of type %s not List at %s" % (primtype, '/'.join(map(str, proppath)))
+            matches.append(RuleMatch(proppath, message))
 
         return matches
 
@@ -105,6 +106,12 @@ def propertycheck(self, text, proptype, parenttype, resourcename, path, root):
                 resourcetype = str.format("{0}.{1}", parenttype, proptype)
 
         resourcespec = specs[resourcetype].get('Properties', {})
+        if text == 'AWS::NoValue':
+            return matches
+        if not isinstance(text, dict):
+            message = "Expecting an object at %s" % ('/'.join(map(str, path)))
+            matches.append(RuleMatch(path, message))
+            return matches
         for prop in text:
             proppath = path[:]
             proppath.append(prop)
@@ -141,7 +148,7 @@ def propertycheck(self, text, proptype, parenttype, resourcename, path, root):
                                 for index, item in enumerate(text[prop]):
                                     arrproppath = proppath[:]
                                     arrproppath.append(index)
-                                    matches.extend(self.primitivetypecheck(text, prop, primtype, arrproppath))
+                                    matches.extend(self.primitivetypecheck(item, primtype, arrproppath))
                             elif isinstance(text[prop], dict):
                                 if 'Ref' in text[prop]:
                                     ref = text[prop]['Ref']
@@ -172,7 +179,7 @@ def propertycheck(self, text, proptype, parenttype, resourcename, path, root):
                                 resourcename, proppath, False))
                 elif 'PrimitiveType' in resourcespec[prop]:
                     primtype = resourcespec[prop]['PrimitiveType']
-                    matches.extend(self.primitivetypecheck(text, prop, primtype, proppath))
+                    matches.extend(self.primitivetypecheck(text[prop], primtype, proppath))
 
         return matches
 

src/cfnlint/rules/resources/properties/Required.py

@@ -52,7 +52,9 @@ def propertycheck(self, text, proptype, parenttype, resourcename, tree, root):
                 resourcetype = str.format("{0}.{1}", parenttype, proptype)
 
         resourcespec = specs[resourcetype]['Properties']
-
+        if not isinstance(text, dict):
+            # Covered with Properties not with Required
+            return matches
         for prop in resourcespec:
             if resourcespec[prop]['Required']:
                 if prop not in text:

test/module/test_rules_collections.py

@@ -60,7 +60,7 @@ def test_fail_run(self):
 
         matches = list()
         matches.extend(self.rules.run(filename, cfn, []))
-        assert(len(matches) == 24)
+        assert(len(matches) == 25)
 
     def test_fail_sub_properties_run(self):
         """Test failure run"""

test/rules/resources/properties/test_properties.py

@@ -35,7 +35,7 @@ def test_file_positive_template(self):
 
     def test_file_negative(self):
         """Test failure"""
-        self.helper_file_negative('templates/bad/generic.yaml', 6)
+        self.helper_file_negative('templates/bad/generic.yaml', 7)
 
     def test_file_negative_2(self):
         """Failure test"""

test/templates/bad/generic.yaml

@@ -34,7 +34,8 @@ Resources:
             VolumeSize: 20
             BadSubX2Key: Not valid
       NetworkInterfaces:
-        - DeviceIndex: "1"
+        - DeviceIndex:
+          - "1"
           BadKey: true
   MyEC2Instance3:
     Type: "AWS::EC2::Instance"

test/templates/good/generic.yaml

@@ -98,7 +98,14 @@ Resources:
             DeleteOnTermination: false
             VolumeSize: 20
       NetworkInterfaces:
-        - DeviceIndex: "1"
+      -
+        DeviceIndex: "1"
+        PrivateIpAddresses:
+        -
+          PrivateIpAddress: 1.1.1.1
+          Primary: true
+        - PrivateIpAddress: 1.1.1.2
+          Primary: false
       UserData:
         Fn::Sub:
         - "yum install ${myPackage}"