Commit d992b50
fix(package): rewrite AWS::Include Location buried inside language-extension functions (aws#9030)
* refactor(package): replace METADATA_EXPORT_LIST and GLOBAL_EXPORT_DICT with typed registries
Migrate two flat package exporter registries to typed dataclass form:
- METADATA_EXPORT_LIST (list of Resource subclasses) -> METADATA_EXPORTS
(List[MetadataExportSpec]). Each spec captures both the metadata_type
('AWS::ServerlessRepo::Application') and the per-property exporter
classes that handle LicenseUrl/ReadmeUrl. Template._export_metadata
now dispatches via a metadata_type lookup instead of a per-class
RESOURCE_TYPE filter.
- GLOBAL_EXPORT_DICT (dict keyed by Fn::Transform) -> GLOBAL_TRANSFORM_EXPORTS
(List[GlobalTransformExportSpec]). Each spec carries a discriminator
callable (e.g. _is_aws_include for matching AWS::Include) plus the
handler. _export_global_artifacts now dispatches through the
discriminator so future Fn::Transform variants can register without
touching the walker.
The typed shape is the contract a follow-up commit will read from to
process AWS::Include before language-extension expansion.
* feat(package): merge SAR Metadata exports back into LE child templates
Extend merge_language_extensions_s3_uris with a registry-driven Metadata
pass that copies rewritten property values (LicenseUrl, ReadmeUrl) from
the exported template back into the original (Fn::ForEach-preserving)
template after LE expansion.
Without this pass, when a child stack uses Transform: AWS::LanguageExtensions
and declares AWS::ServerlessRepo::Application metadata, sam package
silently dropped the License/Readme S3 URLs from the merged output —
they were uploaded but never wired into the template the user deploys.
Implementation iterates METADATA_EXPORTS (the registry added in the prior
commit) so future metadata types pick up merge support without touching
the merge walker.
* fix(package): process AWS::Include before language-extension expansion (aws#9027)
Closes aws#9027.
Symptom: when a template uses both Transform: AWS::LanguageExtensions and
contains Fn::Transform: AWS::Include buried inside an LE function (e.g.
Fn::ToJsonString), sam package fails to rewrite the include's Location to
an S3 URL. CloudFormation then rejects the deploy with 'The location
parameter is not a valid S3 uri.'
Root cause: PackageContext._export ran expand_language_extensions BEFORE
the artifact exporter walked Fn::Transform nodes. LE functions like
Fn::ToJsonString json.dumps() their argument, collapsing the structural
Fn::Transform subtree into a JSON-string literal. By the time the
exporter ran, the include was no longer a structural dict node and was
invisible to the global-transform walker.
Fix: process AWS::Include (and any other GLOBAL_TRANSFORM_EXPORTS
handler) on the original template BEFORE LE expansion runs, mirroring
CloudFormation's own server-side transform ordering — CFN resolves
inline Fn::Transform macros before evaluating AWS::LanguageExtensions.
Implementation:
- Extract Template._export_global_artifacts to a module-level
_export_global_artifacts_pass(template_dict, uploader, template_dir).
The instance method becomes a one-line delegate so existing callers
keep working.
- Call _export_global_artifacts_pass on the original template before
expand_language_extensions in PackageContext._export (root flow) and
in CloudFormationStackResource.do_export (nested-stack child flow).
Dynamic-Location AWS::Include inside Fn::ForEach (e.g.
Location: ./swagger-${Name}.yaml) is not supported by sam package: a
local file path with literal ${...} placeholders does not exist on disk,
so is_local_file fails and the existing InvalidLocalPathError fires —
which is the right user-facing failure. CloudFormation does not
substitute loop variables into Fn::Transform paths server-side either,
so the limitation matches CFN's actual capability.
* test(package): integration coverage for AWS::Include + SAR Metadata in LE templates
Three end-to-end tests that exercise the package_context._export-equivalent
flow on language-extension templates:
- test_le_template_with_top_level_aws_include_merges_location verifies
AWS::Include in Outputs gets its Location rewritten to s3:// after
the pre-LE pass.
- test_le_template_with_serverless_repo_metadata_merges_license_url
verifies SAR LicenseUrl/ReadmeUrl in Metadata get merged back.
- TestPackageContextIssue9027 reproduces the user template from
aws#9027 (Fn::ToJsonString over Fn::Transform: AWS::Include
buried inside AWS::SSM::Parameter) and asserts the buried Location is
rewritten to s3://. Locks down the regression.
* docs(cfn-lang-ext): document AWS::Include processing order vs language extensions
Add a section explaining that sam package processes Fn::Transform: AWS::Include
before language-extension expansion, mirroring CloudFormation's server-side
transform ordering. This means AWS::Include Location rewrites work correctly
even when the include lives buried inside language-extension functions like
Fn::ToJsonString or Fn::ForEach bodies.
* test(package): align LE tests with opt-in API from aws#9033
PR aws#9033 made AWS::LanguageExtensions local processing opt-in. Two API
changes broke three tests on this branch after the merge from develop:
- expand_language_extensions() now requires a keyword-only `enabled` arg
- PackageContext reads self._language_extensions_enabled, set in __init__
Pass enabled=True to expand_language_extensions in the two artifact-exporter
tests, and set _language_extensions_enabled on the PackageContext stub that
bypasses __init__ via __new__. All three tests exercise templates with
Transform: AWS::LanguageExtensions, so enabling LE is the intended behavior.
* test(package): hoist inline imports in TestPackageContextBuriedAWSInclude
Move os, Destination/Uploaders, and yaml_parse to module-level imports;
drop the redundant inline tempfile/PackageContext (already at module level).
Addresses inline-imports review comment on aws#9030.
* refactor(package): hoist InvalidSamDocumentException and expand_language_extensions imports
Move the two inline imports inside _export() to module scope. No
behavior change; this is preparation for the upcoming _export() split
which references these from a new branch method.
* refactor(package): add _export_without_language_extensions (off-path branch)
New private method that mirrors pre-1.160.0 sam-cli behavior: a tight
Template.export() pipeline with no LE machinery. Unused by _export()
until the dispatcher is cut over in a follow-up commit.
Also adds two structural-gate smoke tests that stay red until the
dispatcher is wired up — they assert that the off path never invokes
expand_language_extensions or the pre-LE _export_global_artifacts_pass.
* refactor(package): add _export_with_language_extensions (on-path branch)
New private method containing the existing aws#9027 ordering: pre-LE
include pass, LE expansion, Template.export(), merge, Mappings.
Unused by _export() until the dispatcher is cut over in the next commit.
* refactor(package): split _export() into LE / non-LE branches gated on flag
_export() becomes a thin dispatcher: read the template, branch on
self._language_extensions_enabled, dump. The two branch methods added
in the prior two commits now own the actual work.
Treats --language-extensions as a hard correctness boundary: when off,
no LE machinery is invoked at all (no expand_language_extensions, no
pre-LE _export_global_artifacts_pass, no merge, no Mappings).
Template.export() still handles AWS::Include for non-LE templates via
its own internal _export_global_artifacts pass — that path has worked
since long before the aws#9027 fix and is unchanged by this refactor.
Public surface (PackageContext._export(template_path, use_json) -> str)
is preserved; all existing _export tests pass unchanged.
* test(package): repoint expand_language_extensions patches to package_context use site
The hoist in 756c502 moved `expand_language_extensions` to a
module-level import in `package_context.py`. The use-site binding is
now captured at module load, so existing tests that patched the source
module `samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions`
no longer intercept calls from `_export()`.
Repoint two patches at the use-site
`samcli.commands.package.package_context.expand_language_extensions`:
- `test_phase_separation.py::test_package_context_export_calls_expand_language_extensions`
was failing intermittently in CI depending on test-collection order.
- `test_package_context.py::test_off_path_does_not_invoke_expand_language_extensions`
is retargeted for consistency now that the inline-import shadow is
gone (post-Task-4 cutover); the dead `had_language_extensions=False`
scaffolding can also go since the off path no longer calls expand.
Other source-module patches in `test_artifact_exporter.py` are correct
as-is — those tests cover `Template.export()` paths whose use site is
the artifact_exporter module.
* refactor(package): hoist do_export inline imports and repoint test patches
Hoist expand_language_extensions, IntrinsicsSymbolTable,
generate_and_apply_artifact_mappings, and merge_language_extensions_s3_uris
to module-level imports in artifact_exporter.py — preparation for the
LE / non-LE structural-gate split of CloudFormationStackResource.do_export.
Module-level binding requires existing tests that patched
samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions
(the source module) to repoint at the use-site
samcli.lib.package.artifact_exporter.expand_language_extensions, since
the use-site name is captured at module load and source-module patches
no longer intercept once the inline import is gone. Same lesson as
PR aws#9030 commit 45dea4b for package_context.
* refactor(package): add _do_export_without_language_extensions branch method
LE-off branch for CloudFormationStackResource: path-based Template
construction with no pre-LE pass, no parameter_values, no deepcopy.
Method is unused until the dispatcher rewrite — wiring lands in the
follow-up commit. Adds TestDoExportLanguageExtensionsStructuralGate
with two red structural-gate assertions that the dispatcher rewrite
will turn green.
* refactor(package): add _do_export_with_language_extensions branch method
Lifts the existing LE-expansion body from CloudFormationStackResource.do_export
into a dedicated method (resource_id, template_path, parent_dir,
abs_template_path, resource_dict) -> Dict. No behavior change yet — the
dispatcher in the follow-up commit will route LE-on calls into it.
Threads resource_dict so the branch can read nested-stack Parameters
without the dispatcher passing them separately. Returns the exported
template dict so the dispatcher owns the final yaml_dump + upload tail.
* refactor(package): split do_export into LE / non-LE branches gated on flag
CloudFormationStackResource.do_export is now a thin dispatcher that:
1) does the early-exit guards (None / S3 URL / non-local file)
2) routes to _do_export_with_language_extensions or
_do_export_without_language_extensions based on
self.language_extensions_enabled
3) owns the final yaml_dump + upload + property mutation tail.
The off path is now structurally LE-free: no expand_language_extensions
call, no _export_global_artifacts_pass, no IntrinsicsSymbolTable
pseudo-param plumbing, no parent_parameter_values, no copy.deepcopy.
Template.export() runs its own internal _export_global_artifacts so
AWS::Include still resolves on the off path.
The structural-gate tests added in the previous commit are now green.
Existing LE tests that rely on language_extensions_enabled = True now
set the flag explicitly (where they previously depended on the LE
machinery running unconditionally as a passthrough).
* chore(package): apply make format to do_export refactor
Black collapsed multi-line calls and signatures whose single-line
form fits under the 120-char limit. No behavior change.
* refactor(package): add _build_child_parameter_values helper
Adds a CFN-parity helper that builds the parameter_values dict for a
nested child stack: pseudo-params (with parent pseudo-name overrides
honored) plus parent-rebound values via the nested-stack Parameters
property. Non-pseudo parent names are not copied; child Defaults are
read by the LE expander itself via parsed_template.parameters.
Helper is unused production code in this commit. Wired into
CloudFormationStackResource._do_export_with_language_extensions in
the next commit.
* fix(package): tighten parent_parameter_values scoping in LE path
CloudFormationStackResource._do_export_with_language_extensions used to
bulk-copy the parent stack's full parameter_values into the child, so a
parent's `Foo=parent_foo` could silently shadow an unrelated child
Parameter named Foo. This diverged from CloudFormation's nested-stack
contract (only parent-rebound names reach the child) and from the
non-LE path (which threads no parameters at all).
Replace the merge block with _build_child_parameter_values, which
returns pseudo-params (with parent pseudo-name overrides honored) plus
parent-rebound values from the nested-stack Parameters property. Child
template Defaults still resolve via the LE expander's parsed_template
fallback (resolvers/fn_ref.py:_resolve_parameter).
Adds end-to-end coverage:
- non-pseudo parent param does not leak into child's parameter_values
- child Default still resolves via resolver fallback after the change
* refactor(package): trim Task 2 redundant comments and verbose test docstrings
Code-review polish for the LE parent-param scoping fix:
- drop call-site comment block at _do_export_with_language_extensions
that duplicated _build_child_parameter_values's docstring
- shorten the two new test docstrings in
TestCloudFormationStackResourceChildExpansion to match neighbor
style (no internal module paths)
* refactor(tests): hoist inline imports added in LE parent-param fix
Move imports introduced by the previous two commits from inside test
bodies up to the module-level import block:
- _build_child_parameter_values, IntrinsicsSymbolTable
- yaml_dump (samcli.yamlhelper), shutil, inspect (stdlib)
None of these symbols are @patch targets, so hoisting does not affect
any mock binding (use-site patches in artifact_exporter remain correct).
* refactor(tests): hoist remaining inline imports and extract LE fixtures
Address review feedback on PR aws#9030: inline imports should live at
module scope, and on-the-fly file writes for LE child templates should
use checked-in fixtures instead of tempfile.mkdtemp() + yaml_dump().
Hoist inline imports (LanguageExtensionResult, _resolve_nested_stack_parameters,
yaml_dump, shutil, the packageable_resources block) to the module-level
import block. None are @patch targets, so use-site patches remain correct.
Add tests/unit/lib/package/test_data/ following the precedent in
tests/unit/lib/intrinsic_resolver/test_data, with TEST_DATA_PATH
constant defined as Path(__file__).resolve().parent / "test_data".
Convert the LE parent-param test methods, the buried-AWS::Include test,
the expansion-error-handling pair, and the structural-gate pair to read
their child templates (and the export-events.json include target) from
the fixture tree. The on-the-fly tempdir + yaml_dump scaffolding is
removed entirely along with the corresponding _write_child /
_write_minimal_child helpers.
No production code change; all 110 unit tests in test_artifact_exporter
still pass.
* refactor(tests): extract TestPackageContextBuriedAWSInclude inline fixture
Replace the on-the-fly tempfile + open()+write() scaffolding in
TestPackageContextBuriedAWSInclude.setUp with a checked-in fixture under
tests/unit/commands/package/test_data/buried_aws_include/. Mirrors the
TEST_DATA_PATH pattern already in tests/unit/lib/package/test_data and
tests/unit/lib/intrinsic_resolver/test_data.
setUp now resolves template_path from TEST_DATA_PATH; the export-events
include target lives next to template.yaml so the relative Location in
the template still resolves at sam-package time.
No production code change; all 146 tests in test_package_context still
pass.1 parent abfdb31 commit d992b50
23 files changed
Lines changed: 1262 additions & 260 deletions
File tree
- docs
- samcli
- commands/package
- lib/package
- tests/unit
- commands/package
- test_data/buried_aws_include
- lib
- cfn_language_extensions
- package
- test_data
- buried_aws_include
- child_bar_default
- child_foo_param
- child_minimal_le
- child_minimal_no_le
- child_servicenames
- le_sar_metadata
- code
- le_top_level_include
- code
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
321 | 321 | | |
322 | 322 | | |
323 | 323 | | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
324 | 340 | | |
325 | 341 | | |
326 | 342 | | |
| |||
330 | 346 | | |
331 | 347 | | |
332 | 348 | | |
333 | | - | |
| 349 | + | |
334 | 350 | | |
335 | 351 | | |
336 | 352 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | | - | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
29 | 33 | | |
30 | | - | |
| 34 | + | |
31 | 35 | | |
32 | 36 | | |
33 | 37 | | |
34 | 38 | | |
35 | 39 | | |
36 | 40 | | |
37 | 41 | | |
38 | | - | |
| 42 | + | |
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
| |||
174 | 178 | | |
175 | 179 | | |
176 | 180 | | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
| 181 | + | |
| 182 | + | |
181 | 183 | | |
182 | 184 | | |
183 | 185 | | |
184 | | - | |
185 | | - | |
186 | | - | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
187 | 245 | | |
188 | 246 | | |
189 | 247 | | |
190 | 248 | | |
191 | 249 | | |
192 | | - | |
193 | 250 | | |
194 | | - | |
195 | | - | |
196 | | - | |
| 251 | + | |
197 | 252 | | |
198 | 253 | | |
199 | 254 | | |
200 | | - | |
201 | | - | |
202 | | - | |
203 | | - | |
204 | | - | |
205 | 255 | | |
206 | 256 | | |
207 | 257 | | |
| |||
211 | 261 | | |
212 | 262 | | |
213 | 263 | | |
214 | | - | |
| 264 | + | |
215 | 265 | | |
216 | | - | |
217 | 266 | | |
218 | 267 | | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
224 | | - | |
225 | | - | |
226 | | - | |
227 | | - | |
228 | | - | |
229 | | - | |
230 | | - | |
231 | | - | |
232 | | - | |
233 | | - | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | | - | |
239 | | - | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
| 268 | + | |
| 269 | + | |
244 | 270 | | |
245 | | - | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
246 | 287 | | |
247 | 288 | | |
248 | 289 | | |
| |||
0 commit comments