Skip to content

Commit 2f2ae79

Browse files
aledbfclaude
andcommitted
feat: port upstream #1225 (Dockerfile metadata), #1078 (feature build secrets), #1241 (up --platform)
These three upstream PRs interleave in the up/build feature paths, so they land together. #1225 — preserve a user Dockerfile's `LABEL devcontainer.metadata`: the parser now captures LABEL instructions (Dockerfile.StageLabels), and build/up merge the Dockerfile-declared metadata into the composed label so the CLI's own stamped label no longer clobbers the user's entries. #1078 — build secrets in features: FeatureBuildOptions gains Secrets, forwarded to the feature-install image build (buildx `--secret id=KEY,env=KEY`) and mounted into each feature-install RUN as `--mount=type=secret,id=KEY` (new DockerfileBuilder.RunWithMounts), so a Feature's install.sh can read /run/secrets/KEY. Threaded from --secrets-file at every up/build call site; gated on buildx. Compose feature secrets stay unsupported (compose build does not forward --secret). #1241 — `devcontainer up --platform`: new flag, and for image-based configs the platform is derived from a `--platform` in runArgs when the flag is unset, then used to pull (EngineClient.PullImagePlatform) and feature-build the image for the platform it will run on (e.g. amd64 forced via runArgs on an arm64 host). Tests added for each; the flag inventory is updated for up --platform. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 87da481 commit 2f2ae79

16 files changed

Lines changed: 266 additions & 32 deletions

docs/parity/cli-flags-inventory.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ commands:
6565
choices: [all, detect, none]
6666
default: detect
6767
description: "Availability of GPUs in case the dev container requires any."
68+
platform:
69+
type: string
70+
description: "Target platform (e.g. linux/amd64). Used to resolve, pull and build the image; overrides a --platform in runArgs."
6871
mount-workspace-git-root:
6972
type: boolean
7073
default: true

internal/cli/build.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,10 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai
348348
}
349349
baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger)
350350
metadata := append([]imagemeta.Entry{}, baseMetadata...)
351+
// Preserve a `LABEL devcontainer.metadata` declared in the user's Dockerfile
352+
// (#1225): the CLI stamps its own computed metadata label below, which would
353+
// otherwise clobber the user's entries on the built image.
354+
metadata = append(metadata, imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger)...)
351355
if len(cfg.Features) == 0 {
352356
metadata = append(metadata, configToMetadataEntry(cfg))
353357
}
@@ -442,6 +446,7 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai
442446
if len(cfg.Features) > 0 {
443447
baseImageName := imageNames[0]
444448
return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, &FeatureBuildOptions{
449+
Secrets: buildSecretsFromFile(opts.secretsFile),
445450
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
446451
NoCache: opts.noCache,
447452
CacheFrom: opts.cacheFrom,
@@ -512,6 +517,7 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer,
512517
featureImageNames = []string{folderImageName(resolvePath(opts.workspaceFolder)) + "-features"}
513518
}
514519
return extendImageWithFeatures(ctx, logger, dockerClient, engine, cfg.Image, cfg.Features, useBuildx, featureImageNames, &FeatureBuildOptions{
520+
Secrets: buildSecretsFromFile(opts.secretsFile),
515521
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
516522
NoCache: opts.noCache,
517523
CacheFrom: opts.cacheFrom,
@@ -545,6 +551,7 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer,
545551

546552
if len(cfg.Features) > 0 {
547553
return extendImageWithFeatures(ctx, logger, dockerClient, engine, cfg.Image, cfg.Features, useBuildx, featureImageNames, &FeatureBuildOptions{
554+
Secrets: buildSecretsFromFile(opts.secretsFile),
548555
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
549556
NoCache: opts.noCache,
550557
CacheFrom: opts.cacheFrom,
@@ -794,6 +801,7 @@ func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer
794801
imageNames = []string{baseImageName + "-features"}
795802
}
796803
return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, &FeatureBuildOptions{
804+
Secrets: buildSecretsFromFile(opts.secretsFile),
797805
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
798806
NoCache: opts.noCache,
799807
CacheFrom: opts.cacheFrom,

internal/cli/feature_install.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,18 @@ type FeatureBuildOptions struct {
3333
NoCache bool
3434
// HostSub, when set, resolves ${localEnv:…}/${localWorkspaceFolder}/… in each
3535
// Feature's containerEnv/mounts — same as the config's own substitution.
36-
HostSub *config.HostSubContext
37-
CacheFrom []string
38-
CacheTo string
39-
Labels []string
40-
Platform string
41-
Push bool
42-
Output string
36+
HostSub *config.HostSubContext
37+
CacheFrom []string
38+
CacheTo string
39+
Labels []string
40+
Platform string
41+
Push bool
42+
Output string
43+
// Secrets are Docker build secrets ("KEY=VALUE") forwarded to the
44+
// feature-install image build as buildx `--secret id=KEY,env=KEY`, so a
45+
// Feature's install.sh can use `RUN --mount=type=secret` (#1078). Requires
46+
// buildx; ignored by the legacy builder.
47+
Secrets []string
4348
ContainerEnv map[string]string
4449
SkipFeatureAutoMapping bool
4550
SkipPersistCustoms bool
@@ -598,8 +603,14 @@ func extendImageWithFeatures(
598603
configContainerEnv = fbOpts.ContainerEnv
599604
}
600605

606+
// Build secrets are mounted into each feature-install RUN, but only when the
607+
// build uses buildx (RUN --mount=type=secret + --secret both require BuildKit).
608+
var secretIDs []string
609+
if useBuildx && fbOpts != nil {
610+
secretIDs = buildSecretIDs(fbOpts.Secrets)
611+
}
601612
buildInfo := imagemeta.GenerateExtendImageBuild(
602-
baseImage, featureSets, metadata, containerUser, remoteUser, useBuildx, configContainerEnv,
613+
baseImage, featureSets, metadata, containerUser, remoteUser, useBuildx, configContainerEnv, secretIDs,
603614
)
604615

605616
// Write the Dockerfile
@@ -640,6 +651,7 @@ func extendImageWithFeatures(
640651
buildOpts.Platform = fbOpts.Platform
641652
buildOpts.Push = fbOpts.Push
642653
buildOpts.Output = fbOpts.Output
654+
buildOpts.Secrets = fbOpts.Secrets
643655
}
644656

645657
// Add BuildKit contexts — resolve relative paths to tmpDir

internal/cli/feature_tarball_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ func TestIsGitHubTarballURI(t *testing.T) {
2727

2828
func TestIsPlainHTTPURL(t *testing.T) {
2929
cases := map[string]bool{
30-
"http://example.com/feat.tgz": true,
31-
"https://example.com/feat.tgz": false,
32-
"https://localhost:8080/x.tgz": true, // localhost even over https
33-
"http://localhost/x.tgz": true,
34-
"https://127.0.0.1:5000/x.tgz": false, // only the literal "localhost" host is special
30+
"http://example.com/feat.tgz": true,
31+
"https://example.com/feat.tgz": false,
32+
"https://localhost:8080/x.tgz": true, // localhost even over https
33+
"http://localhost/x.tgz": true,
34+
"https://127.0.0.1:5000/x.tgz": false, // only the literal "localhost" host is special
3535
}
3636
for u, want := range cases {
3737
if got := isPlainHTTPURL(u); got != want {

internal/cli/helpers.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"encoding/json"
55
"fmt"
6+
"strings"
67

78
"github.com/devcontainers/cli/internal/config"
89
"github.com/devcontainers/cli/internal/log"
@@ -89,3 +90,17 @@ func logDimensions(columns, rows int) *log.Dimensions {
8990
}
9091
return nil
9192
}
93+
94+
// findPlatformArg extracts the target platform from runArgs, supporting both
95+
// "--platform=linux/amd64" and "--platform linux/amd64". Returns "" when absent.
96+
func findPlatformArg(runArgs []string) string {
97+
for i, a := range runArgs {
98+
if v, ok := strings.CutPrefix(a, "--platform="); ok {
99+
return v
100+
}
101+
if a == "--platform" && i+1 < len(runArgs) {
102+
return runArgs[i+1]
103+
}
104+
}
105+
return ""
106+
}

internal/cli/metadata_interop_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func TestMetadataInteropGoRoundTrip(t *testing.T) {
192192
// Build the Dockerfile exactly as build.go does for the no-features case
193193
// (base image + metadata label), but pin the base to `scratch` so the build
194194
// needs no network.
195-
info := imagemeta.GenerateExtendImageBuild("scratch", nil, entries, "root", "", false, nil)
195+
info := imagemeta.GenerateExtendImageBuild("scratch", nil, entries, "root", "", false, nil, nil)
196196
dockerfile := info.DockerfilePrefixContent + info.DockerfileContent
197197

198198
tmp := t.TempDir()

internal/cli/pure_helpers_test.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,13 @@ func TestHighestSatisfyingTag(t *testing.T) {
7070

7171
func TestMajorOf(t *testing.T) {
7272
cases := map[string]string{
73-
"": "",
74-
"1.2.3": "1",
75-
"2": "2",
76-
"0.5.0": "0",
77-
"not-semver": "",
78-
"10.20.30": "10",
79-
"v3.1.0": "3", // semver tolerates a leading v
73+
"": "",
74+
"1.2.3": "1",
75+
"2": "2",
76+
"0.5.0": "0",
77+
"not-semver": "",
78+
"10.20.30": "10",
79+
"v3.1.0": "3", // semver tolerates a leading v
8080
}
8181
for in, want := range cases {
8282
if got := majorOf(in); got != want {
@@ -423,3 +423,24 @@ func TestIsBareVersion(t *testing.T) {
423423
}
424424
}
425425
}
426+
427+
// TestFindPlatformArg covers #1241: extracting --platform from runArgs in both
428+
// "--platform=X" and "--platform X" forms.
429+
func TestFindPlatformArg(t *testing.T) {
430+
cases := []struct {
431+
args []string
432+
want string
433+
}{
434+
{[]string{"--platform=linux/amd64"}, "linux/amd64"},
435+
{[]string{"--platform", "linux/arm64"}, "linux/arm64"},
436+
{[]string{"--cap-add=SYS_PTRACE", "--platform=linux/amd64", "--rm"}, "linux/amd64"},
437+
{[]string{"--rm"}, ""},
438+
{nil, ""},
439+
{[]string{"--platform"}, ""}, // dangling flag, no value
440+
}
441+
for _, tc := range cases {
442+
if got := findPlatformArg(tc.args); got != tc.want {
443+
t.Errorf("findPlatformArg(%v) = %q, want %q", tc.args, got, tc.want)
444+
}
445+
}
446+
}

internal/cli/secrets.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,29 @@ func secretValuesFromFile(path string) []string {
4343
}
4444
return values
4545
}
46+
47+
// buildSecretsFromFile reads build secrets ("KEY=VALUE") from a --secrets-file
48+
// path, or nil when unset/unreadable (a read error is non-fatal — the build
49+
// proceeds without secrets, matching the main build path).
50+
func buildSecretsFromFile(path string) []string {
51+
if path == "" {
52+
return nil
53+
}
54+
secrets, err := readSecretsFile(path)
55+
if err != nil {
56+
return nil
57+
}
58+
return secrets
59+
}
60+
61+
// buildSecretIDs extracts the secret id (the KEY of each "KEY=VALUE") so it can
62+
// be mounted into a feature-install RUN as --mount=type=secret,id=KEY.
63+
func buildSecretIDs(secrets []string) []string {
64+
ids := make([]string, 0, len(secrets))
65+
for _, s := range secrets {
66+
if i := strings.IndexByte(s, '='); i > 0 {
67+
ids = append(ids, s[:i])
68+
}
69+
}
70+
return ids
71+
}

internal/cli/up.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ type upOpts struct {
6969
workspaceMountConsistency string
7070
updateRemoteUserUIDDefault string
7171
gpuAvailability string
72+
platform string
7273
defaultUserEnvProbe string
7374
containerSessionDataFolder string
7475
skipFeatureAutoMapping bool
@@ -129,6 +130,7 @@ func newUpCmd() *cobra.Command {
129130
f.String("container-system-data-folder", "", "")
130131
f.StringVar(&opts.workspaceMountConsistency, "workspace-mount-consistency", "cached", "Mount consistency.")
131132
f.StringVar(&opts.gpuAvailability, "gpu-availability", "detect", "GPU availability (all|detect|none).")
133+
f.StringVar(&opts.platform, "platform", "", "Target platform (e.g. linux/amd64). Used to resolve, pull and build the image; overrides a --platform in runArgs.")
132134
f.StringVar(&opts.defaultUserEnvProbe, "default-user-env-probe", "loginInteractiveShell", "Env probe type.")
133135
f.StringVar(&opts.updateRemoteUserUIDDefault, "update-remote-user-uid-default", "on", "UID update default.")
134136
f.BoolVar(&opts.expectExistingContainer, "expect-existing-container", false, "Fail if no existing container is found.")
@@ -764,6 +766,8 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer,
764766
if baseInspect, inspErr := engine.InspectImage(ctx, baseImage); inspErr == nil && baseInspect.Config != nil {
765767
metadata = append(metadata, imagemeta.ReadMetadataFromLabels(baseInspect.Config.Labels, logger)...)
766768
}
769+
// Preserve a `LABEL devcontainer.metadata` from the user's Dockerfile (#1225).
770+
metadata = append(metadata, imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger)...)
767771
metadata = append(metadata, imagemeta.Entry{RemoteUser: cfg.RemoteUser, ContainerUser: cfg.ContainerUser})
768772
metadataLabel := imagemeta.GenerateMetadataLabel(metadata)
769773

@@ -808,6 +812,7 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer,
808812
// Extend with features if any
809813
if len(cfg.Features) > 0 {
810814
names, err := extendImageWithFeatures(ctx, logger, dockerClient, engine, imageName, cfg.Features, useBuildx, nil, &FeatureBuildOptions{
815+
Secrets: buildSecretsFromFile(opts.secretsFile),
811816
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
812817
FeaturesBasePath: filepath.Dir(cfg.ConfigFilePath),
813818
SkipFeatureAutoMapping: opts.skipFeatureAutoMapping,
@@ -837,11 +842,19 @@ func (r *upRunner) fromImage(ctx context.Context, cfg *config.DevContainer, load
837842
return "", fmt.Errorf("no image specified in devcontainer.json")
838843
}
839844

845+
// Target platform: the --platform flag wins, else a --platform in runArgs, so
846+
// the image is pulled and (feature-)built for the platform it will run on —
847+
// e.g. an amd64 image forced via runArgs on an arm64 host (#1241).
848+
platform := opts.platform
849+
if platform == "" {
850+
platform = findPlatformArg(cfg.RunArgs)
851+
}
852+
840853
// Pull if needed
841854
_, err := engine.InspectImage(ctx, cfg.Image)
842855
if err != nil {
843856
logger.Write(fmt.Sprintf("Pulling image %s...", cfg.Image), log.LevelInfo)
844-
if pullErr := engine.PullImage(ctx, cfg.Image); pullErr != nil {
857+
if pullErr := engine.PullImagePlatform(ctx, cfg.Image, platform); pullErr != nil {
845858
return "", fmt.Errorf("pull image %q: %w", cfg.Image, pullErr)
846859
}
847860
}
@@ -850,6 +863,8 @@ func (r *upRunner) fromImage(ctx context.Context, cfg *config.DevContainer, load
850863
imageName := cfg.Image
851864
if len(cfg.Features) > 0 {
852865
names, err := extendImageWithFeatures(ctx, logger, dockerClient, engine, cfg.Image, cfg.Features, useBuildx, nil, &FeatureBuildOptions{
866+
Platform: platform,
867+
Secrets: buildSecretsFromFile(opts.secretsFile),
853868
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
854869
FeaturesBasePath: filepath.Dir(cfg.ConfigFilePath),
855870
SkipFeatureAutoMapping: opts.skipFeatureAutoMapping,
@@ -1342,8 +1357,11 @@ func (r *upRunner) fromCompose(ctx context.Context, cfg *config.DevContainer, lo
13421357
}
13431358

13441359
// Generate feature Dockerfile content for compose injection
1360+
// No build secrets here: `docker compose build` does not forward
1361+
// --secret to the injected feature stage, so mounting them would break
1362+
// the build. Compose feature build secrets remain unsupported for now.
13451363
featureContent := imagemeta.GenerateExtendImageBuildForCompose(
1346-
baseStageName, featureSets, metadata, containerUser, remoteUser, nil,
1364+
baseStageName, featureSets, metadata, containerUser, remoteUser, nil, nil,
13471365
)
13481366
combinedDockerfile := string(dockerfileContent) + "\n" + featureContent
13491367

@@ -1412,6 +1430,7 @@ func (r *upRunner) fromCompose(ctx context.Context, cfg *config.DevContainer, lo
14121430

14131431
logger.Write(fmt.Sprintf("Installing features on compose service %s...", cfg.Service), log.LevelInfo)
14141432
names, extErr := extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, nil, &FeatureBuildOptions{
1433+
Secrets: buildSecretsFromFile(opts.secretsFile),
14151434
OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder,
14161435
FeaturesBasePath: filepath.Dir(cfg.ConfigFilePath),
14171436
SkipFeatureAutoMapping: opts.skipFeatureAutoMapping,

internal/docker/dockerfile.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ func ExtractDockerfile(content string) *Dockerfile {
9494
}
9595
case *instructions.UserCommand:
9696
st.Instructions = append(st.Instructions, Instruction{Instruction: "USER", Name: c.User})
97+
case *instructions.LabelCommand:
98+
for _, kv := range c.Labels {
99+
st.Instructions = append(st.Instructions, Instruction{Instruction: "LABEL", Name: kv.Key, Value: trimQuotes(kv.Value)})
100+
}
97101
}
98102
}
99103
df.Stages = append(df.Stages, st)
@@ -106,6 +110,30 @@ func ExtractDockerfile(content string) *Dockerfile {
106110
return df
107111
}
108112

113+
// StageLabels returns the LABEL key/value pairs declared in the named build
114+
// stage (or the final stage when name is ""), with later labels overriding
115+
// earlier ones. Used to recover a user's `LABEL devcontainer.metadata` from the
116+
// Dockerfile so the CLI-stamped metadata label does not clobber it.
117+
func (d *Dockerfile) StageLabels(name string) map[string]string {
118+
var st *Stage
119+
if name != "" {
120+
st = d.StagesByLabel[name]
121+
}
122+
if st == nil && len(d.Stages) > 0 {
123+
st = &d.Stages[len(d.Stages)-1]
124+
}
125+
if st == nil {
126+
return nil
127+
}
128+
labels := map[string]string{}
129+
for _, ins := range st.Instructions {
130+
if ins.Instruction == "LABEL" {
131+
labels[ins.Name] = ins.Value
132+
}
133+
}
134+
return labels
135+
}
136+
109137
// syntaxVersion extracts the tag of a docker/dockerfile syntax directive
110138
// ("docker/dockerfile:1.4" -> "1.4", no tag -> "latest", other frontend -> "").
111139
func syntaxVersion(syntax string) string {

0 commit comments

Comments
 (0)