diff --git a/docs/cfn-language-extensions.md b/docs/cfn-language-extensions.md index b6b1f60fcb..deaed3e111 100644 --- a/docs/cfn-language-extensions.md +++ b/docs/cfn-language-extensions.md @@ -319,6 +319,22 @@ The following intrinsic functions are resolved locally during expansion: Functions that require deployed resources (`Fn::GetAtt`, `Fn::ImportValue`, `Fn::GetAZs`) are preserved for CloudFormation to resolve at deploy time. +## AWS::Include processing order + +When a template uses both `Fn::Transform: AWS::Include` and +`Transform: AWS::LanguageExtensions`, SAM CLI processes the inline +`AWS::Include` macros **before** running language-extension expansion +locally. This mirrors CloudFormation's server-side transform pipeline, +where `Fn::Transform` macros are resolved before +`AWS::LanguageExtensions`. + +The practical effect is that `AWS::Include` `Location` rewrites work +correctly even when the include lives buried inside language-extension +functions like `Fn::ToJsonString` or `Fn::ForEach` bodies, because the +include is rewritten while still structurally visible — before +`Fn::ToJsonString` collapses subtrees into JSON-string literals or +`Fn::ForEach` expands resources. + ## Validation errors The following template issues are caught locally before the SAM transform runs: @@ -328,7 +344,7 @@ The following template issues are caught locally before the SAM transform runs: | The `Fn::ForEach` value is malformed — not a list, doesn't have exactly 3 elements, or has a non-string loop identifier. | `Fn::ForEach:: layout is incorrect` (raised as `InvalidTemplateException`; see `samcli/lib/cfn_language_extensions/processors/foreach.py`). | | More than 5 levels of `Fn::ForEach` are nested. | `Fn::ForEach nesting depth of exceeds the maximum allowed depth of 5. CloudFormation supports up to 5 nested Fn::ForEach loops.` | | The collection resolves to an empty list (e.g., a `CommaDelimitedList` parameter with `Default: ""`). | No error — the loop is silently skipped and no resources are emitted. | -| The `!Ref` in the collection points at a parameter that is not declared in the template. | No error in the typical `sam build` / `sam package` flow. SAM CLI runs intrinsic resolution in PARTIAL mode and preserves the unresolved `{"Ref": ""}`. CloudFormation will reject the unresolved ref at deploy time. | +| The `!Ref` in the collection points at a parameter that is not declared in the template. | No error in the typical `sam build` / `sam package` flow. SAM CLI runs intrinsic resolution in PARTIAL mode and preserves the unresolved `{"Ref": ""}`. At deploy time, CloudFormation will resolve it server side. | ## Limitations diff --git a/samcli/commands/package/package_context.py b/samcli/commands/package/package_context.py index 17529f60be..ac76e9b427 100644 --- a/samcli/commands/package/package_context.py +++ b/samcli/commands/package/package_context.py @@ -24,10 +24,14 @@ import click from samcli.commands.package.exceptions import PackageFailedError +from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException from samcli.lib.bootstrap.companion_stack.companion_stack_manager import sync_ecr_stack -from samcli.lib.cfn_language_extensions.sam_integration import resolve_language_extensions_enabled +from samcli.lib.cfn_language_extensions.sam_integration import ( + expand_language_extensions, + resolve_language_extensions_enabled, +) from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable -from samcli.lib.package.artifact_exporter import Template +from samcli.lib.package.artifact_exporter import Template, _export_global_artifacts_pass from samcli.lib.package.code_signer import CodeSigner from samcli.lib.package.ecr_uploader import ECRUploader from samcli.lib.package.language_extensions_packaging import ( @@ -35,7 +39,7 @@ merge_language_extensions_s3_uris, ) from samcli.lib.package.s3_uploader import S3Uploader -from samcli.lib.package.uploaders import Uploaders +from samcli.lib.package.uploaders import Destination, Uploaders from samcli.lib.providers.provider import ResourceIdentifier, Stack, get_resource_full_path_by_id from samcli.lib.providers.sam_stack_provider import SamLocalStackProvider from samcli.lib.utils.boto_utils import get_boto_config_with_user_agent @@ -174,34 +178,80 @@ def run(self): raise PackageFailedError(template_file=self.template_file, ex=str(ex)) from ex def _export(self, template_path, use_json): - from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException - from samcli.lib.cfn_language_extensions.sam_integration import expand_language_extensions - - # Read the original template + # Read + parse once. Branch methods receive the parsed dict and + # return the output dict; this dispatcher owns the I/O bookends. with open(template_path, "r", encoding="utf-8") as f: original_template_dict = yaml_parse(f.read()) - # Build combined parameter values for expand_language_extensions - parameter_values = {} - parameter_values.update(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES) + # Structural fork on the opt-in flag (#9033). When LE is disabled + # we never invoke any LE machinery — no expand_language_extensions, + # no pre-LE _export_global_artifacts_pass, no merge or Mappings. + # See aws/aws-sam-cli#9027 for why the on path needs the pre-LE pass. + if self._language_extensions_enabled: + output_template = self._export_with_language_extensions(template_path, original_template_dict) + else: + output_template = self._export_without_language_extensions(template_path, original_template_dict) + + if use_json: + return json.dumps(output_template, indent=4, ensure_ascii=False) + return yaml_dump(output_template) + + def _export_without_language_extensions(self, template_path, original_template_dict): + """Export a template with AWS::LanguageExtensions opt-in disabled. + + Mirrors pre-1.160.0 sam-cli behavior: AWS::Include and other global + Fn::Transform exporters are handled inside Template.export() via its + own _export_global_artifacts pass — no pre-Template pass is needed + because there is no LE expansion step that would collapse Fn::Transform + into a JSON-string literal first (see aws/aws-sam-cli#9027 for why the + LE path differs). + """ + template = Template( + template_path, + os.getcwd(), + self.uploaders, + self.code_signer, + normalize_template=True, + normalize_parameters=True, + template_dict=original_template_dict, + language_extensions_enabled=False, + ) + return template.export() + + def _export_with_language_extensions(self, template_path, original_template_dict): + """Export a template with AWS::LanguageExtensions processing enabled. + + Order matters here: + 1. Run AWS::Include (and any other GLOBAL_TRANSFORM_EXPORTS) on the + original template *before* LE expansion. LE functions like + Fn::ToJsonString json.dumps() their argument and collapse any + structural Fn::Transform subtree into a JSON-string literal, + which would hide the include from the post-export walker. + See aws/aws-sam-cli#9027. + 2. Run expand_language_extensions to materialize Fn::ForEach, + Fn::ToJsonString, etc. into a flat resource list. + 3. Run Template.export() on the expanded copy. + 4. Merge the S3 URIs back into the original Fn::ForEach structure + and apply Mappings for any dynamic artifact properties. + """ + template_dir = os.path.dirname(os.path.abspath(template_path)) + _export_global_artifacts_pass( + original_template_dict, + self.uploaders.get(Destination.S3), + template_dir, + ) + + parameter_values = {**IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES} if self.parameter_overrides: parameter_values.update(self.parameter_overrides) if self._global_parameter_overrides: parameter_values.update(self._global_parameter_overrides) - # Use the canonical expand_language_extensions() entry point (Phase 1) try: - result = expand_language_extensions( - original_template_dict, parameter_values, enabled=self._language_extensions_enabled - ) + result = expand_language_extensions(original_template_dict, parameter_values, enabled=True) except InvalidSamDocumentException as e: raise PackageFailedError(template_file=self.template_file, ex=str(e)) from e - uses_language_extensions = result.had_language_extensions - dynamic_properties = result.dynamic_artifact_properties - - # Create Template with the (possibly expanded) template dict directly, - # avoiding a yaml_dump → yaml_parse round-trip. template = Template( template_path, os.getcwd(), @@ -211,38 +261,29 @@ def _export(self, template_path, use_json): normalize_parameters=True, template_dict=copy.deepcopy(result.expanded_template), parameter_values=parameter_values, - language_extensions_enabled=self._language_extensions_enabled, + language_extensions_enabled=True, ) - exported_template = template.export() - # If using language extensions, we need to preserve the original Fn::ForEach structure - # but update the artifact URIs (CodeUri, ContentUri, etc.) with the S3 locations - if uses_language_extensions: - LOG.debug("Template uses language extensions, preserving Fn::ForEach structure") - output_template = merge_language_extensions_s3_uris( - result.original_template, exported_template, dynamic_properties - ) - - # Generate Mappings for dynamic artifact properties - if dynamic_properties: - LOG.debug("Generating Mappings for %d dynamic artifact properties", len(dynamic_properties)) - - template_dir = os.path.dirname(os.path.abspath(template_path)) - exported_resources = exported_template.get("Resources", {}) - - output_template = generate_and_apply_artifact_mappings( - output_template, dynamic_properties, exported_resources, template_dir - ) - else: - output_template = exported_template - - if use_json: - exported_str = json.dumps(output_template, indent=4, ensure_ascii=False) - else: - exported_str = yaml_dump(output_template) + if not result.had_language_extensions: + return exported_template - return exported_str + LOG.debug("Template uses language extensions, preserving Fn::ForEach structure") + output_template = merge_language_extensions_s3_uris( + result.original_template, exported_template, result.dynamic_artifact_properties + ) + if result.dynamic_artifact_properties: + LOG.debug( + "Generating Mappings for %d dynamic artifact properties", + len(result.dynamic_artifact_properties), + ) + output_template = generate_and_apply_artifact_mappings( + output_template, + result.dynamic_artifact_properties, + exported_template.get("Resources", {}), + template_dir, + ) + return output_template @staticmethod def _warn_preview_runtime(stacks: List[Stack]) -> None: diff --git a/samcli/lib/package/artifact_exporter.py b/samcli/lib/package/artifact_exporter.py index 681a3390fb..05ac43ce31 100644 --- a/samcli/lib/package/artifact_exporter.py +++ b/samcli/lib/package/artifact_exporter.py @@ -17,21 +17,28 @@ import copy import logging import os -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional, Sequence, cast from botocore.utils import set_value_from_jmespath from samcli.commands._utils.experimental import ExperimentalFlag, is_experimental_enabled from samcli.commands.package import exceptions from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException +from samcli.lib.cfn_language_extensions.sam_integration import expand_language_extensions from samcli.lib.cfn_language_extensions.utils import iter_regular_resources +from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable from samcli.lib.package.code_signer import CodeSigner +from samcli.lib.package.language_extensions_packaging import ( + generate_and_apply_artifact_mappings, + merge_language_extensions_s3_uris, +) from samcli.lib.package.local_files_utils import get_uploaded_s3_object_name, mktempfile from samcli.lib.package.packageable_resources import ( - GLOBAL_EXPORT_DICT, - METADATA_EXPORT_LIST, + GLOBAL_TRANSFORM_EXPORTS, + METADATA_EXPORTS, RESOURCES_EXPORT_LIST, ECRResource, + MetadataExportSpec, ResourceZip, ) from samcli.lib.package.uploaders import Destination, Uploaders @@ -118,6 +125,84 @@ def _resolve_nested_stack_parameters(nested_params: Dict, parent_parameter_value return resolved +def _build_child_parameter_values( + parent_parameter_values: Optional[Dict], + nested_stack_parameters: Dict, +) -> Dict: + """Build the parameter_values dict the LE expander should see for a child stack. + + CFN-parity scope: pseudo-params (with parent overrides for the pseudo NAMES + only) and the parent's explicit rebindings via the nested-stack ``Parameters`` + property. Non-pseudo parent names are NOT copied — that would diverge from + CloudFormation's nested-stack contract. + + Child template ``Parameters.X.Default`` values are NOT folded in here. The + LE expander's Fn::Ref resolver reads them itself from + ``context.parsed_template.parameters[X]["Default"]``, which + ``TemplateParsingProcessor`` populates as the first step of the expansion + pipeline. So Defaults still take effect at expansion time; they just don't + pass through this helper's return value. + + Names declared in the child but neither defaulted nor rebound are + intentionally absent everywhere — PARTIAL mode preserves the Ref and + CloudFormation errors at deploy time, matching the non-LE path. + """ + parameter_values: Dict = dict(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES) + + if parent_parameter_values: + for pseudo_name in IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES: + if pseudo_name in parent_parameter_values: + parameter_values[pseudo_name] = parent_parameter_values[pseudo_name] + + resolved_nested = _resolve_nested_stack_parameters( + nested_stack_parameters, + dict(parent_parameter_values or {}), + ) + parameter_values.update(resolved_nested) + + return parameter_values + + +def _export_global_artifacts_pass(template_dict: Any, uploader, template_dir: str) -> Any: + """ + Walk template_dict recursively, dispatching dict nodes to handlers + registered in GLOBAL_TRANSFORM_EXPORTS (today: AWS::Include). + + Mutates template_dict in place AND returns it for caller convenience. + + This is the standalone form of Template._export_global_artifacts — + used by callers that need to run the global-transform export pass + on a template before constructing a Template (e.g., the pre-LE + pass that processes AWS::Include before language-extension + expansion runs). + + No-ops on non-dict input (e.g. yaml_parse returning None for empty + template files), so callers can pass the result of yaml_parse + unconditionally. + """ + if not isinstance(template_dict, dict): + return template_dict + + specs_by_key: Dict[str, list] = {} + for spec in GLOBAL_TRANSFORM_EXPORTS: + specs_by_key.setdefault(spec.template_key, []).append(spec) + + for key, val in template_dict.items(): + if key in specs_by_key: + current = val + for spec in specs_by_key[key]: + if spec.discriminator(current): + template_dict[key] = spec.handler(current, uploader, template_dir) + current = template_dict[key] + elif isinstance(val, dict): + _export_global_artifacts_pass(val, uploader, template_dir) + elif isinstance(val, list): + for item in val: + if isinstance(item, dict): + _export_global_artifacts_pass(item, uploader, template_dir) + return template_dict + + class CloudFormationStackResource(ResourceZip): """ Represents CloudFormation::Stack resource that can refer to a nested @@ -135,21 +220,10 @@ def do_export(self, resource_id, resource_dict, parent_dir): export on the nested template, upload the exported template to S3 and set property to URL of the uploaded S3 template. - When the child template uses CloudFormation Language Extensions - (e.g. Fn::ForEach), the template is first expanded so that - Template.export() can discover and upload all generated artifacts. - The S3 URIs are then merged back into the original template - (preserving the Fn::ForEach structure) before uploading. + Routes to the LE-on or LE-off branch based on + self.language_extensions_enabled. The flag is a hard structural + gate — when off, no language-extension machinery runs. """ - from samcli.lib.cfn_language_extensions.sam_integration import ( - expand_language_extensions, - ) - from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable - from samcli.lib.package.language_extensions_packaging import ( - generate_and_apply_artifact_mappings, - merge_language_extensions_s3_uris, - ) - template_path = resource_dict.get(self.PROPERTY_NAME, None) if template_path is None or is_s3_url(template_path): @@ -162,33 +236,95 @@ def do_export(self, resource_id, resource_dict, parent_dir): property_name=self.PROPERTY_NAME, resource_id=resource_id, template_path=abs_template_path ) - # Read and attempt language extensions expansion on the child template + if self.language_extensions_enabled: + exported_template_dict = self._do_export_with_language_extensions( + resource_id, + template_path, + parent_dir, + abs_template_path, + resource_dict, + ) + else: + exported_template_dict = self._do_export_without_language_extensions(resource_id, template_path, parent_dir) + + exported_template_str = yaml_dump(exported_template_dict) + + with mktempfile() as temporary_file: + temporary_file.write(exported_template_str) + temporary_file.flush() + remote_path = get_uploaded_s3_object_name(file_path=temporary_file.name, extension="template") + url = self.uploader.upload(temporary_file.name, remote_path) + + # TemplateUrl property requires S3 URL to be in path-style format + parts = parse_s3_url(url, version_property="Version") + s3_path_url = self.uploader.to_path_style_s3_url(parts["Key"], parts.get("Version", None)) + set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, s3_path_url) + + def _do_export_without_language_extensions(self, resource_id: str, template_path: str, parent_dir: str) -> Dict: + """LE-off branch: legacy path-based Template construction. + + Template.export() runs its own internal _export_global_artifacts so + AWS::Include still resolves on this path. No pre-LE pass, no + parameter_values, no parent_parameter_values, no copy.deepcopy — + none of them are needed without language extensions. + """ + return Template( + template_path, + parent_dir, + self.uploaders, + self.code_signer, + normalize_template=True, + normalize_parameters=True, + parent_stack_id=resource_id, + language_extensions_enabled=False, + ).export() + + def _do_export_with_language_extensions( + self, + resource_id: str, + template_path: str, + parent_dir: str, + abs_template_path: str, + resource_dict: Dict, + ) -> Dict: + """LE-on branch: dict-based Template construction with full LE machinery. + + Reads the child template to a dict, runs the pre-LE AWS::Include pass + (#9027), threads pseudo-params + parent_parameter_values + nested + Parameters into expand_language_extensions, and merges S3 URIs back + if expansion produced any. + + Falls back to the no-extensions path for InvalidSamDocumentException + (expected user-facing failure) and unexpected exceptions (SAM CLI + bugs — logged at ERROR with traceback). + """ with open(abs_template_path, "r", encoding="utf-8") as f: child_template_dict = yaml_parse(f.read()) child_template_dir = os.path.dirname(abs_template_path) - # Merge pseudo-parameters with: - # 1) values propagated from the parent Template (parent stack's own params - # + CLI --parameter-overrides + pseudo-params), used to resolve intrinsics - # in the nested stack's Parameters property; - # 2) the nested stack's Parameters property on the parent resource, which is - # the authoritative source for the child's parameter values at deploy time. - # Child-local values override parent-propagated ones on key conflict, which - # matches CloudFormation's behavior (a child parameter shadows a same-named - # parent parameter unless explicitly wired). - parent_parameter_values = dict(self.parent_parameter_values or {}) - - nested_params = resource_dict.get("Parameters", {}) or {} - resolved_nested_params = _resolve_nested_stack_parameters(nested_params, parent_parameter_values) + # Process AWS::Include before LE expansion to mirror CFN's transform + # ordering. See aws/aws-sam-cli#9027. + # + # NOTE: mutates child_template_dict in place. Must run before the + # expand_language_extensions call below so result.original_template + # and result.expanded_template both observe the rewrite. + _export_global_artifacts_pass( + child_template_dict, + self.uploaders.get(ResourceZip.EXPORT_DESTINATION), + child_template_dir, + ) - parameter_values = dict(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES) - parameter_values.update(parent_parameter_values) - parameter_values.update(resolved_nested_params) + parameter_values = _build_child_parameter_values( + self.parent_parameter_values, + resource_dict.get("Parameters", {}) or {}, + ) try: result = expand_language_extensions( - child_template_dict, parameter_values, enabled=self.language_extensions_enabled + child_template_dict, + parameter_values, + enabled=self.language_extensions_enabled, ) except InvalidSamDocumentException as e: # Expected failure path: the child template triggered the @@ -207,9 +343,6 @@ def do_export(self, resource_id, resource_dict, parent_dir): ) result = None except Exception as e: # pylint: disable=broad-except - # Unexpected failure: a bug in SAM CLI rather than a malformed template. - # Surface at ERROR with a traceback so users running --debug (and SAM - # telemetry) see it, but don't abort the rest of the package run. LOG.error( "Internal error expanding language extensions for %s. This is a " "SAM CLI bug; please report at " @@ -222,9 +355,11 @@ def do_export(self, resource_id, resource_dict, parent_dir): result = None if result and result.had_language_extensions: - LOG.debug("Child template %s uses language extensions, expanding before export", abs_template_path) + LOG.debug( + "Child template %s uses language extensions, expanding before export", + abs_template_path, + ) - # Create Template from the expanded template string template = Template( template_path, parent_dir, @@ -240,12 +375,12 @@ def do_export(self, resource_id, resource_dict, parent_dir): exported_template = template.export() - # Merge S3 URIs back into the original template (preserving Fn::ForEach) exported_template_dict = merge_language_extensions_s3_uris( - result.original_template, exported_template, result.dynamic_artifact_properties + result.original_template, + exported_template, + result.dynamic_artifact_properties, ) - # Generate and apply Mappings for dynamic artifact properties if result.dynamic_artifact_properties: LOG.debug( "Generating Mappings for %d dynamic artifact properties in child template", @@ -259,7 +394,6 @@ def do_export(self, resource_id, resource_dict, parent_dir): child_template_dir, ) else: - # No language extensions — use existing flow exported_template_dict = Template( template_path, parent_dir, @@ -272,18 +406,7 @@ def do_export(self, resource_id, resource_dict, parent_dir): language_extensions_enabled=self.language_extensions_enabled, ).export() - exported_template_str = yaml_dump(exported_template_dict) - - with mktempfile() as temporary_file: - temporary_file.write(exported_template_str) - temporary_file.flush() - remote_path = get_uploaded_s3_object_name(file_path=temporary_file.name, extension="template") - url = self.uploader.upload(temporary_file.name, remote_path) - - # TemplateUrl property requires S3 URL to be in path-style format - parts = parse_s3_url(url, version_property="Version") - s3_path_url = self.uploader.to_path_style_s3_url(parts["Key"], parts.get("Version", None)) - set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, s3_path_url) + return exported_template_dict class ServerlessApplicationResource(CloudFormationStackResource): @@ -341,7 +464,7 @@ class Template: template_dict: Dict template_dir: str resources_to_export: frozenset - metadata_to_export: frozenset + metadata_to_export: Sequence[MetadataExportSpec] uploaders: Uploaders code_signer: CodeSigner @@ -355,7 +478,7 @@ def __init__( RESOURCES_EXPORT_LIST + [CloudFormationStackResource, CloudFormationStackSetResource, ServerlessApplicationResource] ), - metadata_to_export=frozenset(METADATA_EXPORT_LIST), + metadata_to_export=tuple(METADATA_EXPORTS), template_str: Optional[str] = None, normalize_template: bool = False, normalize_parameters: bool = False, @@ -409,24 +532,18 @@ def __init__( self.language_extensions_enabled = language_extensions_enabled def _export_global_artifacts(self, template_dict: Dict) -> Dict: + """See module-level _export_global_artifacts_pass for the canonical + implementation. This wrapper exists so Template.export() doesn't + need to know about uploader / template_dir plumbing. """ - Template params such as AWS::Include transforms are not specific to - any resource type but contain artifacts that should be exported, - here we iterate through the template dict and export params with a - handler defined in GLOBAL_EXPORT_DICT - """ - for key, val in template_dict.items(): - if key in GLOBAL_EXPORT_DICT: - template_dict[key] = GLOBAL_EXPORT_DICT[key]( - val, self.uploaders.get(ResourceZip.EXPORT_DESTINATION), self.template_dir - ) - elif isinstance(val, dict): - self._export_global_artifacts(val) - elif isinstance(val, list): - for item in val: - if isinstance(item, dict): - self._export_global_artifacts(item) - return template_dict + result = _export_global_artifacts_pass( + template_dict, + self.uploaders.get(ResourceZip.EXPORT_DESTINATION), + self.template_dir, + ) + # template_dict is guaranteed to be a dict here, so the pass returns + # the same dict back. cast() narrows the return type for mypy. + return cast(Dict, result) def _export_metadata(self): """ @@ -436,11 +553,13 @@ def _export_metadata(self): if "Metadata" not in self.template_dict: return - for metadata_type, metadata_dict in self.template_dict["Metadata"].items(): - for exporter_class in self.metadata_to_export: - if exporter_class.RESOURCE_TYPE != metadata_type: - continue + specs_by_type = {spec.metadata_type: spec for spec in self.metadata_to_export} + for metadata_type, metadata_dict in self.template_dict["Metadata"].items(): + spec = specs_by_type.get(metadata_type) + if spec is None: + continue + for exporter_class in spec.exporters: exporter = exporter_class(self.uploaders, self.code_signer) exporter.export(metadata_type, metadata_dict, self.template_dir) diff --git a/samcli/lib/package/language_extensions_packaging.py b/samcli/lib/package/language_extensions_packaging.py index 9eb39366ae..46123ef299 100644 --- a/samcli/lib/package/language_extensions_packaging.py +++ b/samcli/lib/package/language_extensions_packaging.py @@ -27,6 +27,7 @@ substitute_loop_variable, ) from samcli.lib.cfn_language_extensions.utils import FOREACH_REQUIRED_ELEMENTS, is_foreach_key +from samcli.lib.package.packageable_resources import METADATA_EXPORTS LOG = logging.getLogger(__name__) @@ -82,6 +83,8 @@ def merge_language_extensions_s3_uris( _update_resources_with_s3_uris(original_resources, exported_resources, dynamic_prop_keys) + _merge_metadata(result.get("Metadata", {}), exported_template.get("Metadata", {})) + return result @@ -183,6 +186,24 @@ def _resolve_property_paths(prop_names: List[str], properties: Dict[str, Any]) - # --------------------------------------------------------------------------- +def _merge_metadata( + original_metadata: Dict[str, Any], + exported_metadata: Dict[str, Any], +) -> None: + """Copy rewritten property values from exported_metadata into + original_metadata for every (metadata_type, property_name) declared + in METADATA_EXPORTS. Mutates original_metadata in place. + """ + for spec in METADATA_EXPORTS: + original_entry = original_metadata.get(spec.metadata_type) + exported_entry = exported_metadata.get(spec.metadata_type) + if not isinstance(original_entry, dict) or not isinstance(exported_entry, dict): + continue + for prop_name in spec.property_names: + if prop_name in exported_entry: + original_entry[prop_name] = exported_entry[prop_name] + + def _update_resources_with_s3_uris( original_resources: Dict[str, Any], exported_resources: Dict[str, Any], diff --git a/samcli/lib/package/packageable_resources.py b/samcli/lib/package/packageable_resources.py index d63136df8f..60d06fc851 100644 --- a/samcli/lib/package/packageable_resources.py +++ b/samcli/lib/package/packageable_resources.py @@ -5,7 +5,8 @@ import logging import os import shutil -from typing import Dict, Optional, Union, cast +from dataclasses import dataclass, field +from typing import Callable, Dict, List, Optional, Type, Union, cast import jmespath from botocore.utils import set_value_from_jmespath @@ -717,7 +718,29 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st GraphQLApiCodeResource, ] -METADATA_EXPORT_LIST = [ServerlessRepoApplicationReadme, ServerlessRepoApplicationLicense] + +@dataclass(frozen=True) +class MetadataExportSpec: + """How a top-level Metadata. entry is exported and merged back. + + Used by Template._export_metadata to route entries to their exporter + classes, and by merge_language_extensions_s3_uris to copy rewritten + property values back into the original (Fn::ForEach-preserving) + child-stack template after LE expansion. + """ + + metadata_type: str + property_names: List[str] + exporters: List[Type["Resource"]] = field(default_factory=list) + + +METADATA_EXPORTS: List[MetadataExportSpec] = [ + MetadataExportSpec( + metadata_type=AWS_SERVERLESSREPO_APPLICATION, + property_names=["LicenseUrl", "ReadmeUrl"], + exporters=[ServerlessRepoApplicationLicense, ServerlessRepoApplicationReadme], + ), +] def include_transform_export_handler(template_dict, uploader, parent_dir): @@ -741,4 +764,29 @@ def include_transform_export_handler(template_dict, uploader, parent_dir): return template_dict -GLOBAL_EXPORT_DICT = {"Fn::Transform": include_transform_export_handler} +@dataclass(frozen=True) +class GlobalTransformExportSpec: + """How a global (anywhere-in-the-tree) export hook is invoked. + + The discriminator decides whether a given dict node matches this spec + (dispatch on Fn::Transform's "Name" field today; ready for future + Fn::Transform variants). The handler receives the matched node and + mutates it in place to rewrite local paths to S3 URLs. + """ + + template_key: str + discriminator: Callable[[object], bool] + handler: Callable + + +def _is_aws_include(node: object) -> bool: + return isinstance(node, dict) and node.get("Name") == "AWS::Include" + + +GLOBAL_TRANSFORM_EXPORTS: List[GlobalTransformExportSpec] = [ + GlobalTransformExportSpec( + template_key="Fn::Transform", + discriminator=_is_aws_include, + handler=include_transform_export_handler, + ), +] diff --git a/tests/unit/commands/package/test_data/buried_aws_include/export-events.json b/tests/unit/commands/package/test_data/buried_aws_include/export-events.json new file mode 100644 index 0000000000..2eb28cebd5 --- /dev/null +++ b/tests/unit/commands/package/test_data/buried_aws_include/export-events.json @@ -0,0 +1 @@ +[{"event": "demo"}] diff --git a/tests/unit/commands/package/test_data/buried_aws_include/template.yaml b/tests/unit/commands/package/test_data/buried_aws_include/template.yaml new file mode 100644 index 0000000000..2558ea1fcc --- /dev/null +++ b/tests/unit/commands/package/test_data/buried_aws_include/template.yaml @@ -0,0 +1,13 @@ +Transform: AWS::LanguageExtensions +Resources: + Parameter: + Type: AWS::SSM::Parameter + Properties: + Type: String + DataType: text + Value: + Fn::ToJsonString: + Fn::Transform: + Name: AWS::Include + Parameters: + Location: export-events.json diff --git a/tests/unit/commands/package/test_package_context.py b/tests/unit/commands/package/test_package_context.py index 3e641c548b..a9e255dace 100644 --- a/tests/unit/commands/package/test_package_context.py +++ b/tests/unit/commands/package/test_package_context.py @@ -1,10 +1,14 @@ """Test sam package command""" +import os +from pathlib import Path from unittest import TestCase from unittest.mock import patch, MagicMock, Mock, call, ANY from parameterized import parameterized import tempfile +TEST_DATA_PATH = Path(__file__).resolve().parent / "test_data" + from samcli.commands.package.package_context import PackageContext from samcli.commands.package.exceptions import PackageFailedError @@ -27,9 +31,11 @@ _apply_artifact_mappings_to_template, _replace_dynamic_artifact_with_findmap, ) +from samcli.lib.package.uploaders import Destination, Uploaders from samcli.lib.providers.sam_stack_provider import SamLocalStackProvider from samcli.lib.samlib.resource_metadata_normalizer import ResourceMetadataNormalizer from samcli.lib.utils.resources import AWS_LAMBDA_FUNCTION, AWS_SERVERLESS_FUNCTION +from samcli.yamlhelper import yaml_parse class TestPackageCommand(TestCase): @@ -4462,3 +4468,124 @@ def test_apply_mappings_replaces_build_mappings(self): codeuri = body["${FunctionName}Function"]["Properties"]["CodeUri"] self.assertIn("Fn::FindInMap", codeuri) self.assertEqual(codeuri["Fn::FindInMap"][0], "SAMCodeUriFunctions") + + +class TestPackageContextBuriedAWSInclude(TestCase): + """A root-level template that uses Fn::ToJsonString (a language extension) + and contains AWS::Include buried inside AWS::SSM::Parameter must end up + with the include's Location rewritten to the s3:// URL the uploader + returned, matching the behavior of sam-cli 1.159.0. + + Regression coverage for https://github.com/aws/aws-sam-cli/issues/9027. + Nested-stack coverage lives in tests/unit/lib/package/test_artifact_exporter.py. + """ + + def setUp(self): + self.template_path = str(TEST_DATA_PATH / "buried_aws_include" / "template.yaml") + + # Mock uploader with deterministic S3 URL. + self.s3_uploader = MagicMock() + self.s3_uploader.upload_with_dedup.side_effect = ( + lambda local_path, **_: f"s3://my-bucket/{os.path.basename(local_path)}-md5" + ) + self.s3_uploader.EXPORT_DESTINATION = Destination.S3 + + self.uploaders = MagicMock(spec=Uploaders) + self.uploaders.get.return_value = self.s3_uploader + + # Bypass PackageContext.__init__ — wire only the attributes _export reads. + self.ctx = PackageContext.__new__(PackageContext) + self.ctx.template_file = self.template_path + self.ctx.uploaders = self.uploaders + self.ctx.code_signer = MagicMock() + self.ctx.parameter_overrides = {} + self.ctx._global_parameter_overrides = {} + # Language Extensions is opt-in (#9033); the buried-AWS::Include regression + # test exercises a template that uses Transform: AWS::LanguageExtensions, + # so enable LE expansion explicitly. + self.ctx._language_extensions_enabled = True + + def test_buried_aws_include_location_is_rewritten(self): + exported_str = self.ctx._export(self.template_path, use_json=False) + + exported = yaml_parse(exported_str) + + location = exported["Resources"]["Parameter"]["Properties"]["Value"]["Fn::ToJsonString"]["Fn::Transform"][ + "Parameters" + ]["Location"] + self.assertTrue( + location.startswith("s3://"), + f"Issue #9027: AWS::Include Location must be an S3 URI after sam package, " f"got {location!r}.", + ) + self.assertEqual(location, "s3://my-bucket/export-events.json-md5") + + # Sanity: the LE Transform line is preserved on the root. + self.assertEqual(exported.get("Transform"), "AWS::LanguageExtensions") + + +class TestExportLanguageExtensionsStructuralGate(TestCase): + """Lock in the contract that --language-extensions is a hard correctness + boundary, not a passthrough hint. When disabled, _export() must not invoke + any LE machinery (expand_language_extensions, the pre-LE + _export_global_artifacts_pass, merge_language_extensions_s3_uris, or + generate_and_apply_artifact_mappings). + """ + + def _make_off_path_context(self): + """Build a PackageContext with LE disabled and the attributes _export reads.""" + ctx = PackageContext.__new__(PackageContext) + ctx.template_file = "template.yaml" + ctx.uploaders = MagicMock() + ctx.code_signer = MagicMock() + ctx.parameter_overrides = {} + ctx._global_parameter_overrides = {} + ctx._language_extensions_enabled = False + return ctx + + @patch("samcli.commands.package.package_context.Template") + @patch("samcli.commands.package.package_context.yaml_parse") + @patch("builtins.open", create=True) + @patch("samcli.commands.package.package_context.expand_language_extensions") + def test_off_path_does_not_invoke_expand_language_extensions( + self, mock_expand, mock_open, mock_yaml_parse, mock_template_class + ): + """When --language-extensions is off, expand_language_extensions must not be called.""" + ctx = self._make_off_path_context() + + mock_yaml_parse.return_value = {"Resources": {}} + mock_template_instance = MagicMock() + mock_template_instance.export.return_value = {"Resources": {}} + mock_template_class.return_value = mock_template_instance + + mock_file = MagicMock() + mock_open.return_value.__enter__.return_value = mock_file + mock_file.read.return_value = "" + + ctx._export("template.yaml", use_json=False) + + mock_expand.assert_not_called() + + @patch("samcli.commands.package.package_context.Template") + @patch("samcli.commands.package.package_context.yaml_parse") + @patch("builtins.open", create=True) + @patch("samcli.commands.package.package_context._export_global_artifacts_pass") + def test_off_path_does_not_invoke_pre_le_global_transform_pass( + self, mock_pre_le_pass, mock_open, mock_yaml_parse, mock_template_class + ): + """When --language-extensions is off, the pre-LE _export_global_artifacts_pass + in package_context.py must not be called. Template.export() still runs its own + internal _export_global_artifacts pass — that one is not patched here.""" + ctx = self._make_off_path_context() + + mock_yaml_parse.return_value = {"Resources": {}} + mock_template_instance = MagicMock() + mock_template_instance.export.return_value = {"Resources": {}} + mock_template_class.return_value = mock_template_instance + + mock_file = MagicMock() + mock_open.return_value.__enter__.return_value = mock_file + mock_file.read.return_value = "" + + ctx._export("template.yaml", use_json=False) + + mock_pre_le_pass.assert_not_called() diff --git a/tests/unit/lib/cfn_language_extensions/test_phase_separation.py b/tests/unit/lib/cfn_language_extensions/test_phase_separation.py index 7eccaba88c..fccaf61671 100644 --- a/tests/unit/lib/cfn_language_extensions/test_phase_separation.py +++ b/tests/unit/lib/cfn_language_extensions/test_phase_separation.py @@ -581,7 +581,7 @@ def test_sam_template_validator_no_longer_has_process_language_extensions(self): @patch("samcli.commands.package.package_context.yaml_parse") @patch("builtins.open", create=True) - @patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") + @patch("samcli.commands.package.package_context.expand_language_extensions") def test_package_context_export_calls_expand_language_extensions(self, mock_expand, mock_open, mock_yaml_parse): """PackageContext._export() calls expand_language_extensions().""" from samcli.commands.package.package_context import PackageContext diff --git a/tests/unit/lib/package/test_artifact_exporter.py b/tests/unit/lib/package/test_artifact_exporter.py index 59f7be0b1d..430d775855 100644 --- a/tests/unit/lib/package/test_artifact_exporter.py +++ b/tests/unit/lib/package/test_artifact_exporter.py @@ -1,13 +1,17 @@ +import copy import functools +import inspect import json import os import platform import random +import shutil import string import tempfile import unittest import zipfile from contextlib import contextmanager, closing +from pathlib import Path from typing import Optional, Dict from unittest import mock from unittest.mock import call, patch, Mock, MagicMock @@ -16,6 +20,10 @@ from samcli.commands.package import exceptions from samcli.commands.package.exceptions import ExportFailedError from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException +from samcli.lib.cfn_language_extensions.sam_integration import ( + LanguageExtensionResult, + expand_language_extensions, +) from samcli.lib.package.artifact_exporter import ( is_local_folder, make_abs_path, @@ -23,13 +31,20 @@ CloudFormationStackResource, CloudFormationStackSetResource, ServerlessApplicationResource, + _build_child_parameter_values, + _export_global_artifacts_pass, + _resolve_nested_stack_parameters, ) +from samcli.lib.package.language_extensions_packaging import merge_language_extensions_s3_uris +from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable from samcli.lib.package.packageable_resources import ( GraphQLApiCodeResource, GraphQLApiSchemaResource, is_s3_protocol_url, is_local_file, upload_local_artifacts, + METADATA_EXPORTS, + MetadataExportSpec, Resource, ResourceWithS3UrlDict, ServerlessApiResource, @@ -41,7 +56,8 @@ LambdaLayerVersionResource, copy_to_temp_dir, include_transform_export_handler, - GLOBAL_EXPORT_DICT, + GLOBAL_TRANSFORM_EXPORTS, + GlobalTransformExportSpec, ServerlessLayerVersionResource, ServerlessRepoApplicationLicense, ServerlessRepoApplicationReadme, @@ -69,8 +85,11 @@ from samcli.lib.package.utils import zip_folder, make_zip, make_zip_with_lambda_permissions, make_zip_with_permissions from samcli.lib.utils.packagetype import ZIP, IMAGE from samcli.lib.utils.resources import LAMBDA_LOCAL_RESOURCES, RESOURCES_WITH_LOCAL_PATHS +from samcli.yamlhelper import yaml_dump from tests.testing_utils import FileCreator +TEST_DATA_PATH = Path(__file__).resolve().parent / "test_data" + class TestArtifactExporter(unittest.TestCase): def setUp(self): @@ -1171,6 +1190,7 @@ class MockResource(ResourceZip): @patch("samcli.lib.package.artifact_exporter.Template") def test_export_cloudformation_stack(self, TemplateMock): stack_resource = CloudFormationStackResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "id" property_name = stack_resource.PROPERTY_NAME @@ -1205,7 +1225,7 @@ def test_export_cloudformation_stack(self, TemplateMock): normalize_template=True, parent_stack_id="id", parameter_values=mock.ANY, - language_extensions_enabled=False, + language_extensions_enabled=True, ) template_instance_mock.export.assert_called_once_with() self.s3_uploader_mock.upload.assert_called_once_with(mock.ANY, mock.ANY) @@ -1353,6 +1373,7 @@ def test_export_cloudformation_stack_set_no_upload_path_not_file(self): @patch("samcli.lib.package.artifact_exporter.Template") def test_export_serverless_application(self, TemplateMock): stack_resource = ServerlessApplicationResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "id" property_name = stack_resource.PROPERTY_NAME @@ -1387,7 +1408,7 @@ def test_export_serverless_application(self, TemplateMock): normalize_template=True, parent_stack_id="id", parameter_values=mock.ANY, - language_extensions_enabled=False, + language_extensions_enabled=True, ) template_instance_mock.export.assert_called_once_with() self.s3_uploader_mock.upload.assert_called_once_with(mock.ANY, mock.ANY) @@ -1478,7 +1499,18 @@ def test_template_export_metadata(self, yaml_parse_mock): metadata_type2_instance = Mock() metadata_type2_class.return_value = metadata_type2_instance - metadata_to_export = [metadata_type1_class, metadata_type2_class] + metadata_to_export = [ + MetadataExportSpec( + metadata_type="metadata_type1", + property_names=["property_1"], + exporters=[metadata_type1_class], + ), + MetadataExportSpec( + metadata_type="metadata_type2", + property_names=["property_2"], + exporters=[metadata_type2_class], + ), + ] template_dict = {"Metadata": {"metadata_type1": {"property_1": "abc"}, "metadata_type2": {"property_2": "def"}}} open_mock = mock.mock_open() @@ -1854,8 +1886,13 @@ def test_template_global_export(self, yaml_parse_mock): } yaml_parse_mock.return_value = template_dict + mock_spec = GlobalTransformExportSpec( + template_key="Fn::Transform", + discriminator=lambda v: isinstance(v, dict), + handler=include_transform_export_handler_mock, + ) with patch("samcli.lib.package.artifact_exporter.open", open_mock(read_data=template_str)) as open_mock: - with patch.dict(GLOBAL_EXPORT_DICT, {"Fn::Transform": include_transform_export_handler_mock}): + with patch("samcli.lib.package.artifact_exporter.GLOBAL_TRANSFORM_EXPORTS", [mock_spec]): template_exporter = Template(template_path, parent_dir, self.uploaders_mock, resources_to_export) exported_template = template_exporter._export_global_artifacts(template_exporter.template_dict) @@ -1972,6 +2009,186 @@ def test_include_transform_export_handler_non_include_transform(self, is_local_f self.s3_uploader_mock.upload_with_dedup.assert_not_called() self.assertEqual(handler_output, {"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}}) + def test_le_template_with_top_level_aws_include_merges_location(self): + """End-to-end regression: an LE template with a top-level AWS::Include + in Outputs (outside any Fn::ForEach body) must end up with its + Location rewritten to the uploader's S3 URL after the + package_context._export-equivalent flow: + + _export_global_artifacts_pass(template_dict, ...) + -> expand_language_extensions(...) + -> Template(template_dict=expanded).export() + -> merge_language_extensions_s3_uris(original, exported, dynamic) + + The pre-LE global-transform pass is what now rewrites AWS::Include + Locations; the merge pass only handles resource artifact properties + and Metadata. See aws/aws-sam-cli#9027. + """ + # Predictable upload URL keyed on basename so we can assert on it later. + # upload_with_dedup is invoked with absolute paths (for AWS::Include) and + # with absolute paths to temp zips (for ServerlessFunctionResource folder + # uploads); both forms have a basename we can use. + self.s3_uploader_mock.upload_with_dedup.side_effect = ( + lambda local_path, **_: f"s3://bucket/{os.path.basename(local_path)}-md5" + ) + + # Real files on disk so is_local_file / is_local_folder return True + # without needing to mock them. extra.yaml is the AWS::Include target; + # code/ is the CodeUri folder for the (post-expansion) functions. + parent_dir = str(TEST_DATA_PATH / "le_top_level_include") + + template_dict = { + "Transform": "AWS::LanguageExtensions", + "Resources": { + "Fn::ForEach::Services": [ + "Name", + ["A", "B"], + { + "${Name}Function": { + "Type": "AWS::Serverless::Function", + "Properties": { + "CodeUri": "./code", + "Handler": "index.handler", + "Runtime": "python3.11", + }, + } + }, + ] + }, + "Outputs": { + "Extra": { + "Value": { + "Fn::Transform": { + "Name": "AWS::Include", + "Parameters": {"Location": "./extra.yaml"}, + } + } + } + }, + } + + # Mirror PackageContext._export's flow: run the pre-LE + # global-transform pass on the original template before + # language-extension expansion. + _export_global_artifacts_pass(template_dict, self.s3_uploader_mock, parent_dir) + + parameter_values = dict(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES) + + result = expand_language_extensions(template_dict, parameter_values, enabled=True) + self.assertTrue(result.had_language_extensions) + + template = Template( + template_path="template.yaml", + parent_dir=parent_dir, + uploaders=self.uploaders_mock, + code_signer=self.code_signer_mock, + normalize_template=True, + normalize_parameters=True, + template_dict=copy.deepcopy(result.expanded_template), + parameter_values=parameter_values, + ) + exported_template = template.export() + + output = merge_language_extensions_s3_uris( + result.original_template, exported_template, result.dynamic_artifact_properties + ) + + # The original Fn::ForEach is preserved (not re-expanded) in the merged output. + self.assertIn("Fn::ForEach::Services", output["Resources"]) + + # The top-level AWS::Include Location is rewritten to the uploader's S3 URL. + out_location = output["Outputs"]["Extra"]["Value"]["Fn::Transform"]["Parameters"]["Location"] + self.assertEqual(out_location, "s3://bucket/extra.yaml-md5") + + def test_le_template_with_serverless_repo_metadata_merges_license_url(self): + """End-to-end regression: an LE template with + AWS::ServerlessRepo::Application metadata containing local + LicenseUrl/ReadmeUrl paths must end up with both rewritten to + S3 URLs after the package_context._export-equivalent flow: + + expand_language_extensions(...) + -> Template(template_dict=expanded).export() + -> merge_language_extensions_s3_uris(original, exported, dynamic) + + This covers the Metadata merge pass (Task 4) wired into the + artifact-exporter's export plumbing. + """ + # SAR exporters extend ResourceZip, whose do_export ultimately + # routes through upload_local_artifacts -> uploader.upload_with_dedup. + # Same lambda as Task 7: a predictable URL keyed on basename. + self.s3_uploader_mock.upload_with_dedup.side_effect = ( + lambda local_path, **_: f"s3://bucket/{os.path.basename(local_path)}-md5" + ) + + # Real files on disk for the SAR exporters (LicenseUrl / ReadmeUrl) + # and for the Fn::ForEach-expanded SAM function's CodeUri. + parent_dir = str(TEST_DATA_PATH / "le_sar_metadata") + + template_dict = { + "Transform": "AWS::LanguageExtensions", + "Metadata": { + "AWS::ServerlessRepo::Application": { + "Name": "MyApp", + "Description": "Test app", + "Author": "Test", + "SemanticVersion": "1.0.0", + "LicenseUrl": "./LICENSE.txt", + "ReadmeUrl": "./README.md", + } + }, + "Resources": { + "Fn::ForEach::Services": [ + "Name", + ["A"], + { + "${Name}Function": { + "Type": "AWS::Serverless::Function", + "Properties": { + "CodeUri": "./code", + "Handler": "index.handler", + "Runtime": "python3.11", + }, + } + }, + ] + }, + } + + parameter_values = dict(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES) + + result = expand_language_extensions(template_dict, parameter_values, enabled=True) + self.assertTrue(result.had_language_extensions) + + template = Template( + template_path="template.yaml", + parent_dir=parent_dir, + uploaders=self.uploaders_mock, + code_signer=self.code_signer_mock, + normalize_template=True, + normalize_parameters=True, + template_dict=copy.deepcopy(result.expanded_template), + parameter_values=parameter_values, + ) + exported_template = template.export() + + output = merge_language_extensions_s3_uris( + result.original_template, exported_template, result.dynamic_artifact_properties + ) + + # SAR LicenseUrl and ReadmeUrl must both be rewritten to S3 URLs. + sar = output["Metadata"]["AWS::ServerlessRepo::Application"] + self.assertTrue(sar["LicenseUrl"].startswith("s3://"), f"LicenseUrl: {sar['LicenseUrl']!r}") + self.assertTrue(sar["ReadmeUrl"].startswith("s3://"), f"ReadmeUrl: {sar['ReadmeUrl']!r}") + # The basenames identify which file was uploaded to which property, + # confirming the SAR exporter fired once for License and once for Readme. + self.assertIn("LICENSE.txt", sar["LicenseUrl"]) + self.assertIn("README.md", sar["ReadmeUrl"]) + # Unrelated SAR metadata is preserved untouched. + self.assertEqual(sar["Name"], "MyApp") + self.assertEqual(sar["SemanticVersion"], "1.0.0") + # The original Fn::ForEach is preserved (not re-expanded) in the merged output. + self.assertIn("Fn::ForEach::Services", output["Resources"]) + def test_template_export_path_be_folder(self): template_path = "/path/foo" # Set parent_dir to be a non-existent folder @@ -2313,9 +2530,8 @@ def test_export_cloudformation_stack_with_language_extensions(self, TemplateMock When a child template uses Fn::ForEach, do_export should expand it, export the expanded template, then merge S3 URIs back. """ - from samcli.lib.cfn_language_extensions.sam_integration import LanguageExtensionResult - stack_resource = CloudFormationStackResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "NestedStack" property_name = stack_resource.PROPERTY_NAME @@ -2416,7 +2632,7 @@ def test_export_cloudformation_stack_with_language_extensions(self, TemplateMock parent_dir = tempfile.gettempdir() with patch( - "samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions", + "samcli.lib.package.artifact_exporter.expand_language_extensions", return_value=lang_ext_result, ): stack_resource.export(resource_id, resource_dict, parent_dir) @@ -2434,14 +2650,14 @@ def test_export_cloudformation_stack_with_language_extensions(self, TemplateMock os.remove(template_path) @patch("samcli.lib.package.artifact_exporter.Template") - def test_export_cloudformation_stack_without_language_extensions(self, TemplateMock): + def test_export_cloudformation_stack_language_extensions_no_le_in_template(self, TemplateMock): """ - When a child template does NOT use language extensions, - do_export should use the original flow (no expansion). + When LE is opted-in but the child template uses no LE, do_export + should still go through the LE-on branch and exit via the + no-extension sub-path. """ - from samcli.lib.cfn_language_extensions.sam_integration import LanguageExtensionResult - stack_resource = CloudFormationStackResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "NestedStack" property_name = stack_resource.PROPERTY_NAME @@ -2473,7 +2689,7 @@ def test_export_cloudformation_stack_without_language_extensions(self, TemplateM parent_dir = tempfile.gettempdir() with patch( - "samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions", + "samcli.lib.package.artifact_exporter.expand_language_extensions", return_value=no_ext_result, ): stack_resource.export(resource_id, resource_dict, parent_dir) @@ -2495,6 +2711,7 @@ def test_export_cloudformation_stack_language_extensions_expansion_failure(self, do_export should fall back to the original flow. """ stack_resource = CloudFormationStackResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "NestedStack" property_name = stack_resource.PROPERTY_NAME @@ -2519,7 +2736,7 @@ def test_export_cloudformation_stack_language_extensions_expansion_failure(self, parent_dir = tempfile.gettempdir() with patch( - "samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions", + "samcli.lib.package.artifact_exporter.expand_language_extensions", side_effect=InvalidSamDocumentException("expansion failed"), ): stack_resource.export(resource_id, resource_dict, parent_dir) @@ -2539,6 +2756,7 @@ def test_export_cloudformation_stack_unexpected_exception_falls_back(self, Templ it should fall back gracefully to the non-extension flow (not abort the parent packaging). """ stack_resource = CloudFormationStackResource(self.uploaders_mock, self.code_signer_mock) + stack_resource.language_extensions_enabled = True resource_id = "NestedStack" property_name = stack_resource.PROPERTY_NAME @@ -2563,7 +2781,7 @@ def test_export_cloudformation_stack_unexpected_exception_falls_back(self, Templ parent_dir = tempfile.gettempdir() with patch( - "samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions", + "samcli.lib.package.artifact_exporter.expand_language_extensions", side_effect=TypeError("unexpected bug"), ): stack_resource.export(resource_id, resource_dict, parent_dir) @@ -2573,6 +2791,31 @@ def test_export_cloudformation_stack_unexpected_exception_falls_back(self, Templ finally: os.remove(template_path) + def test_metadata_exports_registry_shape(self): + self.assertTrue(all(isinstance(s, MetadataExportSpec) for s in METADATA_EXPORTS)) + + by_type = {s.metadata_type: s for s in METADATA_EXPORTS} + spec = by_type["AWS::ServerlessRepo::Application"] + + self.assertEqual(sorted(spec.property_names), ["LicenseUrl", "ReadmeUrl"]) + self.assertIn(ServerlessRepoApplicationLicense, spec.exporters) + self.assertIn(ServerlessRepoApplicationReadme, spec.exporters) + + def test_global_transform_exports_registry_shape(self): + self.assertTrue(all(isinstance(s, GlobalTransformExportSpec) for s in GLOBAL_TRANSFORM_EXPORTS)) + + aws_include = next( + s for s in GLOBAL_TRANSFORM_EXPORTS if s.discriminator({"Name": "AWS::Include", "Parameters": {}}) + ) + + self.assertEqual(aws_include.template_key, "Fn::Transform") + self.assertIs(aws_include.handler, include_transform_export_handler) + + # Discriminator must reject non-AWS::Include transforms and non-dict inputs + self.assertFalse(aws_include.discriminator({"Name": "AWS::OtherTransform"})) + self.assertFalse(aws_include.discriminator(None)) + self.assertFalse(aws_include.discriminator("not-a-dict")) + class TestResolveNestedStackParameters(unittest.TestCase): """Unit tests for _resolve_nested_stack_parameters helper. @@ -2584,16 +2827,12 @@ class TestResolveNestedStackParameters(unittest.TestCase): """ def test_literal_values_pass_through(self): - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - resolved = _resolve_nested_stack_parameters( {"ServiceNames": "Users,Orders,Products"}, parent_parameter_values={} ) self.assertEqual(resolved, {"ServiceNames": "Users,Orders,Products"}) def test_ref_to_parent_parameter_resolves(self): - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - resolved = _resolve_nested_stack_parameters( {"ServiceNames": {"Ref": "TopLevelServices"}}, parent_parameter_values={"TopLevelServices": "Users,Orders"}, @@ -2601,8 +2840,6 @@ def test_ref_to_parent_parameter_resolves(self): self.assertEqual(resolved, {"ServiceNames": "Users,Orders"}) def test_unresolvable_getatt_is_dropped(self): - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - resolved = _resolve_nested_stack_parameters( {"TopicArn": {"Fn::GetAtt": ["SomeResource", "Arn"]}}, parent_parameter_values={} ) @@ -2611,15 +2848,11 @@ def test_unresolvable_getatt_is_dropped(self): self.assertEqual(resolved, {}) def test_empty_input(self): - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - self.assertEqual(_resolve_nested_stack_parameters({}, {}), {}) self.assertEqual(_resolve_nested_stack_parameters(None, {}), {}) def test_unresolvable_ref_to_resource_is_dropped(self): """Ref to a resource (not a parameter) raises UnresolvableReferenceError and is dropped.""" - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - # Ref to "MyBucket" — not in parameter_values, not a pseudo-param → resource ref → preserved # as {"Ref": "MyBucket"} by partial resolver → dropped by the intrinsic-detection heuristic resolved = _resolve_nested_stack_parameters({"BucketName": {"Ref": "MyBucket"}}, parent_parameter_values={}) @@ -2627,8 +2860,6 @@ def test_unresolvable_ref_to_resource_is_dropped(self): def test_fn_sub_partially_resolves(self): """Fn::Sub with resolvable parts produces a string (not dropped).""" - from samcli.lib.package.artifact_exporter import _resolve_nested_stack_parameters - resolved = _resolve_nested_stack_parameters( {"Endpoint": {"Fn::Sub": "https://${MyApi}.execute-api.${AWS::Region}.amazonaws.com"}}, parent_parameter_values={"AWS::Region": "us-east-1"}, @@ -2670,6 +2901,84 @@ def test_template_str_path_parses_yaml(self): self.assertEqual(t.template_dict["Resources"]["MyTopic"]["Type"], "AWS::SNS::Topic") +class TestBuildChildParameterValues(unittest.TestCase): + """Unit tests for _build_child_parameter_values, the CFN-parity helper that + builds the parameter_values dict threaded into expand_language_extensions + for a nested child stack. + + Contract: result contains pseudo-params (with parent overrides for pseudo + NAMES only) and parent-rebound values from the nested-stack `Parameters:` + property. Non-pseudo parent names are NOT copied. Child template Defaults + are NOT folded in — the LE expander reads them from + parsed_template.parameters[X].Default at resolve time. + """ + + def test_excludes_non_pseudo_parent_params(self): + """Non-pseudo parent names must not leak into the child's scope.""" + result = _build_child_parameter_values( + parent_parameter_values={"Foo": "parent_foo", "AWS::Region": "us-east-1"}, + nested_stack_parameters={}, + ) + + self.assertNotIn("Foo", result) + # Pseudo baseline is present. + self.assertEqual(result.get("AWS::Region"), "us-east-1") + + def test_includes_pseudo_overrides_from_parent(self): + """Parent's override of a pseudo name must propagate to the child.""" + result = _build_child_parameter_values( + parent_parameter_values={"AWS::Region": "eu-west-1"}, + nested_stack_parameters={}, + ) + + self.assertEqual(result["AWS::Region"], "eu-west-1") + + def test_resolved_nested_rebinding_appears_in_result(self): + """Parent-rebound values reach the child.""" + result = _build_child_parameter_values( + parent_parameter_values={"ParentBar": "rebound"}, + nested_stack_parameters={"Bar": {"Ref": "ParentBar"}}, + ) + + self.assertEqual(result["Bar"], "rebound") + + def test_resolved_nested_rebinding_can_override_pseudo_named_key(self): + """Edge case: explicit nested-stack rebinding of a pseudo name wins + over the parent's pseudo-name override (step 3 dominates step 2 in + the merge order). Preserves existing _resolve_nested_stack_parameters + semantics. + """ + result = _build_child_parameter_values( + parent_parameter_values={"AWS::Region": "us-east-1", "MyRegion": "ap-south-1"}, + nested_stack_parameters={"AWS::Region": {"Ref": "MyRegion"}}, + ) + + self.assertEqual(result["AWS::Region"], "ap-south-1") + + def test_no_parent_parameter_values_uses_defaults(self): + """parent_parameter_values=None must not crash and must yield exactly + the default pseudo-param baseline.""" + result = _build_child_parameter_values( + parent_parameter_values=None, + nested_stack_parameters={}, + ) + + self.assertEqual(result, dict(IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES)) + + def test_does_not_fold_child_defaults(self): + """Helper signature does not take a child template dict. Documents the + design choice that child Defaults are picked up by the LE expander's + resolver via parsed_template.parameters[X].Default — not folded in + here. If a future contributor adds Default-folding to this helper, + this test prevents that drift. + """ + sig = inspect.signature(_build_child_parameter_values) + self.assertEqual( + list(sig.parameters.keys()), + ["parent_parameter_values", "nested_stack_parameters"], + ) + + class TestCloudFormationStackResourceChildExpansion(unittest.TestCase): """End-to-end test for threading parent params into child template expansion. @@ -2691,64 +3000,165 @@ def _make_stack_resource(self): def test_child_template_receives_parent_parameters(self): stack_resource, _ = self._make_stack_resource() + stack_resource.language_extensions_enabled = True - # Child template: Fn::ForEach driven by a child parameter. - child_template = { - "AWSTemplateFormatVersion": "2010-09-09", - "Transform": "AWS::LanguageExtensions", - "Parameters": {"ServiceNames": {"Type": "CommaDelimitedList"}}, - "Resources": { - "Fn::ForEach::Services": [ - "Name", - {"Ref": "ServiceNames"}, - {"${Name}Topic": {"Type": "AWS::SNS::Topic", "Properties": {}}}, - ] - }, + parent_dir = str(TEST_DATA_PATH / "child_servicenames") + child_path = str(TEST_DATA_PATH / "child_servicenames" / "child.yaml") + + # Parent-propagated values (what PackageContext._export would send) + stack_resource.parent_parameter_values = { + "AWS::Region": "us-east-1", + "AWS::AccountId": "123456789012", } - tmpdir = tempfile.mkdtemp() - try: - child_path = os.path.join(tmpdir, "child.yaml") - from samcli.yamlhelper import yaml_dump + # Parent's nested-stack resource: Parameters literal list + resource_dict = { + "TemplateURL": child_path, + "Parameters": {"ServiceNames": "Users,Orders,Products"}, + } - with open(child_path, "w", encoding="utf-8") as f: - f.write(yaml_dump(child_template)) + with patch("samcli.lib.package.artifact_exporter.expand_language_extensions") as expand_mock: + # Short-circuit: make expand return a "no-LE" result so do_export + # proceeds via the non-extension path (which doesn't upload since + # we'll intercept the inner Template). + expand_mock.return_value = Mock(had_language_extensions=False) + with patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock: + inner_template = Mock() + inner_template.export.return_value = {"Resources": {}} + TemplateMock.return_value = inner_template + stack_resource.do_export("ChildStack", resource_dict, parent_dir) + + # Verify the parameter_values threaded into expand_language_extensions + # contains the nested-stack Parameters values. + call_args = expand_mock.call_args[0] + call_kwargs = expand_mock.call_args[1] + passed_params = call_args[1] if len(call_args) > 1 else call_kwargs.get("parameter_values", {}) + self.assertEqual(passed_params.get("ServiceNames"), "Users,Orders,Products") + # And still carries propagated parent/pseudo values: + self.assertEqual(passed_params.get("AWS::Region"), "us-east-1") + + def test_parent_param_does_not_leak_into_child_with_same_name(self): + stack_resource, _ = self._make_stack_resource() + stack_resource.language_extensions_enabled = True - # Parent-propagated values (what PackageContext._export would send) - stack_resource.parent_parameter_values = { - "AWS::Region": "us-east-1", - "AWS::AccountId": "123456789012", - } + parent_dir = str(TEST_DATA_PATH / "child_foo_param") + child_path = str(TEST_DATA_PATH / "child_foo_param" / "child.yaml") - # Parent's nested-stack resource: Parameters literal list - resource_dict = { - "TemplateURL": child_path, - "Parameters": {"ServiceNames": "Users,Orders,Products"}, - } + # Parent has a non-pseudo Foo with a value — this is the leak source. + stack_resource.parent_parameter_values = { + "AWS::Region": "us-east-1", + "AWS::AccountId": "123456789012", + "Foo": "parent_foo", + } + # Nested-stack Parameters: does NOT rebind Foo. + resource_dict = { + "TemplateURL": child_path, + "Parameters": {}, + } - with patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") as expand_mock: - # Short-circuit: make expand return a "no-LE" result so do_export - # proceeds via the non-extension path (which doesn't upload since - # we'll intercept the inner Template). - expand_mock.return_value = Mock(had_language_extensions=False) - with patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock: - inner_template = Mock() - inner_template.export.return_value = {"Resources": {}} - TemplateMock.return_value = inner_template - stack_resource.do_export("ChildStack", resource_dict, tmpdir) - - # Verify the parameter_values threaded into expand_language_extensions - # contains the nested-stack Parameters values. - call_args = expand_mock.call_args[0] - call_kwargs = expand_mock.call_args[1] - passed_params = call_args[1] if len(call_args) > 1 else call_kwargs.get("parameter_values", {}) - self.assertEqual(passed_params.get("ServiceNames"), "Users,Orders,Products") - # And still carries propagated parent/pseudo values: - self.assertEqual(passed_params.get("AWS::Region"), "us-east-1") - finally: - import shutil + with patch("samcli.lib.package.artifact_exporter.expand_language_extensions") as expand_mock: + expand_mock.return_value = Mock(had_language_extensions=False) + with patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock: + TemplateMock.return_value.export.return_value = {"Resources": {}} + stack_resource.do_export("ChildStack", resource_dict, parent_dir) + + call_args = expand_mock.call_args[0] + call_kwargs = expand_mock.call_args[1] + passed_params = call_args[1] if len(call_args) > 1 else call_kwargs.get("parameter_values", {}) + + self.assertNotIn( + "Foo", + passed_params, + "Non-pseudo parent param leaked into child's parameter_values " "(CFN-parity violation).", + ) + # Pseudos still propagate. + self.assertEqual(passed_params.get("AWS::Region"), "us-east-1") + self.assertEqual(passed_params.get("AWS::AccountId"), "123456789012") + + def test_child_default_still_resolves_via_resolver_fallback(self): + stack_resource, _ = self._make_stack_resource() + stack_resource.language_extensions_enabled = True + + parent_dir = str(TEST_DATA_PATH / "child_bar_default") + child_path = str(TEST_DATA_PATH / "child_bar_default" / "child.yaml") + + stack_resource.parent_parameter_values = { + "AWS::Region": "us-east-1", + "AWS::AccountId": "123456789012", + } + resource_dict = { + "TemplateURL": child_path, + "Parameters": {}, + } + + captured = {} + + def capture_template_dict(*args, **kwargs): + captured["template_dict"] = kwargs.get("template_dict") + inner = Mock() + inner.export.return_value = {"Resources": {}} + return inner + + with patch( + "samcli.lib.package.artifact_exporter.Template", + side_effect=capture_template_dict, + ): + stack_resource.do_export("ChildStack", resource_dict, parent_dir) + + expanded = captured.get("template_dict") or {} + resources = expanded.get("Resources", {}) + # Default "bar_default" should have been substituted into the + # ForEach loop, producing a "bar_defaultTopic" resource. + self.assertIn( + "bar_defaultTopic", + resources, + "Child Default did not resolve via resolver fallback. " "Expanded Resources: %r" % (resources,), + ) + + +class TestCloudFormationStackResourceBuriedAWSInclude(unittest.TestCase): + """Nested-stack child template containing AWS::Include buried inside an + LE function (e.g. Fn::ToJsonString) must have the include's Location + rewritten by the pre-LE global-transform pass in + CloudFormationStackResource.do_export, before LE expansion would have + collapsed the structural Fn::Transform into a JSON-string literal. + + Regression coverage for https://github.com/aws/aws-sam-cli/issues/9027. + Root-flow coverage lives in tests/unit/commands/package/ + test_package_context.py. + """ + + def test_buried_aws_include_in_le_child_is_rewritten(self): + uploaders_mock = Mock() + s3_uploader_mock = Mock() + s3_uploader_mock.upload_with_dedup.side_effect = ( + lambda local_path, **_: f"s3://bucket/{os.path.basename(local_path)}-md5" + ) + # Template upload (the child template itself) — give it a deterministic URL. + s3_uploader_mock.upload.return_value = "s3://bucket/child-template-md5" + s3_uploader_mock.to_path_style_s3_url.return_value = "https://s3.amazonaws.com/bucket/child-template-md5" + uploaders_mock.get.return_value = s3_uploader_mock + + code_signer_mock = Mock() + code_signer_mock.should_sign_package.return_value = False + + stack_resource = CloudFormationStackResource(uploaders_mock, code_signer_mock) + stack_resource.language_extensions_enabled = True + + # Fixture mirrors the issue #9027 reporter's shape: Fn::ToJsonString + # over Fn::Transform: AWS::Include, buried inside + # AWS::SSM::Parameter.Properties.Value, under + # Transform: AWS::LanguageExtensions so the LE branch is taken. + parent_dir = str(TEST_DATA_PATH / "buried_aws_include") + child_path = str(TEST_DATA_PATH / "buried_aws_include" / "child.yaml") + include_path = str(TEST_DATA_PATH / "buried_aws_include" / "export-events.json") - shutil.rmtree(tmpdir, ignore_errors=True) + resource_dict = {"TemplateURL": child_path} + stack_resource.do_export("ChildStack", resource_dict, parent_dir) + + # Walk every upload_with_dedup call to confirm the include was uploaded. + uploaded_files = [call_args[0][0] for call_args in s3_uploader_mock.upload_with_dedup.call_args_list] + self.assertIn(include_path, uploaded_files) class TestCloudFormationStackResourceExpansionErrorHandling(unittest.TestCase): @@ -2771,64 +3181,43 @@ def _make_stack_resource(self): code_signer_mock.should_sign_package.return_value = False return CloudFormationStackResource(uploaders_mock, code_signer_mock) - def _write_child(self, tmpdir): - child = { - "AWSTemplateFormatVersion": "2010-09-09", - "Transform": "AWS::LanguageExtensions", - "Resources": {"Dummy": {"Type": "AWS::SNS::Topic", "Properties": {}}}, - } - from samcli.yamlhelper import yaml_dump - - path = os.path.join(tmpdir, "child.yaml") - with open(path, "w", encoding="utf-8") as f: - f.write(yaml_dump(child)) - return path - def test_invalid_sam_document_exception_logs_warning_and_falls_back(self): stack_resource = self._make_stack_resource() - tmpdir = tempfile.mkdtemp() - try: - child_path = self._write_child(tmpdir) - with ( - patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") as expand_mock, - patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, - self.assertLogs("samcli.lib.package.artifact_exporter", level="WARNING") as log_ctx, - ): - expand_mock.side_effect = InvalidSamDocumentException("bad template") - TemplateMock.return_value.export.return_value = {"Resources": {}} - stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, tmpdir) - - # Warning emitted (not error) and fallback path taken. - self.assertTrue(any("Language extensions expansion failed" in m for m in log_ctx.output)) - TemplateMock.assert_called_once() # non-extension fallback constructor - finally: - import shutil + stack_resource.language_extensions_enabled = True + parent_dir = str(TEST_DATA_PATH / "child_minimal_le") + child_path = str(TEST_DATA_PATH / "child_minimal_le" / "child.yaml") + with ( + patch("samcli.lib.package.artifact_exporter.expand_language_extensions") as expand_mock, + patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, + self.assertLogs("samcli.lib.package.artifact_exporter", level="WARNING") as log_ctx, + ): + expand_mock.side_effect = InvalidSamDocumentException("bad template") + TemplateMock.return_value.export.return_value = {"Resources": {}} + stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, parent_dir) - shutil.rmtree(tmpdir, ignore_errors=True) + # Warning emitted (not error) and fallback path taken. + self.assertTrue(any("Language extensions expansion failed" in m for m in log_ctx.output)) + TemplateMock.assert_called_once() # non-extension fallback constructor def test_unexpected_exception_logs_error_and_falls_back(self): stack_resource = self._make_stack_resource() - tmpdir = tempfile.mkdtemp() - try: - child_path = self._write_child(tmpdir) - with ( - patch("samcli.lib.cfn_language_extensions.sam_integration.expand_language_extensions") as expand_mock, - patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, - self.assertLogs("samcli.lib.package.artifact_exporter", level="ERROR") as log_ctx, - ): - expand_mock.side_effect = AttributeError("unexpected bug") - TemplateMock.return_value.export.return_value = {"Resources": {}} - stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, tmpdir) - - # ERROR-level log containing the bug-report pointer; fallback still taken. - joined = "\n".join(log_ctx.output) - self.assertIn("Internal error expanding language extensions", joined) - self.assertIn("github.com/aws/aws-sam-cli/issues", joined) - TemplateMock.assert_called_once() - finally: - import shutil + stack_resource.language_extensions_enabled = True + parent_dir = str(TEST_DATA_PATH / "child_minimal_le") + child_path = str(TEST_DATA_PATH / "child_minimal_le" / "child.yaml") + with ( + patch("samcli.lib.package.artifact_exporter.expand_language_extensions") as expand_mock, + patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, + self.assertLogs("samcli.lib.package.artifact_exporter", level="ERROR") as log_ctx, + ): + expand_mock.side_effect = AttributeError("unexpected bug") + TemplateMock.return_value.export.return_value = {"Resources": {}} + stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, parent_dir) - shutil.rmtree(tmpdir, ignore_errors=True) + # ERROR-level log containing the bug-report pointer; fallback still taken. + joined = "\n".join(log_ctx.output) + self.assertIn("Internal error expanding language extensions", joined) + self.assertIn("github.com/aws/aws-sam-cli/issues", joined) + TemplateMock.assert_called_once() class TestTemplateLanguageExtensionsKwarg(unittest.TestCase): @@ -2865,3 +3254,66 @@ def test_default_is_false(self): template_dict={"Resources": {}}, ) self.assertFalse(template.language_extensions_enabled) + + +class TestDoExportLanguageExtensionsStructuralGate(unittest.TestCase): + """Structural-gate guarantees for CloudFormationStackResource.do_export(). + + When language_extensions_enabled is False (the default), do_export must not + invoke any LE machinery: no expand_language_extensions call, no pre-LE + AWS::Include pass, no pseudo-param plumbing. The flag is a hard correctness + boundary, not a passthrough. + + Patch targets are use-site (samcli.lib.package.artifact_exporter.*) per the + lesson from PR #9030's 45dea4b7b — hoisting an import freezes the use-site + binding, so source-module patches stop intercepting and tests fail only + under CI's deterministic collection order. + """ + + def _make_stack_resource(self): + uploaders_mock = Mock() + uploaders_mock.get.return_value = Mock() + uploaders_mock.get.return_value.upload.return_value = "s3://bucket/key" + uploaders_mock.get.return_value.to_path_style_s3_url.return_value = "http://s3.amazonaws.com/bucket/key" + code_signer_mock = Mock() + code_signer_mock.should_sign_package.return_value = False + return CloudFormationStackResource(uploaders_mock, code_signer_mock) + + def test_off_path_does_not_invoke_expand_language_extensions(self): + """LE-off: dispatcher must NOT call expand_language_extensions.""" + stack_resource = self._make_stack_resource() + # Default state: language_extensions_enabled is False. + self.assertFalse(stack_resource.language_extensions_enabled) + + parent_dir = str(TEST_DATA_PATH / "child_minimal_no_le") + child_path = str(TEST_DATA_PATH / "child_minimal_no_le" / "child.yaml") + with ( + patch("samcli.lib.package.artifact_exporter.expand_language_extensions") as mock_expand, + patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, + ): + TemplateMock.return_value.export.return_value = {"Resources": {}} + stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, parent_dir) + + mock_expand.assert_not_called() + + def test_off_path_does_not_invoke_pre_le_global_transform_pass(self): + """LE-off: dispatcher / off branch must NOT call _export_global_artifacts_pass. + + Note: Template.export() runs its own internal _export_global_artifacts_pass, + but here Template is patched, so any call to the patched + _export_global_artifacts_pass would be from the do_export call site + (which we want to NOT happen on the off path). + """ + stack_resource = self._make_stack_resource() + self.assertFalse(stack_resource.language_extensions_enabled) + + parent_dir = str(TEST_DATA_PATH / "child_minimal_no_le") + child_path = str(TEST_DATA_PATH / "child_minimal_no_le" / "child.yaml") + with ( + patch("samcli.lib.package.artifact_exporter._export_global_artifacts_pass") as mock_pre_pass, + patch("samcli.lib.package.artifact_exporter.Template") as TemplateMock, + ): + TemplateMock.return_value.export.return_value = {"Resources": {}} + stack_resource.do_export("ChildStack", {"TemplateURL": child_path}, parent_dir) + + mock_pre_pass.assert_not_called() diff --git a/tests/unit/lib/package/test_data/buried_aws_include/child.yaml b/tests/unit/lib/package/test_data/buried_aws_include/child.yaml new file mode 100644 index 0000000000..15baadfa92 --- /dev/null +++ b/tests/unit/lib/package/test_data/buried_aws_include/child.yaml @@ -0,0 +1,14 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: AWS::LanguageExtensions +Resources: + Parameter: + Type: AWS::SSM::Parameter + Properties: + Type: String + DataType: text + Value: + Fn::ToJsonString: + Fn::Transform: + Name: AWS::Include + Parameters: + Location: export-events.json diff --git a/tests/unit/lib/package/test_data/buried_aws_include/export-events.json b/tests/unit/lib/package/test_data/buried_aws_include/export-events.json new file mode 100644 index 0000000000..2eb28cebd5 --- /dev/null +++ b/tests/unit/lib/package/test_data/buried_aws_include/export-events.json @@ -0,0 +1 @@ +[{"event": "demo"}] diff --git a/tests/unit/lib/package/test_data/child_bar_default/child.yaml b/tests/unit/lib/package/test_data/child_bar_default/child.yaml new file mode 100644 index 0000000000..41d293bfd1 --- /dev/null +++ b/tests/unit/lib/package/test_data/child_bar_default/child.yaml @@ -0,0 +1,13 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: AWS::LanguageExtensions +Parameters: + Bar: + Type: CommaDelimitedList + Default: bar_default +Resources: + Fn::ForEach::Vals: + - Item + - Ref: Bar + - ${Item}Topic: + Type: AWS::SNS::Topic + Properties: {} diff --git a/tests/unit/lib/package/test_data/child_foo_param/child.yaml b/tests/unit/lib/package/test_data/child_foo_param/child.yaml new file mode 100644 index 0000000000..1a8072857f --- /dev/null +++ b/tests/unit/lib/package/test_data/child_foo_param/child.yaml @@ -0,0 +1,6 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: AWS::LanguageExtensions +Parameters: + Foo: + Type: String +Resources: {} diff --git a/tests/unit/lib/package/test_data/child_minimal_le/child.yaml b/tests/unit/lib/package/test_data/child_minimal_le/child.yaml new file mode 100644 index 0000000000..ba8410e284 --- /dev/null +++ b/tests/unit/lib/package/test_data/child_minimal_le/child.yaml @@ -0,0 +1,6 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: AWS::LanguageExtensions +Resources: + Dummy: + Type: AWS::SNS::Topic + Properties: {} diff --git a/tests/unit/lib/package/test_data/child_minimal_no_le/child.yaml b/tests/unit/lib/package/test_data/child_minimal_no_le/child.yaml new file mode 100644 index 0000000000..ccdf30b80e --- /dev/null +++ b/tests/unit/lib/package/test_data/child_minimal_no_le/child.yaml @@ -0,0 +1 @@ +Resources: {} diff --git a/tests/unit/lib/package/test_data/child_servicenames/child.yaml b/tests/unit/lib/package/test_data/child_servicenames/child.yaml new file mode 100644 index 0000000000..19f3d15370 --- /dev/null +++ b/tests/unit/lib/package/test_data/child_servicenames/child.yaml @@ -0,0 +1,12 @@ +AWSTemplateFormatVersion: '2010-09-09' +Transform: AWS::LanguageExtensions +Parameters: + ServiceNames: + Type: CommaDelimitedList +Resources: + Fn::ForEach::Services: + - Name + - Ref: ServiceNames + - ${Name}Topic: + Type: AWS::SNS::Topic + Properties: {} diff --git a/tests/unit/lib/package/test_data/le_sar_metadata/LICENSE.txt b/tests/unit/lib/package/test_data/le_sar_metadata/LICENSE.txt new file mode 100644 index 0000000000..a22a2da24d --- /dev/null +++ b/tests/unit/lib/package/test_data/le_sar_metadata/LICENSE.txt @@ -0,0 +1 @@ +MIT diff --git a/tests/unit/lib/package/test_data/le_sar_metadata/README.md b/tests/unit/lib/package/test_data/le_sar_metadata/README.md new file mode 100644 index 0000000000..85d89a4408 --- /dev/null +++ b/tests/unit/lib/package/test_data/le_sar_metadata/README.md @@ -0,0 +1 @@ +# MyApp diff --git a/tests/unit/lib/package/test_data/le_sar_metadata/code/index.py b/tests/unit/lib/package/test_data/le_sar_metadata/code/index.py new file mode 100644 index 0000000000..5ff82cf6f4 --- /dev/null +++ b/tests/unit/lib/package/test_data/le_sar_metadata/code/index.py @@ -0,0 +1,2 @@ +def handler(event, context): + return event diff --git a/tests/unit/lib/package/test_data/le_top_level_include/code/index.py b/tests/unit/lib/package/test_data/le_top_level_include/code/index.py new file mode 100644 index 0000000000..5ff82cf6f4 --- /dev/null +++ b/tests/unit/lib/package/test_data/le_top_level_include/code/index.py @@ -0,0 +1,2 @@ +def handler(event, context): + return event diff --git a/tests/unit/lib/package/test_data/le_top_level_include/extra.yaml b/tests/unit/lib/package/test_data/le_top_level_include/extra.yaml new file mode 100644 index 0000000000..9f11beba7e --- /dev/null +++ b/tests/unit/lib/package/test_data/le_top_level_include/extra.yaml @@ -0,0 +1 @@ +# stub include target diff --git a/tests/unit/lib/package/test_language_extensions_packaging.py b/tests/unit/lib/package/test_language_extensions_packaging.py new file mode 100644 index 0000000000..7f01fc3151 --- /dev/null +++ b/tests/unit/lib/package/test_language_extensions_packaging.py @@ -0,0 +1,104 @@ +"""Unit tests for samcli.lib.package.language_extensions_packaging. + +Resource-property merge tests live in test_artifact_exporter.py; this file +focuses on the Metadata merge pass added for the registry-driven merge. +""" + +from unittest import TestCase + +from samcli.lib.package.language_extensions_packaging import merge_language_extensions_s3_uris + + +class TestMergeMetadata(TestCase): + def test_serverless_repo_license_url_is_merged(self): + original = { + "Transform": "AWS::LanguageExtensions", + "Metadata": { + "AWS::ServerlessRepo::Application": { + "Name": "MyApp", + "LicenseUrl": "./LICENSE.txt", + "ReadmeUrl": "./README.md", + } + }, + "Resources": {}, + } + exported = { + "Metadata": { + "AWS::ServerlessRepo::Application": { + "Name": "MyApp", + "LicenseUrl": "s3://bucket/license-md5", + "ReadmeUrl": "s3://bucket/readme-md5", + } + }, + "Resources": {}, + } + + result = merge_language_extensions_s3_uris(original, exported) + + sar = result["Metadata"]["AWS::ServerlessRepo::Application"] + self.assertEqual(sar["LicenseUrl"], "s3://bucket/license-md5") + self.assertEqual(sar["ReadmeUrl"], "s3://bucket/readme-md5") + self.assertEqual(sar["Name"], "MyApp") # unrelated keys preserved + + def test_metadata_without_serverless_repo_is_unchanged(self): + original = { + "Metadata": {"OtherKey": {"Foo": "./bar"}}, + "Resources": {}, + } + exported = { + "Metadata": {"OtherKey": {"Foo": "./bar"}}, + "Resources": {}, + } + + result = merge_language_extensions_s3_uris(original, exported) + + self.assertEqual(result["Metadata"], {"OtherKey": {"Foo": "./bar"}}) + + def test_missing_metadata_section_in_either_template_is_safe(self): + # No Metadata in original + result = merge_language_extensions_s3_uris( + {"Resources": {}}, + {"Metadata": {"AWS::ServerlessRepo::Application": {"LicenseUrl": "s3://x"}}, "Resources": {}}, + ) + self.assertNotIn("Metadata", result) + + # No Metadata in exported + original = { + "Metadata": {"AWS::ServerlessRepo::Application": {"LicenseUrl": "./LICENSE"}}, + "Resources": {}, + } + result = merge_language_extensions_s3_uris(original, {"Resources": {}}) + # Original retained, since exporter never wrote anything + self.assertEqual(result["Metadata"]["AWS::ServerlessRepo::Application"]["LicenseUrl"], "./LICENSE") + + def test_partial_serverless_repo_export_preserves_unwritten_properties(self): + """If the exported template has only LicenseUrl written (no ReadmeUrl), + the original's ReadmeUrl must be left untouched. Guards the + `if prop_name in exported_entry:` check in _merge_metadata. + """ + original = { + "Metadata": { + "AWS::ServerlessRepo::Application": { + "Name": "MyApp", + "LicenseUrl": "./LICENSE.txt", + "ReadmeUrl": "./README.md", + } + }, + "Resources": {}, + } + exported = { + "Metadata": { + "AWS::ServerlessRepo::Application": { + "Name": "MyApp", + "LicenseUrl": "s3://bucket/license-md5", + # ReadmeUrl deliberately absent + } + }, + "Resources": {}, + } + + result = merge_language_extensions_s3_uris(original, exported) + + sar = result["Metadata"]["AWS::ServerlessRepo::Application"] + self.assertEqual(sar["LicenseUrl"], "s3://bucket/license-md5") + self.assertEqual(sar["ReadmeUrl"], "./README.md") # untouched