Repository navigation
fix(docker): pass build args in the reference's order - #74
Merged
Merged
Conversation
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]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build args order
TS: config
build.argsgo on thedocker buildcommand line inObject.keysorder: 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 inupdateImageand in the Compose override. The updateUID build passesBASE_IMAGE,REMOTE_USER,NEW_UID,NEW_GID,IMAGE_USER.Before:
docker.BuildOptions.BuildArgswas 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_SOURCEcame before_DEV_CONTAINERS_IMAGE_USER.After:
BuildArgsis ordered ([][2]string). The loader records the member order ofbuild.argsinconfig.DevContainer.BuildArgsOrder, base config then override, throughjsonc.MemberNamesthe same way asContainerEnvOrder. The Dockerfile build emits the args infeatures.PropertyOrder(args, BuildArgsOrder)order. The image-config Features build and the Compose override usefeatureBuildArgOrder, 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 runsTestExtendImageWithFeaturesBuildArgOrderTestBuildAndExtendComposeImageServiceWithFeatures: the existing assertion now also requiresIMAGE_USERbeforeFEATURE_CONTENT_SOURCETestBuildUIDImageBuildArgOrderbuild.dockerfile-build-args-order-success: a--docker-pathwrapper logs the argv, andverify_cmdcompares the--build-arglines. TS gives2, 10, ZED, ALPHA. Before this change the Go binary gave10, 2, ALPHA, ZED.SELinux: no change
TS adds
--security-opt label=disable(containerFeatures.tsisUsingSELinuxLabels) only when all of these hold:docker -voutput containspodman);getenforceis notDisabled;podman info -f {{.Host.Security.SELinuxEnabled}}istrue.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 testTestBuildDockerfileWritesTheReferenceDockerfilealready 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=disableand--userns=keep-idon run, thelocalhost/prefix for updateUID). This PR doesn't do that. It names the gap in the Podman entry of DIVERGENCES.md instead.build --labelon a Dockerfile configurationNo behavior change. TS applies
--labelonly inupdateImage(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 existingTestBuildDockerfileBuildFlags(asserts--label a=bon the Dockerfile build).Checks
go build ./...: okinternal/{docker,config,cli,imagemeta},TMPDIR=/var/tmp/...): ok.TestE2E_CABundlefailed once under shared-Docker load and passed when rerun alone.go tool task lint: okBASE=origin/main go tool task mutate: 127 refused, 0 survived, 21 did not buildTMPDIR=/var/tmp/... go tool task test:race: okgo tool task parity:contract: 114 matched. The only 2 failures areread-configuration.features-configuration-local-feature-{key-order,metadata}-success, which fail locally because of a stray/tmp/.git.go tool task spec:compliance: okgo 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-successandbuild.compose-no-lockfile-writes-none-successpass alone.build.cache-to-local-successandbuild.output-oci-successhit Go timeouts even alone. ghcr.io currently returns 403 forcodspace/myfeatures/helloworld, whichdockerfile-with-targetneeds, 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 (remoteWorkspaceFolderand dotfiles verify, from the local stray/tmp/.git). Three compose cases failed only on the branch, withcontainer is marked for removalafter the TS side's cleanup was killed, and all three pass when rerun alone. The other 6 matched on the rerun.parity-runtimeshards,e2eandmutate.🤖 Generated with Claude Code