Skip to content

Commit f46d86e

Browse files
authored
fix(cfn-lang-ext): preserve inline-source intrinsics through LE-aware build merge (#9029) (#9031)
* 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. * 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. * 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. * refactor(build_context): hoist all inline imports, drop redundant `or {}` 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. * 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.
1 parent fe16ab0 commit f46d86e

7 files changed

Lines changed: 638 additions & 85 deletions

File tree

samcli/commands/build/build_context.py

Lines changed: 152 additions & 60 deletions
Large diffs are not rendered by default.

tests/integration/buildcmd/test_build_cmd_language_extensions.py

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,124 @@ def test_build_dynamic_codeuri_generates_mappings(self):
111111
self.assertEqual(codeuri["Fn::FindInMap"][0], mapping_name)
112112
self.assertEqual(codeuri["Fn::FindInMap"][1], {"Ref": "FunctionName"})
113113

114+
def test_build_zipfile_fnsub_preserves_intrinsic(self):
115+
"""Regression for #9029: `Code.ZipFile: !Sub ...` on a Lambda function under
116+
AWS::LanguageExtensions must round-trip through `sam build` with the Fn::Sub
117+
intact. Before the fix, the LE-aware merge step copied the LE-resolved value
118+
back, baking default pseudo-parameter values (us-east-1, 123456789012) into
119+
the built template."""
120+
self.template_path = str(Path(self.test_data_path, "language-extensions-zipfile-fnsub", "template.yaml"))
121+
122+
cmdlist = self.get_command_list()
123+
command_result = run_command(cmdlist, cwd=self.working_dir)
124+
125+
self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}")
126+
127+
built_template_path = self.default_build_dir.joinpath("template.yaml")
128+
self.assertTrue(built_template_path.exists())
129+
130+
with open(built_template_path, "r") as f:
131+
built_template = yaml.safe_load(f)
132+
133+
function_props = built_template["Resources"]["MyTriggerFunction"]["Properties"]
134+
135+
# Code.ZipFile must remain a dict containing Fn::Sub, not a resolved string.
136+
zipfile_value = function_props["Code"]["ZipFile"]
137+
self.assertIsInstance(
138+
zipfile_value, dict, f"Expected Code.ZipFile to remain a Fn::Sub dict, got: {zipfile_value!r}"
139+
)
140+
self.assertIn("Fn::Sub", zipfile_value)
141+
sub_body = zipfile_value["Fn::Sub"]
142+
self.assertIn("${AWS::Region}", sub_body)
143+
self.assertIn("${AWS::AccountId}", sub_body)
144+
# Confirm no default pseudo-param values leaked through.
145+
self.assertNotIn("us-east-1", sub_body)
146+
self.assertNotIn("123456789012", sub_body)
147+
148+
# Role's Fn::Sub should likewise be preserved (sanity check on the same merge path).
149+
role_value = function_props["Role"]
150+
self.assertIsInstance(role_value, dict)
151+
self.assertIn("Fn::Sub", role_value)
152+
self.assertIn("${AWS::AccountId}", role_value["Fn::Sub"])
153+
154+
def test_build_foreach_static_zipfile_fnsub_preserves_intrinsic(self):
155+
"""Regression for #9029 (ForEach static branch / case B): inside a
156+
Fn::ForEach body, `Code.ZipFile: !Sub ...` whose body does NOT reference
157+
the loop variable must round-trip through `sam build` with the Fn::Sub
158+
intact. The static-branch merge has no build artifact for an inline-source
159+
Lambda, so the user-authored property must be preserved verbatim."""
160+
self.template_path = str(
161+
Path(self.test_data_path, "language-extensions-foreach-zipfile-static", "template.yaml")
162+
)
163+
164+
cmdlist = self.get_command_list() + ["--language-extensions"]
165+
command_result = run_command(cmdlist, cwd=self.working_dir)
166+
167+
self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}")
168+
169+
built_template_path = self.default_build_dir.joinpath("template.yaml")
170+
self.assertTrue(built_template_path.exists())
171+
172+
with open(built_template_path, "r") as f:
173+
built_template = yaml.safe_load(f)
174+
175+
# Fn::ForEach structure must be preserved
176+
resources = built_template.get("Resources", {})
177+
foreach_block = resources.get("Fn::ForEach::Workers")
178+
self.assertIsNotNone(foreach_block, "Fn::ForEach::Workers must survive in built template")
179+
self.assertEqual(foreach_block[0], "WorkerName")
180+
self.assertEqual(foreach_block[1], ["Alpha", "Beta"])
181+
182+
# The body's Code.ZipFile must remain a Fn::Sub dict, not a resolved string,
183+
# and must still reference ${AWS::Region} / ${AWS::AccountId}.
184+
body = foreach_block[2]
185+
worker_props = body["${WorkerName}Worker"]["Properties"]
186+
zipfile_value = worker_props["Code"]["ZipFile"]
187+
self.assertIsInstance(zipfile_value, dict)
188+
self.assertIn("Fn::Sub", zipfile_value)
189+
sub_body = zipfile_value["Fn::Sub"]
190+
self.assertIn("${AWS::Region}", sub_body)
191+
self.assertIn("${AWS::AccountId}", sub_body)
192+
self.assertNotIn("us-east-1", sub_body)
193+
self.assertNotIn("123456789012", sub_body)
194+
195+
def test_build_foreach_dynamic_inline_zipfile_preserved(self):
196+
"""Regression for #9029 (ForEach dynamic branch / case C): inside a
197+
Fn::ForEach body, a property that references the loop variable but produces
198+
no build artifact for any iteration (e.g. inline-source `Code.ZipFile`) is
199+
passed through verbatim. CFN's LanguageExtensions transform expands the
200+
ForEach at deploy time, substituting the loop variable in each per-iteration
201+
copy, so the inline body works correctly without a SAM-built artifact."""
202+
self.template_path = str(
203+
Path(self.test_data_path, "language-extensions-foreach-zipfile-dynamic", "template.yaml")
204+
)
205+
206+
cmdlist = self.get_command_list() + ["--language-extensions"]
207+
command_result = run_command(cmdlist, cwd=self.working_dir)
208+
209+
self.assertEqual(command_result.process.returncode, 0, f"Build failed: {command_result.stderr.decode('utf-8')}")
210+
211+
built_template_path = self.default_build_dir.joinpath("template.yaml")
212+
with open(built_template_path, "r") as f:
213+
built_template = yaml.safe_load(f)
214+
215+
resources = built_template.get("Resources", {})
216+
foreach_block = resources["Fn::ForEach::Workers"]
217+
body = foreach_block[2]
218+
worker_props = body["${WorkerName}Worker"]["Properties"]
219+
zipfile_value = worker_props["Code"]["ZipFile"]
220+
# The inline ZipFile must remain a Fn::Sub dict with both the loop
221+
# variable and pseudo-parameter intrinsics intact — neither resolved
222+
# at build time nor replaced by a Mapping lookup.
223+
self.assertIsInstance(zipfile_value, dict)
224+
self.assertIn("Fn::Sub", zipfile_value)
225+
sub_body = zipfile_value["Fn::Sub"]
226+
self.assertIn("${WorkerName}", sub_body)
227+
role_value = worker_props["Role"]
228+
self.assertIsInstance(role_value, dict)
229+
self.assertIn("Fn::Sub", role_value)
230+
self.assertIn("${AWS::AccountId}", role_value["Fn::Sub"])
231+
114232
def test_build_nested_foreach_dynamic_codeuri_generates_mappings(self):
115233
"""Test that nested Fn::ForEach with dynamic CodeUri generates Mappings."""
116234
self.template_path = str(
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
AWSTemplateFormatVersion: '2010-09-09'
2+
Transform:
3+
- AWS::LanguageExtensions
4+
- AWS::Serverless-2016-10-31
5+
6+
Description: >
7+
Regression test for #9029, ForEach dynamic-branch (case C). Each expanded
8+
Lambda has `Code: {ZipFile: !Sub ...}` whose body references the loop
9+
variable. There is no build artifact for `sam build` to merge, so the
10+
property must be passed through verbatim. CFN's LanguageExtensions transform
11+
expands the ForEach at deploy time and substitutes the loop variable in each
12+
per-iteration copy, so the inline body works correctly without a SAM-built
13+
artifact.
14+
15+
Resources:
16+
Fn::ForEach::Workers:
17+
- WorkerName
18+
- - Alpha
19+
- Beta
20+
- ${WorkerName}Worker:
21+
Type: AWS::Lambda::Function
22+
Properties:
23+
FunctionName: !Sub ${WorkerName}-worker
24+
Runtime: python3.11
25+
Handler: index.handler
26+
Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/worker-role'
27+
Code:
28+
ZipFile: !Sub |
29+
import boto3
30+
def handler(event, context):
31+
print('worker name: ${WorkerName}')
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
AWSTemplateFormatVersion: '2010-09-09'
2+
Transform:
3+
- AWS::LanguageExtensions
4+
- AWS::Serverless-2016-10-31
5+
6+
Description: >
7+
Regression test for #9029, ForEach static-branch (case B). Each expanded
8+
Lambda has `Code: {ZipFile: !Sub ...}` whose body references CFN pseudo-
9+
parameters but NOT the loop variable. The static-branch merge must skip
10+
these resources (no build artifact) so the user-authored `Fn::Sub`
11+
survives intact in the built template.
12+
13+
Resources:
14+
Fn::ForEach::Workers:
15+
- WorkerName
16+
- - Alpha
17+
- Beta
18+
- ${WorkerName}Worker:
19+
Type: AWS::Lambda::Function
20+
Properties:
21+
FunctionName: !Sub ${WorkerName}-worker
22+
Runtime: python3.11
23+
Handler: index.handler
24+
Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/worker-role'
25+
Code:
26+
ZipFile: !Sub |
27+
import boto3
28+
def handler(event, context):
29+
client = boto3.client('logs')
30+
client.put_log_events(
31+
logGroupName='/aws/lambda/worker-${AWS::Region}-${AWS::AccountId}',
32+
)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
AWSTemplateFormatVersion: '2010-09-09'
2+
Transform:
3+
- AWS::LanguageExtensions
4+
- AWS::Serverless-2016-10-31
5+
6+
Description: >
7+
Regression test for #9029. Inline-source `Code: {ZipFile: !Sub ...}` on a
8+
Lambda function under AWS::LanguageExtensions must round-trip through
9+
`sam build` with the `Fn::Sub` intact (no pseudo-parameter resolution).
10+
11+
Resources:
12+
MyTriggerFunction:
13+
Type: AWS::Lambda::Function
14+
Properties:
15+
FunctionName: my-trigger
16+
Runtime: python3.11
17+
Handler: index.handler
18+
Role: !Sub 'arn:aws:iam::${AWS::AccountId}:role/my-role'
19+
Code:
20+
ZipFile: !Sub |
21+
import boto3
22+
def handler(event, context):
23+
client = boto3.client('stepfunctions')
24+
client.start_execution(
25+
stateMachineArn='arn:aws:states:${AWS::Region}:${AWS::AccountId}:stateMachine:my-state-machine',
26+
)

0 commit comments

Comments
 (0)