diff --git a/samcli/commands/deploy/command.py b/samcli/commands/deploy/command.py index c1d05561543..9b2d06c3d30 100644 --- a/samcli/commands/deploy/command.py +++ b/samcli/commands/deploy/command.py @@ -333,7 +333,13 @@ def do_cli( # after we figure out how to enable resolve-images-repos in package if resolve_image_repos: image_repositories = sync_ecr_stack( - template_file, stack_name, region, s3_bucket, s3_prefix, image_repositories + template_file, + stack_name, + region, + s3_bucket, + s3_prefix, + image_repositories, + language_extensions_enabled=language_extensions_enabled, ) with osutils.tempfile_platform_independent() as output_template_file: diff --git a/samcli/commands/deploy/guided_context.py b/samcli/commands/deploy/guided_context.py index 1a3335687ae..03ee6cd1b69 100644 --- a/samcli/commands/deploy/guided_context.py +++ b/samcli/commands/deploy/guided_context.py @@ -189,7 +189,13 @@ def guided_prompts(self, parameter_override_keys): image_repositories = ( sync_ecr_stack( - self.template_file, stack_name, region, managed_s3_bucket, self.s3_prefix, self.image_repositories + self.template_file, + stack_name, + region, + managed_s3_bucket, + self.s3_prefix, + self.image_repositories, + language_extensions_enabled=self._language_extensions_enabled, ) if self.resolve_image_repositories else self.prompt_image_repository( diff --git a/samcli/commands/package/package_context.py b/samcli/commands/package/package_context.py index ac76e9b4279..f02efcf395e 100644 --- a/samcli/commands/package/package_context.py +++ b/samcli/commands/package/package_context.py @@ -121,7 +121,13 @@ def run(self): stack_name = f"sam-app-{template_basename}" self.image_repositories = sync_ecr_stack( - self.template_file, stack_name, self.region, self.s3_bucket, self.s3_prefix, self.image_repositories + self.template_file, + stack_name, + self.region, + self.s3_bucket, + self.s3_prefix, + self.image_repositories, + language_extensions_enabled=self._language_extensions_enabled, ) stacks, _ = SamLocalStackProvider.get_stacks( @@ -269,17 +275,23 @@ def _export_with_language_extensions(self, template_path, original_template_dict return exported_template LOG.debug("Template uses language extensions, preserving Fn::ForEach structure") + deferred_dynamic: List = [] output_template = merge_language_extensions_s3_uris( - result.original_template, exported_template, result.dynamic_artifact_properties + result.original_template, + exported_template, + result.dynamic_artifact_properties, + parameter_values=parameter_values, + deferred_dynamic=deferred_dynamic, ) - if result.dynamic_artifact_properties: + all_dynamic_properties = list(result.dynamic_artifact_properties or []) + deferred_dynamic + if all_dynamic_properties: LOG.debug( "Generating Mappings for %d dynamic artifact properties", - len(result.dynamic_artifact_properties), + len(all_dynamic_properties), ) output_template = generate_and_apply_artifact_mappings( output_template, - result.dynamic_artifact_properties, + all_dynamic_properties, exported_template.get("Resources", {}), template_dir, ) diff --git a/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py b/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py index 66dd6a290e1..52ba15ffc24 100644 --- a/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py +++ b/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py @@ -279,7 +279,13 @@ def is_repo_uri(self, repo_uri: Optional[str], function_logical_id: str) -> bool def sync_ecr_stack( - template_file: str, stack_name: str, region: str, s3_bucket: str, s3_prefix: str, image_repositories: Dict[str, str] + template_file: str, + stack_name: str, + region: str, + s3_bucket: str, + s3_prefix: str, + image_repositories: Dict[str, str], + language_extensions_enabled: bool = False, ) -> Dict[str, str]: """Blocking call to sync local functions with ECR Companion Stack @@ -297,6 +303,8 @@ def sync_ecr_stack( S3 prefix for the bucket image_repositories : Dict[str, str] Mapping between function logical ID and ECR URI + language_extensions_enabled : bool + Whether AWS::LanguageExtensions transform processing (e.g. Fn::ForEach expansion) is enabled Returns ------- @@ -307,7 +315,7 @@ def sync_ecr_stack( image_repositories = image_repositories.copy() if image_repositories else {} manager = CompanionStackManager(stack_name, region, s3_bucket, s3_prefix) - stacks = SamLocalStackProvider.get_stacks(template_file, language_extensions_enabled=False)[0] + stacks = SamLocalStackProvider.get_stacks(template_file, language_extensions_enabled=language_extensions_enabled)[0] function_provider = SamFunctionProvider(stacks, ignore_code_extraction_warnings=True) function_logical_ids = [ function.full_path for function in function_provider.get_all() if function.packagetype == IMAGE diff --git a/samcli/lib/package/artifact_exporter.py b/samcli/lib/package/artifact_exporter.py index 05ac43ce314..3049c5d25cb 100644 --- a/samcli/lib/package/artifact_exporter.py +++ b/samcli/lib/package/artifact_exporter.py @@ -375,21 +375,25 @@ def _do_export_with_language_extensions( exported_template = template.export() + deferred_dynamic: List = [] exported_template_dict = merge_language_extensions_s3_uris( result.original_template, exported_template, result.dynamic_artifact_properties, + parameter_values=parameter_values, + deferred_dynamic=deferred_dynamic, ) - if result.dynamic_artifact_properties: + all_dynamic_properties = list(result.dynamic_artifact_properties or []) + deferred_dynamic + if all_dynamic_properties: LOG.debug( "Generating Mappings for %d dynamic artifact properties in child template", - len(result.dynamic_artifact_properties), + len(all_dynamic_properties), ) exported_resources = exported_template.get("Resources", {}) exported_template_dict = generate_and_apply_artifact_mappings( exported_template_dict, - result.dynamic_artifact_properties, + all_dynamic_properties, exported_resources, child_template_dir, ) diff --git a/samcli/lib/package/language_extensions_packaging.py b/samcli/lib/package/language_extensions_packaging.py index 46123ef2994..6cf2d4b43ad 100644 --- a/samcli/lib/package/language_extensions_packaging.py +++ b/samcli/lib/package/language_extensions_packaging.py @@ -23,6 +23,7 @@ ) from samcli.lib.cfn_language_extensions.sam_integration import ( contains_loop_variable, + resolve_collection, sanitize_resource_key_for_mapping, substitute_loop_variable, ) @@ -44,6 +45,8 @@ def merge_language_extensions_s3_uris( original_template: Dict[str, Any], exported_template: Dict[str, Any], dynamic_properties: Optional[List[DynamicArtifactProperty]] = None, + parameter_values: Optional[Dict[str, Any]] = None, + deferred_dynamic: Optional[List] = None, ) -> Dict[str, Any]: """ Update the original template (with Fn::ForEach intact) with S3 URIs from the exported template. @@ -81,7 +84,14 @@ def merge_language_extensions_s3_uris( original_resources = result.get("Resources", {}) exported_resources = exported_template.get("Resources", {}) - _update_resources_with_s3_uris(original_resources, exported_resources, dynamic_prop_keys) + _update_resources_with_s3_uris( + original_resources, + exported_resources, + dynamic_prop_keys, + template=original_template, + parameter_values=parameter_values, + deferred_dynamic=deferred_dynamic, + ) _merge_metadata(result.get("Metadata", {}), exported_template.get("Metadata", {})) @@ -208,6 +218,9 @@ def _update_resources_with_s3_uris( original_resources: Dict[str, Any], exported_resources: Dict[str, Any], dynamic_prop_keys: Optional[set] = None, + template: Optional[Dict[str, Any]] = None, + parameter_values: Optional[Dict[str, Any]] = None, + deferred_dynamic: Optional[List] = None, ) -> None: """ Update resources in the original template with S3 URIs from the exported template. @@ -216,7 +229,15 @@ def _update_resources_with_s3_uris( """ for resource_key, resource_value in original_resources.items(): if is_foreach_key(resource_key): - _update_foreach_with_s3_uris(resource_key, resource_value, exported_resources, dynamic_prop_keys) + _update_foreach_with_s3_uris( + resource_key, + resource_value, + exported_resources, + dynamic_prop_keys, + template=template, + parameter_values=parameter_values, + deferred_dynamic=deferred_dynamic, + ) elif isinstance(resource_value, dict) and resource_key in exported_resources: exported_resource = exported_resources.get(resource_key, {}) _copy_artifact_uris(resource_value, exported_resource) @@ -228,6 +249,9 @@ def _update_foreach_with_s3_uris( exported_resources: Dict[str, Any], dynamic_prop_keys: Optional[set] = None, outer_context: Optional[List[Tuple[str, List[str]]]] = None, + template: Optional[Dict[str, Any]] = None, + parameter_values: Optional[Dict[str, Any]] = None, + deferred_dynamic: Optional[List] = None, ) -> None: """ Update artifact URIs in a Fn::ForEach construct. @@ -245,9 +269,18 @@ def _update_foreach_with_s3_uris( if not isinstance(loop_variable, str) or not isinstance(body, dict): return - collection_values: List[str] = [] - if isinstance(collection, list): - collection_values = [str(item) for item in collection if item is not None] + collection_values = resolve_collection(collection, template or {}, parameter_values) + + # Detect whether the collection is a parameter reference so deferred artifacts + # carry the metadata that warn_parameter_based_collections uses to advise + # re-packaging (mirrors detect_foreach_dynamic_properties in sam_integration.py). + collection_is_parameter_ref = False + collection_parameter_name: Optional[str] = None + if isinstance(collection, dict) and "Ref" in collection: + param_name = collection["Ref"] + if param_name in (template or {}).get("Parameters", {}): + collection_is_parameter_ref = True + collection_parameter_name = param_name if outer_context is None: outer_context = [] @@ -261,6 +294,9 @@ def _update_foreach_with_s3_uris( exported_resources, dynamic_prop_keys, outer_context=current_outer_context, + template=template, + parameter_values=parameter_values, + deferred_dynamic=deferred_dynamic, ) continue @@ -281,13 +317,102 @@ def _update_foreach_with_s3_uris( exported_resource = exported_resources[expanded_key] if not isinstance(exported_resource, dict): continue - exported_props = exported_resource.get("Properties", {}) - _copy_artifact_uris_for_type( - properties, exported_props, resource_template.get("Type", ""), foreach_key, dynamic_prop_keys + resource_type = resource_template.get("Type", "") + _merge_or_defer_foreach_artifacts( + properties=properties, + resource_type=resource_type, + resource_template_key=resource_template_key, + foreach_key=foreach_key, + loop_variable=loop_variable, + collection_values=collection_values, + outer_context=outer_context, + exported_resources=exported_resources, + dynamic_prop_keys=dynamic_prop_keys, + deferred_dynamic=deferred_dynamic, + collection_is_parameter_ref=collection_is_parameter_ref, + collection_parameter_name=collection_parameter_name, ) +def _merge_or_defer_foreach_artifacts( + properties: Dict[str, Any], + resource_type: str, + resource_template_key: str, + foreach_key: str, + loop_variable: str, + collection_values: List[str], + outer_context: Optional[List[Tuple[str, List[str]]]], + exported_resources: Dict[str, Any], + dynamic_prop_keys: Optional[set], + deferred_dynamic: Optional[List], + collection_is_parameter_ref: bool = False, + collection_parameter_name: Optional[str] = None, +) -> None: + """Decide, per artifact property, whether all Fn::ForEach iterations resolved + to the same exported URI (static copy) or to distinct URIs (defer to Mappings). + + - All iterations identical -> copy the shared raw value onto the ForEach body. + - Iterations differ -> append a synthetic DynamicArtifactProperty to + ``deferred_dynamic`` (handled later by generate_and_apply_artifact_mappings) + and leave the body value untouched. + - No accumulator / nested loop -> fall back to copying the first resolved + iteration's value (legacy behavior); nested ForEach with static differing + values is a documented limitation. + """ + prop_names = PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.get(resource_type) + if not prop_names: + return + + for prop_name in _resolve_property_paths(prop_names, properties): + # Loop-variable properties are handled by the existing dynamic path. + if dynamic_prop_keys and (foreach_key, prop_name) in dynamic_prop_keys: + continue + + # Track each iteration's normalized URI (for the identical-vs-differing + # comparison) paired with the RAW exported value (for copying). The + # comparison uses the normalized string, but the value written back must + # preserve the original shape (e.g. a {S3Bucket, S3Key} object) so the + # resulting CloudFormation stays valid. + resolved: List[Tuple[str, Any]] = [] + for value in collection_values: + expanded_key = _build_expanded_key(resource_template_key, loop_variable, [value], outer_context) + if not expanded_key: + continue + uri = _find_artifact_uri_for_resource(exported_resources, expanded_key, resource_type, prop_name) + if uri is None: + continue + exported_resource = exported_resources.get(expanded_key, {}) + raw = _get_prop_value(exported_resource.get("Properties", {}), prop_name) + resolved.append((uri, raw)) + + if not resolved: + continue + + distinct_uris = {uri for uri, _ in resolved} + first_raw = resolved[0][1] + if len(distinct_uris) == 1: + _set_prop_value(properties, prop_name, first_raw) + elif deferred_dynamic is not None and not outer_context: + deferred_dynamic.append( + DynamicArtifactProperty( + foreach_key=foreach_key, + loop_name=foreach_key.replace("Fn::ForEach::", ""), + loop_variable=loop_variable, + collection=collection_values, + resource_key=resource_template_key, + resource_type=resource_type, + property_name=prop_name, + property_value=_get_prop_value(properties, prop_name), + outer_loops=[], + collection_is_parameter_ref=collection_is_parameter_ref, + collection_parameter_name=collection_parameter_name, + ) + ) + else: + _set_prop_value(properties, prop_name, first_raw) + + def _build_expanded_key( resource_template_key: str, loop_variable: str, diff --git a/tests/unit/commands/package/test_package_context.py b/tests/unit/commands/package/test_package_context.py index a9e255dace8..50cc2a48565 100644 --- a/tests/unit/commands/package/test_package_context.py +++ b/tests/unit/commands/package/test_package_context.py @@ -1,5 +1,6 @@ """Test sam package command""" +import copy import os from pathlib import Path from unittest import TestCase @@ -10,14 +11,19 @@ TEST_DATA_PATH = Path(__file__).resolve().parent / "test_data" +import samcli.lib.package.packageable_resources as pr +from botocore.utils import set_value_from_jmespath from samcli.commands.package.package_context import PackageContext from samcli.commands.package.exceptions import PackageFailedError from samcli.lib.cfn_language_extensions.sam_integration import ( contains_loop_variable, detect_dynamic_artifact_properties, + expand_language_extensions, ) +from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable from samcli.lib.package.artifact_exporter import Template from samcli.lib.package.language_extensions_packaging import ( + generate_and_apply_artifact_mappings, merge_language_extensions_s3_uris, warn_parameter_based_collections, _update_resources_with_s3_uris, @@ -35,7 +41,7 @@ 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 +from samcli.yamlhelper import yaml_dump, yaml_parse class TestPackageCommand(TestCase): @@ -4589,3 +4595,186 @@ def test_off_path_does_not_invoke_pre_le_global_transform_pass( ctx._export("template.yaml", use_json=False) mock_pre_le_pass.assert_not_called() + + +class TestForEachImagePackagingEndToEnd(TestCase): + """End-to-end pipeline: Fn::ForEach + !Ref collection + PackageType: Image (#9117).""" + + def _run(self, template_dict, param_values): + result = expand_language_extensions(template_dict, param_values, enabled=True) + + def fake_image_export(self, resource_id, resource_dict, parent_dir): + set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, f"repo:{resource_id}-latest") + + with patch.object(pr.ResourceImage, "do_export", fake_image_export): + template = Template( + "t.yaml", + ".", + Uploaders(object(), object()), + None, + normalize_template=True, + normalize_parameters=True, + template_dict=copy.deepcopy(result.expanded_template), + parameter_values=param_values, + language_extensions_enabled=True, + ) + exported = template.export() + + deferred = [] + output = merge_language_extensions_s3_uris( + result.original_template, + exported, + result.dynamic_artifact_properties, + parameter_values=param_values, + deferred_dynamic=deferred, + ) + all_dynamic = list(result.dynamic_artifact_properties or []) + deferred + if all_dynamic: + output = generate_and_apply_artifact_mappings(output, all_dynamic, exported.get("Resources", {}), ".") + return output + + def test_multivalue_ref_image_generates_findinmap(self): + template = { + "Transform": ["AWS::LanguageExtensions", "AWS::Serverless-2016-10-31"], + "Parameters": {"FuncType": {"Type": "CommaDelimitedList", "Default": "func1,func2"}}, + "Resources": { + "Fn::ForEach::LoopFunction": [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"PackageType": "Image", "ImageUri": "foo"}, + } + }, + ] + }, + } + output = self._run(template, {**IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES}) + + body = output["Resources"]["Fn::ForEach::LoopFunction"][2]["${FunctionName}Function"]["Properties"] + self.assertIn("Fn::FindInMap", body["ImageUri"]) + mapping_name = body["ImageUri"]["Fn::FindInMap"][0] + mapping = output["Mappings"][mapping_name] + self.assertEqual(mapping["func1"]["ImageUri"], "repo:func1Function-latest") + self.assertEqual(mapping["func2"]["ImageUri"], "repo:func2Function-latest") + + def test_package_context_wires_deferred_image_mappings(self): + # Drives the REAL production caller (_export_with_language_extensions), + # not the helpers directly, so it guards the caller wiring itself: + # parameter_values passed + deferred_dynamic collected + combined into + # generate_and_apply_artifact_mappings. + template_str = yaml_dump( + { + "AWSTemplateFormatVersion": "2010-09-09", + "Transform": ["AWS::LanguageExtensions", "AWS::Serverless-2016-10-31"], + "Parameters": {"FuncType": {"Type": "CommaDelimitedList", "Default": "func1,func2"}}, + "Resources": { + "Fn::ForEach::LoopFunction": [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"PackageType": "Image", "ImageUri": "foo"}, + } + }, + ] + }, + } + ) + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as tf: + tf.write(template_str) + template_path = tf.name + + def fake_image_export(self, resource_id, resource_dict, parent_dir): + set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, f"repo:{resource_id}-latest") + + try: + ctx = PackageContext( + template_file=template_path, + s3_bucket="bucket", + s3_prefix="prefix", + image_repository=None, + image_repositories=None, + output_template_file=None, + kms_key_id=None, + use_json=False, + force_upload=False, + no_progressbar=False, + metadata=None, + region=None, + profile=None, + language_extensions=True, + ) + ctx.uploaders = MagicMock() + ctx.code_signer = MagicMock() + with patch.object(pr.ResourceImage, "do_export", fake_image_export): + original_template_dict = yaml_parse(template_str) + output = ctx._export_with_language_extensions(template_path, original_template_dict) + finally: + os.unlink(template_path) + + body = output["Resources"]["Fn::ForEach::LoopFunction"][2]["${FunctionName}Function"]["Properties"] + self.assertIn("Fn::FindInMap", body["ImageUri"]) + mapping_name = body["ImageUri"]["Fn::FindInMap"][0] + mapping = output["Mappings"][mapping_name] + self.assertEqual(mapping["func1"]["ImageUri"], "repo:func1Function-latest") + self.assertEqual(mapping["func2"]["ImageUri"], "repo:func2Function-latest") + + def _run_zip(self, template_dict, param_values): + result = expand_language_extensions(template_dict, param_values, enabled=True) + + def fake_zip_export(self, resource_id, resource_dict, parent_dir): + set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, "s3://bucket/SAME") + + with patch.object(pr.ResourceWithS3UrlDict, "do_export", fake_zip_export): + template = Template( + "t.yaml", + ".", + Uploaders(object(), object()), + None, + normalize_template=True, + normalize_parameters=True, + template_dict=copy.deepcopy(result.expanded_template), + parameter_values=param_values, + language_extensions_enabled=True, + ) + exported = template.export() + + deferred = [] + output = merge_language_extensions_s3_uris( + result.original_template, + exported, + result.dynamic_artifact_properties, + parameter_values=param_values, + deferred_dynamic=deferred, + ) + all_dynamic = list(result.dynamic_artifact_properties or []) + deferred + if all_dynamic: + output = generate_and_apply_artifact_mappings(output, all_dynamic, exported.get("Resources", {}), ".") + return output + + def test_ref_collection_statemachine_definitionuri_rewritten(self): + template = { + "Transform": ["AWS::LanguageExtensions", "AWS::Serverless-2016-10-31"], + "Parameters": {"Names": {"Type": "CommaDelimitedList", "Default": "Alpha,Beta"}}, + "Resources": { + "Fn::ForEach::SM": [ + "Name", + {"Ref": "Names"}, + { + "${Name}Machine": { + "Type": "AWS::Serverless::StateMachine", + "Properties": {"DefinitionUri": "sm.asl.json"}, + } + }, + ] + }, + } + output = self._run_zip(template, {**IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES}) + body = output["Resources"]["Fn::ForEach::SM"][2]["${Name}Machine"]["Properties"] + # Identical S3 URI across iterations -> shared static copy, not a Mapping. + self.assertEqual(body["DefinitionUri"], "s3://bucket/SAME") + self.assertNotIn("Mappings", output) diff --git a/tests/unit/commands/package/test_package_context_language_extensions.py b/tests/unit/commands/package/test_package_context_language_extensions.py index 3ae83ade7ec..88a34ad8700 100644 --- a/tests/unit/commands/package/test_package_context_language_extensions.py +++ b/tests/unit/commands/package/test_package_context_language_extensions.py @@ -10,6 +10,7 @@ from samcli.lib.cfn_language_extensions.exceptions import InvalidTemplateException from samcli.lib.package.language_extensions_packaging import ( + merge_language_extensions_s3_uris, _compute_mapping_name, _copy_artifact_uris_for_type, _nesting_path, @@ -1436,3 +1437,437 @@ def test_cross_context_same_resource_key_suffix(self): # No suffixed variants self.assertNotIn("SAMDefinitionUriRegionAPIsServicesApi", mappings) self.assertNotIn("SAMDefinitionUriEnvAPIsServicesApi", mappings) + + +class TestForEachRefCollectionResolution(TestCase): + """Ref-based Fn::ForEach collections must resolve so static artifacts get rewritten.""" + + def test_ref_collection_single_value_rewrites_static_imageuri(self): + foreach_value = [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "foo"}, + } + }, + ] + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "123.dkr.ecr.us-east-1.amazonaws.com/r:func1Function-latest"}, + }, + } + template = {"Parameters": {"FuncType": {"Default": "func1"}}} + _update_foreach_with_s3_uris( + "Fn::ForEach::LoopFunction", + foreach_value, + exported_resources, + None, + template=template, + ) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["ImageUri"], "123.dkr.ecr.us-east-1.amazonaws.com/r:func1Function-latest") + + def test_ref_collection_uses_parameter_overrides(self): + foreach_value = [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"CodeUri": "src"}, + } + }, + ] + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"CodeUri": "s3://bucket/SAMEHASH"}, + }, + } + template = {"Parameters": {"FuncType": {"Default": "unused"}}} + _update_foreach_with_s3_uris( + "Fn::ForEach::LoopFunction", + foreach_value, + exported_resources, + None, + template=template, + parameter_values={"FuncType": ["func1"]}, + ) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["CodeUri"], "s3://bucket/SAMEHASH") + + +class TestForEachValueDrivenMerge(TestCase): + """Per-iteration URI comparison: identical -> copy; differing -> defer to Mappings.""" + + def _foreach(self, prop_name, static_value): + return [ + "FunctionName", + ["func1", "func2"], + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {prop_name: static_value}, + } + }, + ] + + def test_identical_uris_copied_as_static(self): + foreach_value = self._foreach("CodeUri", "src") + exported_resources = { + "func1Function": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "s3://bucket/SAME"}}, + "func2Function": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "s3://bucket/SAME"}}, + } + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + None, + deferred_dynamic=deferred, + ) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["CodeUri"], "s3://bucket/SAME") + self.assertEqual(deferred, []) + + def test_differing_uris_deferred_not_copied(self): + foreach_value = self._foreach("ImageUri", "foo") + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func1Function-latest"}, + }, + "func2Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func2Function-latest"}, + }, + } + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + None, + deferred_dynamic=deferred, + ) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["ImageUri"], "foo") + self.assertEqual(len(deferred), 1) + self.assertEqual(deferred[0].property_name, "ImageUri") + self.assertEqual(deferred[0].resource_key, "${FunctionName}Function") + self.assertEqual(deferred[0].loop_variable, "FunctionName") + self.assertEqual(deferred[0].collection, ["func1", "func2"]) + + def test_differing_uris_without_accumulator_falls_back_to_copy(self): + foreach_value = self._foreach("ImageUri", "foo") + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func1Function-latest"}, + }, + "func2Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func2Function-latest"}, + }, + } + _update_foreach_with_s3_uris("Fn::ForEach::L", foreach_value, exported_resources, None) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["ImageUri"], "repo:func1Function-latest") + + def test_dynamic_prop_key_still_skipped(self): + foreach_value = self._foreach("CodeUri", "${FunctionName}/") + exported_resources = { + "func1Function": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "s3://bucket/func1"}}, + "func2Function": {"Type": "AWS::Serverless::Function", "Properties": {"CodeUri": "s3://bucket/func2"}}, + } + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + {("Fn::ForEach::L", "CodeUri")}, + deferred_dynamic=deferred, + ) + props = foreach_value[2]["${FunctionName}Function"]["Properties"] + self.assertEqual(props["CodeUri"], "${FunctionName}/") + self.assertEqual(deferred, []) + + def test_identical_dict_form_artifact_preserves_object_shape(self): + # Raw AWS::Lambda::Function.Code is a {S3Bucket,S3Key} object, not a string. + # Identical across iterations must copy the OBJECT, not a normalized s3:// string. + foreach_value = [ + "FunctionName", + ["func1", "func2"], + { + "${FunctionName}Function": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"S3Bucket": "b", "S3Key": "local"}}, + } + }, + ] + exported_resources = { + "func1Function": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"S3Bucket": "b", "S3Key": "SAME"}}, + }, + "func2Function": { + "Type": "AWS::Lambda::Function", + "Properties": {"Code": {"S3Bucket": "b", "S3Key": "SAME"}}, + }, + } + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + None, + deferred_dynamic=deferred, + ) + code = foreach_value[2]["${FunctionName}Function"]["Properties"]["Code"] + self.assertEqual(code, {"S3Bucket": "b", "S3Key": "SAME"}) + self.assertEqual(deferred, []) + + def test_deferred_ref_collection_marks_parameter_ref(self): + # Deferred artifacts from a !Ref collection must carry the parameter-ref + # metadata so warn_parameter_based_collections can advise re-packaging. + foreach_value = [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "foo"}, + } + }, + ] + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func1Function-latest"}, + }, + "func2Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func2Function-latest"}, + }, + } + template = {"Parameters": {"FuncType": {"Default": "func1,func2"}}} + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + None, + template=template, + deferred_dynamic=deferred, + ) + self.assertEqual(len(deferred), 1) + self.assertTrue(deferred[0].collection_is_parameter_ref) + self.assertEqual(deferred[0].collection_parameter_name, "FuncType") + + def test_deferred_literal_collection_not_parameter_ref(self): + foreach_value = [ + "FunctionName", + ["func1", "func2"], + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "foo"}, + } + }, + ] + exported_resources = { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func1Function-latest"}, + }, + "func2Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func2Function-latest"}, + }, + } + deferred = [] + _update_foreach_with_s3_uris( + "Fn::ForEach::L", + foreach_value, + exported_resources, + None, + deferred_dynamic=deferred, + ) + self.assertEqual(len(deferred), 1) + self.assertFalse(deferred[0].collection_is_parameter_ref) + self.assertIsNone(deferred[0].collection_parameter_name) + + +class TestMergeThreadsParametersAndDeferred(TestCase): + """merge_language_extensions_s3_uris forwards parameter_values + accumulator.""" + + def test_merge_resolves_ref_and_collects_deferred(self): + original = { + "Parameters": {"FuncType": {"Default": "func1,func2"}}, + "Resources": { + "Fn::ForEach::L": [ + "FunctionName", + {"Ref": "FuncType"}, + { + "${FunctionName}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "foo"}, + } + }, + ] + }, + } + exported = { + "Resources": { + "func1Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func1Function-latest"}, + }, + "func2Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:func2Function-latest"}, + }, + } + } + deferred = [] + result = merge_language_extensions_s3_uris(original, exported, None, deferred_dynamic=deferred) + body = result["Resources"]["Fn::ForEach::L"][2]["${FunctionName}Function"]["Properties"] + # Differing image URIs deferred; body value untouched pre-Mapping. + self.assertEqual(body["ImageUri"], "foo") + self.assertEqual(len(deferred), 1) + self.assertEqual(deferred[0].property_name, "ImageUri") + + +class TestForEachArtifactShapes(TestCase): + """Decision matrix across the packageable artifact value shapes.""" + + def _body(self, resource_type, prop_path, value): + # prop_path may be dotted (e.g. "Code.ImageUri"); build nested dict. + props: dict = {} + cur = props + parts = prop_path.split(".") + for p in parts[:-1]: + cur = cur.setdefault(p, {}) + cur[parts[-1]] = value + return [ + "Name", + ["a", "b"], + {"${Name}Res": {"Type": resource_type, "Properties": props}}, + ] + + def _exported(self, resource_type, prop_path, val_a, val_b): + def mk(v): + props: dict = {} + cur = props + parts = prop_path.split(".") + for p in parts[:-1]: + cur = cur.setdefault(p, {}) + cur[parts[-1]] = v + return {"Type": resource_type, "Properties": props} + + return {"aRes": mk(val_a), "bRes": mk(val_b)} + + def _leaf(self, props, prop_path): + cur = props + for p in prop_path.split("."): + cur = cur[p] + return cur + + # --- string URI shape (CodeUri, DefinitionUri) --- + def test_string_uri_identical_copies(self): + fe = self._body("AWS::Serverless::StateMachine", "DefinitionUri", "def.asl.json") + exp = self._exported("AWS::Serverless::StateMachine", "DefinitionUri", "s3://b/SAME", "s3://b/SAME") + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::L", fe, exp, None, deferred_dynamic=deferred) + self.assertEqual(self._leaf(fe[2]["${Name}Res"]["Properties"], "DefinitionUri"), "s3://b/SAME") + self.assertEqual(deferred, []) + + # --- dotted path shape (Code.ImageUri, Command.ScriptLocation) --- + def test_dotted_imageuri_differing_defers(self): + fe = self._body("AWS::Lambda::Function", "Code.ImageUri", "foo") + exp = self._exported("AWS::Lambda::Function", "Code.ImageUri", "repo:aRes", "repo:bRes") + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::L", fe, exp, None, deferred_dynamic=deferred) + self.assertEqual(len(deferred), 1) + self.assertEqual(deferred[0].property_name, "Code.ImageUri") + + def test_dotted_scriptlocation_identical_copies(self): + fe = self._body("AWS::Glue::Job", "Command.ScriptLocation", "script.py") + exp = self._exported("AWS::Glue::Job", "Command.ScriptLocation", "s3://b/SAME", "s3://b/SAME") + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::L", fe, exp, None, deferred_dynamic=deferred) + self.assertEqual(self._leaf(fe[2]["${Name}Res"]["Properties"], "Command.ScriptLocation"), "s3://b/SAME") + self.assertEqual(deferred, []) + + # --- image tag shape (ImageUri) --- + def test_imageuri_differing_defers(self): + fe = self._body("AWS::Serverless::Function", "ImageUri", "foo") + exp = self._exported("AWS::Serverless::Function", "ImageUri", "repo:aFunc", "repo:bFunc") + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::L", fe, exp, None, deferred_dynamic=deferred) + self.assertEqual(len(deferred), 1) + + # --- structured object shape ({S3Bucket,S3Key}) --- + def test_structured_content_identical_preserves_object(self): + fe = self._body("AWS::Lambda::LayerVersion", "Content", "layer/") + exp = self._exported( + "AWS::Lambda::LayerVersion", + "Content", + {"S3Bucket": "b", "S3Key": "SAME"}, + {"S3Bucket": "b", "S3Key": "SAME"}, + ) + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::L", fe, exp, None, deferred_dynamic=deferred) + # Identical across iterations -> copy the RAW object shape (not a normalized s3:// string). + self.assertEqual( + self._leaf(fe[2]["${Name}Res"]["Properties"], "Content"), + {"S3Bucket": "b", "S3Key": "SAME"}, + ) + self.assertEqual(deferred, []) + + +class TestForEachNestedStaticImageLimitation(TestCase): + """Nested ForEach + static image value falls back to legacy copy (documented limitation). + + Image identity varies by outer AND inner loop, but a static value carries no + loop variable, so the walk cannot build compound Mapping keys. This guards the + documented fallback. Nested ForEach with dynamic values is unaffected and + continues through the existing dynamic path. + """ + + def test_nested_static_image_falls_back_to_copy(self): + foreach_value = [ + "Env", + ["dev", "prod"], + { + "Fn::ForEach::Svc": [ + "Svc", + ["Users", "Orders"], + { + "${Env}${Svc}Function": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "foo"}, + } + }, + ] + }, + ] + exported = { + "devUsersFunction": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:devUsersFunction"}, + }, + "devOrdersFunction": { + "Type": "AWS::Serverless::Function", + "Properties": {"ImageUri": "repo:devOrdersFunction"}, + }, + } + deferred = [] + _update_foreach_with_s3_uris("Fn::ForEach::Env", foreach_value, exported, None, deferred_dynamic=deferred) + inner = foreach_value[2]["Fn::ForEach::Svc"][2]["${Env}${Svc}Function"]["Properties"] + # Legacy fallback copies the first inner iteration's URI; nothing deferred. + self.assertEqual(inner["ImageUri"], "repo:devUsersFunction") + self.assertEqual(deferred, []) diff --git a/tests/unit/lib/bootstrap/companion_stack/test_companion_stack_manager.py b/tests/unit/lib/bootstrap/companion_stack/test_companion_stack_manager.py index 69f7d76ff65..1414c12dfbf 100644 --- a/tests/unit/lib/bootstrap/companion_stack/test_companion_stack_manager.py +++ b/tests/unit/lib/bootstrap/companion_stack/test_companion_stack_manager.py @@ -281,3 +281,41 @@ def test_sync_ecr_stack(self, function_provider_mock, stack_provider_mock, manag manager_mock.return_value.sync_repos.assert_called_once_with() self.assertEqual(result, {"Function1": "uri1", "Function2": "uri2"}) + + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.CompanionStackManager") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamLocalStackProvider") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamFunctionProvider") + def test_sync_ecr_stack_language_extensions_enabled( + self, function_provider_mock, stack_provider_mock, manager_mock + ): + image_repositories = {"Function1": "uri1"} + stacks = MagicMock() + stack_provider_mock.get_stacks.return_value = (stacks, None) + manager_mock.return_value.get_repository_mapping.return_value = {"Function2": "uri2"} + + sync_ecr_stack( + "template.yaml", + "stack-name", + "region", + "s3-bucket", + "s3-prefix", + image_repositories, + language_extensions_enabled=True, + ) + + stack_provider_mock.get_stacks.assert_called_once_with("template.yaml", language_extensions_enabled=True) + + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.CompanionStackManager") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamLocalStackProvider") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamFunctionProvider") + def test_sync_ecr_stack_language_extensions_disabled_by_default( + self, function_provider_mock, stack_provider_mock, manager_mock + ): + image_repositories = {"Function1": "uri1"} + stacks = MagicMock() + stack_provider_mock.get_stacks.return_value = (stacks, None) + manager_mock.return_value.get_repository_mapping.return_value = {"Function2": "uri2"} + + sync_ecr_stack("template.yaml", "stack-name", "region", "s3-bucket", "s3-prefix", image_repositories) + + stack_provider_mock.get_stacks.assert_called_once_with("template.yaml", language_extensions_enabled=False)