Add rule E1032 to validate ForEach with transform (#2809)

Author: kddejong

src/cfnlint/rules/functions/ForEach.py

@@ -4,6 +4,7 @@
 """
 import logging
 
+from cfnlint.languageExtensions import LanguageExtensions
 from cfnlint.rules import CloudFormationLintRule
 
 LOGGER = logging.getLogger("cfnlint")
@@ -18,4 +19,19 @@ class ForEach(CloudFormationLintRule):
 
     # pylint: disable=unused-argument
     def match(self, cfn):
-        return []
+        matches = []
+        intrinsic_function = "Fn::ForEach"
+        for_eaches = cfn.transform_pre["Fn::ForEach"]
+
+        for for_each in for_eaches:
+            has_language_extensions_transform = cfn.has_language_extensions_transform()
+
+            LanguageExtensions.validate_transform_is_declared(
+                self,
+                has_language_extensions_transform,
+                matches,
+                for_each[:-1],
+                intrinsic_function,
+            )
+
+        return matches

src/cfnlint/rules/functions/GetAtt.py

@@ -98,6 +98,15 @@ def match(self, cfn):
             # only check resname if its set.  if it isn't it is because of bad structure
             # and an error is already provided
             if resname:
+                if not isinstance(resname, str):
+                    message = "Invalid GetAtt structure {0} at {1}"
+                    matches.append(
+                        RuleMatch(
+                            getatt,
+                            message.format(getatt[-1], "/".join(map(str, getatt[:-1]))),
+                        )
+                    )
+                    continue
                 if resname in valid_getatts:
                     if restype is not None:
                         # Check for maps

src/cfnlint/template/template.py

@@ -46,6 +46,9 @@ def __init__(self, filename, template, regions=["us-east-1"]):
         self.transform_pre["Fn::Sub"] = self.search_deep_keys("Fn::Sub")
         self.transform_pre["Fn::FindInMap"] = self.search_deep_keys("Fn::FindInMap")
         self.transform_pre["Transform"] = self.template.get("Transform", [])
+        self.transform_pre["Fn::ForEach"] = self.search_deep_keys(
+            cfnlint.helpers.FUNCTION_FOR_EACH
+        )
 
         self.conditions = cfnlint.conditions.Conditions(self)
         self.__cache_search_deep_class = {}
@@ -460,12 +463,13 @@ def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path):
                         # dict and list checks
                         pathprop = pathprop[:-1]
                 elif isinstance(searchText, re.Pattern):
-                    if re.match(searchText, key):
-                        pathprop.append(cfndict[key])
-                        keys.append(pathprop)
-                        # pop the last element off for nesting of found elements for
-                        # dict and list checks
-                        pathprop = pathprop[:-1]
+                    if isinstance(key, str):
+                        if re.match(searchText, key):
+                            pathprop.append(cfndict[key])
+                            keys.append(pathprop)
+                            # pop the last element off for nesting of found elements for
+                            # dict and list checks
+                            pathprop = pathprop[:-1]
                 if isinstance(cfndict[key], dict):
                     keys.extend(
                         self._search_deep_keys(searchText, cfndict[key], pathprop)

src/cfnlint/template/transforms/_language_extensions.py

@@ -43,7 +43,6 @@ def __init__(self, message: str, key: str) -> None:
 def language_extension(cfn: Any) -> TransformResult:
     transform = _Transform()
     try:
-        cfn.transform_pre["Fn::ForEach"] = []
         return transform.transform(cfn)
     except (_ValueError, _TypeError, _ResolveError) as e:
         # pylint: disable=import-outside-toplevel
@@ -115,7 +114,6 @@ def _walk(self, item: Any, params: MutableMapping[str, Any], cfn: Any):
             for k, v in deepcopy(obj).items():
                 # see if key matches Fn::ForEach
                 if re.match(FUNCTION_FOR_EACH, k):
-                    cfn.transform_pre["Fn::ForEach"].append({k: v})
                     # only translate the foreach if its valid
                     foreach = _ForEach(k, v, self._collections)
                     # get the values will flatten the foreach

test/fixtures/templates/bad/functions/foreach_no_transform.yaml

@@ -0,0 +1,40 @@
+AWSTemplateFormatVersion: 2010-09-09
+
+Parameters:
+    Environment:
+        Type: String
+Mappings:
+    Buckets:
+        Production:
+            Identifiers: [A, B, C]
+Resources:
+    "Fn::ForEach::Buckets":
+        - Identifier
+        - !FindInMap [Buckets, !Ref Environment, Identifiers]
+        - "S3Bucket${Identifier}":
+              Type: "AWS::S3::Bucket"
+              Properties:
+                  AccessControl: PublicRead
+                  MetricsConfigurations:
+                      - Id: !Sub "EntireBucket${Identifier}"
+                  WebsiteConfiguration:
+                      IndexDocument: index.html
+                      ErrorDocument: error.html
+                      RoutingRules:
+                          - RoutingRuleCondition:
+                                HttpErrorCodeReturnedEquals: "404"
+                                KeyPrefixEquals: out1/
+                            RedirectRule:
+                                HostName: ec2-11-22-333-44.compute-1.amazonaws.com
+                                ReplaceKeyPrefixWith: report-404/
+              DeletionPolicy: Retain
+              UpdateReplacePolicy: Retain
+Outputs:
+    "Fn::ForEach::BucketOutputs":
+        - Identifier
+        - !FindInMap [Buckets, !Ref Environment, Identifiers]
+        - "Fn::ForEach::GetAttLoop":
+              - Property
+              - [Arn, DomainName, WebsiteURL]
+              - "S3Bucket${Identifier}${Property}":
+                    Value: !GetAtt [!Sub "S3Bucket${Identifier}", {"Ref": "Property"}]

test/fixtures/templates/bad/functions/getatt.yaml

@@ -58,3 +58,5 @@ Outputs:
     Value: !GetAtt ProvisionedProduct.Output.Example
   OutputBadName:
     Value: !GetAtt myElasticSearch.DomainEndpoint.Example
+  BadStructure:
+    Value: !GetAtt [!Sub "${ProjectS3Root}", !Sub "${SSMLookup}"]

test/fixtures/templates/good/functions/foreach.yaml

@@ -0,0 +1,40 @@
+AWSTemplateFormatVersion: 2010-09-09
+Transform: "AWS::LanguageExtensions"
+Parameters:
+    Environment:
+        Type: String
+Mappings:
+    Buckets:
+        Production:
+            Identifiers: [A, B, C]
+Resources:
+    "Fn::ForEach::Buckets":
+        - Identifier
+        - !FindInMap [Buckets, !Ref Environment, Identifiers]
+        - "S3Bucket${Identifier}":
+              Type: "AWS::S3::Bucket"
+              Properties:
+                  AccessControl: PublicRead
+                  MetricsConfigurations:
+                      - Id: !Sub "EntireBucket${Identifier}"
+                  WebsiteConfiguration:
+                      IndexDocument: index.html
+                      ErrorDocument: error.html
+                      RoutingRules:
+                          - RoutingRuleCondition:
+                                HttpErrorCodeReturnedEquals: "404"
+                                KeyPrefixEquals: out1/
+                            RedirectRule:
+                                HostName: ec2-11-22-333-44.compute-1.amazonaws.com
+                                ReplaceKeyPrefixWith: report-404/
+              DeletionPolicy: Retain
+              UpdateReplacePolicy: Retain
+Outputs:
+    "Fn::ForEach::BucketOutputs":
+        - Identifier
+        - !FindInMap [Buckets, !Ref Environment, Identifiers]
+        - "Fn::ForEach::GetAttLoop":
+              - Property
+              - [Arn, DomainName, WebsiteURL]
+              - "S3Bucket${Identifier}${Property}":
+                    Value: !GetAtt [!Sub "S3Bucket${Identifier}", {"Ref": "Property"}]

test/unit/rules/functions/test_for_each.py

@@ -0,0 +1,24 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""
+from test.unit.rules import BaseRuleTestCase
+
+from cfnlint.rules.functions.ForEach import ForEach
+
+
+class TestForEach(BaseRuleTestCase):
+    def setUp(self):
+        super(TestForEach, self).setUp()
+        self.collection.register(ForEach())
+        self.success_templates = [
+            "test/fixtures/templates/good/functions/foreach.yaml",
+        ]
+
+    def test_file_positive(self):
+        self.helper_file_positive()
+
+    def test_file_negative_missing_transform(self):
+        self.helper_file_negative(
+            "test/fixtures/templates/bad/functions/foreach_no_transform.yaml", 3
+        )

test/unit/rules/functions/test_get_att.py

@@ -30,5 +30,5 @@ def test_file_negative(self):
     def test_file_negative_getatt(self):
         """Test failure"""
         self.helper_file_negative(
-            "test/fixtures/templates/bad/functions/getatt.yaml", 5
+            "test/fixtures/templates/bad/functions/getatt.yaml", 7
         )