Skip to content

fix(docker): pass build args in the reference's order - #74

Merged
aledbf merged 2 commits into
mainfrom
build-args-and-selinux
Nov 13, 2026
Merged

aledbf merged 2 commits into
mainfrom
build-args-and-selinux

Conversation

@aledbf

@aledbf aledbf commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Build args order

TS: config build.args go on the docker build command line in Object.keys order: integer-like keys first (ascending), then the rest as declared (singleContainer.ts, for (const key in buildArgs)). The Features build args are _DEV_CONTAINERS_BASE_IMAGE, _DEV_CONTAINERS_IMAGE_USER, _DEV_CONTAINERS_FEATURE_CONTENT_SOURCE, in that order (getFeaturesBuildOptions). That order is used in updateImage and in the Compose override. The updateUID build passes BASE_IMAGE, REMOTE_USER, NEW_UID, NEW_GID, IMAGE_USER.

Before: docker.BuildOptions.BuildArgs was a Go map. So the config args, the image-config Features args and the updateUID args came out in map order. The Compose override sorted the Features args, so _DEV_CONTAINERS_FEATURE_CONTENT_SOURCE came before _DEV_CONTAINERS_IMAGE_USER.

After: BuildArgs is ordered ([][2]string). The loader records the member order of build.args in config.DevContainer.BuildArgsOrder, base config then override, through jsonc.MemberNames the same way as ContainerEnvOrder. The Dockerfile build emits the args in features.PropertyOrder(args, BuildArgsOrder) order. The image-config Features build and the Compose override use featureBuildArgOrder, and updateUID passes its args in TS's order. Base-image resolution only looks values up, so order doesn't matter there and it is unchanged.

Tests (each fails before the fix):

  • TestLoadKeepsBuildArgsOrder (config, base + override)
  • TestBuildDockerfileBuildArgOrder: compares the whole argv with what TS runs
  • TestExtendImageWithFeaturesBuildArgOrder
  • TestBuildAndExtendComposeImageServiceWithFeatures: the existing assertion now also requires IMAGE_USER before FEATURE_CONTENT_SOURCE
  • TestBuildUIDImageBuildArgOrder
  • New runtime parity case build.dockerfile-build-args-order-success: a --docker-path wrapper logs the argv, and verify_cmd compares the --build-arg lines. TS gives 2, 10, ZED, ALPHA. Before this change the Go binary gave 10, 2, ALPHA, ZED.

SELinux: no change

TS adds --security-opt label=disable (containerFeatures.ts isUsingSELinuxLabels) only when all of these hold:

  • BuildKit build contexts are used (BuildKit >= 0.8.0);
  • the CLI variant is Podman (docker -v output contains podman);
  • the platform is linux;
  • getenforce is not Disabled;
  • podman info -f {{.Host.Security.SELinuxEnabled}} is true.

When those hold, the flag goes on the Dockerfile build (buildAndExtendImage) and on the image-config Features build (updateImage), not on Compose. With Docker, TS never adds it. This CLI supports Docker only (docs/DIVERGENCES.md "Runtime: Docker only"), so Go already matches TS on every supported runtime. The existing argv test TestBuildDockerfileWritesTheReferenceDockerfile already pins the Docker Features build without --security-opt. Implementing the check would mean adding Podman detection for this one behavior and leaving out the rest of TS's Podman handling (label=disable and --userns=keep-id on run, the localhost/ prefix for updateUID). This PR doesn't do that. It names the gap in the Podman entry of DIVERGENCES.md instead.

build --label on a Dockerfile configuration

No behavior change. TS applies --label only in updateImage (image configs with Features) and drops it for a Dockerfile configuration without a word. Go keeps applying it, the same decision as for Compose (#930). docs/DIVERGENCES.md has an entry next to the Compose one, pinned by the existing TestBuildDockerfileBuildFlags (asserts --label a=b on the Dockerfile build).

Checks

  • go build ./...: ok
  • affected package tests (internal/{docker,config,cli,imagemeta}, TMPDIR=/var/tmp/...): ok. TestE2E_CABundle failed once under shared-Docker load and passed when rerun alone.
  • go tool task lint: ok
  • BASE=origin/main go tool task mutate: 127 refused, 0 survived, 21 did not build
  • TMPDIR=/var/tmp/... go tool task test:race: ok
  • go tool task parity:contract: 114 matched. The only 2 failures are read-configuration.features-configuration-local-feature-{key-order,metadata}-success, which fail locally because of a stray /tmp/.git.
  • go tool task spec:compliance: ok
  • go tool task test:e2e: ok (13/13). The first run hit the task's 5m package timeout on a contended Docker host. It was rerun with -- -timeout 25m.
  • PARITY_COMMAND=build go tool task parity:runtime: 50 matched, including the new case. Of the 4 failures, build.labels-success and build.compose-no-lockfile-writes-none-success pass alone. build.cache-to-local-success and build.output-oci-success hit Go timeouts even alone. ghcr.io currently returns 403 for codspace/myfeatures/helloworld, which dockerfile-with-target needs, and Docker is shared with other agents. Both fixtures have a single config build arg, so this change cannot alter their argv.
  • PARITY_COMMAND=up go tool task parity:runtime: 76 matched, 24 failed, 2 inconclusive (compose-dockerfile-with-features: TS infra). I reran the 24 failures against an origin/main binary and against this branch. 15 fail the same way on both (remoteWorkspaceFolder and dotfiles verify, from the local stray /tmp/.git). Three compose cases failed only on the branch, with container is marked for removal after the TS side's cleanup was killed, and all three pass when rerun alone. The other 6 matched on the rerun.
  • CI: all checks pass, including both parity-runtime shards, e2e and mutate.

🤖 Generated with Claude Code

aledbf and others added 2 commits November 13, 2026 01:02
Config build.args reached docker build from a Go map, so their
--build-arg order changed from run to run. They now follow the order
TS gives them (Object.keys: integer keys ascending, then the others as
declared, base config then override). The Features build args
(_DEV_CONTAINERS_BASE_IMAGE, _DEV_CONTAINERS_IMAGE_USER,
_DEV_CONTAINERS_FEATURE_CONTENT_SOURCE) keep TS's order in the image
build and the Compose override, and the updateUID build passes
BASE_IMAGE, REMOTE_USER, NEW_UID, NEW_GID, IMAGE_USER in that order.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aledbf
aledbf merged commit f05caa9 into main Nov 13, 2026
13 checks passed
@aledbf
aledbf deleted the build-args-and-selinux branch November 13, 2026 11:54
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.

1 participant