From a882ae31f533afbbbe2fcff208dc8b71effb6ebc Mon Sep 17 00:00:00 2001 From: Harold Sun Date: Mon, 18 May 2026 23:58:39 -0700 Subject: [PATCH 1/5] fix(cfn-lang-ext): preserve inline-source intrinsics through LE-aware build merge (#9029) Under `Transform: AWS::LanguageExtensions`, `sam build` was overwriting `Code: {ZipFile: !Sub ...}` with the LE-resolved value, baking default pseudo-parameter substitutions (`us-east-1`, `123456789012`) into the built template. The same merge path covered three sites; gate all three on a single artifacts-lookup primitive matching the non-LE `ApplicationBuilder.update_template` early-skip: - Root-level merge (`_update_original_template_paths`): skip resources whose `get_full_path(stack_path, key)` is absent from `artifacts`. - ForEach static branch (`_merge_static_artifact_path`): same check per expanded key. - ForEach dynamic branch (`_collect_dynamic_mapping_entries` + nested): same check; reject with `UserException` naming the non-buildable property when no iteration produced an artifact, since Mappings hold only static strings and the loop variable cannot survive deploy. Adds case A/B/C integration tests + testdata templates and unit tests covering the new skip and refusal paths. --- samcli/commands/build/build_context.py | 203 +++++++++++++++--- .../test_build_cmd_language_extensions.py | 100 +++++++++ .../template.yaml | 30 +++ .../template.yaml | 32 +++ .../template.yaml | 26 +++ .../commands/buildcmd/test_build_context.py | 107 +++++++-- .../test_build_context_language_extensions.py | 198 ++++++++++++++++- 7 files changed, 641 insertions(+), 55 deletions(-) create mode 100644 tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml create mode 100644 tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-static/template.yaml create mode 100644 tests/integration/testdata/buildcmd/language-extensions-zipfile-fnsub/template.yaml diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 1c6355b030c..30d3c4c8619 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -417,11 +417,19 @@ def _handle_build_post_processing(self, builder: ApplicationBuilder, build_resul # Determine which template to write to disk # If the stack has an original template (with Fn::ForEach intact), use it # Otherwise, use the modified (expanded) template - template_to_write = self._get_template_for_output(stack, modified_template, artifacts) + template_to_write = self._get_template_for_output( + stack, modified_template, artifacts, stack_output_template_path_by_stack_path + ) move_template(stack.location, output_template_path, template_to_write) - def _get_template_for_output(self, stack: Stack, modified_template: Dict, artifacts: Dict[str, str]) -> Dict: + def _get_template_for_output( + self, + stack: Stack, + modified_template: Dict, + artifacts: Dict[str, str], + stack_output_template_path_by_stack_path: Optional[Dict[str, str]] = None, + ) -> Dict: """ Get the template to write to the build output directory. @@ -438,6 +446,8 @@ def _get_template_for_output(self, stack: Stack, modified_template: Dict, artifa The expanded template with updated artifact paths artifacts : Dict[str, str] Map of resource full paths to their built artifact locations + stack_output_template_path_by_stack_path : Optional[Dict[str, str]] + Map of stack paths to their output template paths (for nested stacks). Returns ------- @@ -454,7 +464,13 @@ def _get_template_for_output(self, stack: Stack, modified_template: Dict, artifa original_template: Dict = copy.deepcopy(stack.original_template_dict) # Update artifact paths in the original template - self._update_original_template_paths(original_template, modified_template, stack) + self._update_original_template_paths( + original_template, + modified_template, + stack, + artifacts, + stack_output_template_path_by_stack_path or {}, + ) # Propagate the auto dependency layer nested stack resource from the expanded template # into the original template. This resource is a regular AWS::CloudFormation::Stack type @@ -465,7 +481,14 @@ def _get_template_for_output(self, stack: Stack, modified_template: Dict, artifa return original_template - def _update_original_template_paths(self, original_template: Dict, modified_template: Dict, stack: Stack) -> None: + def _update_original_template_paths( + self, + original_template: Dict, + modified_template: Dict, + stack: Stack, + artifacts: Optional[Dict[str, str]] = None, + stack_output_template_path_by_stack_path: Optional[Dict[str, str]] = None, + ) -> None: """ Update artifact paths in the original template based on the modified template. @@ -481,7 +504,20 @@ def _update_original_template_paths(self, original_template: Dict, modified_temp The expanded template with updated artifact paths stack : Stack The stack being processed + artifacts : Optional[Dict[str, str]] + Map of resource full paths to built artifact locations. Used to + skip merge for resources that produced no build artifact (e.g. + inline-source ``ZipFile`` Lambdas). Mirrors the non-LE + ``ApplicationBuilder.update_template`` skip. See #9029. + stack_output_template_path_by_stack_path : Optional[Dict[str, str]] + Map of stack paths to their output template paths (for nested stacks). """ + from samcli.lib.providers.provider import get_full_path + + artifacts = artifacts if artifacts is not None else {} + stack_outputs = stack_output_template_path_by_stack_path or {} + stack_path = stack.stack_path if isinstance(stack.stack_path, str) else "" + # Get the resources section from both templates original_resources = original_template.get("Resources", {}) modified_resources = modified_template.get("Resources", {}) @@ -499,9 +535,19 @@ def _update_original_template_paths(self, original_template: Dict, modified_temp modified_resources, template=original_template, parameter_values=stack.parameters, + artifacts=artifacts, + stack_path=stack_path, ) all_generated_mappings.update(generated_mappings) elif isinstance(resource_value, dict) and resource_key in modified_resources: + # Skip resources that produced no build artifact and are not nested + # stacks — there is nothing to merge back. Mirrors the non-LE + # ``ApplicationBuilder.update_template`` early-skip and prevents + # the LE-resolved value from clobbering user intrinsics on, e.g., + # ``Code: {ZipFile: !Sub ...}`` Lambdas. See #9029. + full_path = get_full_path(stack_path, resource_key) + if full_path not in artifacts and full_path not in stack_outputs: + continue # Regular resource - copy updated paths from modified template modified_resource = modified_resources.get(resource_key, {}) self._copy_artifact_paths(resource_value, modified_resource) @@ -521,6 +567,8 @@ def _update_foreach_artifact_paths( template: Optional[Dict] = None, parameter_values: Optional[Dict] = None, parent_nesting_path: str = "", + artifacts: Optional[Dict[str, str]] = None, + stack_path: str = "", ) -> Dict[str, Dict[str, Dict[str, str]]]: """ Update artifact paths in a Fn::ForEach construct. @@ -545,6 +593,13 @@ def _update_foreach_artifact_paths( parent_nesting_path : str Accumulated nesting path from parent ForEach loops (e.g., "Envs" when nested under Fn::ForEach::Envs). + artifacts : Optional[Dict[str, str]] + Map of resource full paths to built artifact locations. Used to skip + the static-branch merge for expanded resources that produced no build + artifact (e.g. inline-source ``ZipFile`` Lambdas inside a ForEach body). + Mirrors the non-LE early-skip. See #9029. + stack_path : str + The owning stack's path (for computing the ``full_path`` lookup key). Returns ------- @@ -560,6 +615,8 @@ def _update_foreach_artifact_paths( _set_prop_value, ) + artifacts = artifacts if artifacts is not None else {} + generated_mappings: Dict[str, Dict[str, Dict[str, str]]] = {} if outer_context is None: @@ -593,6 +650,8 @@ def _update_foreach_artifact_paths( template=template, parameter_values=parameter_values, parent_nesting_path=nesting_path, + artifacts=artifacts, + stack_path=stack_path, ) generated_mappings.update(nested_mappings) continue @@ -632,39 +691,53 @@ def _update_foreach_artifact_paths( modified_resources, outer_context, referenced_outer_vars=referenced_outer_vars, + artifacts=artifacts, + stack_path=stack_path, ) - if mapping_entries: - mapping_name = f"SAM{leaf_name}{nesting_path}" - # Collision count is keyed on the leaf so dotted paths - # that share a leaf (e.g. "ImageUri" vs "Code.ImageUri") - # get the resource-key suffix and don't overwrite each - # other's Mapping entries. - if dynamic_props_count.get(leaf_name, 0) > 1: - suffix = sanitize_resource_key_for_mapping(resource_template_key) - mapping_name = f"{mapping_name}{suffix}" - generated_mappings[mapping_name] = mapping_entries - - lookup_key: Any - if referenced_outer_vars: - # Compound key: join outer + inner variable refs with "-" - ref_parts = [{"Ref": ovar} for ovar, _ in referenced_outer_vars] - ref_parts.append({"Ref": loop_variable}) - lookup_key = {"Fn::Join": ["-", ref_parts]} - else: - lookup_key = {"Ref": loop_variable} - - _set_prop_value(properties, prop_name, {"Fn::FindInMap": [mapping_name, lookup_key, leaf_name]}) + if not mapping_entries: + # No per-iteration expanded resource produced a build + # artifact — the property is non-packageable for every + # iteration (e.g. ``Code: {ZipFile: ...}`` Lambdas have + # no ``CodeUri`` to build). Mappings hold only static + # strings, so deploy-time intrinsics like ``Fn::Sub`` + # cannot be expressed per-iteration; leaving the loop + # variable in the original would fail at deploy time + # (the variable isn't a CFN parameter). Reject. Mirrors + # the artifacts-lookup discipline of the static branch + # and the root-level merge. See #9029. + self._raise_dynamic_no_artifacts_unsupported(resource_template_key, loop_variable, leaf_name) + mapping_name = f"SAM{leaf_name}{nesting_path}" + # Collision count is keyed on the leaf so dotted paths + # that share a leaf (e.g. "ImageUri" vs "Code.ImageUri") + # get the resource-key suffix and don't overwrite each + # other's Mapping entries. + if dynamic_props_count.get(leaf_name, 0) > 1: + suffix = sanitize_resource_key_for_mapping(resource_template_key) + mapping_name = f"{mapping_name}{suffix}" + generated_mappings[mapping_name] = mapping_entries + + lookup_key: Any + if referenced_outer_vars: + # Compound key: join outer + inner variable refs with "-" + ref_parts = [{"Ref": ovar} for ovar, _ in referenced_outer_vars] + ref_parts.append({"Ref": loop_variable}) + lookup_key = {"Fn::Join": ["-", ref_parts]} + else: + lookup_key = {"Ref": loop_variable} + + _set_prop_value(properties, prop_name, {"Fn::FindInMap": [mapping_name, lookup_key, leaf_name]}) else: - expanded_key = self._build_expanded_key( + self._merge_static_artifact_path( resource_template_key, loop_variable, collection_values, outer_context, + modified_resources, + properties, + prop_name, + artifacts, + stack_path, ) - if expanded_key: - artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) - if artifact_value is not None: - _set_prop_value(properties, prop_name, artifact_value) # Propagate auto dependency layer references from expanded functions # to the ForEach body. Each expanded function may have Layers entries @@ -711,6 +784,54 @@ def _update_foreach_artifact_paths( return generated_mappings + def _merge_static_artifact_path( + self, + resource_template_key: str, + loop_variable: str, + collection_values: List[str], + outer_context: List[Tuple[str, List[str]]], + modified_resources: Dict, + properties: Dict, + prop_name: str, + artifacts: Dict[str, str], + stack_path: str, + ) -> None: + """Static-property merge inside a Fn::ForEach body. Skips resources that + produced no build artifact — mirrors the non-LE + ``ApplicationBuilder.update_template`` early-skip so that, e.g., + ``Code: {ZipFile: !Sub ...}`` Lambdas survive verbatim. See #9029. + """ + from samcli.lib.package.language_extensions_packaging import _set_prop_value + from samcli.lib.providers.provider import get_full_path + + expanded_key = self._build_expanded_key( + resource_template_key, + loop_variable, + collection_values, + outer_context, + ) + if not expanded_key: + return + if get_full_path(stack_path, expanded_key) not in artifacts: + return + artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) + if artifact_value is not None: + _set_prop_value(properties, prop_name, artifact_value) + + @staticmethod + def _raise_dynamic_no_artifacts_unsupported(resource_template_key: str, loop_variable: str, leaf_name: str) -> None: + """Refuse a Fn::ForEach body whose dynamic artifact property produced no + build artifacts for any iteration (e.g. an inline-source ``Code: {ZipFile: + ...}`` Lambda has no ``CodeUri`` to build). Mappings hold only static + strings, so we cannot emit a per-iteration value that preserves deploy-time + intrinsics, and we cannot leave the loop variable unresolved. See #9029. + """ + raise UserException( + f"Resource '{resource_template_key}' uses Fn::ForEach loop variable " + f"'{loop_variable}' in non-buildable property '{leaf_name}'. " + f"This is not supported." + ) + @staticmethod def _count_dynamic_properties( body: Dict, @@ -776,6 +897,8 @@ def _collect_dynamic_mapping_entries( modified_resources: Dict, outer_context: List[Tuple[str, List[str]]], referenced_outer_vars: Optional[List[Tuple[str, List[str]]]] = None, + artifacts: Optional[Dict[str, str]] = None, + stack_path: str = "", ) -> Dict[str, Dict[str, str]]: """ Collect Mapping entries for a dynamic artifact property by looking up @@ -787,11 +910,18 @@ def _collect_dynamic_mapping_entries( ``prop_name`` may be a jmespath dotted path; the value-dict key stored in the Mapping uses the leaf segment so it stays alphanumeric and matches the third argument of the generated Fn::FindInMap. + + ``artifacts`` filters per-iteration expanded keys to only those that + produced a build artifact — same discipline as the static branch and + the root-level merge. Iterations whose ``get_full_path(stack_path, + expanded_key)`` is absent from ``artifacts`` are skipped. See #9029. """ from samcli.lib.package.language_extensions_packaging import _leaf_prop_name + from samcli.lib.providers.provider import get_full_path mapping_entries: Dict[str, Dict[str, str]] = {} leaf_name = _leaf_prop_name(prop_name) + artifacts = artifacts if artifacts is not None else {} for coll_value in collection_values: if outer_context: @@ -804,9 +934,13 @@ def _collect_dynamic_mapping_entries( outer_context, mapping_entries, referenced_outer_vars=referenced_outer_vars, + artifacts=artifacts, + stack_path=stack_path, ) else: expanded_key = substitute_loop_variable(resource_template_key, loop_variable, coll_value) + if get_full_path(stack_path, expanded_key) not in artifacts: + continue artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) if artifact_value is not None: mapping_entries[coll_value] = {leaf_name: artifact_value} @@ -823,18 +957,25 @@ def _collect_nested_mapping_entry( outer_context: List[Tuple[str, List[str]]], mapping_entries: Dict[str, Dict[str, str]], referenced_outer_vars: Optional[List[Tuple[str, List[str]]]] = None, + artifacts: Optional[Dict[str, str]] = None, + stack_path: str = "", ) -> None: """Enumerate outer value combinations to find expanded resource for a nested ForEach. ``prop_name`` may be a jmespath dotted path; the value-dict key uses the leaf segment. + + ``artifacts`` filters per-iteration expanded keys (see + ``_collect_dynamic_mapping_entries``). """ import itertools from samcli.lib.package.language_extensions_packaging import _leaf_prop_name + from samcli.lib.providers.provider import get_full_path leaf_name = _leaf_prop_name(prop_name) outer_collections = [oc[1] for oc in outer_context] outer_vars = [oc[0] for oc in outer_context] + artifacts = artifacts if artifacts is not None else {} # Determine which outer vars need compound keys compound_outer_vars = {ovar for ovar, _ in (referenced_outer_vars or [])} @@ -845,6 +986,8 @@ def _collect_nested_mapping_entry( expanded_key = substitute_loop_variable(expanded_key, ovar, oval) expanded_key = substitute_loop_variable(expanded_key, loop_variable, coll_value) + if get_full_path(stack_path, expanded_key) not in artifacts: + continue artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) if artifact_value is None: continue diff --git a/tests/integration/buildcmd/test_build_cmd_language_extensions.py b/tests/integration/buildcmd/test_build_cmd_language_extensions.py index 829c9a2c576..a71a91a55ff 100644 --- a/tests/integration/buildcmd/test_build_cmd_language_extensions.py +++ b/tests/integration/buildcmd/test_build_cmd_language_extensions.py @@ -111,6 +111,106 @@ def test_build_dynamic_codeuri_generates_mappings(self): self.assertEqual(codeuri["Fn::FindInMap"][0], mapping_name) self.assertEqual(codeuri["Fn::FindInMap"][1], {"Ref": "FunctionName"}) + def test_build_zipfile_fnsub_preserves_intrinsic(self): + """Regression for #9029: `Code.ZipFile: !Sub ...` on a Lambda function under + AWS::LanguageExtensions must round-trip through `sam build` with the Fn::Sub + intact. Before the fix, the LE-aware merge step copied the LE-resolved value + back, baking default pseudo-parameter values (us-east-1, 123456789012) into + the built template.""" + self.template_path = str(Path(self.test_data_path, "language-extensions-zipfile-fnsub", "template.yaml")) + + cmdlist = self.get_command_list() + command_result = run_command(cmdlist, cwd=self.working_dir) + + self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}") + + built_template_path = self.default_build_dir.joinpath("template.yaml") + self.assertTrue(built_template_path.exists()) + + with open(built_template_path, "r") as f: + built_template = yaml.safe_load(f) + + function_props = built_template["Resources"]["MyTriggerFunction"]["Properties"] + + # Code.ZipFile must remain a dict containing Fn::Sub, not a resolved string. + zipfile_value = function_props["Code"]["ZipFile"] + self.assertIsInstance( + zipfile_value, dict, f"Expected Code.ZipFile to remain a Fn::Sub dict, got: {zipfile_value!r}" + ) + self.assertIn("Fn::Sub", zipfile_value) + sub_body = zipfile_value["Fn::Sub"] + self.assertIn("${AWS::Region}", sub_body) + self.assertIn("${AWS::AccountId}", sub_body) + # Confirm no default pseudo-param values leaked through. + self.assertNotIn("us-east-1", sub_body) + self.assertNotIn("123456789012", sub_body) + + # Role's Fn::Sub should likewise be preserved (sanity check on the same merge path). + role_value = function_props["Role"] + self.assertIsInstance(role_value, dict) + self.assertIn("Fn::Sub", role_value) + self.assertIn("${AWS::AccountId}", role_value["Fn::Sub"]) + + def test_build_foreach_static_zipfile_fnsub_preserves_intrinsic(self): + """Regression for #9029 (ForEach static branch / case B): inside a + Fn::ForEach body, `Code.ZipFile: !Sub ...` whose body does NOT reference + the loop variable must round-trip through `sam build` with the Fn::Sub + intact. The static-branch merge has no build artifact for an inline-source + Lambda, so the user-authored property must be preserved verbatim.""" + self.template_path = str( + Path(self.test_data_path, "language-extensions-foreach-zipfile-static", "template.yaml") + ) + + cmdlist = self.get_command_list() + command_result = run_command(cmdlist, cwd=self.working_dir) + + self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}") + + built_template_path = self.default_build_dir.joinpath("template.yaml") + self.assertTrue(built_template_path.exists()) + + with open(built_template_path, "r") as f: + built_template = yaml.safe_load(f) + + # Fn::ForEach structure must be preserved + resources = built_template.get("Resources", {}) + foreach_block = resources.get("Fn::ForEach::Workers") + self.assertIsNotNone(foreach_block, "Fn::ForEach::Workers must survive in built template") + self.assertEqual(foreach_block[0], "WorkerName") + self.assertEqual(foreach_block[1], ["Alpha", "Beta"]) + + # The body's Code.ZipFile must remain a Fn::Sub dict, not a resolved string, + # and must still reference ${AWS::Region} / ${AWS::AccountId}. + body = foreach_block[2] + worker_props = body["${WorkerName}Worker"]["Properties"] + zipfile_value = worker_props["Code"]["ZipFile"] + self.assertIsInstance(zipfile_value, dict) + self.assertIn("Fn::Sub", zipfile_value) + sub_body = zipfile_value["Fn::Sub"] + self.assertIn("${AWS::Region}", sub_body) + self.assertIn("${AWS::AccountId}", sub_body) + self.assertNotIn("us-east-1", sub_body) + self.assertNotIn("123456789012", sub_body) + + def test_build_foreach_dynamic_non_buildable_property_rejected(self): + """Regression for #9029 (ForEach dynamic branch / case C): inside a + Fn::ForEach body, a property that references the loop variable but produces + no build artifact for any iteration (e.g. inline-source `Code.ZipFile`) + cannot be expressed as a per-iteration CloudFormation Mapping. `sam build` + must reject the template with InvalidTemplateException naming the + non-buildable property.""" + self.template_path = str( + Path(self.test_data_path, "language-extensions-foreach-zipfile-dynamic", "template.yaml") + ) + + cmdlist = self.get_command_list() + command_result = run_command(cmdlist, cwd=self.working_dir) + + self.assertNotEqual(command_result.process.returncode, 0, "Build should have failed") + stderr = command_result.stderr.decode("utf-8") + self.assertIn("non-buildable property", stderr) + self.assertIn("WorkerName", stderr) + def test_build_nested_foreach_dynamic_codeuri_generates_mappings(self): """Test that nested Fn::ForEach with dynamic CodeUri generates Mappings.""" self.template_path = str( diff --git a/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml new file mode 100644 index 00000000000..c5164b39799 --- /dev/null +++ b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml @@ -0,0 +1,30 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: + - AWS::LanguageExtensions + - AWS::Serverless-2016-10-31 + +Description: > + Regression test for #9029, ForEach dynamic-branch (case C). Each expanded + Lambda has `Code: {ZipFile: ...}` whose body references the loop variable. + The dynamic-branch merge cannot express a per-iteration ZipFile via a + CloudFormation Mapping (Mappings hold only static strings) and there is no + build artifact to fall back on, so `sam build` must reject the template + with InvalidTemplateException naming the non-buildable property. + +Resources: + Fn::ForEach::Workers: + - WorkerName + - - Alpha + - Beta + - ${WorkerName}Worker: + Type: AWS::Lambda::Function + Properties: + FunctionName: !Sub ${WorkerName}-worker + Runtime: python3.11 + Handler: index.handler + Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/worker-role' + Code: + ZipFile: !Sub | + import boto3 + def handler(event, context): + print('worker name: ${WorkerName}') diff --git a/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-static/template.yaml b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-static/template.yaml new file mode 100644 index 00000000000..48d7630e90f --- /dev/null +++ b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-static/template.yaml @@ -0,0 +1,32 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: + - AWS::LanguageExtensions + - AWS::Serverless-2016-10-31 + +Description: > + Regression test for #9029, ForEach static-branch (case B). Each expanded + Lambda has `Code: {ZipFile: !Sub ...}` whose body references CFN pseudo- + parameters but NOT the loop variable. The static-branch merge must skip + these resources (no build artifact) so the user-authored `Fn::Sub` + survives intact in the built template. + +Resources: + Fn::ForEach::Workers: + - WorkerName + - - Alpha + - Beta + - ${WorkerName}Worker: + Type: AWS::Lambda::Function + Properties: + FunctionName: !Sub ${WorkerName}-worker + Runtime: python3.11 + Handler: index.handler + Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/worker-role' + Code: + ZipFile: !Sub | + import boto3 + def handler(event, context): + client = boto3.client('logs') + client.put_log_events( + logGroupName='/aws/lambda/worker-${AWS::Region}-${AWS::AccountId}', + ) diff --git a/tests/integration/testdata/buildcmd/language-extensions-zipfile-fnsub/template.yaml b/tests/integration/testdata/buildcmd/language-extensions-zipfile-fnsub/template.yaml new file mode 100644 index 00000000000..b08507947b1 --- /dev/null +++ b/tests/integration/testdata/buildcmd/language-extensions-zipfile-fnsub/template.yaml @@ -0,0 +1,26 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: + - AWS::LanguageExtensions + - AWS::Serverless-2016-10-31 + +Description: > + Regression test for #9029. Inline-source `Code: {ZipFile: !Sub ...}` on a + Lambda function under AWS::LanguageExtensions must round-trip through + `sam build` with the `Fn::Sub` intact (no pseudo-parameter resolution). + +Resources: + MyTriggerFunction: + Type: AWS::Lambda::Function + Properties: + FunctionName: my-trigger + Runtime: python3.11 + Handler: index.handler + Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/my-role' + Code: + ZipFile: !Sub | + import boto3 + def handler(event, context): + client = boto3.client('stepfunctions') + client.start_execution( + stateMachineArn='arn:aws:states:${AWS::Region}:${AWS::AccountId}:stateMachine:my-state-machine', + ) diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index 657bb0cb9e2..af6c5793e09 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -1,6 +1,6 @@ import os from unittest import TestCase -from unittest.mock import MagicMock, patch, Mock, ANY, call +from unittest.mock import ANY, MagicMock, Mock, call, patch from parameterized import parameterized @@ -9,10 +9,10 @@ from samcli.commands.build.utils import MountMode from samcli.commands.exceptions import UserException from samcli.lib.build.app_builder import ( + ApplicationBuildResult, BuildError, - UnsupportedBuilderLibraryVersionError, BuildInsideContainerError, - ApplicationBuildResult, + UnsupportedBuilderLibraryVersionError, ) from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR from samcli.lib.build.bundler import EsbuildBundlerManager @@ -20,9 +20,8 @@ from samcli.lib.providers.provider import Function, get_function_build_info from samcli.lib.telemetry.event import EventName, UsedFeature from samcli.lib.utils.osutils import BUILD_DIR_PERMISSIONS -from samcli.lib.utils.packagetype import ZIP, IMAGE -from samcli.local.lambdafn.exceptions import FunctionNotFound -from samcli.local.lambdafn.exceptions import ResourceNotFound +from samcli.lib.utils.packagetype import IMAGE, ZIP +from samcli.local.lambdafn.exceptions import FunctionNotFound, ResourceNotFound class DeepWrap(Exception): @@ -1502,6 +1501,7 @@ def test_returns_original_template_when_present(self): """When stack has original_template_dict, return a copy of it with updated paths.""" stack = Mock() stack.location = "/path/to/template.yaml" + stack.stack_path = "" original_template = { "Resources": { "Fn::ForEach::Functions": [ @@ -1538,7 +1538,7 @@ def test_returns_original_template_when_present(self): }, } } - artifacts = {} + artifacts = {"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"} result = self.build_context._get_template_for_output(stack, modified_template, artifacts) @@ -1555,6 +1555,7 @@ def test_original_template_is_deep_copied(self): """Ensure the original template is deep copied and not modified in place.""" stack = Mock() stack.location = "/path/to/template.yaml" + stack.stack_path = "" original_template = { "Resources": { "Function": { @@ -1576,7 +1577,8 @@ def test_original_template_is_deep_copied(self): } } } - artifacts = {} + # Function was built — ensure the merge is not skipped by the #9029 guard. + artifacts = {"Function": "/path/to/built/Function"} result = self.build_context._get_template_for_output(stack, modified_template, artifacts) @@ -1589,6 +1591,7 @@ def test_dynamic_codeuri_generates_mappings_in_output(self): """When Fn::ForEach has dynamic CodeUri, the output template should have Mappings.""" stack = Mock() stack.location = "/path/to/template.yaml" + stack.stack_path = "" original_template = { "Resources": { "Fn::ForEach::Functions": [ @@ -1625,7 +1628,7 @@ def test_dynamic_codeuri_generates_mappings_in_output(self): }, } } - artifacts = {} + artifacts = {"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"} result = self.build_context._get_template_for_output(stack, modified_template, artifacts) @@ -1691,7 +1694,12 @@ def test_updates_static_codeuri_in_foreach_body(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"}, + ) # Static CodeUri: should be updated to first matching expanded resource's path self.assertEqual(foreach_value[2]["${Name}Function"]["Properties"]["CodeUri"], "../build/AlphaFunction") @@ -1732,7 +1740,12 @@ def test_dynamic_codeuri_generates_mappings(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"}, + ) # Mappings should be generated self.assertIn("SAMCodeUriFunctions", mappings) @@ -1788,7 +1801,12 @@ def test_handles_layer_resources(self): } } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"Layer1Layer": "/built/Layer1Layer", "Layer2Layer": "/built/Layer2Layer"}, + ) self.assertEqual(foreach_value[2]["${Name}Layer"]["Properties"]["ContentUri"], "../build/Layer1Layer") self.assertEqual(mappings, {}) @@ -1827,7 +1845,12 @@ def test_dynamic_codeuri_with_fn_sub(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"UsersService": "/built/UsersService", "OrdersService": "/built/OrdersService"}, + ) self.assertIn("SAMCodeUriServices", mappings) self.assertEqual(mappings["SAMCodeUriServices"]["Users"]["CodeUri"], "UsersService") @@ -1877,6 +1900,7 @@ def test_parameter_ref_collection_resolves_to_values(self): modified_resources, template=template, parameter_values=parameter_values, + artifacts={"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"}, ) # Should resolve the Ref collection and generate mappings @@ -1954,7 +1978,17 @@ def test_nested_foreach_compound_keys_when_property_references_outer_var(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={ + "DevApiFunction": "/built/DevApiFunction", + "DevWorkerFunction": "/built/DevWorkerFunction", + "ProdApiFunction": "/built/ProdApiFunction", + "ProdWorkerFunction": "/built/ProdWorkerFunction", + }, + ) # Mappings should use compound keys (outer-inner) and nesting path self.assertIn("SAMCodeUriEnvsServices", mappings) @@ -2019,7 +2053,17 @@ def test_nested_foreach_simple_keys_when_property_references_inner_only(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={ + "DevApiFunction": "/built/DevApiFunction", + "DevWorkerFunction": "/built/DevWorkerFunction", + "ProdApiFunction": "/built/ProdApiFunction", + "ProdWorkerFunction": "/built/ProdWorkerFunction", + }, + ) # Mappings should use simple keys (inner only) and nesting path self.assertIn("SAMCodeUriEnvsServices", mappings) @@ -2060,7 +2104,12 @@ def test_static_dotted_glue_script_location(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"Etl1Job": "/built/Etl1Job", "Etl2Job": "/built/Etl2Job"}, + ) body = foreach_value[2] self.assertEqual(body["${Name}Job"]["Properties"]["Command"]["ScriptLocation"], "../build/Etl1Job/job.py") @@ -2098,7 +2147,12 @@ def test_dynamic_dotted_glue_script_location_generates_mapping(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"Etl1Job": "/built/Etl1Job", "Etl2Job": "/built/Etl2Job"}, + ) # Mapping name uses the leaf "ScriptLocation", not "Command.ScriptLocation" self.assertIn("SAMScriptLocationJobs", mappings) @@ -2150,7 +2204,12 @@ def test_dynamic_dotted_lambda_image_uri_generates_mapping(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"}, + ) # Mapping name uses leaf "ImageUri", not "Code.ImageUri" self.assertIn("SAMImageUriFuncs", mappings) @@ -2210,7 +2269,17 @@ def test_leaf_collision_across_resource_types_disambiguates_mapping_name(self): }, } - mappings = self.build_context._update_foreach_artifact_paths(foreach_key, foreach_value, modified_resources) + mappings = self.build_context._update_foreach_artifact_paths( + foreach_key, + foreach_value, + modified_resources, + artifacts={ + "AlphaSam": "/built/AlphaSam", + "BetaSam": "/built/BetaSam", + "AlphaLambda": "/built/AlphaLambda", + "BetaLambda": "/built/BetaLambda", + }, + ) # Two distinct Mapping names should exist — one per resource — disambiguated # by the resource-key suffix because both leaf to "ImageUri". diff --git a/tests/unit/commands/buildcmd/test_build_context_language_extensions.py b/tests/unit/commands/buildcmd/test_build_context_language_extensions.py index cf65a021a2b..60e81913036 100644 --- a/tests/unit/commands/buildcmd/test_build_context_language_extensions.py +++ b/tests/unit/commands/buildcmd/test_build_context_language_extensions.py @@ -6,7 +6,7 @@ """ from unittest import TestCase -from unittest.mock import patch, MagicMock, PropertyMock +from unittest.mock import MagicMock, patch from samcli.commands.build.build_context import BuildContext @@ -35,12 +35,13 @@ def test_serverless_function_imageuri(self): ctx._copy_artifact_paths(original, modified) self.assertEqual(original["Properties"]["ImageUri"], "123.dkr.ecr.us-east-1.amazonaws.com/repo") - def test_lambda_function_code(self): + def test_lambda_function_code_packageable(self): + """Packageable Code (S3 location, no ZipFile): merge wins.""" ctx = self._make_context() - original = {"Type": "AWS::Lambda::Function", "Properties": {"Code": {"ZipFile": "code"}}} - modified = {"Type": "AWS::Lambda::Function", "Properties": {"Code": {"S3Bucket": "b", "S3Key": "k"}}} + original = {"Type": "AWS::Lambda::Function", "Properties": {"Code": {"S3Bucket": "old", "S3Key": "k"}}} + modified = {"Type": "AWS::Lambda::Function", "Properties": {"Code": {"S3Bucket": "new", "S3Key": "k"}}} ctx._copy_artifact_paths(original, modified) - self.assertEqual(original["Properties"]["Code"]["S3Bucket"], "b") + self.assertEqual(original["Properties"]["Code"]["S3Bucket"], "new") def test_serverless_layer_contenturi(self): ctx = self._make_context() @@ -107,6 +108,186 @@ def test_glue_job_dotted_path(self): self.assertEqual(original["Properties"]["Command"]["Name"], "glueetl") +class TestUpdateOriginalTemplatePathsZipFile(TestCase): + """#9029: At the root level, _update_original_template_paths must skip resources + that produced no build artifact, mirroring the non-LE + ApplicationBuilder.update_template skip. Inline-source ``ZipFile`` Lambdas have + no CodeUri and are never built, so they must not be overwritten by the LE-resolved + expansion (which strips intrinsics like Fn::Sub against pseudo-parameter defaults). + """ + + def _make_context(self): + with patch.object(BuildContext, "__init__", lambda self: None): + return BuildContext() + + def test_zipfile_resource_not_in_artifacts_is_skipped(self): + """Lambda with Code.ZipFile is not in artifacts (no CodeUri to build); + merge must skip it so the original Fn::Sub survives.""" + ctx = self._make_context() + original_code = {"ZipFile": {"Fn::Sub": "arn:aws:states:${AWS::Region}:..."}} + # LE-expanded template has Fn::Sub already resolved against defaults + modified_code = {"ZipFile": "arn:aws:states:us-east-1:..."} + original_template = { + "Resources": {"MyFunc": {"Type": "AWS::Lambda::Function", "Properties": {"Code": original_code}}} + } + modified_template = { + "Resources": {"MyFunc": {"Type": "AWS::Lambda::Function", "Properties": {"Code": modified_code}}} + } + stack = MagicMock() + stack.parameters = {} + stack.stack_path = "" + + # MyFunc not in artifacts (no CodeUri to build) + ctx._update_original_template_paths( + original_template, + modified_template, + stack, + artifacts={}, + stack_output_template_path_by_stack_path={}, + ) + + self.assertEqual(original_template["Resources"]["MyFunc"]["Properties"]["Code"], original_code) + + def test_packageable_resource_in_artifacts_is_merged(self): + """Lambda with packageable Code is in artifacts; merge proceeds normally.""" + ctx = self._make_context() + original_template = { + "Resources": { + "MyFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"S3Bucket": "old", "S3Key": "k"}}, + } + } + } + modified_template = { + "Resources": { + "MyFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"S3Bucket": "new", "S3Key": "k"}}, + } + } + } + stack = MagicMock() + stack.parameters = {} + stack.stack_path = "" + + ctx._update_original_template_paths( + original_template, + modified_template, + stack, + artifacts={"MyFunc": "/path/to/built/MyFunc"}, + stack_output_template_path_by_stack_path={}, + ) + + self.assertEqual(original_template["Resources"]["MyFunc"]["Properties"]["Code"]["S3Bucket"], "new") + + +class TestForEachStaticArtifactsSkip(TestCase): + """#9029 (case b): A ForEach body whose static property holds inline source + (e.g. ``Code: {ZipFile: ...}``) is never built, so its expanded children are + absent from ``artifacts``. The static-branch merge must skip them so the + original property survives verbatim. + """ + + def _make_context(self): + with patch.object(BuildContext, "__init__", lambda self: None): + return BuildContext() + + def test_static_zipfile_in_foreach_skipped_when_not_in_artifacts(self): + ctx = self._make_context() + original_code = {"ZipFile": {"Fn::Sub": "print('${AWS::Region}')"}} + foreach_value = [ + "Name", + ["Alpha", "Beta"], + { + "${Name}Func": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": original_code}, + } + }, + ] + # Modified resources have the LE-expanded values (us-east-1 baked in) + modified_resources = { + "AlphaFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"ZipFile": "print('us-east-1')"}}, + }, + "BetaFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"ZipFile": "print('us-east-1')"}}, + }, + } + # No artifacts entries — these functions had no CodeUri to build. + ctx._update_foreach_artifact_paths( + "Fn::ForEach::Names", + foreach_value, + modified_resources, + template={}, + parameter_values={}, + artifacts={}, + ) + + # Original Code (with Fn::Sub intact) must be untouched + body = foreach_value[2] + self.assertEqual(body["${Name}Func"]["Properties"]["Code"], original_code) + + +class TestForEachDynamicNoArtifactsRefusal(TestCase): + """#9029: a dynamic Fn::ForEach property whose iterations produced no build + artifacts cannot be expressed as a per-iteration CloudFormation Mapping + (Mappings hold only static strings, so deploy-time intrinsics would be + silently lost) and cannot leave the loop variable unresolved. Mirrors the + artifacts-lookup discipline of the static branch and the root-level merge — + refuse rather than emit a misleading template. + """ + + def _make_context(self): + with patch.object(BuildContext, "__init__", lambda self: None): + return BuildContext() + + def test_dynamic_property_with_no_artifacts_raises(self): + from samcli.commands.exceptions import UserException + + ctx = self._make_context() + foreach_value = [ + "Name", + ["Alpha", "Beta"], + { + "${Name}Func": { + "Type": "AWS::Lambda::Function", + "Properties": { + # ZipFile contains the loop variable; an inline-source + # Lambda has no CodeUri, so neither AlphaFunc nor BetaFunc + # appears in `artifacts`. + "Code": {"ZipFile": {"Fn::Sub": "print('${Name}')"}}, + }, + } + }, + ] + modified_resources = { + "AlphaFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"ZipFile": "print('Alpha')"}}, + }, + "BetaFunc": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"ZipFile": "print('Beta')"}}, + }, + } + with self.assertRaises(UserException) as cm: + ctx._update_foreach_artifact_paths( + "Fn::ForEach::Names", + foreach_value, + modified_resources, + template={}, + parameter_values={}, + artifacts={}, + ) + message = str(cm.exception) + self.assertIn("Name", message) + self.assertIn("non-buildable property", message) + + class TestGetTemplateForOutputForEachExploration(TestCase): """ Bug condition exploration tests for _get_template_for_output with ForEach templates. @@ -543,6 +724,7 @@ def test_build_without_sync_foreach_template_unchanged(self): stack = MagicMock() stack.original_template_dict = original_template stack.parameters = {"FunctionNames": "Alpha,Beta"} + stack.stack_path = "" # Expanded template with built artifact paths expanded_template = { @@ -567,7 +749,11 @@ def test_build_without_sync_foreach_template_unchanged(self): }, } - result = ctx._get_template_for_output(stack, expanded_template, {}) + result = ctx._get_template_for_output( + stack, + expanded_template, + {"AlphaFunction": "/built/AlphaFunction", "BetaFunction": "/built/BetaFunction"}, + ) # Fn::ForEach structure must be preserved self.assertIn("Fn::ForEach::Functions", result.get("Resources", {})) From 6eb395dca44e90df6525a64a78971a45213495a5 Mon Sep 17 00:00:00 2001 From: Harold Sun Date: Tue, 19 May 2026 14:27:48 -0700 Subject: [PATCH 2/5] refactor(cfn-lang-ext): address PR #9031 review feedback - Hoist inline `get_full_path` imports to the top of build_context.py. - Allow inline `Code: {ZipFile: !Sub ...}` inside Fn::ForEach bodies that reference the loop variable: skip the merge instead of raising, since CFN's LanguageExtensions transform expands the body at deploy time and substitutes the loop variable per iteration. Drop the unused `_raise_dynamic_no_artifacts_unsupported` helper and flip the related integration test to assert pass-through preservation. - Stop coercing `Optional[Dict[str, str]]` artifacts to `{}` in functions that only forward the value. Where it's used directly, fold the None guard into the existing membership check (`not artifacts or ...`). `_merge_static_artifact_path` becomes Optional to match its callers. --- samcli/commands/build/build_context.py | 51 ++++--------------- .../test_build_cmd_language_extensions.py | 36 +++++++++---- .../template.yaml | 11 ++-- 3 files changed, 44 insertions(+), 54 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 30d3c4c8619..dbf14cbf81f 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -43,7 +43,7 @@ ) from samcli.lib.cfn_language_extensions.utils import is_foreach_key from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable -from samcli.lib.providers.provider import LayerVersion, ResourcesToBuildCollector, Stack +from samcli.lib.providers.provider import LayerVersion, ResourcesToBuildCollector, Stack, get_full_path from samcli.lib.providers.sam_api_provider import SamApiProvider from samcli.lib.providers.sam_function_provider import SamFunctionProvider from samcli.lib.providers.sam_layer_provider import SamLayerProvider @@ -512,9 +512,6 @@ def _update_original_template_paths( stack_output_template_path_by_stack_path : Optional[Dict[str, str]] Map of stack paths to their output template paths (for nested stacks). """ - from samcli.lib.providers.provider import get_full_path - - artifacts = artifacts if artifacts is not None else {} stack_outputs = stack_output_template_path_by_stack_path or {} stack_path = stack.stack_path if isinstance(stack.stack_path, str) else "" @@ -546,7 +543,7 @@ def _update_original_template_paths( # the LE-resolved value from clobbering user intrinsics on, e.g., # ``Code: {ZipFile: !Sub ...}`` Lambdas. See #9029. full_path = get_full_path(stack_path, resource_key) - if full_path not in artifacts and full_path not in stack_outputs: + if (not artifacts or full_path not in artifacts) and full_path not in stack_outputs: continue # Regular resource - copy updated paths from modified template modified_resource = modified_resources.get(resource_key, {}) @@ -615,8 +612,6 @@ def _update_foreach_artifact_paths( _set_prop_value, ) - artifacts = artifacts if artifacts is not None else {} - generated_mappings: Dict[str, Dict[str, Dict[str, str]]] = {} if outer_context is None: @@ -696,16 +691,11 @@ def _update_foreach_artifact_paths( ) if not mapping_entries: # No per-iteration expanded resource produced a build - # artifact — the property is non-packageable for every - # iteration (e.g. ``Code: {ZipFile: ...}`` Lambdas have - # no ``CodeUri`` to build). Mappings hold only static - # strings, so deploy-time intrinsics like ``Fn::Sub`` - # cannot be expressed per-iteration; leaving the loop - # variable in the original would fail at deploy time - # (the variable isn't a CFN parameter). Reject. Mirrors - # the artifacts-lookup discipline of the static branch - # and the root-level merge. See #9029. - self._raise_dynamic_no_artifacts_unsupported(resource_template_key, loop_variable, leaf_name) + # artifact (e.g. inline ``Code: {ZipFile: !Sub ...}`` + # has no ``CodeUri`` to build). Leave the property as + # written in the source so deploy-time intrinsics + # survive. See #9029. + continue mapping_name = f"SAM{leaf_name}{nesting_path}" # Collision count is keyed on the leaf so dotted paths # that share a leaf (e.g. "ImageUri" vs "Code.ImageUri") @@ -793,7 +783,7 @@ def _merge_static_artifact_path( modified_resources: Dict, properties: Dict, prop_name: str, - artifacts: Dict[str, str], + artifacts: Optional[Dict[str, str]], stack_path: str, ) -> None: """Static-property merge inside a Fn::ForEach body. Skips resources that @@ -802,7 +792,6 @@ def _merge_static_artifact_path( ``Code: {ZipFile: !Sub ...}`` Lambdas survive verbatim. See #9029. """ from samcli.lib.package.language_extensions_packaging import _set_prop_value - from samcli.lib.providers.provider import get_full_path expanded_key = self._build_expanded_key( resource_template_key, @@ -812,26 +801,12 @@ def _merge_static_artifact_path( ) if not expanded_key: return - if get_full_path(stack_path, expanded_key) not in artifacts: + if not artifacts or get_full_path(stack_path, expanded_key) not in artifacts: return artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) if artifact_value is not None: _set_prop_value(properties, prop_name, artifact_value) - @staticmethod - def _raise_dynamic_no_artifacts_unsupported(resource_template_key: str, loop_variable: str, leaf_name: str) -> None: - """Refuse a Fn::ForEach body whose dynamic artifact property produced no - build artifacts for any iteration (e.g. an inline-source ``Code: {ZipFile: - ...}`` Lambda has no ``CodeUri`` to build). Mappings hold only static - strings, so we cannot emit a per-iteration value that preserves deploy-time - intrinsics, and we cannot leave the loop variable unresolved. See #9029. - """ - raise UserException( - f"Resource '{resource_template_key}' uses Fn::ForEach loop variable " - f"'{loop_variable}' in non-buildable property '{leaf_name}'. " - f"This is not supported." - ) - @staticmethod def _count_dynamic_properties( body: Dict, @@ -917,11 +892,9 @@ def _collect_dynamic_mapping_entries( expanded_key)`` is absent from ``artifacts`` are skipped. See #9029. """ from samcli.lib.package.language_extensions_packaging import _leaf_prop_name - from samcli.lib.providers.provider import get_full_path mapping_entries: Dict[str, Dict[str, str]] = {} leaf_name = _leaf_prop_name(prop_name) - artifacts = artifacts if artifacts is not None else {} for coll_value in collection_values: if outer_context: @@ -939,7 +912,7 @@ def _collect_dynamic_mapping_entries( ) else: expanded_key = substitute_loop_variable(resource_template_key, loop_variable, coll_value) - if get_full_path(stack_path, expanded_key) not in artifacts: + if not artifacts or get_full_path(stack_path, expanded_key) not in artifacts: continue artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) if artifact_value is not None: @@ -970,12 +943,10 @@ def _collect_nested_mapping_entry( import itertools from samcli.lib.package.language_extensions_packaging import _leaf_prop_name - from samcli.lib.providers.provider import get_full_path leaf_name = _leaf_prop_name(prop_name) outer_collections = [oc[1] for oc in outer_context] outer_vars = [oc[0] for oc in outer_context] - artifacts = artifacts if artifacts is not None else {} # Determine which outer vars need compound keys compound_outer_vars = {ovar for ovar, _ in (referenced_outer_vars or [])} @@ -986,7 +957,7 @@ def _collect_nested_mapping_entry( expanded_key = substitute_loop_variable(expanded_key, ovar, oval) expanded_key = substitute_loop_variable(expanded_key, loop_variable, coll_value) - if get_full_path(stack_path, expanded_key) not in artifacts: + if not artifacts or get_full_path(stack_path, expanded_key) not in artifacts: continue artifact_value = self._get_artifact_value(modified_resources, expanded_key, prop_name) if artifact_value is None: diff --git a/tests/integration/buildcmd/test_build_cmd_language_extensions.py b/tests/integration/buildcmd/test_build_cmd_language_extensions.py index a71a91a55ff..a344c9ff4c2 100644 --- a/tests/integration/buildcmd/test_build_cmd_language_extensions.py +++ b/tests/integration/buildcmd/test_build_cmd_language_extensions.py @@ -192,13 +192,13 @@ def test_build_foreach_static_zipfile_fnsub_preserves_intrinsic(self): self.assertNotIn("us-east-1", sub_body) self.assertNotIn("123456789012", sub_body) - def test_build_foreach_dynamic_non_buildable_property_rejected(self): + def test_build_foreach_dynamic_inline_zipfile_preserved(self): """Regression for #9029 (ForEach dynamic branch / case C): inside a Fn::ForEach body, a property that references the loop variable but produces - no build artifact for any iteration (e.g. inline-source `Code.ZipFile`) - cannot be expressed as a per-iteration CloudFormation Mapping. `sam build` - must reject the template with InvalidTemplateException naming the - non-buildable property.""" + no build artifact for any iteration (e.g. inline-source `Code.ZipFile`) is + passed through verbatim. CFN's LanguageExtensions transform expands the + ForEach at deploy time, substituting the loop variable in each per-iteration + copy, so the inline body works correctly without a SAM-built artifact.""" self.template_path = str( Path(self.test_data_path, "language-extensions-foreach-zipfile-dynamic", "template.yaml") ) @@ -206,10 +206,28 @@ def test_build_foreach_dynamic_non_buildable_property_rejected(self): cmdlist = self.get_command_list() command_result = run_command(cmdlist, cwd=self.working_dir) - self.assertNotEqual(command_result.process.returncode, 0, "Build should have failed") - stderr = command_result.stderr.decode("utf-8") - self.assertIn("non-buildable property", stderr) - self.assertIn("WorkerName", stderr) + self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}") + + built_template_path = self.default_build_dir.joinpath("template.yaml") + with open(built_template_path, "r") as f: + built_template = yaml.safe_load(f) + + resources = built_template.get("Resources", {}) + foreach_block = resources["Fn::ForEach::Workers"] + body = foreach_block[2] + worker_props = body["${WorkerName}Worker"]["Properties"] + zipfile_value = worker_props["Code"]["ZipFile"] + # The inline ZipFile must remain a Fn::Sub dict with both the loop + # variable and pseudo-parameter intrinsics intact — neither resolved + # at build time nor replaced by a Mapping lookup. + self.assertIsInstance(zipfile_value, dict) + self.assertIn("Fn::Sub", zipfile_value) + sub_body = zipfile_value["Fn::Sub"] + self.assertIn("${WorkerName}", sub_body) + role_value = worker_props["Role"] + self.assertIsInstance(role_value, dict) + self.assertIn("Fn::Sub", role_value) + self.assertIn("${AWS::AccountId}", role_value["Fn::Sub"]) def test_build_nested_foreach_dynamic_codeuri_generates_mappings(self): """Test that nested Fn::ForEach with dynamic CodeUri generates Mappings.""" diff --git a/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml index c5164b39799..bd7d79d2925 100644 --- a/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml +++ b/tests/integration/testdata/buildcmd/language-extensions-foreach-zipfile-dynamic/template.yaml @@ -5,11 +5,12 @@ Transform: Description: > Regression test for #9029, ForEach dynamic-branch (case C). Each expanded - Lambda has `Code: {ZipFile: ...}` whose body references the loop variable. - The dynamic-branch merge cannot express a per-iteration ZipFile via a - CloudFormation Mapping (Mappings hold only static strings) and there is no - build artifact to fall back on, so `sam build` must reject the template - with InvalidTemplateException naming the non-buildable property. + Lambda has `Code: {ZipFile: !Sub ...}` whose body references the loop + variable. There is no build artifact for `sam build` to merge, so the + property must be passed through verbatim. CFN's LanguageExtensions transform + expands the ForEach at deploy time and substitutes the loop variable in each + per-iteration copy, so the inline body works correctly without a SAM-built + artifact. Resources: Fn::ForEach::Workers: From eba63241a06463ed1e4b134973acae278615f3d6 Mon Sep 17 00:00:00 2001 From: Harold Sun Date: Tue, 19 May 2026 14:32:46 -0700 Subject: [PATCH 3/5] test(cfn-lang-ext): flip dynamic-no-artifact unit test to pass-through Companion to the prior commit: the dynamic-branch path no longer raises when an inline-source Fn::ForEach iteration has no build artifact. Update the unit test to assert that no Mapping is generated and the original Code (with Fn::Sub intact) is left untouched, so CFN's LanguageExtensions transform can substitute the loop variable per-iteration at deploy time. --- .../test_build_context_language_extensions.py | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/tests/unit/commands/buildcmd/test_build_context_language_extensions.py b/tests/unit/commands/buildcmd/test_build_context_language_extensions.py index 60e81913036..0e77e449118 100644 --- a/tests/unit/commands/buildcmd/test_build_context_language_extensions.py +++ b/tests/unit/commands/buildcmd/test_build_context_language_extensions.py @@ -232,23 +232,21 @@ def test_static_zipfile_in_foreach_skipped_when_not_in_artifacts(self): self.assertEqual(body["${Name}Func"]["Properties"]["Code"], original_code) -class TestForEachDynamicNoArtifactsRefusal(TestCase): +class TestForEachDynamicNoArtifactsPassthrough(TestCase): """#9029: a dynamic Fn::ForEach property whose iterations produced no build - artifacts cannot be expressed as a per-iteration CloudFormation Mapping - (Mappings hold only static strings, so deploy-time intrinsics would be - silently lost) and cannot leave the loop variable unresolved. Mirrors the - artifacts-lookup discipline of the static branch and the root-level merge — - refuse rather than emit a misleading template. + artifacts (e.g. inline-source ``Code: {ZipFile: !Sub ...}``) is passed + through verbatim. CFN's LanguageExtensions transform expands the ForEach at + deploy time, substituting the loop variable in each per-iteration copy, so + no per-iteration Mapping is needed. """ def _make_context(self): with patch.object(BuildContext, "__init__", lambda self: None): return BuildContext() - def test_dynamic_property_with_no_artifacts_raises(self): - from samcli.commands.exceptions import UserException - + def test_dynamic_property_with_no_artifacts_skipped(self): ctx = self._make_context() + original_code = {"ZipFile": {"Fn::Sub": "print('${Name}')"}} foreach_value = [ "Name", ["Alpha", "Beta"], @@ -259,7 +257,7 @@ def test_dynamic_property_with_no_artifacts_raises(self): # ZipFile contains the loop variable; an inline-source # Lambda has no CodeUri, so neither AlphaFunc nor BetaFunc # appears in `artifacts`. - "Code": {"ZipFile": {"Fn::Sub": "print('${Name}')"}}, + "Code": original_code, }, } }, @@ -274,18 +272,19 @@ def test_dynamic_property_with_no_artifacts_raises(self): "Properties": {"Code": {"ZipFile": "print('Beta')"}}, }, } - with self.assertRaises(UserException) as cm: - ctx._update_foreach_artifact_paths( - "Fn::ForEach::Names", - foreach_value, - modified_resources, - template={}, - parameter_values={}, - artifacts={}, - ) - message = str(cm.exception) - self.assertIn("Name", message) - self.assertIn("non-buildable property", message) + generated_mappings = ctx._update_foreach_artifact_paths( + "Fn::ForEach::Names", + foreach_value, + modified_resources, + template={}, + parameter_values={}, + artifacts={}, + ) + # No Mapping is generated for a non-buildable dynamic property and the + # original Code (with the Fn::Sub intrinsic intact) is left untouched. + self.assertEqual(generated_mappings, {}) + body = foreach_value[2] + self.assertEqual(body["${Name}Func"]["Properties"]["Code"], original_code) class TestGetTemplateForOutputForEachExploration(TestCase): From 9d2af43e7f2827adc81e03cb36b01b28a28baca4 Mon Sep 17 00:00:00 2001 From: Harold Sun Date: Tue, 19 May 2026 14:57:25 -0700 Subject: [PATCH 4/5] refactor(build_context): hoist all inline imports, drop redundant `or {}` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoist every inline import in build_context.py to the module-level import block (`itertools`, the cfn_language_extensions models/sam_integration symbols, and all language_extensions_packaging helpers). None of these introduce a circular dependency — language_extensions_packaging and cfn_language_extensions only import from samcli.lib.* and samcli.commands. validate, never from samcli.commands.build.build_context. Drop the redundant `stack_output_template_path_by_stack_path or {}` at the `_update_original_template_paths` call site: the receiver declares the parameter Optional and already coerces to {} internally. --- samcli/commands/build/build_context.py | 42 ++++++-------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index dbf14cbf81f..c77036278b7 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -3,6 +3,7 @@ """ import copy +import itertools import logging import os import pathlib @@ -36,13 +37,21 @@ InvalidBuildGraphException, ) from samcli.lib.build.workflow_config import UnsupportedRuntimeException +from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES from samcli.lib.cfn_language_extensions.sam_integration import ( contains_loop_variable, + resolve_collection, sanitize_resource_key_for_mapping, substitute_loop_variable, ) from samcli.lib.cfn_language_extensions.utils import is_foreach_key from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable +from samcli.lib.package.language_extensions_packaging import ( + _get_prop_value, + _leaf_prop_name, + _resolve_property_paths, + _set_prop_value, +) from samcli.lib.providers.provider import LayerVersion, ResourcesToBuildCollector, Stack, get_full_path from samcli.lib.providers.sam_api_provider import SamApiProvider from samcli.lib.providers.sam_function_provider import SamFunctionProvider @@ -469,7 +478,7 @@ def _get_template_for_output( modified_template, stack, artifacts, - stack_output_template_path_by_stack_path or {}, + stack_output_template_path_by_stack_path, ) # Propagate the auto dependency layer nested stack resource from the expanded template @@ -603,15 +612,6 @@ def _update_foreach_artifact_paths( Dict[str, Dict[str, Dict[str, str]]] Generated Mappings section for dynamic artifact properties (empty dict if none) """ - from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES - from samcli.lib.cfn_language_extensions.sam_integration import resolve_collection - from samcli.lib.package.language_extensions_packaging import ( - _get_prop_value, - _leaf_prop_name, - _resolve_property_paths, - _set_prop_value, - ) - generated_mappings: Dict[str, Dict[str, Dict[str, str]]] = {} if outer_context is None: @@ -791,8 +791,6 @@ def _merge_static_artifact_path( ``ApplicationBuilder.update_template`` early-skip so that, e.g., ``Code: {ZipFile: !Sub ...}`` Lambdas survive verbatim. See #9029. """ - from samcli.lib.package.language_extensions_packaging import _set_prop_value - expanded_key = self._build_expanded_key( resource_template_key, loop_variable, @@ -818,13 +816,6 @@ def _count_dynamic_properties( Used to detect collisions where multiple resources in the same ForEach body share the same property name (e.g., two resources both with DefinitionUri). """ - from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES - from samcli.lib.package.language_extensions_packaging import ( - _get_prop_value, - _leaf_prop_name, - _resolve_property_paths, - ) - count: Counter = Counter() for rtk, rt in body.items(): if is_foreach_key(rtk): @@ -891,8 +882,6 @@ def _collect_dynamic_mapping_entries( the root-level merge. Iterations whose ``get_full_path(stack_path, expanded_key)`` is absent from ``artifacts`` are skipped. See #9029. """ - from samcli.lib.package.language_extensions_packaging import _leaf_prop_name - mapping_entries: Dict[str, Dict[str, str]] = {} leaf_name = _leaf_prop_name(prop_name) @@ -940,10 +929,6 @@ def _collect_nested_mapping_entry( ``artifacts`` filters per-iteration expanded keys (see ``_collect_dynamic_mapping_entries``). """ - import itertools - - from samcli.lib.package.language_extensions_packaging import _leaf_prop_name - leaf_name = _leaf_prop_name(prop_name) outer_collections = [oc[1] for oc in outer_context] outer_vars = [oc[0] for oc in outer_context] @@ -990,8 +975,6 @@ def _collect_foreach_layer_mappings( Returns a dict mapping each collection value (or compound key for nested ForEach) to its layer output key, e.g. {"Alpha": {"LayerOutputKey": "Outputs.AlphaFunction...DepLayer"}}. """ - import itertools - mapping_entries: Dict[str, Dict[str, str]] = {} for coll_value in collection_values: @@ -1050,8 +1033,6 @@ def _get_artifact_value(modified_resources: Dict, expanded_key: str, prop_name: ``prop_name`` may be a jmespath dotted path (e.g. "Command.ScriptLocation"). """ - from samcli.lib.package.language_extensions_packaging import _get_prop_value - modified_resource = modified_resources.get(expanded_key, {}) if not isinstance(modified_resource, dict): return None @@ -1074,9 +1055,6 @@ def _copy_artifact_paths(self, original_resource: Dict, modified_resource: Dict) modified_resource : Dict The modified resource with updated artifact paths """ - from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES - from samcli.lib.package.language_extensions_packaging import _get_prop_value, _set_prop_value - original_props = original_resource.get("Properties", {}) modified_props = modified_resource.get("Properties", {}) resource_type = original_resource.get("Type", "") From 944571ea9e6b1217c434018e3f6df177ecccc6ec Mon Sep 17 00:00:00 2001 From: Harold Sun Date: Wed, 20 May 2026 18:57:11 -0700 Subject: [PATCH 5/5] test(integ): opt-in to --language-extensions for foreach zipfile tests After #9033, language extensions are opt-in. The two ForEach zipfile tests added by this PR omitted the flag, causing SAM transform plugins to choke on unexpanded Fn::ForEach blocks. --- .../buildcmd/test_build_cmd_language_extensions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/buildcmd/test_build_cmd_language_extensions.py b/tests/integration/buildcmd/test_build_cmd_language_extensions.py index cd527c83f36..8250260ef15 100644 --- a/tests/integration/buildcmd/test_build_cmd_language_extensions.py +++ b/tests/integration/buildcmd/test_build_cmd_language_extensions.py @@ -161,7 +161,7 @@ def test_build_foreach_static_zipfile_fnsub_preserves_intrinsic(self): Path(self.test_data_path, "language-extensions-foreach-zipfile-static", "template.yaml") ) - cmdlist = self.get_command_list() + cmdlist = self.get_command_list() + ["--language-extensions"] command_result = run_command(cmdlist, cwd=self.working_dir) self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}") @@ -203,7 +203,7 @@ def test_build_foreach_dynamic_inline_zipfile_preserved(self): Path(self.test_data_path, "language-extensions-foreach-zipfile-dynamic", "template.yaml") ) - cmdlist = self.get_command_list() + cmdlist = self.get_command_list() + ["--language-extensions"] command_result = run_command(cmdlist, cwd=self.working_dir) self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}")