Skip to content

Commit 1ebbfe0

Browse files
hext-devhext-dev
committed
Merge branch 'arg-fix-improved'
2 parents 7feb5c0 + f5bd1f4 commit 1ebbfe0

3 files changed

Lines changed: 206 additions & 34 deletions

File tree

‎devcontainer/devcontainer.go‎

Lines changed: 91 additions & 32 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, buildArgs)
208208
if err != nil {
209209
return nil, fmt.Errorf("user from dockerfile: %w", err)
210210
}
@@ -306,14 +306,75 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
306306
return strings.Join(lines, "\n"), featureContexts, err
307307
}
308308

309+
// buildArgsWithDefaults merges external build args with ARG defaults from a Dockerfile.
310+
// External args take precedence over Dockerfile defaults.
311+
func buildArgsWithDefaults(dockerfileContent string, externalArgs []string) ([]string, error) {
312+
lexer := shell.NewLex('\\')
313+
314+
// Start with external args (these have highest precedence)
315+
result := make([]string, len(externalArgs))
316+
copy(result, externalArgs)
317+
318+
// Build a set of externally-provided arg names for quick lookup
319+
externalArgNames := make(map[string]struct{})
320+
for _, arg := range externalArgs {
321+
if parts := strings.SplitN(arg, "=", 2); len(parts) == 2 {
322+
externalArgNames[parts[0]] = struct{}{}
323+
}
324+
}
325+
326+
// Process ARG instructions to add default values if not overridden
327+
for _, line := range strings.Split(dockerfileContent, "\n") {
328+
arg, ok := strings.CutPrefix(line, "ARG ")
329+
if !ok {
330+
continue
331+
}
332+
arg = strings.TrimSpace(arg)
333+
if !strings.Contains(arg, "=") {
334+
continue
335+
}
336+
337+
parts := strings.SplitN(arg, "=", 2)
338+
key, _, err := lexer.ProcessWord(parts[0], shell.EnvsFromSlice(result))
339+
if err != nil {
340+
return nil, fmt.Errorf("processing %q: %w", line, err)
341+
}
342+
343+
// Only use the default value if no external arg was provided
344+
if _, exists := externalArgNames[key]; exists {
345+
continue
346+
}
347+
348+
val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(result))
349+
if err != nil {
350+
return nil, fmt.Errorf("processing %q: %w", line, err)
351+
}
352+
result = append(result, key+"="+val)
353+
}
354+
355+
return result, nil
356+
}
357+
309358
// UserFromDockerfile inspects the contents of a provided Dockerfile
310359
// and returns the user that will be used to run the container.
311-
func UserFromDockerfile(dockerfileContent string) (user string, err error) {
360+
// Optionally accepts build args that may override default values in the Dockerfile.
361+
func UserFromDockerfile(dockerfileContent string, buildArgs ...[]string) (user string, err error) {
362+
var args []string
363+
if len(buildArgs) > 0 {
364+
args = buildArgs[0]
365+
}
366+
312367
res, err := parser.Parse(strings.NewReader(dockerfileContent))
313368
if err != nil {
314369
return "", fmt.Errorf("parse dockerfile: %w", err)
315370
}
316371

372+
resolvedArgs, err := buildArgsWithDefaults(dockerfileContent, args)
373+
if err != nil {
374+
return "", err
375+
}
376+
lexer := shell.NewLex('\\')
377+
317378
// Parse stages and user commands to determine the relevant user
318379
// from the final stage.
319380
var (
@@ -371,10 +432,16 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) {
371432
}
372433

373434
// If we can't find a user command, try to find the user from
374-
// the image.
375-
ref, err := name.ParseReference(strings.TrimSpace(stage.BaseName))
435+
// the image. First, substitute any ARG variables in the image name.
436+
imageRef := stage.BaseName
437+
imageRef, _, err := lexer.ProcessWord(imageRef, shell.EnvsFromSlice(resolvedArgs))
438+
if err != nil {
439+
return "", fmt.Errorf("processing image ref %q: %w", stage.BaseName, err)
440+
}
441+
442+
ref, err := name.ParseReference(strings.TrimSpace(imageRef))
376443
if err != nil {
377-
return "", fmt.Errorf("parse image ref %q: %w", stage.BaseName, err)
444+
return "", fmt.Errorf("parse image ref %q: %w", imageRef, err)
378445
}
379446
user, err := UserFromImage(ref)
380447
if err != nil {
@@ -388,40 +455,32 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) {
388455

389456
// ImageFromDockerfile inspects the contents of a provided Dockerfile
390457
// and returns the image that will be used to run the container.
391-
func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) {
392-
lexer := shell.NewLex('\\')
458+
// Optionally accepts build args that may override default values in the Dockerfile.
459+
func ImageFromDockerfile(dockerfileContent string, buildArgs ...[]string) (name.Reference, error) {
393460
var args []string
461+
if len(buildArgs) > 0 {
462+
args = buildArgs[0]
463+
}
464+
465+
resolvedArgs, err := buildArgsWithDefaults(dockerfileContent, args)
466+
if err != nil {
467+
return nil, err
468+
}
469+
470+
// Find the FROM instruction
394471
var imageRef string
395-
lines := strings.Split(dockerfileContent, "\n")
396-
// Iterate over lines in reverse
397-
for i := len(lines) - 1; i >= 0; i-- {
398-
line := lines[i]
399-
if arg, ok := strings.CutPrefix(line, "ARG "); ok {
400-
arg = strings.TrimSpace(arg)
401-
if strings.Contains(arg, "=") {
402-
parts := strings.SplitN(arg, "=", 2)
403-
key, _, err := lexer.ProcessWord(parts[0], shell.EnvsFromSlice(args))
404-
if err != nil {
405-
return nil, fmt.Errorf("processing %q: %w", line, err)
406-
}
407-
val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(args))
408-
if err != nil {
409-
return nil, fmt.Errorf("processing %q: %w", line, err)
410-
}
411-
args = append(args, key+"="+val)
412-
}
413-
continue
414-
}
415-
if imageRef == "" {
416-
if fromArgs, ok := strings.CutPrefix(line, "FROM "); ok {
417-
imageRef = fromArgs
418-
}
472+
for _, line := range strings.Split(dockerfileContent, "\n") {
473+
if fromArgs, ok := strings.CutPrefix(line, "FROM "); ok {
474+
imageRef = fromArgs
475+
break
419476
}
420477
}
421478
if imageRef == "" {
422479
return nil, fmt.Errorf("no FROM directive found")
423480
}
424-
imageRef, _, err := lexer.ProcessWord(imageRef, shell.EnvsFromSlice(args))
481+
482+
lexer := shell.NewLex('\\')
483+
imageRef, _, err = lexer.ProcessWord(imageRef, shell.EnvsFromSlice(resolvedArgs))
425484
if err != nil {
426485
return nil, fmt.Errorf("processing %q: %w", imageRef, err)
427486
}

‎devcontainer/devcontainer_test.go‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ func TestImageFromDockerfile(t *testing.T) {
204204
}, {
205205
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}",
206206
image: "mcr.microsoft.com/devcontainers/python:0-3.10",
207+
}, {
208+
content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}",
209+
image: "mcr.microsoft.com/devcontainers/python:1-3-bookworm",
207210
}, {
208211
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ",
209212
image: "mcr.microsoft.com/devcontainers/python:0-3.10",
@@ -218,6 +221,117 @@ func TestImageFromDockerfile(t *testing.T) {
218221
}
219222
}
220223

224+
func TestImageFromDockerfileWithArgs(t *testing.T) {
225+
t.Parallel()
226+
for _, tc := range []struct {
227+
default_image string
228+
content string
229+
image string
230+
}{{
231+
default_image: "mcr.microsoft.com/devcontainers/python:1-3-bookworm",
232+
content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}",
233+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
234+
}, {
235+
default_image: "mcr.microsoft.com/devcontainers/python:1-3.10",
236+
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT",
237+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
238+
}, {
239+
default_image: "mcr.microsoft.com/devcontainers/python:1-3.10",
240+
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT\nUSER app",
241+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
242+
}} {
243+
tc := tc
244+
t.Run(tc.image, func(t *testing.T) {
245+
t.Parallel()
246+
dc := &devcontainer.Spec{
247+
Build: devcontainer.BuildSpec{
248+
Dockerfile: "Dockerfile",
249+
Context: ".",
250+
Args: map[string]string{
251+
"VARIANT": "3.11-bookworm",
252+
},
253+
},
254+
}
255+
fs := memfs.New()
256+
dcDir := "/workspaces/coder/.devcontainer"
257+
err := fs.MkdirAll(dcDir, 0o755)
258+
require.NoError(t, err)
259+
file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644)
260+
require.NoError(t, err)
261+
_, err = io.WriteString(file, tc.content)
262+
require.NoError(t, err)
263+
_ = file.Close()
264+
params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv)
265+
require.NoError(t, err)
266+
require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0])
267+
require.Equal(t, params.DockerfileContent, tc.content)
268+
ref, err := devcontainer.ImageFromDockerfile(tc.content, params.BuildArgs)
269+
require.NoError(t, err)
270+
require.Equal(t, tc.image, ref.Name())
271+
// Test without args (using defaults)
272+
ref1, err := devcontainer.ImageFromDockerfile(tc.content)
273+
require.NoError(t, err)
274+
require.Equal(t, tc.default_image, ref1.Name())
275+
})
276+
}
277+
}
278+
279+
func TestUserFromDockerfileWithArgs(t *testing.T) {
280+
t.Parallel()
281+
for _, tc := range []struct {
282+
user string
283+
content string
284+
image string
285+
}{{
286+
user: "root",
287+
content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}",
288+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
289+
}, {
290+
user: "root",
291+
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT",
292+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
293+
}, {
294+
user: "app",
295+
content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT\nUSER app",
296+
image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm",
297+
}} {
298+
tc := tc
299+
t.Run(tc.image, func(t *testing.T) {
300+
t.Parallel()
301+
dc := &devcontainer.Spec{
302+
Build: devcontainer.BuildSpec{
303+
Dockerfile: "Dockerfile",
304+
Context: ".",
305+
Args: map[string]string{
306+
"VARIANT": "3.11-bookworm",
307+
},
308+
},
309+
}
310+
fs := memfs.New()
311+
dcDir := "/workspaces/coder/.devcontainer"
312+
err := fs.MkdirAll(dcDir, 0o755)
313+
require.NoError(t, err)
314+
file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644)
315+
require.NoError(t, err)
316+
_, err = io.WriteString(file, tc.content)
317+
require.NoError(t, err)
318+
_ = file.Close()
319+
params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv)
320+
require.NoError(t, err)
321+
require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0])
322+
require.Equal(t, params.DockerfileContent, tc.content)
323+
// Test UserFromDockerfile without args
324+
user1, err := devcontainer.UserFromDockerfile(tc.content)
325+
require.NoError(t, err)
326+
require.Equal(t, tc.user, user1)
327+
// Test UserFromDockerfile with args
328+
user2, err := devcontainer.UserFromDockerfile(tc.content, params.BuildArgs)
329+
require.NoError(t, err)
330+
require.Equal(t, tc.user, user2)
331+
})
332+
}
333+
}
334+
221335
func TestUserFrom(t *testing.T) {
222336
t.Parallel()
223337

@@ -310,7 +424,6 @@ func TestUserFrom(t *testing.T) {
310424
require.NoError(t, err)
311425
parsed.Path = "coder/test:" + tag
312426
ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://"))
313-
fmt.Println(ref)
314427
require.NoError(t, err)
315428
err = remote.Write(ref, image)
316429
require.NoError(t, err)

‎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)