Skip to content

fix(cfn-lang-ext): preserve inline-source intrinsics through LE-aware build merge (#9029)#9031

Merged
bnusunny merged 7 commits into
developfrom
fix/9029-zipfile-fnsub
May 21, 2026
Merged

fix(cfn-lang-ext): preserve inline-source intrinsics through LE-aware build merge (#9029)#9031
bnusunny merged 7 commits into
developfrom
fix/9029-zipfile-fnsub

Conversation

@bnusunny
Copy link
Copy Markdown
Contributor

@bnusunny bnusunny commented May 19, 2026

Summary

Fixes #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 merge had three sites that all need the same discipline as the non-LE ApplicationBuilder.update_template early-skip — "if no build artifact, don't overwrite":

  • Root-level merge (_update_original_template_paths): skip when 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 per expanded key.

Test plan

  • Unit: pytest tests/unit/commands/buildcmd/test_build_context.py tests/unit/commands/buildcmd/test_build_context_language_extensions.py — 93 tests pass; new coverage for skip (cases A/B) and refusal (case C).
  • Integration testdata + tests added for all three cases:
    • test_build_zipfile_fnsub_preserves_intrinsic (case A: root-level)
    • test_build_foreach_static_zipfile_fnsub_preserves_intrinsic (case B: ForEach static)
    • test_build_foreach_dynamic_inline_zipfile_preserved (case C: ForEach dynamic,)
  • black --check clean on touched files.

… 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.
@bnusunny bnusunny requested a review from a team as a code owner May 19, 2026 07:01
@github-actions github-actions Bot added area/build sam build command pr/internal labels May 19, 2026
@bnusunny bnusunny requested review from roger-zhangg and vicheey May 19, 2026 16:01
Comment thread samcli/commands/build/build_context.py Outdated
Comment thread samcli/commands/build/build_context.py Outdated
Comment thread samcli/commands/build/build_context.py Outdated
Comment thread samcli/commands/build/build_context.py Outdated
bnusunny added 2 commits May 19, 2026 14:27
- 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.
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.
Copy link
Copy Markdown
Collaborator

@aws-sam-cli-bot aws-sam-cli-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Results

Reviewed: 9ffa5ed..eba6324
Files: 7
Comments: 2

Comment thread samcli/commands/build/build_context.py Outdated
Comment thread samcli/commands/build/build_context.py Outdated
… {}`

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.
@bnusunny bnusunny requested a review from valerena May 19, 2026 22:06
roger-zhangg
roger-zhangg previously approved these changes May 19, 2026
valerena
valerena previously approved these changes May 20, 2026
@bnusunny bnusunny dismissed stale reviews from roger-zhangg and valerena via 17195cd May 21, 2026 01:45
@bnusunny bnusunny requested review from roger-zhangg and valerena May 21, 2026 01:46
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.
@bnusunny bnusunny added this pull request to the merge queue May 21, 2026
Merged via the queue into develop with commit f46d86e May 21, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build sam build command pr/internal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Fn::Sub inside ZipFile inline Lambda code is evaluated at build time with wrong pseudo-parameter values in 1.160.x

4 participants