Skip to content

Commit 4ed8396

Browse files
committed
feat: support dynamic artifact properties in nested Fn::ForEach blocks
Make detection, build output, and package output recursive so that dynamic artifact properties (e.g., CodeUri: ./services/${Service}) inside nested Fn::ForEach blocks are correctly handled with Mappings transformation. Changes: - Add outer_loops field to DynamicArtifactProperty for tracking enclosing ForEach loops - Make detect_foreach_dynamic_properties() recursive with outer_loops parameter to detect dynamic properties at any nesting depth - Make _update_foreach_artifact_paths() (build) recursive with outer_context parameter; extract helper methods to satisfy ruff PLR0912 branch limit - Make _update_foreach_with_s3_uris() (package) recursive for nested ForEach blocks - Update _generate_artifact_mappings() to support compound keys when property references both outer and inner loop variables - Update _replace_dynamic_artifact_with_findmap() to traverse through outer_loops chain to locate nested ForEach body - Add unit tests, property tests, and integration tests - Update spec (Requirement 25, Tasks 38-43)
1 parent f64177c commit 4ed8396

14 files changed

Lines changed: 979 additions & 93 deletions

File tree

samcli/commands/build/build_context.py

Lines changed: 129 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -497,17 +497,18 @@ def _update_original_template_paths(self, original_template: Dict, modified_temp
497497
original_template["Mappings"].update(all_generated_mappings)
498498

499499
def _update_foreach_artifact_paths(
500-
self, foreach_key: str, foreach_value: list, modified_resources: Dict, original_dir
500+
self,
501+
foreach_key: str,
502+
foreach_value: list,
503+
modified_resources: Dict,
504+
original_dir,
505+
outer_context: Optional[List[Tuple[str, List[str]]]] = None,
501506
) -> Dict[str, Dict[str, Dict[str, str]]]:
502507
"""
503508
Update artifact paths in a Fn::ForEach construct.
504509
505-
For Fn::ForEach with static CodeUri, all generated functions share the same
506-
artifact path. We find one of the expanded functions and use its path.
507-
508-
For Fn::ForEach with dynamic CodeUri (containing the loop variable), we generate
509-
a Mappings section that maps each collection value to its expanded resource's
510-
build artifact path, and replace the dynamic property with Fn::FindInMap.
510+
Recurses into nested Fn::ForEach blocks, passing outer loop context so that
511+
expanded resource names can be fully resolved.
511512
512513
Parameters
513514
----------
@@ -519,6 +520,8 @@ def _update_foreach_artifact_paths(
519520
The expanded resources with updated artifact paths
520521
original_dir : pathlib.Path
521522
The directory containing the original template
523+
outer_context : list of tuples, optional
524+
Enclosing loop variables and their collections for nested ForEach.
522525
523526
Returns
524527
-------
@@ -529,92 +532,160 @@ def _update_foreach_artifact_paths(
529532

530533
generated_mappings: Dict[str, Dict[str, Dict[str, str]]] = {}
531534

535+
if outer_context is None:
536+
outer_context = []
537+
532538
if not isinstance(foreach_value, list) or len(foreach_value) < FOREACH_REQUIRED_ELEMENTS:
533539
return generated_mappings
534540

535-
# Fn::ForEach structure: [loop_var, collection, body]
536541
loop_variable = foreach_value[0]
537542
collection = foreach_value[1]
538543
body = foreach_value[2]
539544

540545
if not isinstance(loop_variable, str) or not isinstance(body, dict):
541546
return generated_mappings
542547

543-
# Resolve collection values
544548
collection_values: List[str] = []
545549
if isinstance(collection, list):
546550
collection_values = [str(item) for item in collection if item is not None]
547551

548-
# Extract loop name from foreach_key
549552
loop_name = foreach_key.replace("Fn::ForEach::", "")
553+
current_outer_context = outer_context + [(loop_variable, collection_values)]
550554

551-
# The body contains resource definitions with ${loop_var} placeholders
552555
for resource_template_key, resource_template in body.items():
556+
if isinstance(resource_template_key, str) and resource_template_key.startswith("Fn::ForEach::"):
557+
nested_mappings = self._update_foreach_artifact_paths(
558+
resource_template_key,
559+
resource_template,
560+
modified_resources,
561+
original_dir,
562+
outer_context=current_outer_context,
563+
)
564+
generated_mappings.update(nested_mappings)
565+
continue
566+
553567
if not isinstance(resource_template, dict):
554568
continue
555569

556570
resource_type = resource_template.get("Type", "")
557571
properties = resource_template.get("Properties", {})
558-
559572
if not isinstance(properties, dict):
560573
continue
561574

562-
# Check if this is a packageable resource type
563-
artifact_props = PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.get(resource_type, [])
564-
565-
for prop_name in artifact_props:
575+
for prop_name in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.get(resource_type, []):
566576
prop_value = properties.get(prop_name)
567577
if prop_value is None:
568578
continue
569579

570-
is_dynamic = self._contains_loop_variable(prop_value, loop_variable)
580+
if self._contains_loop_variable(prop_value, loop_variable) and collection_values:
581+
mapping_entries = self._collect_dynamic_mapping_entries(
582+
resource_template_key,
583+
prop_name,
584+
loop_variable,
585+
collection_values,
586+
modified_resources,
587+
outer_context,
588+
)
589+
if mapping_entries:
590+
mapping_name = f"SAM{prop_name}{loop_name}"
591+
generated_mappings[mapping_name] = mapping_entries
592+
properties[prop_name] = {"Fn::FindInMap": [mapping_name, {"Ref": loop_variable}, prop_name]}
593+
else:
594+
self._copy_static_artifact_property(properties, prop_name, resource_type, modified_resources)
595+
596+
return generated_mappings
597+
598+
def _collect_dynamic_mapping_entries(
599+
self,
600+
resource_template_key: str,
601+
prop_name: str,
602+
loop_variable: str,
603+
collection_values: List[str],
604+
modified_resources: Dict,
605+
outer_context: List[Tuple[str, List[str]]],
606+
) -> Dict[str, Dict[str, str]]:
607+
"""
608+
Collect Mapping entries for a dynamic artifact property by looking up
609+
expanded resources in modified_resources.
610+
611+
For nested ForEach, enumerates all outer value combinations to find
612+
the fully-expanded resource name.
613+
"""
614+
mapping_entries: Dict[str, Dict[str, str]] = {}
615+
616+
for coll_value in collection_values:
617+
if outer_context:
618+
self._collect_nested_mapping_entry(
619+
resource_template_key,
620+
prop_name,
621+
loop_variable,
622+
coll_value,
623+
modified_resources,
624+
outer_context,
625+
mapping_entries,
626+
)
627+
else:
628+
expanded_key = self._substitute_loop_variable(resource_template_key, loop_variable, coll_value)
629+
artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name)
630+
if artifact_value is not None:
631+
mapping_entries[coll_value] = {prop_name: artifact_value}
571632

572-
if is_dynamic and collection_values:
573-
# Dynamic artifact property — generate Mappings
574-
mapping_name = f"SAM{prop_name}{loop_name}"
575-
mapping_entries: Dict[str, Dict[str, str]] = {}
633+
return mapping_entries
576634

577-
for coll_value in collection_values:
578-
# Find the expanded resource name
579-
expanded_key = self._substitute_loop_variable(resource_template_key, loop_variable, coll_value)
580-
modified_resource = modified_resources.get(expanded_key, {})
581-
if not isinstance(modified_resource, dict):
582-
continue
635+
def _collect_nested_mapping_entry(
636+
self,
637+
resource_template_key: str,
638+
prop_name: str,
639+
loop_variable: str,
640+
coll_value: str,
641+
modified_resources: Dict,
642+
outer_context: List[Tuple[str, List[str]]],
643+
mapping_entries: Dict[str, Dict[str, str]],
644+
) -> None:
645+
"""Enumerate outer value combinations to find expanded resource for a nested ForEach."""
646+
import itertools
583647

584-
modified_props = modified_resource.get("Properties", {})
585-
if not isinstance(modified_props, dict):
586-
continue
648+
outer_collections = [oc[1] for oc in outer_context]
649+
outer_vars = [oc[0] for oc in outer_context]
587650

588-
artifact_value = modified_props.get(prop_name)
589-
if artifact_value is not None:
590-
mapping_entries[coll_value] = {prop_name: artifact_value}
651+
for outer_combo in itertools.product(*outer_collections):
652+
expanded_key = resource_template_key
653+
for ovar, oval in zip(outer_vars, outer_combo):
654+
expanded_key = self._substitute_loop_variable(expanded_key, ovar, oval)
655+
expanded_key = self._substitute_loop_variable(expanded_key, loop_variable, coll_value)
591656

592-
if mapping_entries:
593-
generated_mappings[mapping_name] = mapping_entries
594-
# Replace the dynamic property with Fn::FindInMap
595-
# Use {"Ref": loop_variable} so ForEach substitutes the collection value
596-
# into the FindInMap lookup (bare ${Var} strings are NOT resolved by
597-
# ForEach inside FindInMap arguments)
598-
properties[prop_name] = {
599-
"Fn::FindInMap": [
600-
mapping_name,
601-
{"Ref": loop_variable},
602-
prop_name,
603-
]
604-
}
605-
else:
606-
# Static artifact property — copy from first matching expanded resource
607-
for modified_key, modified_resource in modified_resources.items():
608-
if not isinstance(modified_resource, dict):
609-
continue
610-
if modified_resource.get("Type", "") != resource_type:
611-
continue
612-
modified_props = modified_resource.get("Properties", {})
613-
if prop_name in modified_props:
614-
properties[prop_name] = modified_props[prop_name]
615-
break
657+
artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name)
658+
if artifact_value is not None and coll_value not in mapping_entries:
659+
mapping_entries[coll_value] = {prop_name: artifact_value}
616660

617-
return generated_mappings
661+
@staticmethod
662+
def _get_artifact_value(modified_resources: Dict, expanded_key: str, prop_name: str) -> Optional[Any]:
663+
"""Extract an artifact property value from an expanded resource, or return None."""
664+
modified_resource = modified_resources.get(expanded_key, {})
665+
if not isinstance(modified_resource, dict):
666+
return None
667+
modified_props = modified_resource.get("Properties", {})
668+
if not isinstance(modified_props, dict):
669+
return None
670+
return modified_props.get(prop_name)
671+
672+
@staticmethod
673+
def _copy_static_artifact_property(
674+
properties: Dict,
675+
prop_name: str,
676+
resource_type: str,
677+
modified_resources: Dict,
678+
) -> None:
679+
"""Copy a static artifact property from the first matching expanded resource."""
680+
for modified_resource in modified_resources.values():
681+
if not isinstance(modified_resource, dict):
682+
continue
683+
if modified_resource.get("Type", "") != resource_type:
684+
continue
685+
modified_props = modified_resource.get("Properties", {})
686+
if prop_name in modified_props:
687+
properties[prop_name] = modified_props[prop_name]
688+
break
618689

619690
@staticmethod
620691
def _contains_loop_variable(value: Any, loop_variable: str) -> bool:

0 commit comments

Comments
 (0)