Add fallback to CONTAINER_DEFAULT_PLATFORM for image architecture

[Willjianger9]

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Motivation and Context

Closes #1252 (part of #913).

Commands that accept --platform currently require it to be passed explicitly every time. Users working consistently with a non-native platform (e.g. linux/amd64 on Apple Silicon) have no way to set a default, unlike Docker's DOCKER_DEFAULT_PLATFORM.

This adds support for the CONTAINER_DEFAULT_PLATFORM environment variable as a fallback when --platform is not explicitly provided.

Changes

• New file DefaultPlatform.swift — centralized logic for reading, validating, and resolving the environment variable with clear precedence rules.
• Wired into all commands with --platform: run, create, image pull, image push, image save, and build.
• Precedence: explicit --platform flag → --os/--arch flags → CONTAINER_DEFAULT_PLATFORM → native host architecture.
• User feedback: prints a notice to stderr when the environment variable is active (e.g. Notice: Using platform "linux/amd64" from CONTAINER_DEFAULT_PLATFORM).
• Help text: all --platform options now show [environment: CONTAINER_DEFAULT_PLATFORM].
• Error handling: invalid values in the environment variable produce a clear error message naming the variable.

Testing

• Tested locally
• Added/updated tests
• Added/updated docs

20 unit tests added in DefaultPlatformTests.swift covering:

• Environment variable parsing (valid platforms, empty, missing, invalid)
• Precedence for resolve() (pull/push/save): explicit flags override env var
• Precedence for resolveWithDefaults() (run/create): env var overrides os/arch defaults
• Invalid env var is skipped when explicit --platform is provided
• All 134 pre-existing unit tests pass with no regressions

[jglogan]

@Willjianger9 Thanks for the contribution! Could you make sure that your commit is signed and verified? See https://github.com/apple/containerization/blob/main/CONTRIBUTING.md#pull-requests for details.

[Willjianger9]

Thanks for bringing that to my attention! I've signed the commit and force-pushed so it should now show as verified. Please let me know if there's anything else that needs to be addressed 😊

[jglogan]

@Willjianger9 thanks, this looks like a nice refactor!

Signature looks good now, just a couple of changes suggested so that we have ContainerClient code that works as expected with the CLI, but makes output optional for non-CLI clients.

[jglogan]

A couple of things here:

• For the CLI we manage stderr output through a Logger, and use structured logging. Furthermore, if it's something other than container that's using the APIs, stderr output may not be desired. What do you think about the following?
  • Make printNotice private, and add a log: Logger parameter and log the result at warn level. Have a look at the logging statements in ContainerCommand for structured logging examples. The log message should contain no interpolated values.
  • For the other functions, add a log: Logger? parameter and move the printNotice invocations in resolve and resolveWithDefaults into fromEnvironment.
• Add docc for the public func parameters and return values.

[Willjianger9]

Thanks for the feedback! I do agree that using a Logger makes the API much more flexible for non-CLI clients and cleans up the stderr output, which is something I overlooked in my changes. I'll get started on updating the logging structure, adjusting the parameters, and adding the missing docc comments

[Ronitsabhaya75]

@Willjianger9 @jglogan can you help me understand here:

I noticed that BuildCommand manually calls fromEnvironment() inline, whereas the other commands (ImagePull/Push/Save) delgating to DefaultPlatform.resolve(). Should BuildCommand also use a centralized helper so the precedence logic doesn't drift out of sync?

resolve() currently uses the raw Platform(from:) OCI initializer, while resolveWithDefaults() uses Parser.platform(from:). Since the original code used Parser.platform(), could this cause any subtle behavioral changes?

I saw some tests cases were missing

• A test for a platform with a variant (like linux/arm/v7) via the environment variable.
• A test for resolve() where an invalid env var is provided but --platform is explicit.

[Willjianger9]

@Ronitsabhaya75 Thanks for the feedback!

Here is my thinking on these (and @jglogan, feel free to jump in if I missed any context):

• BuildCommand vs. others: BuildCommand needs to produce a Set<Platform> from an array of strings, while the other commands only need a single Platform?, which is why it can't use resolve() directly. However, it does use the centralized DefaultPlatform.fromEnvironment() to read the env var, so the validation logic won't drift. The precedence is still explicit --platform > CONTAINER_DEFAULT_PLATFORM > --os/--arch
• Parser logic: Parser.platform(from:) is just a thin wrapper that calls try .init(from: platform), so it’s functionally identical to using Platform(from:). There shouldn't be any subtle behavioral changes, and I just kept them separate to preserve the original behavior of the specific callsites (pull/push/save vs. run/create)

I totally agree on the tests, I have gone ahead and added two missing test cases

[Ronitsabhaya75]

@Willjianger9 Thanks for the explanation!

That all makes sense.

You're completely right about Set<Platform> vs Platform?. Since it still goes through fromEnvironment() to parse the env var, the validation logic is safely centralized.

Thanks for confirming that Parser.platform(from:) is just a thin wrapper around the OCI initializer. Preserving the original behavior of the local call sites is definitely the safest move here.

[jglogan]

• BuildCommand vs. others: BuildCommand needs to produce a Set<Platform> from an array of strings, while the other commands only need a single Platform?

this.

[jglogan]

@Willjianger9 Been hitting consistent CI failures for this PR, pretty sure it's not your changes.

I'm going to merge main into the PR to see if the most recent test changes will help. Apologies for the delay on this.

[jglogan]

@Willjianger9 The merge took care of the test problems. Thank you for this contribution!