Fix GetAtt to ForEach modules using identifiers

Author: ericzbeard

awscli/customizations/cloudformation/modules/maps.py

@@ -40,6 +40,7 @@
 ORIGINAL_MAP_NAME = "OriginalMapName"
 VALUE = "Value"
 FOREACH_PREFIX = "Fn::ForEach::"
+MAP_NAME_IS_I = "map_name_is_i"
 
 
 def replace_str(s, m, i):
@@ -289,6 +290,7 @@ def vf(vis):
 
                     # Remember the original name for GetAtt resolution
                     modules[module_id][ORIGINAL_MAP_NAME] = loop_name
+                    modules[module_id][MAP_NAME_IS_I] = False
 
             # Remove the ForEach entry
             del modules[k]

awscli/customizations/cloudformation/modules/process.py

@@ -36,6 +36,7 @@
     resolve_mapped_lists,
     getatt_map_list,
     ORIGINAL_MAP_NAME,
+    MAP_NAME_IS_I,
 )
 from awscli.customizations.cloudformation.modules.read import (
     is_url,
@@ -146,6 +147,8 @@ def make_module(
         module_config[OVERRIDES] = config[OVERRIDES]
     if ORIGINAL_MAP_NAME in config:
         module_config[ORIGINAL_MAP_NAME] = config[ORIGINAL_MAP_NAME]
+    if MAP_NAME_IS_I in config:
+        module_config[MAP_NAME_IS_I] = config[MAP_NAME_IS_I]
     module_config[NO_SOURCE_MAP] = no_source_map
     return Module(template, module_config, parent_module, s3_client)
 
@@ -283,6 +286,11 @@ def __init__(self, template, module_config, parent_module, s3_client=None):
         self.parent_path = module_config.get(PARENT_PATH, "")
         self.invoked = module_config.get(INVOKED, False)
 
+        # Default behavior for Map/ForEach is to use array indexes
+        self.map_name_is_i = True
+        if MAP_NAME_IS_I in module_config:
+            self.map_name_is_i = module_config[MAP_NAME_IS_I]
+
         if RESOURCES not in self.template:
             # The parent might only have Modules
             self.template[RESOURCES] = {}
@@ -700,14 +708,25 @@ def resolve_output_getatt(self, v, d, n):
                     self.resolve_output_getatt_map(mapped, name, prop_name)
                 return False
 
+        # index might be a number like 0: Content[0].Arn
+        # or it might be a key like A: Content[A].Arn
+        # The name of the mapped module might be Content0 or ContentA,
+        # depending on if we used an Fn::ForEach identifier.
+
         if index != -1:
-            if num.isdigit():
+            if isinstance(index, int):
                 name = f"{name}{index}"
             else:
                 # Find the array index for the key
-                for i, k in enumerate(mapped[name]):
-                    if index == k:
-                        name = f"{name}{i}"
+                if not self.map_name_is_i:
+                    # If Fn::ForEach was used with an identifier for
+                    # the logical id, we need to use that instead of the index
+                    name = f"{name}{index}"
+                else:
+                    # The mapped name uses the array index
+                    for i, k in enumerate(mapped[name]):
+                        if index == k:
+                            name = f"{name}{i}"
 
         reffed_prop = None
         if name == self.name:

tests/unit/customizations/cloudformation/modules/foreach-expect.yaml

@@ -23,3 +23,8 @@ Resources:
     Type: A::B::C
     Properties:
       Name: b-C
+Outputs:
+  TestGetAttA:
+    Value: !GetAtt A0Foo.Arn
+  TestGetAttB:
+    Value: !GetAtt BBFoo.Arn

tests/unit/customizations/cloudformation/modules/foreach-module.yaml

@@ -6,3 +6,6 @@ Resources:
     Type: A::B::C
     Properties:
       Name: !Ref Name
+Outputs:
+  Out1:
+    Value: !GetAtt Foo.Arn

tests/unit/customizations/cloudformation/modules/foreach-template.yaml

@@ -13,6 +13,11 @@ Modules:
         Source: foreach-module.yaml
         Properties:
           Name: b-${TheKey}
+Outputs:
+  TestGetAttA:
+    Value: !GetAtt A[0].Out1
+  TestGetAttB:
+    Value: !GetAtt B[B].Out1