Skip to content

Commit 20e3937

Browse files
committed
fix: substitute devcontainer.json build args into Dockerfile ARG variables (#455)
ImageFromDockerfile and UserFromDockerfile now accept external build args from devcontainer.json, allowing them to override ARG defaults and supply values for ARGs without defaults. This fixes the error when a Dockerfile uses ARG VARIANT + FROM ...${VARIANT} with the value provided in devcontainer.json build.args.
1 parent 602df39 commit 20e3937

3 files changed

Lines changed: 79 additions & 7 deletions

File tree

devcontainer/devcontainer.go

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func (s *Spec) Compile(fs billy.Filesystem, devcontainerDir, scratchDir string,
204204
// We should make a best-effort attempt to find the user.
205205
// Features must be executed as root, so we need to swap back
206206
// to the running user afterwards.
207-
params.User, err = UserFromDockerfile(params.DockerfileContent)
207+
params.User, err = UserFromDockerfile(params.DockerfileContent, params.BuildArgs)
208208
if err != nil {
209209
return nil, fmt.Errorf("user from dockerfile: %w", err)
210210
}
@@ -308,12 +308,42 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
308308

309309
// UserFromDockerfile inspects the contents of a provided Dockerfile
310310
// and returns the user that will be used to run the container.
311-
func UserFromDockerfile(dockerfileContent string) (user string, err error) {
311+
func UserFromDockerfile(dockerfileContent string, buildArgs []string) (user string, err error) {
312312
res, err := parser.Parse(strings.NewReader(dockerfileContent))
313313
if err != nil {
314314
return "", fmt.Errorf("parse dockerfile: %w", err)
315315
}
316316

317+
// Collect ARG values (defaults + overrides from buildArgs) for
318+
// substitution into FROM image refs.
319+
lexer := shell.NewLex('\\')
320+
var argEnvs []string
321+
for _, child := range res.AST.Children {
322+
if strings.EqualFold(child.Value, "arg") {
323+
arg := strings.TrimSpace(child.Original[len("ARG "):])
324+
if strings.Contains(arg, "=") {
325+
parts := strings.SplitN(arg, "=", 2)
326+
key := parts[0]
327+
val := parts[1]
328+
// Allow buildArgs to override.
329+
for _, ba := range buildArgs {
330+
if k, v, ok := strings.Cut(ba, "="); ok && k == key {
331+
val = v
332+
break
333+
}
334+
}
335+
argEnvs = append(argEnvs, key+"="+val)
336+
} else {
337+
for _, ba := range buildArgs {
338+
if k, v, ok := strings.Cut(ba, "="); ok && k == arg {
339+
argEnvs = append(argEnvs, k+"="+v)
340+
break
341+
}
342+
}
343+
}
344+
}
345+
}
346+
317347
// Parse stages and user commands to determine the relevant user
318348
// from the final stage.
319349
var (
@@ -330,6 +360,11 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) {
330360

331361
switch i := inst.(type) {
332362
case *instructions.Stage:
363+
// Substitute ARG values in the base image name.
364+
baseName, _, err := lexer.ProcessWord(i.BaseName, shell.EnvsFromSlice(argEnvs))
365+
if err == nil {
366+
i.BaseName = baseName
367+
}
333368
stages = append(stages, i)
334369
if i.Name != "" {
335370
stageNames[i.Name] = i
@@ -388,7 +423,7 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) {
388423

389424
// ImageFromDockerfile inspects the contents of a provided Dockerfile
390425
// and returns the image that will be used to run the container.
391-
func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) {
426+
func ImageFromDockerfile(dockerfileContent string, buildArgs []string) (name.Reference, error) {
392427
lexer := shell.NewLex('\\')
393428
var args []string
394429
var imageRef string
@@ -408,7 +443,22 @@ func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) {
408443
if err != nil {
409444
return nil, fmt.Errorf("processing %q: %w", line, err)
410445
}
446+
// Allow buildArgs to override Dockerfile ARG defaults.
447+
for _, ba := range buildArgs {
448+
if k, v, ok := strings.Cut(ba, "="); ok && k == key {
449+
val = v
450+
break
451+
}
452+
}
411453
args = append(args, key+"="+val)
454+
} else {
455+
// ARG without a default — look up in buildArgs.
456+
for _, ba := range buildArgs {
457+
if k, v, ok := strings.Cut(ba, "="); ok && k == arg {
458+
args = append(args, k+"="+v)
459+
break
460+
}
461+
}
412462
}
413463
continue
414464
}

devcontainer/devcontainer_test.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,35 @@ func TestImageFromDockerfile(t *testing.T) {
213213
tc := tc
214214
t.Run(tc.image, func(t *testing.T) {
215215
t.Parallel()
216-
ref, err := devcontainer.ImageFromDockerfile(tc.content)
216+
ref, err := devcontainer.ImageFromDockerfile(tc.content, nil)
217217
require.NoError(t, err)
218218
require.Equal(t, tc.image, ref.Name())
219219
})
220220
}
221221
}
222222

223+
func TestImageFromDockerfile_BuildArgs(t *testing.T) {
224+
t.Parallel()
225+
226+
// Test that build args override ARG defaults.
227+
t.Run("OverridesDefault", func(t *testing.T) {
228+
t.Parallel()
229+
content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}"
230+
ref, err := devcontainer.ImageFromDockerfile(content, []string{"VARIANT=3.11-bookworm"})
231+
require.NoError(t, err)
232+
require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name())
233+
})
234+
235+
// Test that build args supply values for ARGs without defaults.
236+
t.Run("SuppliesArgWithoutDefault", func(t *testing.T) {
237+
t.Parallel()
238+
content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}"
239+
ref, err := devcontainer.ImageFromDockerfile(content, []string{"VARIANT=3.11-bookworm"})
240+
require.NoError(t, err)
241+
require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name())
242+
})
243+
}
244+
223245
func TestUserFrom(t *testing.T) {
224246
t.Parallel()
225247

@@ -287,7 +309,7 @@ func TestUserFrom(t *testing.T) {
287309
for _, tt := range tests {
288310
t.Run(tt.name, func(t *testing.T) {
289311
t.Parallel()
290-
user, err := devcontainer.UserFromDockerfile(tt.content)
312+
user, err := devcontainer.UserFromDockerfile(tt.content, nil)
291313
require.NoError(t, err)
292314
require.Equal(t, tt.user, user)
293315
})
@@ -364,7 +386,7 @@ FROM a`,
364386

365387
content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test")
366388

367-
user, err := devcontainer.UserFromDockerfile(content)
389+
user, err := devcontainer.UserFromDockerfile(content, nil)
368390
require.NoError(t, err)
369391
require.Equal(t, tt.user, user)
370392
})

envbuilder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
493493
defer cleanupBuildContext()
494494
if runtimeData.Built && opts.SkipRebuild {
495495
endStage := startStage("🏗️ Skipping build because of cache...")
496-
imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent)
496+
imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent, buildParams.BuildArgs)
497497
if err != nil {
498498
return nil, fmt.Errorf("image from dockerfile: %w", err)
499499
}

0 commit comments

Comments
 (0)