diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bfae5e813fe..ccc2279c906 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -197,7 +197,6 @@ jobs: run: pytest -vv ${{ matrix.tests_config.params }} env: FORCE_RUN_DOCKER_TEST: ${{ matrix.tests_config.name == 'durable-functions' && '1' || '' }} - DURABLE_EXECUTIONS_EMULATOR_IMAGE_TAG: ${{ matrix.tests_config.name == 'durable-functions' && 'v1.1.1' || '' }} smoke-and-functional-tests: name: ${{ matrix.tests_config.name }} / ${{ matrix.tests_config.os }} / ${{ matrix.python }} diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 00000000000..2afc1823192 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,68 @@ +#### Which issue(s) does this change fix? + +Fixes #8933 + +#### Why is this change necessary? + +SAM CLI provides an excellent developer experience for Lambda Image functions (`sam build && sam deploy`), but users deploying containerized workloads to ECS (Fargate) or Bedrock AgentCore must manage their Docker build/push/deploy pipeline separately — even when these resources live in the same CloudFormation template. This creates a fragmented workflow requiring external tooling for an identical operation: build image → push to ECR → deploy. + +#### How does it address the issue? + +Extends the existing Lambda Image build pipeline to recognize `AWS::ECS::TaskDefinition` and `AWS::BedrockAgentCore::Runtime` resources with a `Metadata` block containing `Dockerfile` and `DockerContext`. No new commands — `sam build`, `sam package`, `sam deploy`, and `sam sync` gain awareness of these resource types. + +**Template example:** +```yaml +Resources: + MyAgent: + Type: AWS::BedrockAgentCore::Runtime + Metadata: + Dockerfile: Dockerfile + DockerContext: ./agent + Architecture: arm64 + Properties: + AgentRuntimeArtifact: + ContainerConfiguration: + ContainerUri: placeholder + + MyTask: + Type: AWS::ECS::TaskDefinition + Metadata: + Dockerfile: Dockerfile + DockerContext: ./app + ContainerName: web + Properties: + ContainerDefinitions: + - Name: web + Image: placeholder +``` + +**Key implementation details:** +- Reuses `_build_lambda_image()` — same Docker build logic, buildkit support included +- `--resolve-image-repos` auto-creates ECR repos via companion stack +- `ContainerName` metadata targets specific containers in multi-container TaskDefinitions +- `Architecture` metadata sets `--platform` (e.g., `arm64` for AgentCore) +- `ARTIFACT_TYPE = ZIP` to pass the `PackageType` filter (these resources don't have `PackageType`) +- No SAM Transform changes needed — uses native CloudFormation resource types + +**Design document:** `designs/container_image_builds_ecs_agentcore.md` + +#### What side effects does this change have? + +- `sam build` logs "Found N container service resource(s) to build" when applicable resources are present. No behavior change for templates without these resources. +- `--resolve-image-repos` creates ECR repos for ECS/AgentCore in addition to Lambda Image functions. +- `_update_built_resource` adds an optional `metadata` parameter (backward compatible, defaults to `None`). + +#### Mandatory Checklist +**PRs will only be reviewed after checklist is complete** + +- [x] Review the [generative AI contribution guidelines](https://github.com/aws/aws-sam-cli/blob/develop/CONTRIBUTING.md#ai-usage) +- [x] Add input/output [type hints](https://docs.python.org/3/library/typing.html) to new functions/methods +- [x] Write design document if needed ([Do I need to write a design document?](https://github.com/aws/aws-sam-cli/blob/develop/DEVELOPMENT_GUIDE.md#design-document)) +- [x] Write/update unit tests +- [x] Write/update integration tests +- [x] Write/update functional tests if needed +- [x] `make pr` passes +- [x] `make update-reproducible-reqs` if dependencies were changed +- [ ] Write documentation + +By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license](https://www.apache.org/licenses/LICENSE-2.0). diff --git a/designs/container_image_builds_ecs_agentcore.md b/designs/container_image_builds_ecs_agentcore.md new file mode 100644 index 00000000000..1d67bbf21f4 --- /dev/null +++ b/designs/container_image_builds_ecs_agentcore.md @@ -0,0 +1,234 @@ +Container Image Builds for ECS and AgentCore +============================================= + +This is the design for extending `sam build`, `sam package`, `sam deploy`, and `sam sync` +to support building and deploying container images for non-Lambda resources: +`AWS::ECS::TaskDefinition` and `AWS::BedrockAgentCore::Runtime`. + +What is the problem? +-------------------- + +SAM CLI provides an excellent developer experience for Lambda Image functions: a single +`sam build && sam deploy` builds the Docker image, pushes to ECR, and deploys via +CloudFormation. However, users deploying containerized workloads to ECS (Fargate) or +Bedrock AgentCore must manage their Docker build/push/deploy pipeline separately, even +when these resources live in the same CloudFormation template alongside Lambda functions. + +This creates a fragmented workflow where developers need external tooling (shell scripts, +Makefiles, or CI/CD steps) for the identical operation: build image → push to ECR → +update template with ECR URI → deploy. + +What will be changed? +--------------------- + +We extend the existing Lambda Image build pipeline to recognize `AWS::ECS::TaskDefinition` +and `AWS::BedrockAgentCore::Runtime` resources that have a `Metadata` block with +`Dockerfile` and `DockerContext`. No new commands are introduced — the existing +`sam build`, `sam package`, `sam deploy`, and `sam sync` gain awareness of these +resource types. + +### Design Principles + +1. **Same convention** — Uses the identical Metadata block as Lambda Image functions + (Dockerfile, DockerContext, DockerTag, DockerBuildArgs, DockerBuildTarget) +2. **No Transform changes** — Works with native CloudFormation resource types +3. **Opt-in** — Only resources with the Metadata block are affected; existing templates + work unchanged +4. **Reuse** — Delegates to the same `_build_lambda_image()` Docker build logic + +Success criteria for the change +------------------------------- + +1. `sam build` discovers and builds container images for ECS TaskDefinitions and + AgentCore Runtimes that have Dockerfile metadata +2. `sam deploy --resolve-image-repos` auto-creates ECR repos for these resources +3. `sam package` / `sam deploy` pushes images to ECR and rewrites the template with + the ECR URI at the correct property path +4. `sam sync` builds, pushes, and triggers redeployment for these resources +5. Multi-container ECS TaskDefinitions can target a specific container via `ContainerName` +6. Architecture can be specified via `Architecture` metadata (e.g., `arm64`) +7. Buildkit support works automatically (shared with Lambda Image builds) +8. No regressions for existing Lambda, Layer, or API builds + +User Experience +--------------- + +### Template Format + +```yaml +Resources: + # AgentCore Runtime + MyAgent: + Type: AWS::BedrockAgentCore::Runtime + Metadata: + Dockerfile: Dockerfile + DockerContext: ./agent + DockerTag: latest + Architecture: arm64 + Properties: + AgentRuntimeName: my_agent + AgentRuntimeArtifact: + ContainerConfiguration: + ContainerUri: placeholder + NetworkConfiguration: + NetworkMode: PUBLIC + RoleArn: !GetAtt AgentRole.Arn + + # ECS TaskDefinition (multi-container) + MyTask: + Type: AWS::ECS::TaskDefinition + Metadata: + Dockerfile: Dockerfile + DockerContext: ./app + DockerTag: latest + ContainerName: web + Properties: + Family: my-app + ContainerDefinitions: + - Name: envoy + Image: public.ecr.aws/envoy:latest + - Name: web + Image: placeholder +``` + +### CLI Usage + +```bash +# Build container images +sam build + +# Deploy with auto ECR repo creation +sam deploy --resolve-image-repos + +# Or with explicit repo +sam deploy --image-repositories SimpleAgent=123456789012.dkr.ecr.us-east-1.amazonaws.com/repo + +# Live sync +sam sync --stack-name my-stack --watch --resolve-image-repos +``` + +### Metadata Fields + +| Field | Required | Description | +|-------|----------|-------------| +| `Dockerfile` | Yes | Path to Dockerfile relative to DockerContext | +| `DockerContext` | Yes | Build context directory relative to template | +| `DockerTag` | No | Image tag (default: `latest`) | +| `DockerBuildArgs` | No | Dict of build args | +| `DockerBuildTarget` | No | Multi-stage build target | +| `DockerBuildExtraParams` | No | List of extra docker build params | +| `Architecture` | No | Target platform: `x86_64` (default) or `arm64` | +| `ContainerName` | No | ECS only: target container in multi-container TaskDefinition | + +Implementation +-------------- + +### Architecture + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ sam build │ +├─────────────────────────────────────────────────────────────────┤ +│ BuildContext.run() │ +│ ├── builder.build() → Lambda functions + layers │ +│ └── _build_container_images() → ECS + AgentCore containers │ +│ ├── SamContainerServiceProvider (discovery) │ +│ ├── ContainerBuildDefinition (build graph) │ +│ └── ApplicationBuilder.build_container_images() │ +│ └── _build_lambda_image() (shared Docker logic) │ +├─────────────────────────────────────────────────────────────────┤ +│ sam package / sam deploy │ +│ ├── sync_ecr_stack() → auto-creates ECR repos (companion) │ +│ ├── ECSTaskDefinitionImageResource.export() → push + rewrite │ +│ └── AgentCoreRuntimeImageResource.export() → push + rewrite │ +├─────────────────────────────────────────────────────────────────┤ +│ sam sync │ +│ └── ECSContainerSyncFlow │ +│ ├── gather_resources() → build image │ +│ ├── sync() → push to ECR + force ECS deployment │ +│ └── SyncFlowFactory (registered for both types) │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Key Components + +**`samcli/lib/providers/sam_container_provider.py`** (new) +- `SamContainerServiceProvider`: Scans stacks for ECS/AgentCore resources with + Dockerfile+DockerContext metadata. Returns `ContainerService` NamedTuples. + +**`samcli/lib/build/build_graph.py`** (modified) +- `ContainerBuildDefinition`: Parallel to `FunctionBuildDefinition`. Holds resource + identifier, type, metadata, and architecture. Reads `Architecture` from metadata. + +**`samcli/lib/build/app_builder.py`** (modified) +- `build_container_images()`: Iterates container definitions and builds each. +- `_build_container_image()`: Delegates to `_build_lambda_image()` — same Docker logic. +- `_update_built_resource()`: Extended for ECS (`ContainerDefinitions[N].Image`) and + AgentCore (`AgentRuntimeArtifact.ContainerConfiguration.ContainerUri`). Accepts + optional `metadata` param for `ContainerName` targeting. + +**`samcli/lib/package/packageable_resources.py`** (modified) +- `ECSTaskDefinitionImageResource`: Custom export for nested `ContainerDefinitions[0].Image`. +- `AgentCoreRuntimeImageResource`: Export using jmespath for deeply nested property path. +- Both use `ARTIFACT_TYPE = ZIP` to pass the `PackageType` filter (these resources + don't have a `PackageType` property). + +**`samcli/lib/sync/flows/ecs_container_sync_flow.py`** (new) +- `ECSContainerSyncFlow`: Builds image, pushes to ECR, forces ECS service redeployment + by finding services using the task definition family. + +**`samcli/lib/bootstrap/companion_stack/companion_stack_manager.py`** (modified) +- `sync_ecr_stack()`: Extended to include container service resources when creating + ECR repos via the companion stack. + +### Property Path Mapping + +| Resource Type | Property Path for Image URI | +|---------------|----------------------------| +| `AWS::Serverless::Function` | `ImageUri` | +| `AWS::Lambda::Function` | `Code.ImageUri` | +| `AWS::ECS::TaskDefinition` | `ContainerDefinitions[N].Image` | +| `AWS::BedrockAgentCore::Runtime` | `AgentRuntimeArtifact.ContainerConfiguration.ContainerUri` | + +Alternatives Considered +----------------------- + +### 1. New SAM Transform resource type (e.g., `AWS::Serverless::ContainerService`) + +**Rejected because:** +- Requires changes to the SAM Transform (separate repo, separate approval process) +- Adds coupling between SAM CLI and the Transform +- Users would need to wait for Transform support in all regions +- Native CFN types already work and are well-understood + +### 2. Separate `sam container build` command + +**Rejected because:** +- Fragments the workflow — users would need to remember different commands +- Doesn't integrate with `sam deploy` and `sam sync` naturally +- The existing `sam build` already handles image builds for Lambda + +### 3. Using `PackageType: Image` on ECS/AgentCore resources + +**Rejected because:** +- `PackageType` is a Lambda-specific concept not present on ECS or AgentCore resources +- Would require CloudFormation schema changes +- The Metadata-based approach is already the established pattern + +Breaking Changes +---------------- + +None. This is purely additive: +- Templates without ECS/AgentCore Metadata are unaffected +- The `_update_built_resource` signature change is backward compatible (optional param) +- No existing CLI flags or behaviors change + +Future Extensions +----------------- + +1. **Multiple Dockerfiles per ECS TaskDefinition** — Build multiple containers from + one resource using a list of Metadata entries +2. **`sam local start-ecs`** — Local testing of ECS containers (similar to `sam local start-api`) +3. **Health check integration** — Wait for container health before marking sync complete +4. **Build caching** — Layer-aware caching for container builds (currently rebuilds fully) +5. **`sam init` templates** — Starter templates for ECS+SAM and AgentCore+SAM projects diff --git a/docs/cfn-language-extensions.md b/docs/cfn-language-extensions.md index 9fa4f2035c1..7de428e0070 100644 --- a/docs/cfn-language-extensions.md +++ b/docs/cfn-language-extensions.md @@ -321,6 +321,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: @@ -330,7 +346,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/__init__.py b/samcli/__init__.py index 8280958891b..caa430809d9 100644 --- a/samcli/__init__.py +++ b/samcli/__init__.py @@ -2,4 +2,4 @@ SAM CLI version """ -__version__ = "1.161.0" +__version__ = "1.161.1" diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index fd8285accb2..41c664f4071 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -30,7 +30,7 @@ BuildError, UnsupportedBuilderLibraryVersionError, ) -from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR +from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR, ContainerBuildDefinition from samcli.lib.build.bundler import EsbuildBundlerManager from samcli.lib.build.exceptions import ( BuildInsideContainerError, @@ -55,6 +55,7 @@ ) from samcli.lib.providers.provider import LayerVersion, ResourcesToBuildCollector, Stack, get_full_path from samcli.lib.providers.sam_api_provider import SamApiProvider +from samcli.lib.providers.sam_container_provider import SamContainerServiceProvider from samcli.lib.providers.sam_function_provider import SamFunctionProvider from samcli.lib.providers.sam_layer_provider import SamLayerProvider from samcli.lib.providers.sam_stack_provider import SamLocalStackProvider @@ -319,6 +320,11 @@ def run(self) -> None: self._build_result = builder.build() + # Build container images for ECS/AgentCore resources + container_artifacts = self._build_container_images(builder) + if container_artifacts: + self._build_result.artifacts.update(container_artifacts) + self._handle_build_post_processing(builder, self._build_result) click.secho("\nBuild Succeeded", fg="green") @@ -1296,6 +1302,33 @@ def collect_all_build_resources(self) -> ResourcesToBuildCollector: ) return result + def _build_container_images(self, builder: ApplicationBuilder) -> Dict[str, str]: + """ + Discover and build container images for ECS/AgentCore resources. + + Returns + ------- + Dict[str, str] + Map of resource full_path to built image tag + """ + container_provider = SamContainerServiceProvider(self.stacks) + container_services = list(container_provider.get_all()) + if not container_services: + return {} + + LOG.info("Found %d container service resource(s) to build", len(container_services)) + + container_build_defs = [] + for service in container_services: + build_def = ContainerBuildDefinition( + resource_identifier=service.full_path, + resource_type=service.resource_type, + metadata=service.metadata, + ) + container_build_defs.append(build_def) + + return builder.build_container_images(container_build_defs) + @property def is_building_specific_resource(self) -> bool: """ diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py index 8ec13607e70..501261169c4 100644 --- a/samcli/commands/build/command.py +++ b/samcli/commands/build/command.py @@ -55,6 +55,8 @@ 2. AWS::Lambda::Function\n 3. AWS::Serverless::LayerVersion\n 4. AWS::Lambda::LayerVersion\n + 5. AWS::ECS::TaskDefinition (container image)\n + 6. AWS::BedrockAgentCore::Runtime (container image)\n \b Supported Runtimes ------------------ diff --git a/samcli/commands/package/package_context.py b/samcli/commands/package/package_context.py index 17529f60bef..ac76e9b4279 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/hook_packages/terraform/copy_terraform_built_artifacts.py b/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py index add87f60eb2..b6f6062e066 100644 --- a/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py +++ b/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py @@ -195,20 +195,11 @@ def search(self, data): def copytree(src, dst): - """Modified copytree method - Note: before python3.8 there is no `dir_exists_ok` argument, therefore - this function explicitly creates one if it does not exist. + """Copy a directory tree from src to dst, merging into dst if it already exists. + + Symbolic links are preserved as symbolic links in the destination. """ - if not os.path.exists(dst): - os.makedirs(dst) - for item in os.listdir(src): - src_item = os.path.join(src, item) - dst_item = os.path.join(dst, item) - if os.path.isdir(src_item): - # recursively call itself. - copytree(src_item, dst_item) - else: - shutil.copy2(src_item, dst_item) + shutil.copytree(src, dst, symlinks=True, dirs_exist_ok=True) def cli_exit(): diff --git a/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py b/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py index 66dd6a290e1..10d56f97302 100644 --- a/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py +++ b/samcli/lib/bootstrap/companion_stack/companion_stack_manager.py @@ -312,6 +312,14 @@ def sync_ecr_stack( function_logical_ids = [ function.full_path for function in function_provider.get_all() if function.packagetype == IMAGE ] + + # Also include ECS/AgentCore container resources that need ECR repos + from samcli.lib.providers.sam_container_provider import SamContainerServiceProvider + + container_provider = SamContainerServiceProvider(stacks) + container_logical_ids = [service.full_path for service in container_provider.get_all()] + function_logical_ids.extend(container_logical_ids) + manager.set_functions(function_logical_ids, image_repositories) image_repositories.update(manager.get_repository_mapping()) manager.sync_repos() diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index c30c948c007..35d0ef1f50c 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -19,7 +19,12 @@ from aws_lambda_builders.exceptions import LambdaBuilderError from samcli.commands._utils.experimental import get_enabled_experimental_flags -from samcli.lib.build.build_graph import BuildGraph, FunctionBuildDefinition, LayerBuildDefinition +from samcli.lib.build.build_graph import ( + BuildGraph, + ContainerBuildDefinition, + FunctionBuildDefinition, + LayerBuildDefinition, +) from samcli.lib.build.build_strategy import ( BuildStrategy, CachedOrIncrementalBuildStrategyWrapper, @@ -51,7 +56,9 @@ from samcli.lib.utils.packagetype import IMAGE, ZIP from samcli.lib.utils.path_utils import check_path_valid_type, convert_path_to_unix_path from samcli.lib.utils.resources import ( + AWS_BEDROCK_AGENTCORE_RUNTIME, AWS_CLOUDFORMATION_STACK, + AWS_ECS_TASK_DEFINITION, AWS_LAMBDA_FUNCTION, AWS_LAMBDA_LAYERVERSION, AWS_SERVERLESS_APPLICATION, @@ -378,7 +385,11 @@ def update_template( if has_build_artifact: ApplicationBuilder._update_built_resource( - built_artifacts[full_path], properties, resource_type or "", store_path + built_artifacts[full_path], + properties, + resource_type or "", + store_path, + resource.get("Metadata"), ) if is_stack: @@ -391,7 +402,9 @@ def update_template( return template_dict @staticmethod - def _update_built_resource(path: str, resource_properties: Dict, resource_type: str, absolute_path: str) -> None: + def _update_built_resource( + path: str, resource_properties: Dict, resource_type: str, absolute_path: str, metadata: Optional[Dict] = None + ) -> None: if resource_type == AWS_SERVERLESS_FUNCTION and resource_properties.get("PackageType", ZIP) == ZIP: resource_properties["CodeUri"] = absolute_path if resource_type == AWS_LAMBDA_FUNCTION and resource_properties.get("PackageType", ZIP) == ZIP: @@ -404,6 +417,29 @@ def _update_built_resource(path: str, resource_properties: Dict, resource_type: resource_properties["Code"] = {"ImageUri": path} if resource_type == AWS_SERVERLESS_FUNCTION and resource_properties.get("PackageType", ZIP) == IMAGE: resource_properties["ImageUri"] = path + if resource_type == AWS_ECS_TASK_DEFINITION: + container_defs = resource_properties.get("ContainerDefinitions", []) + if container_defs: + target_name = metadata.get("ContainerName") if isinstance(metadata, dict) else None + if target_name: + for container_def in container_defs: + if container_def.get("Name") == target_name: + container_def["Image"] = path + break + else: + raise DockerBuildFailed( + f"Metadata.ContainerName '{target_name}' does not match any " + f"container in ContainerDefinitions" + ) + elif len(container_defs) > 1: + raise DockerBuildFailed( + f"TaskDefinition has {len(container_defs)} containers but Metadata.ContainerName is not set. " + f"Add 'ContainerName: ' to the resource Metadata to specify which container to build." + ) + else: + container_defs[0]["Image"] = path + if resource_type == AWS_BEDROCK_AGENTCORE_RUNTIME: + resource_properties["AgentRuntimeArtifact"] = {"ContainerConfiguration": {"ContainerUri": path}} def _build_lambda_image(self, function_name: str, metadata: Dict, architecture: str) -> str: """ @@ -538,6 +574,53 @@ def _load_lambda_image(self, image_archive_path: str) -> str: except (docker.errors.APIError, OSError, ContainerArchiveImageLoadFailedException) as ex: raise DockerBuildFailed(msg=str(ex)) from ex + def build_container_images(self, container_build_definitions: List[ContainerBuildDefinition]) -> Dict[str, str]: + """ + Build container images for non-Lambda resources (ECS, AgentCore). + + Parameters + ---------- + container_build_definitions : List[ContainerBuildDefinition] + List of container build definitions to build + + Returns + ------- + Dict[str, str] + Map of resource identifier to the built image tag + """ + artifacts: Dict[str, str] = {} + for container_def in container_build_definitions: + if not container_def.metadata: + continue + image_tag = self._build_container_image( + container_def.resource_identifier, + container_def.metadata, + container_def.architecture, + ) + artifacts[container_def.resource_identifier] = image_tag + return artifacts + + def _build_container_image(self, resource_name: str, metadata: Dict, architecture: str) -> str: + """ + Build a container image for an ECS/AgentCore resource. Uses the same Metadata + convention as Lambda Image builds (Dockerfile, DockerContext, DockerBuildArgs, etc.) + + Parameters + ---------- + resource_name : str + Logical ID or identifier of the resource + metadata : dict + Metadata block from the resource + architecture : str + Target architecture + + Returns + ------- + str + The full tag of the built image + """ + return self._build_lambda_image(resource_name, metadata, architecture) + def _build_layer( self, layer_name: str, diff --git a/samcli/lib/build/build_graph.py b/samcli/lib/build/build_graph.py index c31ee039438..48625a184b3 100644 --- a/samcli/lib/build/build_graph.py +++ b/samcli/lib/build/build_graph.py @@ -701,3 +701,67 @@ def __eq__(self, other: Any) -> bool: and self.env_vars == other.env_vars and self.architecture == other.architecture ) + + +class ContainerBuildDefinition(AbstractBuildDefinition): + """ + ContainerBuildDefinition holds information about each unique container image build + for non-Lambda resources (ECS TaskDefinitions, AgentCore, etc.) + """ + + def __init__( + self, + resource_identifier: str, + resource_type: str, + metadata: Optional[Dict], + source_hash: str = "", + manifest_hash: str = "", + env_vars: Optional[Dict] = None, + architecture: str = X86_64, + ) -> None: + # Allow metadata to override architecture (e.g., "arm64" for AgentCore) + if metadata and metadata.get("Architecture"): + architecture = metadata["Architecture"] + super().__init__(source_hash, manifest_hash, env_vars, architecture) + self.resource_identifier = resource_identifier + self.resource_type = resource_type + + metadata_copied = deepcopy(metadata) if metadata else {} + metadata_copied.pop(SAM_RESOURCE_ID_KEY, "") + metadata_copied.pop(SAM_IS_NORMALIZED, "") + self.metadata = metadata_copied + + @property + def dockerfile(self) -> Optional[str]: + return self.metadata.get("Dockerfile") if self.metadata else None + + @property + def docker_context(self) -> Optional[str]: + return self.metadata.get("DockerContext") if self.metadata else None + + @property + def docker_build_args(self) -> Dict: + return self.metadata.get("DockerBuildArgs", {}) if self.metadata else {} + + @property + def docker_build_target(self) -> Optional[str]: + return self.metadata.get("DockerBuildTarget") if self.metadata else None + + def get_resource_full_paths(self) -> str: + return self.resource_identifier + + def __str__(self) -> str: + return ( + f"ContainerBuildDefinition({self.resource_identifier}, {self.resource_type}, " + f"{self.source_hash}, {self.uuid}, {self.metadata}, {self.architecture})" + ) + + def __eq__(self, other: Any) -> bool: + if not isinstance(other, ContainerBuildDefinition): + return False + return ( + self.resource_identifier == other.resource_identifier + and self.resource_type == other.resource_type + and self.metadata == other.metadata + and self.architecture == other.architecture + ) diff --git a/samcli/lib/package/artifact_exporter.py b/samcli/lib/package/artifact_exporter.py index 681a3390fbb..02276b25d70 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) @@ -499,6 +618,7 @@ def export(self) -> Dict: # Export code resources exporter = exporter_class(self.uploaders, self.code_signer, cache) exporter.parent_parameter_values = self.parameter_values + exporter.resource_metadata = resource.get("Metadata") exporter.language_extensions_enabled = self.language_extensions_enabled exporter.export(full_path, resource_dict, self.template_dir) @@ -527,6 +647,7 @@ def delete(self, retain_resources: List): continue # Delete code resources exporter = exporter_class(self.uploaders, None) + exporter.resource_metadata = resource.get("Metadata") exporter.delete(resource_id, resource_dict) def get_ecr_repos(self): diff --git a/samcli/lib/package/language_extensions_packaging.py b/samcli/lib/package/language_extensions_packaging.py index 9eb39366aed..46123ef2994 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 d63136df8fb..2c82959be4f 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 @@ -33,9 +34,11 @@ AWS_APPSYNC_FUNCTIONCONFIGURATION, AWS_APPSYNC_GRAPHQLSCHEMA, AWS_APPSYNC_RESOLVER, + AWS_BEDROCK_AGENTCORE_RUNTIME, AWS_CLOUDFORMATION_MODULEVERSION, AWS_CLOUDFORMATION_RESOURCEVERSION, AWS_ECR_REPOSITORY, + AWS_ECS_TASK_DEFINITION, AWS_ELASTICBEANSTALK_APPLICATIONVERSION, AWS_GLUE_JOB, AWS_LAMBDA_FUNCTION, @@ -688,6 +691,110 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st shutil.rmtree(temp_dir) +class ECSTaskDefinitionImageResource(ResourceImage): + """ + Represents an ECS TaskDefinition with a container image that needs to be pushed to ECR. + For single-container TaskDefinitions the first container is used by default; for + multi-container TaskDefinitions Metadata.ContainerName must be set. + """ + + RESOURCE_TYPE = AWS_ECS_TASK_DEFINITION + PROPERTY_NAME = RESOURCES_WITH_IMAGE_COMPONENT[RESOURCE_TYPE][0] + ARTIFACT_TYPE = ZIP # No PackageType property; defaults to ZIP in the filter + + def _get_target_index(self, container_defs): + """Find the target container index using ContainerName from Metadata.""" + metadata = getattr(self, "resource_metadata", None) + target_name = metadata.get("ContainerName") if isinstance(metadata, dict) else None + if target_name: + for i, cd in enumerate(container_defs): + if cd.get("Name") == target_name: + return i + raise exceptions.ExportFailedError( + resource_id="", + property_name=self.PROPERTY_NAME, + property_value=target_name, + ex=ValueError( + f"Metadata.ContainerName '{target_name}' does not match any container in ContainerDefinitions" + ), + ) + if len(container_defs) > 1: + raise exceptions.ExportFailedError( + resource_id="", + property_name=self.PROPERTY_NAME, + property_value="", + ex=ValueError( + f"TaskDefinition has {len(container_defs)} containers but Metadata.ContainerName is not set. " + f"Add 'ContainerName: ' to the resource Metadata to specify which container to package." + ), + ) + return 0 + + def export(self, resource_id, resource_dict, parent_dir): + if resource_dict is None: + return + + container_defs = resource_dict.get("ContainerDefinitions", []) + if not container_defs: + return + + target_idx = self._get_target_index(container_defs) + property_value = container_defs[target_idx].get("Image") + if not property_value or isinstance(property_value, dict) or is_ecr_url(property_value): + return + + try: + uploaded_url = self.uploader.upload(property_value, resource_id) + container_defs[target_idx]["Image"] = uploaded_url + except Exception as ex: + LOG.debug("Unable to export", exc_info=ex) + raise exceptions.ExportFailedError( + resource_id=resource_id, + property_name=self.PROPERTY_NAME, + property_value=property_value, + ex=ex, + ) + + def delete(self, resource_id, resource_dict): + if resource_dict is None: + return + container_defs = resource_dict.get("ContainerDefinitions", []) + if not container_defs: + return + target_idx = self._get_target_index(container_defs) + remote_path = container_defs[target_idx].get("Image") + if isinstance(remote_path, str) and is_ecr_url(remote_path): + self.uploader.delete_artifact( + image_uri=remote_path, resource_id=resource_id, property_name=self.PROPERTY_NAME + ) + + +class AgentCoreRuntimeImageResource(ResourceImage): + """ + Represents a Bedrock AgentCore Runtime resource with a container image. + Property path: AgentRuntimeArtifact.ContainerConfiguration.ContainerUri + """ + + RESOURCE_TYPE = AWS_BEDROCK_AGENTCORE_RUNTIME + PROPERTY_NAME = RESOURCES_WITH_IMAGE_COMPONENT[RESOURCE_TYPE][0] + ARTIFACT_TYPE = ZIP # No PackageType property; defaults to ZIP in the filter + + def do_export(self, resource_id, resource_dict, parent_dir): + uploaded_url = upload_local_image_artifacts( + resource_id, resource_dict, self.PROPERTY_NAME, parent_dir, self.uploader + ) + set_value_from_jmespath(resource_dict, self.PROPERTY_NAME, uploaded_url) + + def delete(self, resource_id, resource_dict): + if resource_dict is None: + return + remote_path = jmespath.search(self.PROPERTY_NAME, resource_dict) + if isinstance(remote_path, str) and is_ecr_url(remote_path): + self.uploader.delete_artifact( + image_uri=remote_path, resource_id=resource_id, property_name=self.PROPERTY_NAME + ) + + RESOURCES_EXPORT_LIST = [ ServerlessFunctionResource, ServerlessFunctionImageResource, @@ -713,11 +820,35 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st CloudFormationModuleVersionModulePackage, CloudFormationResourceVersionSchemaHandlerPackage, ECRResource, + ECSTaskDefinitionImageResource, + AgentCoreRuntimeImageResource, GraphQLApiSchemaResource, 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 +872,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/samcli/lib/providers/sam_container_provider.py b/samcli/lib/providers/sam_container_provider.py new file mode 100644 index 00000000000..70e576d6484 --- /dev/null +++ b/samcli/lib/providers/sam_container_provider.py @@ -0,0 +1,100 @@ +""" +Class that provides container service resources (ECS, AgentCore) from a given SAM/CFN template +""" + +import logging +from typing import Dict, Iterator, List, NamedTuple, Optional + +from samcli.lib.providers.provider import Stack, get_full_path +from samcli.lib.utils.resources import ( + AWS_BEDROCK_AGENTCORE_RUNTIME, + AWS_ECS_TASK_DEFINITION, +) + +LOG = logging.getLogger(__name__) + +# Resource types that support container image builds +CONTAINER_IMAGE_RESOURCE_TYPES = [ + AWS_ECS_TASK_DEFINITION, + AWS_BEDROCK_AGENTCORE_RUNTIME, +] + + +class ContainerService(NamedTuple): + """ + Represents a container service resource that needs an image built. + """ + + # Logical ID of the resource + resource_id: str + # Full path including stack path + full_path: str + # Resource type (e.g., AWS::ECS::TaskDefinition) + resource_type: str + # Metadata dict containing Dockerfile, DockerContext, etc. + metadata: Dict + # Resource properties + properties: Dict + # Stack path + stack_path: str = "" + + +class SamContainerServiceProvider: + """ + Extracts container service resources from SAM/CFN templates that have + Metadata with Dockerfile and DockerContext, indicating they need image builds. + """ + + def __init__(self, stacks: List[Stack]) -> None: + self._stacks = stacks + self._container_services: Dict[str, ContainerService] = {} + self._extract_container_services() + + def _extract_container_services(self) -> None: + for stack in self._stacks: + resources = getattr(stack, "resources", None) + if not resources or not isinstance(resources, dict): + continue + for logical_id, resource in resources.items(): + resource_type = resource.get("Type", "") + if resource_type not in CONTAINER_IMAGE_RESOURCE_TYPES: + continue + + metadata = resource.get("Metadata", {}) + if not self._has_container_build_metadata(metadata): + continue + + full_path = get_full_path(stack.stack_path, logical_id) + properties = resource.get("Properties", {}) + + container_service = ContainerService( + resource_id=logical_id, + full_path=full_path, + resource_type=resource_type, + metadata=metadata, + properties=properties, + stack_path=stack.stack_path, + ) + self._container_services[full_path] = container_service + LOG.debug("Found container service resource: %s (%s)", full_path, resource_type) + + @staticmethod + def _has_container_build_metadata(metadata: Optional[Dict]) -> bool: + """Check if metadata contains the required fields for a container image build.""" + if not isinstance(metadata, dict): + return False + return bool(metadata.get("Dockerfile") and metadata.get("DockerContext")) + + def get(self, name: str) -> Optional[ContainerService]: + """Get a container service by full path or logical ID.""" + if name in self._container_services: + return self._container_services[name] + # Try matching by logical ID alone + for full_path, service in self._container_services.items(): + if service.resource_id == name: + return service + return None + + def get_all(self) -> Iterator[ContainerService]: + """Return all container services.""" + yield from self._container_services.values() diff --git a/samcli/lib/sync/flows/ecs_container_sync_flow.py b/samcli/lib/sync/flows/ecs_container_sync_flow.py new file mode 100644 index 00000000000..106cdd27e37 --- /dev/null +++ b/samcli/lib/sync/flows/ecs_container_sync_flow.py @@ -0,0 +1,300 @@ +"""SyncFlow for ECS TaskDefinition container image resources""" + +import logging +from typing import TYPE_CHECKING, Any, Dict, List, Optional + +from botocore.exceptions import ClientError +from docker.client import DockerClient + +from samcli.lib.build.app_builder import ApplicationBuilder, ApplicationBuildResult +from samcli.lib.build.build_graph import ContainerBuildDefinition +from samcli.lib.package.ecr_uploader import ECRUploader +from samcli.lib.providers.provider import ResourcesToBuildCollector, Stack +from samcli.lib.providers.sam_container_provider import SamContainerServiceProvider +from samcli.lib.sync.sync_flow import ApiCallTypes, ResourceAPICall, SyncFlow +from samcli.local.docker.utils import get_validated_container_client + +if TYPE_CHECKING: # pragma: no cover + from samcli.commands.build.build_context import BuildContext + from samcli.commands.deploy.deploy_context import DeployContext + from samcli.commands.sync.sync_context import SyncContext + +LOG = logging.getLogger(__name__) + +# ECS service ARN format: arn:aws:ecs:::service// +_ECS_SERVICE_ARN_PREFIX = "arn:aws:ecs:" +_ECS_SERVICE_ARN_SERVICE_SEGMENT = ":service/" + +# Minimum number of slash-separated parts after splitting a service ARN +# e.g. arn:aws:ecs:region:account:service/cluster/name -> [..., "cluster", "name"] +_ECS_SERVICE_ARN_PARTS = 3 + + +class ECSContainerSyncFlow(SyncFlow): + """SyncFlow for ECS TaskDefinition container image resources. + + Builds the container image, pushes to ECR, registers a new TaskDefinition + revision with the updated image URI, then updates any associated ECS services + to that revision. + """ + + _resource_identifier: str + _ecr_client: Optional[Any] + _docker_client: Optional[DockerClient] + _ecs_client: Optional[Any] + _image_name: Optional[str] + + def __init__( + self, + resource_identifier: str, + build_context: "BuildContext", + deploy_context: "DeployContext", + sync_context: "SyncContext", + physical_id_mapping: Dict[str, str], + stacks: List[Stack], + application_build_result: Optional[ApplicationBuildResult], + ): + super().__init__( + build_context, + deploy_context, + sync_context, + physical_id_mapping, + f"ECSContainer {resource_identifier}", + stacks, + application_build_result, + ) + self._resource_identifier = resource_identifier + self._ecr_client = None + self._docker_client = None + self._ecs_client = None + self._image_name = None + + @property + def sync_state_identifier(self) -> str: + return self._resource_identifier + + def _get_docker_client(self) -> DockerClient: + if not self._docker_client: + self._docker_client = get_validated_container_client() + return self._docker_client + + def _get_ecr_client(self) -> Any: + if not self._ecr_client: + self._ecr_client = self._boto_client("ecr") + return self._ecr_client + + def _get_ecs_client(self) -> Any: + if not self._ecs_client: + self._ecs_client = self._boto_client("ecs") + return self._ecs_client + + def gather_resources(self) -> None: + """Build the container image.""" + if self._application_build_result: + self._image_name = self._application_build_result.artifacts.get(self._resource_identifier) + else: + self._build_from_scratch() + + if self._image_name: + docker_img = self._get_docker_client().images.get(self._image_name) + if docker_img and docker_img.attrs.get("Id"): + self._local_sha = str(docker_img.attrs.get("Id")) + + def _build_from_scratch(self) -> None: + """Build the container image from scratch using the provider.""" + container_provider = SamContainerServiceProvider(self._stacks or []) + service = container_provider.get(self._resource_identifier) + if not service: + LOG.warning("Cannot find container service resource '%s'", self._resource_identifier) + return + + build_def = ContainerBuildDefinition( + resource_identifier=service.full_path, + resource_type=service.resource_type, + metadata=service.metadata, + ) + + builder = ApplicationBuilder( + ResourcesToBuildCollector(), + self._build_context.build_dir, + self._build_context.base_dir, + self._build_context.cache_dir, + cached=False, + is_building_specific_resource=True, + manifest_path_override=self._build_context.manifest_path_override, + container_manager=self._build_context.container_manager, + mode=self._build_context.mode, + build_in_source=self._build_context.build_in_source, + ) + artifacts = builder.build_container_images([build_def]) + self._image_name = artifacts.get(self._resource_identifier) + + def compare_remote(self) -> bool: + return False + + def sync(self) -> None: + if not self._image_name: + LOG.debug("%sSkipping sync. Image name is None.", self.log_prefix) + return + + ecr_repo = self._deploy_context.image_repository + if ( + not ecr_repo + and self._deploy_context.image_repositories + and isinstance(self._deploy_context.image_repositories, dict) + ): + ecr_repo = self._deploy_context.image_repositories.get(self._resource_identifier) + + if not ecr_repo: + LOG.warning( + "%sNo ECR repository configured for %s. Use --image-repository or --image-repositories.", + self.log_prefix, + self._resource_identifier, + ) + return + + ecr_uploader = ECRUploader(self._get_docker_client(), self._get_ecr_client(), ecr_repo, None) + image_uri = ecr_uploader.upload(self._image_name, self._resource_identifier) + + new_task_def_arn = self._register_updated_task_definition(image_uri) + if new_task_def_arn: + self._update_services_to_task_definition(new_task_def_arn) + + def _register_updated_task_definition(self, image_uri: str) -> Optional[str]: + """Describe the current task definition, swap the image URI, and register a new revision.""" + physical_id = self._physical_id_mapping.get(self._resource_identifier) + if not physical_id: + LOG.debug( + "%sNo physical ID for %s; cannot register new task definition revision.", + self.log_prefix, + self._resource_identifier, + ) + return None + + ecs_client = self._get_ecs_client() + try: + response = ecs_client.describe_task_definition(taskDefinition=physical_id, include=["TAGS"]) + except ClientError: + LOG.warning("%sFailed to describe task definition %s", self.log_prefix, physical_id, exc_info=True) + return None + + task_def = response.get("taskDefinition", {}) + tags = response.get("tags", []) + + # Fields that are output-only and must be stripped before re-registering + _READONLY_FIELDS = { + "taskDefinitionArn", + "revision", + "status", + "requiresAttributes", + "compatibilities", + "registeredAt", + "registeredBy", + } + register_input = {k: v for k, v in task_def.items() if k not in _READONLY_FIELDS} + + # Update the image in the matching container definition + container_name = self._get_container_name() + container_defs = register_input.get("containerDefinitions", []) + updated = False + for cd in container_defs: + if container_name is None or cd.get("name") == container_name: + cd["image"] = image_uri + updated = True + if container_name is not None: + break + + if not updated: + LOG.warning( + "%sContainerName '%s' not found in task definition; skipping registration.", + self.log_prefix, + container_name, + ) + return None + + if tags: + register_input["tags"] = tags + + try: + reg_response = ecs_client.register_task_definition(**register_input) + except ClientError: + LOG.warning("%sFailed to register new task definition revision", self.log_prefix, exc_info=True) + return None + + new_arn: Optional[str] = reg_response.get("taskDefinition", {}).get("taskDefinitionArn") + LOG.info("%sRegistered new task definition revision: %s", self.log_prefix, new_arn) + return new_arn + + def _get_container_name(self) -> Optional[str]: + """Return ContainerName from resource metadata, if set.""" + if not self._stacks: + return None + provider = SamContainerServiceProvider(self._stacks) + service = provider.get(self._resource_identifier) + if service and isinstance(service.metadata, dict): + return service.metadata.get("ContainerName") + return None + + def _update_services_to_task_definition(self, task_def_arn: str) -> None: + """Update ECS services that reference this task definition family to the new revision.""" + physical_id = self._physical_id_mapping.get(self._resource_identifier) + if not physical_id: + return + + ecs_client = self._get_ecs_client() + my_family = physical_id.rsplit("/", 1)[-1].split(":", 1)[0] + + for resource_id, resource_physical_id in self._physical_id_mapping.items(): + if not resource_physical_id: + continue + # Only consider ECS service ARNs; skip everything else (incl. App Runner, VPC Lattice, etc.) + if not ( + str(resource_physical_id).startswith(_ECS_SERVICE_ARN_PREFIX) + and _ECS_SERVICE_ARN_SERVICE_SEGMENT in str(resource_physical_id) + ): + continue + + parts = resource_physical_id.rsplit("/", 2) + if len(parts) < _ECS_SERVICE_ARN_PARTS: + continue + cluster = parts[-2] + service_name = parts[-1] + + try: + svc_response = ecs_client.describe_services(cluster=cluster, services=[service_name]) + for svc in svc_response.get("services", []): + svc_task_def = svc.get("taskDefinition", "") + svc_family = svc_task_def.rsplit("/", 1)[-1].split(":", 1)[0] + if svc_family and svc_family == my_family: + ecs_client.update_service( + cluster=cluster, + service=service_name, + taskDefinition=task_def_arn, + ) + LOG.info( + "%sUpdated service %s to task definition %s", + self.log_prefix, + service_name, + task_def_arn, + ) + except ClientError: + LOG.warning( + "%sFailed to update service %s", + self.log_prefix, + resource_id, + exc_info=True, + ) + + def gather_dependencies(self) -> List[SyncFlow]: + return [] + + def _get_resource_api_calls(self) -> List[ResourceAPICall]: + return [ + ResourceAPICall( + self._resource_identifier, + [ApiCallTypes.UPDATE_CONTAINER_IMAGE], + ) + ] + + def _equality_keys(self) -> Any: + return self._resource_identifier diff --git a/samcli/lib/sync/sync_flow.py b/samcli/lib/sync/sync_flow.py index 1f5fec38ade..a64cdee8725 100644 --- a/samcli/lib/sync/sync_flow.py +++ b/samcli/lib/sync/sync_flow.py @@ -43,6 +43,7 @@ class ApiCallTypes(Enum): UPDATE_FUNCTION_CONFIGURATION = "UpdateFunctionConfiguration" UPDATE_FUNCTION_CODE = "UpdateFunctionCode" PUBLISH_VERSION = "PublishVersion" + UPDATE_CONTAINER_IMAGE = "UpdateContainerImage" class ResourceAPICall(NamedTuple): diff --git a/samcli/lib/sync/sync_flow_factory.py b/samcli/lib/sync/sync_flow_factory.py index 4fbdda54b3b..4a20829e8e5 100644 --- a/samcli/lib/sync/sync_flow_factory.py +++ b/samcli/lib/sync/sync_flow_factory.py @@ -12,6 +12,7 @@ from samcli.lib.package.utils import is_local_folder, is_zip_file from samcli.lib.providers.provider import Function, FunctionBuildInfo, ResourceIdentifier, Stack from samcli.lib.sync.flows.auto_dependency_layer_sync_flow import AutoDependencyLayerParentSyncFlow +from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow from samcli.lib.sync.flows.function_sync_flow import FunctionSyncFlow from samcli.lib.sync.flows.http_api_sync_flow import HttpApiSyncFlow from samcli.lib.sync.flows.image_function_sync_flow import ImageFunctionSyncFlow @@ -39,6 +40,8 @@ from samcli.lib.utils.resources import ( AWS_APIGATEWAY_RESTAPI, AWS_APIGATEWAY_V2_API, + AWS_BEDROCK_AGENTCORE_RUNTIME, + AWS_ECS_TASK_DEFINITION, AWS_LAMBDA_FUNCTION, AWS_LAMBDA_LAYERVERSION, AWS_SERVERLESS_API, @@ -72,6 +75,7 @@ class SyncCodeResources: AWS_APIGATEWAY_V2_API, AWS_SERVERLESS_STATEMACHINE, AWS_STEPFUNCTIONS_STATEMACHINE, + AWS_ECS_TASK_DEFINITION, ] @classmethod @@ -338,6 +342,34 @@ def _create_stepfunctions_flow( self._stacks, ) + def _create_ecs_container_flow( + self, + resource_identifier: ResourceIdentifier, + application_build_result: Optional[ApplicationBuildResult], + ) -> Optional[SyncFlow]: + return ECSContainerSyncFlow( + str(resource_identifier), + self._build_context, + self._deploy_context, + self._sync_context, + self._physical_id_mapping, + self._stacks, + application_build_result, + ) + + def _create_agentcore_flow( + self, + resource_identifier: ResourceIdentifier, + application_build_result: Optional[ApplicationBuildResult], + ) -> Optional[SyncFlow]: + # AgentCore sync requires UpdateAgentRuntime, which is not yet implemented. + LOG.warning( + "sam sync is not yet supported for AWS::BedrockAgentCore::Runtime resource '%s'. " + "Use sam deploy to update AgentCore resources.", + str(resource_identifier), + ) + return None + GeneratorFunction = Callable[ ["SyncFlowFactory", ResourceIdentifier, Optional[ApplicationBuildResult]], Optional[SyncFlow] ] @@ -352,6 +384,8 @@ def _create_stepfunctions_flow( AWS_APIGATEWAY_V2_API: _create_api_flow, AWS_SERVERLESS_STATEMACHINE: _create_stepfunctions_flow, AWS_STEPFUNCTIONS_STATEMACHINE: _create_stepfunctions_flow, + AWS_ECS_TASK_DEFINITION: _create_ecs_container_flow, + AWS_BEDROCK_AGENTCORE_RUNTIME: _create_agentcore_flow, } # SyncFlow mapping between resource type and creation function diff --git a/samcli/lib/utils/osutils.py b/samcli/lib/utils/osutils.py index 4887de2eea2..74cf67199b1 100644 --- a/samcli/lib/utils/osutils.py +++ b/samcli/lib/utils/osutils.py @@ -2,7 +2,6 @@ Common OS utilities """ -import errno import io import logging import os @@ -117,7 +116,7 @@ def stderr() -> io.TextIOWrapper: def remove(path): if path: try: - os.remove(path) + Path(path).unlink(missing_ok=True) except OSError: pass @@ -134,60 +133,27 @@ def tempfile_platform_independent(): remove(_tempfile.name) -# NOTE: Py3.8 or higher has a ``dir_exist_ok=True`` parameter to provide this functionality. -# This method can be removed if we stop supporting Py37 def copytree(source, destination, ignore=None): """ - Similar to shutil.copytree except that it removes the limitation that the destination directory should - be present. + Recursively copy a directory tree from source to destination, allowing the destination + to already exist. Symbolic links in the source tree are preserved as symbolic links + in the destination. + + Delegates to ``shutil.copytree`` with ``dirs_exist_ok=True`` and ``symlinks=True``. + :type source: str :param source: - Path to the source folder to copy + Path to the source directory to copy :type destination: str :param destination: - Path to destination folder - :type ignore: function + Path to the destination directory. Will be created if it does not exist; + existing files will be overwritten by corresponding files from source. + :type ignore: callable, optional :param ignore: - A function that returns a set of file names to ignore, given a list of available file names. Similar to the - ``ignore`` property of ``shutils.copytree`` method + A callable that receives the directory being visited and a list of its contents, + and returns a set of names to ignore. See ``shutil.ignore_patterns`` for a helper. """ - - if not os.path.exists(destination): - os.makedirs(destination) - - try: - # Let's try to copy the directory metadata from source to destination - shutil.copystat(source, destination) - except OSError as ex: - # Can't copy file access times in Windows - LOG.debug("Unable to copy file access times from %s to %s", source, destination, exc_info=ex) - - names = os.listdir(source) - if ignore is not None: - ignored_names = ignore(source, names) - else: - ignored_names = set() - - for name in names: - # Skip ignored names - if name in ignored_names: - continue - - new_source = os.path.join(source, name) - new_destination = os.path.join(destination, name) - - if os.path.isdir(new_source): - copytree(new_source, new_destination, ignore=ignore) - else: - try: - shutil.copy2(new_source, new_destination, follow_symlinks=False) - except OSError as e: - if e.errno != errno.EINVAL: - raise e - - # Symlinks do not get copied for Windows using shutil.copy2, which is why - # they are handled separately here. - create_symlink_or_copy(new_source, new_destination) + shutil.copytree(source, destination, symlinks=True, ignore=ignore, dirs_exist_ok=True) def convert_files_to_unix_line_endings(path: str, target_files: Optional[List[str]] = None) -> None: diff --git a/samcli/lib/utils/resources.py b/samcli/lib/utils/resources.py index cb99abc8d61..0d8a75e681b 100644 --- a/samcli/lib/utils/resources.py +++ b/samcli/lib/utils/resources.py @@ -58,6 +58,14 @@ AWS_SERVERLESS_STATEMACHINE = "AWS::Serverless::StateMachine" AWS_STEPFUNCTIONS_STATEMACHINE = "AWS::StepFunctions::StateMachine" AWS_ECR_REPOSITORY = "AWS::ECR::Repository" + +# ECS +AWS_ECS_TASK_DEFINITION = "AWS::ECS::TaskDefinition" +AWS_ECS_SERVICE = "AWS::ECS::Service" + +# AgentCore +AWS_BEDROCK_AGENTCORE_RUNTIME = "AWS::BedrockAgentCore::Runtime" + AWS_APPLICATION_INSIGHTS = "AWS::ApplicationInsights::Application" AWS_RESOURCE_GROUP = "AWS::ResourceGroups::Group" @@ -99,6 +107,8 @@ AWS_SERVERLESS_FUNCTION: ["ImageUri"], AWS_LAMBDA_FUNCTION: ["Code.ImageUri"], AWS_ECR_REPOSITORY: ["RepositoryName"], + AWS_ECS_TASK_DEFINITION: ["ContainerDefinitions.Image"], + AWS_BEDROCK_AGENTCORE_RUNTIME: ["AgentRuntimeArtifact.ContainerConfiguration.ContainerUri"], } NESTED_STACKS_RESOURCES = { diff --git a/schema/samcli.json b/schema/samcli.json index b7e3f1ec7e3..1c53b509f7a 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -1779,7 +1779,7 @@ "properties": { "parameters": { "title": "Parameters for the sync command", - "description": "Available parameters for the sync command:\n* template_file:\nAWS SAM template file.\n* code:\nSync ONLY code resources. This includes Lambda Functions, API Gateway, and Step Functions.\n* watch:\nWatch local files and automatically sync with cloud.\n* resource_id:\nSync code for all the resources with the ID. To sync a resource within a nested stack, use the following pattern {ChildStack}/{logicalId}.\n* resource:\nSync code for all resources of the given resource type. Accepted values are ['AWS::Serverless::Function', 'AWS::Lambda::Function', 'AWS::Serverless::LayerVersion', 'AWS::Lambda::LayerVersion', 'AWS::Serverless::Api', 'AWS::ApiGateway::RestApi', 'AWS::Serverless::HttpApi', 'AWS::ApiGatewayV2::Api', 'AWS::Serverless::StateMachine', 'AWS::StepFunctions::StateMachine']\n* dependency_layer:\nSeparate dependencies of individual function into a Lambda layer for improved performance.\n* skip_deploy_sync:\nThis option will skip the initial infrastructure deployment if it is not required by comparing the local template with the template deployed in cloud.\n* container_env_var_file:\nEnvironment variables json file (e.g., env_vars.json) to be passed to containers.\n* watch_exclude:\nExcludes a file or folder from being observed for file changes. Files and folders that are excluded will not trigger a sync workflow. This option can be provided multiple times.\n\nExamples:\n\nHelloWorldFunction=package-lock.json\n\nChildStackA/FunctionName=database.sqlite3\n* stack_name:\nName of the AWS CloudFormation stack.\n* base_dir:\nResolve relative paths to function's source code with respect to this directory. Use this if SAM template and source code are not in same enclosing folder. By default, relative paths are resolved with respect to the SAM template's location.\n* use_container:\nBuild functions within an AWS Lambda-like container.\n* build_in_source:\nOpts in to build project in the source folder. The following workflows support building in source: ['nodejs16.x', 'nodejs18.x', 'nodejs20.x', 'nodejs22.x', 'Makefile', 'esbuild']\n* build_image:\nContainer image URIs for building functions/layers. You can specify for all functions/layers with just the image URI (--build-image public.ecr.aws/sam/build-nodejs18.x:latest). You can specify for each individual function with (--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs18.x:latest). A combination of the two can be used. If a function does not have build image specified or an image URI for all functions, the default SAM CLI build images will be used.\n* image_repository:\nAWS ECR repository URI where artifacts referenced in the template are uploaded.\n* image_repositories:\nMapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.\n* s3_bucket:\nAWS S3 bucket where artifacts referenced in the template are uploaded.\n* s3_prefix:\nPrefix name that is added to the artifact's name when it is uploaded to the AWS S3 bucket.\n* kms_key_id:\nThe ID of an AWS KMS key that is used to encrypt artifacts that are at rest in the AWS S3 bucket.\n* role_arn:\nARN of an IAM role that AWS Cloudformation assumes when executing a deployment change set.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* metadata:\nMap of metadata to attach to ALL the artifacts that are referenced in the template.\n* notification_arns:\nARNs of SNS topics that AWS Cloudformation associates with the stack.\n* tags:\nList of tags to associate with the stack.\n* capabilities:\nList of capabilities that one must specify before AWS Cloudformation can create certain stacks.\n\nAccepted Values: CAPABILITY_IAM, CAPABILITY_NAMED_IAM, CAPABILITY_RESOURCE_POLICY, CAPABILITY_AUTO_EXPAND.\n\nLearn more at: https://docs.aws.amazon.com/serverlessrepo/latest/devguide/acknowledging-application-capabilities.html\n* language_extensions:\nExpand AWS::LanguageExtensions transforms (Fn::ForEach, Fn::Length, Fn::ToJsonString, Fn::FindInMap with DefaultValue) locally before running SAM transforms. Off by default. Equivalent env var: SAM_CLI_ENABLE_LANGUAGE_EXTENSIONS=1.\n* save_params:\nSave the parameters provided via the command line to the configuration file.", + "description": "Available parameters for the sync command:\n* template_file:\nAWS SAM template file.\n* code:\nSync ONLY code resources. This includes Lambda Functions, API Gateway, and Step Functions.\n* watch:\nWatch local files and automatically sync with cloud.\n* resource_id:\nSync code for all the resources with the ID. To sync a resource within a nested stack, use the following pattern {ChildStack}/{logicalId}.\n* resource:\nSync code for all resources of the given resource type. Accepted values are ['AWS::Serverless::Function', 'AWS::Lambda::Function', 'AWS::Serverless::LayerVersion', 'AWS::Lambda::LayerVersion', 'AWS::Serverless::Api', 'AWS::ApiGateway::RestApi', 'AWS::Serverless::HttpApi', 'AWS::ApiGatewayV2::Api', 'AWS::Serverless::StateMachine', 'AWS::StepFunctions::StateMachine', 'AWS::ECS::TaskDefinition']\n* dependency_layer:\nSeparate dependencies of individual function into a Lambda layer for improved performance.\n* skip_deploy_sync:\nThis option will skip the initial infrastructure deployment if it is not required by comparing the local template with the template deployed in cloud.\n* container_env_var_file:\nEnvironment variables json file (e.g., env_vars.json) to be passed to containers.\n* watch_exclude:\nExcludes a file or folder from being observed for file changes. Files and folders that are excluded will not trigger a sync workflow. This option can be provided multiple times.\n\nExamples:\n\nHelloWorldFunction=package-lock.json\n\nChildStackA/FunctionName=database.sqlite3\n* stack_name:\nName of the AWS CloudFormation stack.\n* base_dir:\nResolve relative paths to function's source code with respect to this directory. Use this if SAM template and source code are not in same enclosing folder. By default, relative paths are resolved with respect to the SAM template's location.\n* use_container:\nBuild functions within an AWS Lambda-like container.\n* build_in_source:\nOpts in to build project in the source folder. The following workflows support building in source: ['nodejs16.x', 'nodejs18.x', 'nodejs20.x', 'nodejs22.x', 'Makefile', 'esbuild']\n* build_image:\nContainer image URIs for building functions/layers. You can specify for all functions/layers with just the image URI (--build-image public.ecr.aws/sam/build-nodejs18.x:latest). You can specify for each individual function with (--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs18.x:latest). A combination of the two can be used. If a function does not have build image specified or an image URI for all functions, the default SAM CLI build images will be used.\n* image_repository:\nAWS ECR repository URI where artifacts referenced in the template are uploaded.\n* image_repositories:\nMapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.\n* s3_bucket:\nAWS S3 bucket where artifacts referenced in the template are uploaded.\n* s3_prefix:\nPrefix name that is added to the artifact's name when it is uploaded to the AWS S3 bucket.\n* kms_key_id:\nThe ID of an AWS KMS key that is used to encrypt artifacts that are at rest in the AWS S3 bucket.\n* role_arn:\nARN of an IAM role that AWS Cloudformation assumes when executing a deployment change set.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* metadata:\nMap of metadata to attach to ALL the artifacts that are referenced in the template.\n* notification_arns:\nARNs of SNS topics that AWS Cloudformation associates with the stack.\n* tags:\nList of tags to associate with the stack.\n* capabilities:\nList of capabilities that one must specify before AWS Cloudformation can create certain stacks.\n\nAccepted Values: CAPABILITY_IAM, CAPABILITY_NAMED_IAM, CAPABILITY_RESOURCE_POLICY, CAPABILITY_AUTO_EXPAND.\n\nLearn more at: https://docs.aws.amazon.com/serverlessrepo/latest/devguide/acknowledging-application-capabilities.html\n* language_extensions:\nExpand AWS::LanguageExtensions transforms (Fn::ForEach, Fn::Length, Fn::ToJsonString, Fn::FindInMap with DefaultValue) locally before running SAM transforms. Off by default. Equivalent env var: SAM_CLI_ENABLE_LANGUAGE_EXTENSIONS=1.\n* save_params:\nSave the parameters provided via the command line to the configuration file.", "type": "object", "properties": { "template_file": { @@ -1806,10 +1806,11 @@ "resource": { "title": "resource", "type": "string", - "description": "Sync code for all resources of the given resource type. Accepted values are ['AWS::Serverless::Function', 'AWS::Lambda::Function', 'AWS::Serverless::LayerVersion', 'AWS::Lambda::LayerVersion', 'AWS::Serverless::Api', 'AWS::ApiGateway::RestApi', 'AWS::Serverless::HttpApi', 'AWS::ApiGatewayV2::Api', 'AWS::Serverless::StateMachine', 'AWS::StepFunctions::StateMachine']", + "description": "Sync code for all resources of the given resource type. Accepted values are ['AWS::Serverless::Function', 'AWS::Lambda::Function', 'AWS::Serverless::LayerVersion', 'AWS::Lambda::LayerVersion', 'AWS::Serverless::Api', 'AWS::ApiGateway::RestApi', 'AWS::Serverless::HttpApi', 'AWS::ApiGatewayV2::Api', 'AWS::Serverless::StateMachine', 'AWS::StepFunctions::StateMachine', 'AWS::ECS::TaskDefinition']", "enum": [ "AWS::ApiGateway::RestApi", "AWS::ApiGatewayV2::Api", + "AWS::ECS::TaskDefinition", "AWS::Lambda::Function", "AWS::Lambda::LayerVersion", "AWS::Serverless::Api", diff --git a/tests/integration/buildcmd/test_build_cmd_container_image.py b/tests/integration/buildcmd/test_build_cmd_container_image.py new file mode 100644 index 00000000000..8d49fa61693 --- /dev/null +++ b/tests/integration/buildcmd/test_build_cmd_container_image.py @@ -0,0 +1,65 @@ +"""Integration test for ECS/AgentCore container image builds""" + +import shutil +import tempfile +from pathlib import Path +from unittest import TestCase +from unittest import skipIf + +import yaml + +from samcli.commands.build.build_context import BuildContext +from tests.testing_utils import IS_WINDOWS + + +@skipIf(IS_WINDOWS, "Linux container image builds are not supported on Windows CI runners") +class TestContainerImageBuild(TestCase): + """Test that samdev build correctly handles ECS and AgentCore container resources.""" + + template_path = str( + Path(__file__).resolve().parents[1] / "testdata" / "buildcmd" / "container_image" / "template.yaml" + ) + + def setUp(self): + self.build_dir = tempfile.mkdtemp() + self.cache_dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.build_dir, ignore_errors=True) + shutil.rmtree(self.cache_dir, ignore_errors=True) + + def test_build_discovers_and_builds_container_resources(self): + """Verify that sam build discovers AgentCore and ECS resources and produces built artifacts.""" + with BuildContext( + resource_identifier=None, + template_file=self.template_path, + base_dir=None, + build_dir=self.build_dir, + cache_dir=self.cache_dir, + cached=False, + parallel=False, + mode=None, + ) as ctx: + ctx.run() + + # Verify the output template was created + output_template = Path(self.build_dir) / "template.yaml" + self.assertTrue(output_template.exists(), "Built template should exist") + + with open(output_template) as f: + built_template = yaml.safe_load(f) + + resources = built_template["Resources"] + + # AgentCore: ContainerUri should be replaced with the built image tag + agent_uri = resources["SimpleAgent"]["Properties"]["AgentRuntimeArtifact"]["ContainerConfiguration"][ + "ContainerUri" + ] + self.assertNotEqual(agent_uri, "placeholder") + self.assertIn("simpleagent", agent_uri.lower()) + + # ECS: The 'web' container (second one) should be updated, sidecar unchanged + container_defs = resources["ECSTask"]["Properties"]["ContainerDefinitions"] + self.assertEqual(container_defs[0]["Image"], "public.ecr.aws/envoy:latest") # sidecar untouched + self.assertNotEqual(container_defs[1]["Image"], "placeholder") # web updated + self.assertIn("ecstask", container_defs[1]["Image"].lower()) diff --git a/tests/integration/testdata/buildcmd/container_image/agent/Dockerfile b/tests/integration/testdata/buildcmd/container_image/agent/Dockerfile new file mode 100644 index 00000000000..57f45c64404 --- /dev/null +++ b/tests/integration/testdata/buildcmd/container_image/agent/Dockerfile @@ -0,0 +1,4 @@ +FROM python:3.12-slim +WORKDIR /app +RUN echo "hello" > /app/test.txt +CMD ["python", "-c", "print('test')"] diff --git a/tests/integration/testdata/buildcmd/container_image/template.yaml b/tests/integration/testdata/buildcmd/container_image/template.yaml new file mode 100644 index 00000000000..b261dc611fa --- /dev/null +++ b/tests/integration/testdata/buildcmd/container_image/template.yaml @@ -0,0 +1,33 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Integration test for ECS/AgentCore container builds + +Resources: + SimpleAgent: + Type: AWS::BedrockAgentCore::Runtime + Metadata: + Dockerfile: Dockerfile + DockerContext: ./agent + DockerTag: latest + Properties: + AgentRuntimeName: test_agent + AgentRuntimeArtifact: + ContainerConfiguration: + ContainerUri: placeholder + NetworkConfiguration: + NetworkMode: PUBLIC + RoleArn: arn:aws:iam::123456789012:role/TestRole + + ECSTask: + Type: AWS::ECS::TaskDefinition + Metadata: + Dockerfile: Dockerfile + DockerContext: ./agent + DockerTag: latest + ContainerName: web + Properties: + Family: test-task + ContainerDefinitions: + - Name: sidecar + Image: public.ecr.aws/envoy:latest + - Name: web + Image: placeholder 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 00000000000..2eb28cebd55 --- /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 00000000000..2558ea1fcc1 --- /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 3e641c548b6..a9e255dace8 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/build_module/test_container_build_definition.py b/tests/unit/lib/build_module/test_container_build_definition.py new file mode 100644 index 00000000000..1ab39fc7b95 --- /dev/null +++ b/tests/unit/lib/build_module/test_container_build_definition.py @@ -0,0 +1,103 @@ +"""Tests for ContainerBuildDefinition in build_graph.py""" + +from unittest import TestCase + +from samcli.lib.build.build_graph import ContainerBuildDefinition +from samcli.lib.utils.architecture import ARM64, X86_64 +from samcli.lib.utils.resources import AWS_BEDROCK_AGENTCORE_RUNTIME, AWS_ECS_TASK_DEFINITION + + +class TestContainerBuildDefinition(TestCase): + def test_basic_properties(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app", "DockerBuildArgs": {"ENV": "prod"}} + cbd = ContainerBuildDefinition( + resource_identifier="MyTask", + resource_type=AWS_ECS_TASK_DEFINITION, + metadata=metadata, + ) + self.assertEqual(cbd.resource_identifier, "MyTask") + self.assertEqual(cbd.resource_type, AWS_ECS_TASK_DEFINITION) + self.assertEqual(cbd.dockerfile, "Dockerfile") + self.assertEqual(cbd.docker_context, "./app") + self.assertEqual(cbd.docker_build_args, {"ENV": "prod"}) + self.assertIsNone(cbd.docker_build_target) + self.assertEqual(cbd.architecture, X86_64) + + def test_architecture_from_metadata(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./agent", "Architecture": "arm64"} + cbd = ContainerBuildDefinition( + resource_identifier="MyAgent", + resource_type=AWS_BEDROCK_AGENTCORE_RUNTIME, + metadata=metadata, + ) + self.assertEqual(cbd.architecture, ARM64) + + def test_architecture_default_when_not_in_metadata(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app"} + cbd = ContainerBuildDefinition( + resource_identifier="MyTask", + resource_type=AWS_ECS_TASK_DEFINITION, + metadata=metadata, + ) + self.assertEqual(cbd.architecture, X86_64) + + def test_docker_build_target(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app", "DockerBuildTarget": "release"} + cbd = ContainerBuildDefinition( + resource_identifier="MyTask", + resource_type=AWS_ECS_TASK_DEFINITION, + metadata=metadata, + ) + self.assertEqual(cbd.docker_build_target, "release") + + def test_equality_same(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app"} + cbd1 = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, metadata) + cbd2 = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, metadata) + self.assertEqual(cbd1, cbd2) + + def test_equality_different_resource_id(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app"} + cbd1 = ContainerBuildDefinition("MyTask1", AWS_ECS_TASK_DEFINITION, metadata) + cbd2 = ContainerBuildDefinition("MyTask2", AWS_ECS_TASK_DEFINITION, metadata) + self.assertNotEqual(cbd1, cbd2) + + def test_equality_different_type(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app"} + cbd1 = ContainerBuildDefinition("MyRes", AWS_ECS_TASK_DEFINITION, metadata) + cbd2 = ContainerBuildDefinition("MyRes", AWS_BEDROCK_AGENTCORE_RUNTIME, metadata) + self.assertNotEqual(cbd1, cbd2) + + def test_equality_different_metadata(self): + cbd1 = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, {"Dockerfile": "A", "DockerContext": "."}) + cbd2 = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, {"Dockerfile": "B", "DockerContext": "."}) + self.assertNotEqual(cbd1, cbd2) + + def test_equality_not_container_build_definition(self): + metadata = {"Dockerfile": "Dockerfile", "DockerContext": "./app"} + cbd = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, metadata) + self.assertNotEqual(cbd, "not a build def") + + def test_get_resource_full_paths(self): + cbd = ContainerBuildDefinition( + "Stack/MyTask", AWS_ECS_TASK_DEFINITION, {"Dockerfile": "D", "DockerContext": "."} + ) + self.assertEqual(cbd.get_resource_full_paths(), "Stack/MyTask") + + def test_str_representation(self): + cbd = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, {"Dockerfile": "D", "DockerContext": "."}) + result = str(cbd) + self.assertIn("ContainerBuildDefinition", result) + self.assertIn("MyTask", result) + self.assertIn(AWS_ECS_TASK_DEFINITION, result) + + def test_none_metadata(self): + cbd = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, None) + self.assertIsNone(cbd.dockerfile) + self.assertIsNone(cbd.docker_context) + self.assertEqual(cbd.docker_build_args, {}) + + def test_strips_sam_metadata_keys(self): + metadata = {"Dockerfile": "D", "DockerContext": ".", "SamResourceId": "should_be_removed"} + cbd = ContainerBuildDefinition("MyTask", AWS_ECS_TASK_DEFINITION, metadata) + self.assertNotIn("SamResourceId", cbd.metadata) diff --git a/tests/unit/lib/build_module/test_container_build_integration.py b/tests/unit/lib/build_module/test_container_build_integration.py new file mode 100644 index 00000000000..b75389338f0 --- /dev/null +++ b/tests/unit/lib/build_module/test_container_build_integration.py @@ -0,0 +1,261 @@ +"""Tests for ECS/AgentCore container build integration across modules""" + +from unittest import TestCase +from unittest.mock import MagicMock, patch, Mock +from copy import deepcopy + +from samcli.lib.build.app_builder import ApplicationBuilder +from samcli.lib.build.exceptions import DockerBuildFailed +from samcli.lib.build.build_graph import ContainerBuildDefinition +from samcli.lib.package.packageable_resources import ( + AgentCoreRuntimeImageResource, + ECSTaskDefinitionImageResource, +) +from samcli.lib.sync.sync_flow_factory import SyncFlowFactory +from samcli.lib.utils.packagetype import IMAGE, ZIP +from samcli.lib.utils.resources import ( + AWS_BEDROCK_AGENTCORE_RUNTIME, + AWS_ECS_TASK_DEFINITION, + RESOURCES_WITH_IMAGE_COMPONENT, +) + + +class TestResourceConstants(TestCase): + def test_ecs_task_definition_in_image_components(self): + self.assertIn(AWS_ECS_TASK_DEFINITION, RESOURCES_WITH_IMAGE_COMPONENT) + self.assertEqual(RESOURCES_WITH_IMAGE_COMPONENT[AWS_ECS_TASK_DEFINITION], ["ContainerDefinitions.Image"]) + + def test_agentcore_in_image_components(self): + self.assertIn(AWS_BEDROCK_AGENTCORE_RUNTIME, RESOURCES_WITH_IMAGE_COMPONENT) + self.assertEqual( + RESOURCES_WITH_IMAGE_COMPONENT[AWS_BEDROCK_AGENTCORE_RUNTIME], + ["AgentRuntimeArtifact.ContainerConfiguration.ContainerUri"], + ) + + def test_resource_type_values(self): + self.assertEqual(AWS_ECS_TASK_DEFINITION, "AWS::ECS::TaskDefinition") + self.assertEqual(AWS_BEDROCK_AGENTCORE_RUNTIME, "AWS::BedrockAgentCore::Runtime") + + +class TestECSTaskDefinitionImageResource(TestCase): + def test_resource_type(self): + self.assertEqual(ECSTaskDefinitionImageResource.RESOURCE_TYPE, AWS_ECS_TASK_DEFINITION) + + def test_property_name(self): + self.assertEqual(ECSTaskDefinitionImageResource.PROPERTY_NAME, "ContainerDefinitions.Image") + + def test_artifact_type_is_zip(self): + # ZIP so it passes the PackageType filter (ECS has no PackageType property) + self.assertEqual(ECSTaskDefinitionImageResource.ARTIFACT_TYPE, ZIP) + + +class TestAgentCoreRuntimeImageResource(TestCase): + def test_resource_type(self): + self.assertEqual(AgentCoreRuntimeImageResource.RESOURCE_TYPE, AWS_BEDROCK_AGENTCORE_RUNTIME) + + def test_property_name(self): + self.assertEqual( + AgentCoreRuntimeImageResource.PROPERTY_NAME, + "AgentRuntimeArtifact.ContainerConfiguration.ContainerUri", + ) + + def test_artifact_type_is_zip(self): + self.assertEqual(AgentCoreRuntimeImageResource.ARTIFACT_TYPE, ZIP) + + +class TestUpdateBuiltResource(TestCase): + def test_ecs_task_definition_updates_first_container(self): + properties = {"ContainerDefinitions": [{"Name": "web", "Image": "placeholder"}]} + ApplicationBuilder._update_built_resource("myimage:latest", properties, AWS_ECS_TASK_DEFINITION, "/path") + self.assertEqual(properties["ContainerDefinitions"][0]["Image"], "myimage:latest") + + def test_ecs_task_definition_empty_container_defs(self): + properties = {"ContainerDefinitions": []} + # Should not raise + ApplicationBuilder._update_built_resource("myimage:latest", properties, AWS_ECS_TASK_DEFINITION, "/path") + + def test_ecs_task_definition_targets_container_by_name(self): + properties = { + "ContainerDefinitions": [ + {"Name": "sidecar", "Image": "sidecar:latest"}, + {"Name": "web", "Image": "placeholder"}, + ] + } + metadata = {"ContainerName": "web"} + ApplicationBuilder._update_built_resource( + "myimage:latest", properties, AWS_ECS_TASK_DEFINITION, "/path", metadata + ) + self.assertEqual(properties["ContainerDefinitions"][0]["Image"], "sidecar:latest") # unchanged + self.assertEqual(properties["ContainerDefinitions"][1]["Image"], "myimage:latest") # updated + + def test_ecs_task_definition_raises_when_multi_container_without_container_name(self): + properties = { + "ContainerDefinitions": [ + {"Name": "web", "Image": "placeholder"}, + {"Name": "sidecar", "Image": "sidecar:latest"}, + ] + } + with self.assertRaises(DockerBuildFailed): + ApplicationBuilder._update_built_resource("myimage:latest", properties, AWS_ECS_TASK_DEFINITION, "/path") + + def test_agentcore_updates_nested_container_uri(self): + properties = {"AgentRuntimeArtifact": {"ContainerConfiguration": {"ContainerUri": "placeholder"}}} + ApplicationBuilder._update_built_resource("myimage:latest", properties, AWS_BEDROCK_AGENTCORE_RUNTIME, "/path") + self.assertEqual(properties["AgentRuntimeArtifact"]["ContainerConfiguration"]["ContainerUri"], "myimage:latest") + + def test_agentcore_creates_nested_structure_if_missing(self): + properties = {} + ApplicationBuilder._update_built_resource("myimage:latest", properties, AWS_BEDROCK_AGENTCORE_RUNTIME, "/path") + self.assertEqual(properties["AgentRuntimeArtifact"]["ContainerConfiguration"]["ContainerUri"], "myimage:latest") + + +class TestBuildContainerImages(TestCase): + @patch.object(ApplicationBuilder, "_build_lambda_image", return_value="myimage:latest") + def test_builds_all_definitions(self, mock_build): + builder = ApplicationBuilder.__new__(ApplicationBuilder) + builder._base_dir = "/base" + builder._stream_writer = MagicMock() + builder._use_buildkit = False + builder._image_build_client = None + + defs = [ + ContainerBuildDefinition("Task1", AWS_ECS_TASK_DEFINITION, {"Dockerfile": "D", "DockerContext": "."}), + ContainerBuildDefinition( + "Agent1", AWS_BEDROCK_AGENTCORE_RUNTIME, {"Dockerfile": "D", "DockerContext": "."} + ), + ] + result = builder.build_container_images(defs) + self.assertEqual(len(result), 2) + self.assertIn("Task1", result) + self.assertIn("Agent1", result) + self.assertEqual(mock_build.call_count, 2) + + @patch.object(ApplicationBuilder, "_build_lambda_image", return_value="img:tag") + def test_skips_definition_without_metadata(self, mock_build): + builder = ApplicationBuilder.__new__(ApplicationBuilder) + builder._base_dir = "/base" + builder._stream_writer = MagicMock() + builder._use_buildkit = False + builder._image_build_client = None + + defs = [ContainerBuildDefinition("Task1", AWS_ECS_TASK_DEFINITION, None)] + result = builder.build_container_images(defs) + self.assertEqual(len(result), 0) + mock_build.assert_not_called() + + +class TestSyncFlowFactoryMapping(TestCase): + def test_ecs_task_definition_registered(self): + self.assertIn(AWS_ECS_TASK_DEFINITION, SyncFlowFactory.GENERATOR_MAPPING) + + def test_agentcore_registered(self): + self.assertIn(AWS_BEDROCK_AGENTCORE_RUNTIME, SyncFlowFactory.GENERATOR_MAPPING) + + def test_ecs_and_agentcore_use_different_flow_creators(self): + # ECS uses _create_ecs_container_flow; AgentCore uses _create_agentcore_flow + # (AgentCore sync is not yet implemented — it logs a warning and returns None) + self.assertNotEqual( + SyncFlowFactory.GENERATOR_MAPPING[AWS_ECS_TASK_DEFINITION], + SyncFlowFactory.GENERATOR_MAPPING[AWS_BEDROCK_AGENTCORE_RUNTIME], + ) + + +class TestSyncEcrStackIncludesContainerResources(TestCase): + @patch("samcli.lib.providers.sam_container_provider.SamContainerServiceProvider") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamFunctionProvider") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamLocalStackProvider") + @patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.CompanionStackManager") + def test_includes_container_services( + self, mock_manager_cls, mock_stack_provider, mock_func_provider, mock_container_provider + ): + from samcli.lib.bootstrap.companion_stack.companion_stack_manager import sync_ecr_stack + + # Setup mocks + mock_stack_provider.get_stacks.return_value = ([MagicMock()], None) + + mock_func = MagicMock() + mock_func.packagetype = IMAGE + mock_func.full_path = "MyFunction" + mock_func_provider.return_value.get_all.return_value = [mock_func] + + mock_service = MagicMock() + mock_service.full_path = "MyAgent" + mock_container_provider.return_value.get_all.return_value = [mock_service] + + mock_manager = MagicMock() + mock_manager.get_repository_mapping.return_value = {"MyFunction": "uri1", "MyAgent": "uri2"} + mock_manager_cls.return_value = mock_manager + + result = sync_ecr_stack("template.yaml", "stack", "us-east-1", "bucket", "prefix", {}) + + # Verify both function and container service were passed + call_args = mock_manager.set_functions.call_args[0] + logical_ids = call_args[0] + self.assertIn("MyFunction", logical_ids) + self.assertIn("MyAgent", logical_ids) + + +class TestECSContainerSyncFlow(TestCase): + def test_sync_state_identifier(self): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + flow._resource_identifier = "MyTask" + self.assertEqual(flow.sync_state_identifier, "MyTask") + + def test_equality_keys(self): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + flow._resource_identifier = "MyTask" + self.assertEqual(flow._equality_keys(), "MyTask") + + def test_gather_dependencies_empty(self): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + self.assertEqual(flow.gather_dependencies(), []) + + def test_compare_remote_always_false(self): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + self.assertFalse(flow.compare_remote()) + + @patch("samcli.lib.sync.flows.ecs_container_sync_flow.ECRUploader") + @patch("samcli.lib.sync.flows.ecs_container_sync_flow.get_validated_container_client") + def test_sync_pushes_image(self, mock_docker, mock_ecr_uploader_cls): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + flow._resource_identifier = "MyAgent" + flow._log_name = "ECSContainer MyAgent" + flow._image_name = "myimage:latest" + flow._docker_client = None + flow._ecr_client = None + flow._ecs_client = None + flow._physical_id_mapping = {} + flow._deploy_context = MagicMock() + flow._deploy_context.image_repository = "123.dkr.ecr.us-east-1.amazonaws.com/repo" + flow._deploy_context.image_repositories = None + + mock_docker.return_value = MagicMock() + + flow._get_session = MagicMock() + flow._boto_client = MagicMock() + + flow.sync() + + mock_ecr_uploader_cls.assert_called_once() + mock_ecr_uploader_cls.return_value.upload.assert_called_once_with("myimage:latest", "MyAgent") + + def test_sync_skips_when_no_image(self): + from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow + + flow = ECSContainerSyncFlow.__new__(ECSContainerSyncFlow) + flow._resource_identifier = "MyAgent" + flow._log_name = "ECSContainer MyAgent" + flow._image_name = None + flow._physical_id_mapping = {} + # Should not raise + flow.sync() 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 7eccaba88c1..fccaf616714 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 59f7be0b1d3..430d775855f 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 00000000000..15baadfa922 --- /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 00000000000..2eb28cebd55 --- /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 00000000000..41d293bfd16 --- /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 00000000000..1a8072857f9 --- /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 00000000000..ba8410e2845 --- /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 00000000000..ccdf30b80eb --- /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 00000000000..19f3d153707 --- /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 00000000000..a22a2da24d1 --- /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 00000000000..85d89a44082 --- /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 00000000000..5ff82cf6f4a --- /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 00000000000..5ff82cf6f4a --- /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 00000000000..9f11beba7ed --- /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 00000000000..7f01fc31518 --- /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 diff --git a/tests/unit/lib/providers/test_sam_container_provider.py b/tests/unit/lib/providers/test_sam_container_provider.py new file mode 100644 index 00000000000..a9f64a2857d --- /dev/null +++ b/tests/unit/lib/providers/test_sam_container_provider.py @@ -0,0 +1,141 @@ +"""Tests for SamContainerServiceProvider""" + +from unittest import TestCase +from unittest.mock import MagicMock + +from samcli.lib.providers.sam_container_provider import SamContainerServiceProvider, CONTAINER_IMAGE_RESOURCE_TYPES +from samcli.lib.utils.resources import AWS_BEDROCK_AGENTCORE_RUNTIME, AWS_ECS_TASK_DEFINITION + + +def _make_stack(resources, stack_path=""): + stack = MagicMock() + stack.resources = resources + stack.stack_path = stack_path + return stack + + +class TestSamContainerServiceProvider(TestCase): + def test_discovers_ecs_task_definition(self): + resources = { + "MyTask": { + "Type": AWS_ECS_TASK_DEFINITION, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./app"}, + "Properties": {"ContainerDefinitions": [{"Name": "web", "Image": "placeholder"}]}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 1) + self.assertEqual(services[0].resource_id, "MyTask") + self.assertEqual(services[0].resource_type, AWS_ECS_TASK_DEFINITION) + + def test_discovers_agentcore_runtime(self): + resources = { + "MyAgent": { + "Type": AWS_BEDROCK_AGENTCORE_RUNTIME, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./agent"}, + "Properties": {"AgentRuntimeArtifact": {"ContainerConfiguration": {"ContainerUri": "placeholder"}}}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 1) + self.assertEqual(services[0].resource_id, "MyAgent") + self.assertEqual(services[0].resource_type, AWS_BEDROCK_AGENTCORE_RUNTIME) + + def test_skips_resource_without_metadata(self): + resources = { + "MyTask": { + "Type": AWS_ECS_TASK_DEFINITION, + "Properties": {"ContainerDefinitions": [{"Name": "web", "Image": "some-image"}]}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 0) + + def test_skips_resource_without_dockerfile(self): + resources = { + "MyTask": { + "Type": AWS_ECS_TASK_DEFINITION, + "Metadata": {"DockerContext": "./app"}, # Missing Dockerfile + "Properties": {}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 0) + + def test_skips_resource_without_docker_context(self): + resources = { + "MyTask": { + "Type": AWS_ECS_TASK_DEFINITION, + "Metadata": {"Dockerfile": "Dockerfile"}, # Missing DockerContext + "Properties": {}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 0) + + def test_skips_unsupported_resource_type(self): + resources = { + "MyFunction": { + "Type": "AWS::Lambda::Function", + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./app"}, + "Properties": {}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 0) + + def test_get_by_full_path(self): + resources = { + "MyAgent": { + "Type": AWS_BEDROCK_AGENTCORE_RUNTIME, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./agent"}, + "Properties": {}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + result = provider.get("MyAgent") + self.assertIsNotNone(result) + self.assertEqual(result.resource_id, "MyAgent") + + def test_get_returns_none_for_unknown(self): + provider = SamContainerServiceProvider([_make_stack({})]) + self.assertIsNone(provider.get("NonExistent")) + + def test_nested_stack_full_path(self): + resources = { + "MyTask": { + "Type": AWS_ECS_TASK_DEFINITION, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./app"}, + "Properties": {}, + } + } + provider = SamContainerServiceProvider([_make_stack(resources, stack_path="ChildStack")]) + services = list(provider.get_all()) + self.assertEqual(services[0].full_path, "ChildStack/MyTask") + + def test_multiple_resources(self): + resources = { + "Task1": { + "Type": AWS_ECS_TASK_DEFINITION, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./app1"}, + "Properties": {}, + }, + "Agent1": { + "Type": AWS_BEDROCK_AGENTCORE_RUNTIME, + "Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "./agent"}, + "Properties": {}, + }, + } + provider = SamContainerServiceProvider([_make_stack(resources)]) + services = list(provider.get_all()) + self.assertEqual(len(services), 2) + + def test_container_image_resource_types_list(self): + self.assertIn(AWS_ECS_TASK_DEFINITION, CONTAINER_IMAGE_RESOURCE_TYPES) + self.assertIn(AWS_BEDROCK_AGENTCORE_RUNTIME, CONTAINER_IMAGE_RESOURCE_TYPES) diff --git a/tests/unit/lib/sync/flows/test_ecs_container_sync_flow.py b/tests/unit/lib/sync/flows/test_ecs_container_sync_flow.py new file mode 100644 index 00000000000..f467a7dccc9 --- /dev/null +++ b/tests/unit/lib/sync/flows/test_ecs_container_sync_flow.py @@ -0,0 +1,303 @@ +from unittest import TestCase +from unittest.mock import MagicMock, patch, call + +from botocore.exceptions import ClientError + +from samcli.lib.sync.flows.ecs_container_sync_flow import ECSContainerSyncFlow +from samcli.lib.sync.sync_flow import ApiCallTypes + + +def _make_client_error(code="SomeError"): + err = ClientError({"Error": {"Code": code, "Message": "msg"}}, "op") + return err + + +def _make_flow(image_repository="123456789012.dkr.ecr.us-east-1.amazonaws.com/myrepo", stacks=None): + build_context = MagicMock() + deploy_context = MagicMock() + deploy_context.image_repository = image_repository + deploy_context.image_repositories = None + flow = ECSContainerSyncFlow( + "MyTask", + build_context=build_context, + deploy_context=deploy_context, + sync_context=MagicMock(), + physical_id_mapping={"MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5"}, + stacks=stacks or [], + application_build_result=None, + ) + flow._image_name = "myapp:latest" + return flow + + +class TestECSContainerSyncFlowApiCallTypes(TestCase): + def test_get_resource_api_calls_uses_update_container_image(self): + flow = _make_flow() + calls = flow._get_resource_api_calls() + self.assertEqual(1, len(calls)) + self.assertIn(ApiCallTypes.UPDATE_CONTAINER_IMAGE, calls[0].api_calls) + + def test_equality_keys(self): + flow = _make_flow() + self.assertEqual("MyTask", flow._equality_keys()) + + def test_sync_state_identifier(self): + flow = _make_flow() + self.assertEqual("MyTask", flow.sync_state_identifier) + + +class TestECSContainerSyncFlowSync(TestCase): + def _make_ecs_client(self, container_name="app", image_uri="old-image:1"): + ecs_client = MagicMock() + task_def = { + "taskDefinitionArn": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + "family": "mytask", + "revision": 5, + "status": "ACTIVE", + "containerDefinitions": [{"name": container_name, "image": image_uri}], + "requiresAttributes": [], + "compatibilities": ["EC2"], + } + ecs_client.describe_task_definition.return_value = {"taskDefinition": task_def, "tags": []} + ecs_client.register_task_definition.return_value = { + "taskDefinition": {"taskDefinitionArn": "arn:aws:ecs:us-east-1:123:task-definition/mytask:6"} + } + ecs_client.describe_services.return_value = { + "services": [{"taskDefinition": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5"}] + } + return ecs_client + + def test_sync_registers_new_task_definition_and_updates_service(self): + flow = _make_flow() + # Include a service ARN so _update_services_to_task_definition can match it + flow._physical_id_mapping = { + "MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + "MyService": "arn:aws:ecs:us-east-1:123:service/mycluster/mysvc", + } + ecr_client = MagicMock() + docker_client = MagicMock() + ecs_client = self._make_ecs_client() + + flow._ecr_client = ecr_client + flow._docker_client = docker_client + flow._ecs_client = ecs_client + + with ( + patch("samcli.lib.sync.flows.ecs_container_sync_flow.ECRUploader") as MockUploader, + patch.object(flow, "_get_container_name", return_value=None), + ): + mock_uploader_instance = MagicMock() + mock_uploader_instance.upload.return_value = "123456789012.dkr.ecr.us-east-1.amazonaws.com/myrepo:newtag" + MockUploader.return_value = mock_uploader_instance + + flow.sync() + + ecs_client.describe_task_definition.assert_called_once_with( + taskDefinition="arn:aws:ecs:us-east-1:123:task-definition/mytask:5", include=["TAGS"] + ) + # register_task_definition should be called with updated image + reg_call_kwargs = ecs_client.register_task_definition.call_args[1] + self.assertEqual( + "123456789012.dkr.ecr.us-east-1.amazonaws.com/myrepo:newtag", + reg_call_kwargs["containerDefinitions"][0]["image"], + ) + # update_service should target the new revision + ecs_client.update_service.assert_called_once() + update_kwargs = ecs_client.update_service.call_args[1] + self.assertEqual("arn:aws:ecs:us-east-1:123:task-definition/mytask:6", update_kwargs["taskDefinition"]) + + def test_sync_skips_when_no_image_name(self): + flow = _make_flow() + flow._image_name = None + ecs_client = MagicMock() + flow._ecs_client = ecs_client + + flow.sync() + + ecs_client.describe_task_definition.assert_not_called() + + def test_sync_skips_when_no_ecr_repo(self): + flow = _make_flow(image_repository=None) + flow._image_name = "myapp:latest" + ecs_client = MagicMock() + flow._ecs_client = ecs_client + + flow.sync() + + ecs_client.describe_task_definition.assert_not_called() + + def test_register_returns_none_when_describe_fails(self): + flow = _make_flow() + ecs_client = MagicMock() + ecs_client.describe_task_definition.side_effect = _make_client_error("AccessDenied") + flow._ecs_client = ecs_client + + result = flow._register_updated_task_definition("new-image:uri") + self.assertIsNone(result) + + def test_register_returns_none_when_no_physical_id(self): + flow = _make_flow() + flow._physical_id_mapping = {} + result = flow._register_updated_task_definition("new-image:uri") + self.assertIsNone(result) + + def test_update_services_skips_non_ecs_arns(self): + flow = _make_flow() + flow._physical_id_mapping = { + "MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + # App Runner ARN — must NOT trigger describe_services + "AppRunnerSvc": "arn:aws:apprunner:us-east-1:123:service/mysvc/abc123", + } + ecs_client = MagicMock() + flow._ecs_client = ecs_client + + flow._update_services_to_task_definition("arn:aws:ecs:us-east-1:123:task-definition/mytask:6") + + ecs_client.describe_services.assert_not_called() + + def test_update_services_updates_matching_service(self): + flow = _make_flow() + flow._physical_id_mapping = { + "MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + "MyService": "arn:aws:ecs:us-east-1:123:service/mycluster/mysvc", + } + ecs_client = MagicMock() + ecs_client.describe_services.return_value = { + "services": [{"taskDefinition": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5"}] + } + flow._ecs_client = ecs_client + + new_arn = "arn:aws:ecs:us-east-1:123:task-definition/mytask:6" + flow._update_services_to_task_definition(new_arn) + + ecs_client.update_service.assert_called_once_with(cluster="mycluster", service="mysvc", taskDefinition=new_arn) + + def test_update_services_skips_mismatched_family(self): + flow = _make_flow() + flow._physical_id_mapping = { + "MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + "OtherService": "arn:aws:ecs:us-east-1:123:service/mycluster/othersvc", + } + ecs_client = MagicMock() + ecs_client.describe_services.return_value = { + "services": [{"taskDefinition": "arn:aws:ecs:us-east-1:123:task-definition/othertask:3"}] + } + flow._ecs_client = ecs_client + + flow._update_services_to_task_definition("arn:aws:ecs:us-east-1:123:task-definition/mytask:6") + + ecs_client.update_service.assert_not_called() + + def test_update_services_handles_describe_client_error(self): + flow = _make_flow() + flow._physical_id_mapping = { + "MyTask": "arn:aws:ecs:us-east-1:123:task-definition/mytask:5", + "MyService": "arn:aws:ecs:us-east-1:123:service/mycluster/mysvc", + } + ecs_client = MagicMock() + ecs_client.describe_services.side_effect = _make_client_error("AccessDenied") + flow._ecs_client = ecs_client + + # Should not raise + flow._update_services_to_task_definition("arn:aws:ecs:us-east-1:123:task-definition/mytask:6") + ecs_client.update_service.assert_not_called() + + def test_register_strips_readonly_fields(self): + flow = _make_flow() + ecs_client = self._make_ecs_client() + flow._ecs_client = ecs_client + + with patch.object(flow, "_get_container_name", return_value=None): + flow._register_updated_task_definition("new-image:uri") + + reg_kwargs = ecs_client.register_task_definition.call_args[1] + for field in ("taskDefinitionArn", "revision", "status", "requiresAttributes", "compatibilities"): + self.assertNotIn(field, reg_kwargs, f"readonly field '{field}' should be stripped") + + def test_register_tags_included_when_present(self): + flow = _make_flow() + ecs_client = self._make_ecs_client() + ecs_client.describe_task_definition.return_value = { + "taskDefinition": { + "family": "mytask", + "containerDefinitions": [{"name": "app", "image": "old"}], + }, + "tags": [{"key": "env", "value": "prod"}], + } + flow._ecs_client = ecs_client + + with patch.object(flow, "_get_container_name", return_value=None): + flow._register_updated_task_definition("new-image:uri") + + reg_kwargs = ecs_client.register_task_definition.call_args[1] + self.assertEqual([{"key": "env", "value": "prod"}], reg_kwargs.get("tags")) + + def test_register_updates_named_container(self): + flow = _make_flow() + ecs_client = MagicMock() + ecs_client.describe_task_definition.return_value = { + "taskDefinition": { + "family": "mytask", + "containerDefinitions": [ + {"name": "sidecar", "image": "sidecar:old"}, + {"name": "app", "image": "app:old"}, + ], + }, + "tags": [], + } + ecs_client.register_task_definition.return_value = { + "taskDefinition": {"taskDefinitionArn": "arn:aws:ecs:us-east-1:123:task-definition/mytask:6"} + } + flow._ecs_client = ecs_client + + with patch.object(flow, "_get_container_name", return_value="app"): + flow._register_updated_task_definition("new-image:uri") + + reg_kwargs = ecs_client.register_task_definition.call_args[1] + containers = {cd["name"]: cd["image"] for cd in reg_kwargs["containerDefinitions"]} + self.assertEqual("new-image:uri", containers["app"]) + self.assertEqual("sidecar:old", containers["sidecar"]) + + def test_register_returns_none_when_named_container_missing(self): + flow = _make_flow() + ecs_client = MagicMock() + ecs_client.describe_task_definition.return_value = { + "taskDefinition": { + "family": "mytask", + "containerDefinitions": [{"name": "other", "image": "img"}], + }, + "tags": [], + } + flow._ecs_client = ecs_client + + with patch.object(flow, "_get_container_name", return_value="app"): + result = flow._register_updated_task_definition("new-image:uri") + + self.assertIsNone(result) + ecs_client.register_task_definition.assert_not_called() + + +class TestECSContainerSyncFlowGatherResources(TestCase): + def test_gather_resources_uses_prebuilt_artifacts(self): + build_result = MagicMock() + build_result.artifacts = {"MyTask": "prebuilt-image:tag"} + + docker_client = MagicMock() + img = MagicMock() + img.attrs = {"Id": "sha256:abc"} + docker_client.images.get.return_value = img + + flow = ECSContainerSyncFlow( + "MyTask", + build_context=MagicMock(), + deploy_context=MagicMock(), + sync_context=MagicMock(), + physical_id_mapping={}, + stacks=[], + application_build_result=build_result, + ) + flow._docker_client = docker_client + flow.gather_resources() + + self.assertEqual("prebuilt-image:tag", flow._image_name) + self.assertEqual("sha256:abc", flow._local_sha) diff --git a/tests/unit/lib/sync/test_sync_flow.py b/tests/unit/lib/sync/test_sync_flow.py index f4a4a6f85bc..2ea2ae207af 100644 --- a/tests/unit/lib/sync/test_sync_flow.py +++ b/tests/unit/lib/sync/test_sync_flow.py @@ -299,7 +299,7 @@ def test_compare_local(self, patched_session, patched_sync_state_identifier): def test_api_call_types(self): """Test cases for ApiCallTypes enum values""" # Test enum members are unique - self.assertEqual(len(set(ApiCallTypes)), 4) + self.assertEqual(len(set(ApiCallTypes)), 5) # Test enum members exist self.assertTrue(hasattr(ApiCallTypes, "BUILD")) @@ -310,3 +310,5 @@ def test_api_call_types(self): self.assertEqual(ApiCallTypes.UPDATE_FUNCTION_CODE.value, "UpdateFunctionCode") self.assertTrue(hasattr(ApiCallTypes, "PUBLISH_VERSION")) self.assertEqual(ApiCallTypes.PUBLISH_VERSION.value, "PublishVersion") + self.assertTrue(hasattr(ApiCallTypes, "UPDATE_CONTAINER_IMAGE")) + self.assertEqual(ApiCallTypes.UPDATE_CONTAINER_IMAGE.value, "UpdateContainerImage") diff --git a/tests/unit/lib/sync/test_sync_flow_factory.py b/tests/unit/lib/sync/test_sync_flow_factory.py index 92d30c8a09a..98e930be95e 100644 --- a/tests/unit/lib/sync/test_sync_flow_factory.py +++ b/tests/unit/lib/sync/test_sync_flow_factory.py @@ -19,6 +19,7 @@ AWS_APIGATEWAY_V2_API, AWS_SERVERLESS_STATEMACHINE, AWS_STEPFUNCTIONS_STATEMACHINE, + AWS_ECS_TASK_DEFINITION, ) @@ -275,5 +276,6 @@ def test_values(self): AWS_APIGATEWAY_V2_API, AWS_SERVERLESS_STATEMACHINE, AWS_STEPFUNCTIONS_STATEMACHINE, + AWS_ECS_TASK_DEFINITION, ] self.assertEqual(expected, output) diff --git a/tests/unit/lib/utils/test_osutils.py b/tests/unit/lib/utils/test_osutils.py index 3d7fe3c47a3..7fdcb2d71ab 100644 --- a/tests/unit/lib/utils/test_osutils.py +++ b/tests/unit/lib/utils/test_osutils.py @@ -3,7 +3,9 @@ """ import os +import shutil import sys +import tempfile from unittest import TestCase from unittest.mock import patch, Mock @@ -91,66 +93,74 @@ def test_must_delete_if_path_exist(self, patched_rmtree, patched_path): class Test_copytree(TestCase): - @patch("samcli.lib.utils.osutils.Path") - @patch("samcli.lib.utils.osutils.os") - @patch("samcli.lib.utils.osutils.shutil.copy2") - def test_must_copytree(self, patched_copy2, patched_os, patched_path): + @patch("samcli.lib.utils.osutils.shutil.copytree") + def test_must_copytree(self, patched_copytree): source_path = "mock-source/path" destination_path = "mock-destination/path" - mock_path_obj = Mock() - patched_path.exists.return_value = True - patched_os.path.return_value = mock_path_obj - patched_os.path.join.side_effect = [source_path, destination_path] - patched_os.path.isdir.return_value = False - patched_os.listdir.return_value = ["mock-source-file1"] osutils.copytree(source_path, destination_path) - patched_os.path.join.assert_called() - patched_copy2.assert_called_with(source_path, destination_path, follow_symlinks=False) + patched_copytree.assert_called_once_with( + source_path, destination_path, symlinks=True, ignore=None, dirs_exist_ok=True + ) - @patch("samcli.lib.utils.osutils.Path") - @patch("samcli.lib.utils.osutils.os") - @patch("samcli.lib.utils.osutils.shutil.copy2") - def test_copytree_throws_oserror_path_exists(self, patched_copy2, patched_os, patched_path): + @patch("samcli.lib.utils.osutils.shutil.copytree") + def test_copytree_with_ignore(self, patched_copytree): source_path = "mock-source/path" destination_path = "mock-destination/path" - mock_path_obj = Mock() - patched_path.exists.return_value = True - patched_os.path.return_value = mock_path_obj - patched_copy2.side_effect = OSError("mock-os-error") + mock_ignore = Mock() + + osutils.copytree(source_path, destination_path, ignore=mock_ignore) + + patched_copytree.assert_called_once_with( + source_path, destination_path, symlinks=True, ignore=mock_ignore, dirs_exist_ok=True + ) + + @patch("samcli.lib.utils.osutils.shutil.copytree") + def test_copytree_throws_oserror(self, patched_copytree): + patched_copytree.side_effect = OSError("mock-os-error") - patched_os.path.join.side_effect = [source_path, destination_path] - patched_os.path.isdir.return_value = False - patched_os.listdir.return_value = ["mock-source-file1"] with self.assertRaises(OSError): - osutils.copytree(source_path, destination_path) + osutils.copytree("mock-source/path", "mock-destination/path") - patched_os.path.join.assert_called() - patched_copy2.assert_called_with(source_path, destination_path, follow_symlinks=False) - @patch("samcli.lib.utils.osutils.create_symlink_or_copy") - @patch("samcli.lib.utils.osutils.Path") - @patch("samcli.lib.utils.osutils.os") - @patch("samcli.lib.utils.osutils.shutil.copy2") - def test_copytree_symlink_copy_error_handling( - self, patched_copy2, patched_os, patched_path, patched_create_symlink_or_copy - ): - source_path = "mock-source/path" - destination_path = "mock-destination/path" - mock_path_obj = Mock() - patched_path.exists.return_value = True - patched_os.path.return_value = mock_path_obj - patched_copy2.side_effect = OSError(22, "mock-os-error") +class Test_copytree_symlinks(TestCase): + """Integration-style tests verifying symlinks=True preserves symlinks on disk.""" - patched_os.path.join.side_effect = [source_path, destination_path] - patched_os.path.isdir.return_value = False - patched_os.listdir.return_value = ["mock-source-file1"] - osutils.copytree(source_path, destination_path) + def setUp(self): + self.src = tempfile.mkdtemp() + self.dst = tempfile.mkdtemp() + # clean dst so copytree creates it + shutil.rmtree(self.dst) + + def tearDown(self): + shutil.rmtree(self.src, ignore_errors=True) + shutil.rmtree(self.dst, ignore_errors=True) + + def test_preserves_file_symlink(self): + real_file = os.path.join(self.src, "real.txt") + with open(real_file, "w") as f: + f.write("content") + os.symlink("real.txt", os.path.join(self.src, "link.txt")) + + osutils.copytree(self.src, self.dst) + + link = os.path.join(self.dst, "link.txt") + self.assertTrue(os.path.islink(link)) + self.assertEqual(os.readlink(link), "real.txt") + + def test_preserves_directory_symlink(self): + subdir = os.path.join(self.src, "subdir") + os.makedirs(subdir) + with open(os.path.join(subdir, "file.txt"), "w") as f: + f.write("content") + os.symlink("subdir", os.path.join(self.src, "link_dir")) + + osutils.copytree(self.src, self.dst) - patched_os.path.join.assert_called() - patched_copy2.assert_called_with(source_path, destination_path, follow_symlinks=False) - patched_create_symlink_or_copy.assert_called_with(source_path, destination_path) + link = os.path.join(self.dst, "link_dir") + self.assertTrue(os.path.islink(link)) + self.assertEqual(os.readlink(link), "subdir") class Test_create_symlink_or_copy(TestCase):