Skip to content

Support target platform for container builds#140

Merged
danegsta merged 2 commits intomicrosoft:mainfrom
McDonaldSean:users/sean/build-context-platform
May 6, 2026
Merged

Support target platform for container builds#140
danegsta merged 2 commits intomicrosoft:mainfrom
McDonaldSean:users/sean/build-context-platform

Conversation

@McDonaldSean
Copy link
Copy Markdown
Contributor

Summary

Adds an optional Platform field to ContainerBuildContext that DCP forwards to docker/podman build via --platform. This lets Aspire (and other DCP clients) request a specific build target architecture for Dockerfile builds during local dev, instead of always defaulting to the host architecture.

Motivation

Aspire's AddDockerfile resource attaches a ContainerBuildOptionsCallbackAnnotation whose TargetPlatform is honored by the publishing path (DockerContainerRuntime.RunDockerBuildAsync) but not the local-dev DCP path, because DCP's build spec had no field to carry the platform value. On hosts whose architecture differs from the resource's target platform (e.g. arm64 hosts with a linux/amd64 target), the build defaulted to the host arch and could produce images for the wrong platform.

Context: microsoft/aspire#16699, microsoft/aspire#16700.

Changes

  • api/v1/container_types.go: add Platform field to ContainerBuildContext; update the Equal method to compare it.
  • api/v1/container_types.go: include Build.Platform in GetLifecycleKey so persistent containers rebuild when their platform changes.
  • internal/docker/cli_orchestrator.go and internal/podman/cli_orchestrator.go: append --platform <value> when set.
  • pkg/generated/openapi/zz_generated.openapi.go: regenerated via make generate.
  • api/v1/container_types_test.go: new TestContainerBuildContextEqual covering platform-equality semantics, and TestGetLifecycleKeyIncludesBuildPlatform covering the lifecycle-key hash.

Compatibility

The change is additive and opt-in. Existing workloads have Platform="" which is omitted from JSON (omitempty) and skipped by the orchestrators (if options.Platform != ""), so today's behavior is preserved bit-for-bit, including the FNV lifecycle-key value (writing a zero-length slice is a no-op).

Test plan

  • make generate clean (only pre-existing API rule warnings)
  • go test -count 1 -parallel 32 ./... — all packages pass, including integration tests
  • make lint — 0 issues
  • Verified end-to-end with the corresponding Aspire change (Pass target platform to DCP for Dockerfile builds aspire#16700) once that side is updated to populate the new field

🤖 Generated with Claude Code

Add an optional Platform field to ContainerBuildContext that DCP forwards
to docker/podman build via --platform. This lets Aspire (and other DCP
clients) request a specific build target architecture for Dockerfile
builds during local dev, instead of always defaulting to the host
architecture.

The change is additive and opt-in: existing workloads have Platform=""
which is omitted from the build command, preserving today's behavior
exactly. The Equal method and GetLifecycleKey hash both include the new
field so spec changes propagate correctly: changing Platform on a
persistent container will trigger a rebuild.

Context: microsoft/aspire#16699 / microsoft/aspire#16700.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for specifying a target build platform in DCP’s container build spec and forwards it through the Docker/Podman CLI orchestrators, ensuring lifecycle keys change when platform changes so persistent containers rebuild appropriately.

Changes:

  • Add optional platform to ContainerBuildContext, include it in Equal, and incorporate it into ContainerSpec.GetLifecycleKey().
  • Forward --platform <value> to docker build and podman build when set.
  • Regenerate OpenAPI and add tests for build-context equality and lifecycle-key platform sensitivity.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
api/v1/container_types.go Adds Platform, compares it in Equal, and hashes it into lifecycle key.
api/v1/container_types_test.go Adds unit tests for Platform equality and lifecycle-key impact.
internal/docker/cli_orchestrator.go Appends --platform to Docker build args when provided.
internal/podman/cli_orchestrator.go Appends --platform to Podman build args when provided.
pkg/generated/openapi/zz_generated.openapi.go Updates generated OpenAPI schema to include platform.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread api/v1/container_types.go Outdated
Address PR review: writing Stage and Platform as adjacent raw byte slices
made the boundary ambiguous (e.g., Stage="ab",Platform="c" hashed the
same as Stage="a",Platform="bc"). Encode Platform via gob (length-framed)
and only when non-empty, both to disambiguate the hash and to preserve
the legacy lifecycle-key value for existing workloads where Platform is
unset.

Add a test asserting the boundary case produces distinct keys.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@McDonaldSean
Copy link
Copy Markdown
Contributor Author

While addressing the Stage/Platform ambiguity flagged in review, I noticed the same kind of adjacency exists today between Context / Dockerfile (when the Dockerfile isn't readable and the path is written instead, around line 852) / Stage. They're written as raw byte slices back-to-back, so e.g. (Context="ab", Stage="c") and (Context="a", Stage="bc") produce the same hash.

Pre-existing, not caused by this PR — and worth flagging separately because fixing it would change lifecycle keys for all existing workloads (forcing a rebuild on upgrade). Happy to file a follow-up issue if you'd like, or leave it for the team to schedule.

@karolz-ms
Copy link
Copy Markdown
Collaborator

While addressing the Stage/Platform ambiguity flagged in review, I noticed the same kind of adjacency exists today between Context / Dockerfile (when the Dockerfile isn't readable and the path is written instead, around line 852) / Stage. They're written as raw byte slices back-to-back, so e.g. (Context="ab", Stage="c") and (Context="a", Stage="bc") produce the same hash.

Pre-existing, not caused by this PR — and worth flagging separately because fixing it would change lifecycle keys for all existing workloads (forcing a rebuild on upgrade). Happy to file a follow-up issue if you'd like, or leave it for the team to schedule.

@McDonaldSean thank you for this contribution and yes, a follow-up PR for the Docker context ambiguity wrt/lifecycle key would be fantastic.

@McDonaldSean
Copy link
Copy Markdown
Contributor Author

I don't think I have permissions to complete the merge, but will raise another PR with the fixes mentioned above.

@danegsta danegsta merged commit c7f49a1 into microsoft:main May 6, 2026
6 checks passed
@danegsta
Copy link
Copy Markdown
Member

danegsta commented May 6, 2026

I've merged it; we likely won't get a new DCP build inserted into Aspire until after the upcoming 13.3 release ships in a few days, but once we do you'll be able to pickup your PR there where you left off.

@McDonaldSean McDonaldSean deleted the users/sean/build-context-platform branch May 6, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants