From 1f48684ca7849699cd06ff557e8018bbd34eef29 Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 17:22:35 +0200 Subject: [PATCH 1/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index d304e76..5ebd0bf 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -204,6 +204,9 @@ func TestImageFromDockerfile(t *testing.T) { }, { content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}", image: "mcr.microsoft.com/devcontainers/python:0-3.10", + }, { + content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}", + image: "mcr.microsoft.com/devcontainers/python:1-3-bookworm", }, { content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ", image: "mcr.microsoft.com/devcontainers/python:0-3.10", From b8b3dcc7c9d04d92ed209110cbc8d466f21c5c26 Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 17:41:07 +0200 Subject: [PATCH 2/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 5ebd0bf..dab7d35 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -221,6 +221,41 @@ func TestImageFromDockerfile(t *testing.T) { } } +func TestImageFromDockerfileWithArgs(t *testing.T) { + t.Parallel() + dc := &devcontainer.Spec{ + Build: devcontainer.BuildSpec{ + Dockerfile: "Dockerfile", + Context: ".", + Args: map[string]string{ + "VARIANT": "3.11-bookworm", + }, + }, + } + fs := memfs.New() + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) + + for _, tc := range []struct { + content string + image string + }{{ + content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }, { + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT ", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }} { + tc := tc + t.Run(tc.image, func(t *testing.T) { + t.Parallel() + ref, err := devcontainer.ImageFromDockerfile(tc.content) + require.NoError(t, err) + require.Equal(t, tc.image, ref.Name()) + }) + } +} + func TestUserFrom(t *testing.T) { t.Parallel() From 1fa47ec9b86b5a91509be37c86531c4cd75d16be Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 17:52:31 +0200 Subject: [PATCH 3/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index dab7d35..a43fb05 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -232,9 +232,6 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { }, }, } - fs := memfs.New() - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) for _, tc := range []struct { content string @@ -249,6 +246,21 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { tc := tc t.Run(tc.image, func(t *testing.T) { t.Parallel() + + fs := memfs.New() + dcDir := "/workspaces/coder/.devcontainer" + err := fs.MkdirAll(dcDir, 0o755) + require.NoError(t, err) + file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) + require.NoError(t, err) + _, err = io.WriteString(file, tc.content) + require.NoError(t, err) + _ = file.Close() + params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) + + require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) + require.Equal(t, params.DockerfileContent, tc.content) + ref, err := devcontainer.ImageFromDockerfile(tc.content) require.NoError(t, err) require.Equal(t, tc.image, ref.Name()) From 3d03c86751ee82125b28c8a08014814fe99f456e Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:01:24 +0200 Subject: [PATCH 4/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index a43fb05..d0cf973 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -222,17 +222,7 @@ func TestImageFromDockerfile(t *testing.T) { } func TestImageFromDockerfileWithArgs(t *testing.T) { - t.Parallel() - dc := &devcontainer.Spec{ - Build: devcontainer.BuildSpec{ - Dockerfile: "Dockerfile", - Context: ".", - Args: map[string]string{ - "VARIANT": "3.11-bookworm", - }, - }, - } - + t.Parallel() for _, tc := range []struct { content string image string @@ -246,7 +236,15 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { tc := tc t.Run(tc.image, func(t *testing.T) { t.Parallel() - + dc := &devcontainer.Spec{ + Build: devcontainer.BuildSpec{ + Dockerfile: "Dockerfile", + Context: ".", + Args: map[string]string{ + "VARIANT": "3.11-bookworm", + }, + }, + } fs := memfs.New() dcDir := "/workspaces/coder/.devcontainer" err := fs.MkdirAll(dcDir, 0o755) @@ -257,10 +255,8 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { require.NoError(t, err) _ = file.Close() params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) - require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) require.Equal(t, params.DockerfileContent, tc.content) - ref, err := devcontainer.ImageFromDockerfile(tc.content) require.NoError(t, err) require.Equal(t, tc.image, ref.Name()) From e503d3feca8f9574892cdf8bbaaf359085d139d6 Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:08:49 +0200 Subject: [PATCH 5/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index d0cf973..949e541 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -255,6 +255,7 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { require.NoError(t, err) _ = file.Close() params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) + require.NoError(t, err) require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) require.Equal(t, params.DockerfileContent, tc.content) ref, err := devcontainer.ImageFromDockerfile(tc.content) From f60fff8784c6c383c6ee6f7909993e3d6e25288a Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:10:48 +0200 Subject: [PATCH 6/8] Update devcontainer_test.go --- devcontainer/devcontainer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 949e541..9582c99 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -222,7 +222,7 @@ func TestImageFromDockerfile(t *testing.T) { } func TestImageFromDockerfileWithArgs(t *testing.T) { - t.Parallel() + t.Parallel() for _, tc := range []struct { content string image string From 5784366c3f7bec3a5a8f23017a3e0e1715afa748 Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Sat, 24 May 2025 21:35:03 +0000 Subject: [PATCH 7/8] Attempt to fix arg parsing --- devcontainer/devcontainer.go | 102 ++++++++++++++++++++++++++---- devcontainer/devcontainer_test.go | 77 +++++++++++++++++++++- envbuilder.go | 2 +- 3 files changed, 163 insertions(+), 18 deletions(-) diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index 0bf7cc3..c57e490 100644 --- a/devcontainer/devcontainer.go +++ b/devcontainer/devcontainer.go @@ -204,7 +204,7 @@ func (s *Spec) Compile(fs billy.Filesystem, devcontainerDir, scratchDir string, // We should make a best-effort attempt to find the user. // Features must be executed as root, so we need to swap back // to the running user afterwards. - params.User, err = UserFromDockerfile(params.DockerfileContent) + params.User, err = UserFromDockerfile(params.DockerfileContent, buildArgs) if err != nil { return nil, fmt.Errorf("user from dockerfile: %w", err) } @@ -308,12 +308,57 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir // UserFromDockerfile inspects the contents of a provided Dockerfile // and returns the user that will be used to run the container. -func UserFromDockerfile(dockerfileContent string) (user string, err error) { +// Optionally accepts build args that may override default values in the Dockerfile. +func UserFromDockerfile(dockerfileContent string, buildArgs ...[]string) (user string, err error) { + var args []string + if len(buildArgs) > 0 { + args = buildArgs[0] + } + res, err := parser.Parse(strings.NewReader(dockerfileContent)) if err != nil { return "", fmt.Errorf("parse dockerfile: %w", err) } + // Parse build args and ARG instructions to build the substitution context + lexer := shell.NewLex('\\') + + // Start with build args provided externally (e.g., from devcontainer.json) + argsCopy := make([]string, len(args)) + copy(argsCopy, args) + + // Parse build args into a map for easy lookup + buildArgsMap := make(map[string]string) + for _, arg := range args { + if parts := strings.SplitN(arg, "=", 2); len(parts) == 2 { + buildArgsMap[parts[0]] = parts[1] + } + } + + // Process ARG instructions to add default values if not overridden + lines := strings.Split(dockerfileContent, "\n") + for _, line := range lines { + if arg, ok := strings.CutPrefix(line, "ARG "); ok { + arg = strings.TrimSpace(arg) + if strings.Contains(arg, "=") { + parts := strings.SplitN(arg, "=", 2) + key, _, err := lexer.ProcessWord(parts[0], shell.EnvsFromSlice(argsCopy)) + if err != nil { + return "", fmt.Errorf("processing %q: %w", line, err) + } + + // Only use the default value if no build arg was provided + if _, exists := buildArgsMap[key]; !exists { + val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(argsCopy)) + if err != nil { + return "", fmt.Errorf("processing %q: %w", line, err) + } + argsCopy = append(argsCopy, key+"="+val) + } + } + } + } + // Parse stages and user commands to determine the relevant user // from the final stage. var ( @@ -371,10 +416,16 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) { } // If we can't find a user command, try to find the user from - // the image. - ref, err := name.ParseReference(strings.TrimSpace(stage.BaseName)) + // the image. First, substitute any ARG variables in the image name. + imageRef := stage.BaseName + imageRef, _, err := lexer.ProcessWord(imageRef, shell.EnvsFromSlice(argsCopy)) if err != nil { - return "", fmt.Errorf("parse image ref %q: %w", stage.BaseName, err) + return "", fmt.Errorf("processing image ref %q: %w", stage.BaseName, err) + } + + ref, err := name.ParseReference(strings.TrimSpace(imageRef)) + if err != nil { + return "", fmt.Errorf("parse image ref %q: %w", imageRef, err) } user, err := UserFromImage(ref) if err != nil { @@ -388,27 +439,50 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) { // ImageFromDockerfile inspects the contents of a provided Dockerfile // and returns the image that will be used to run the container. -func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) { - lexer := shell.NewLex('\\') +// Optionally accepts build args that may override default values in the Dockerfile. +func ImageFromDockerfile(dockerfileContent string, buildArgs ...[]string) (name.Reference, error) { var args []string + if len(buildArgs) > 0 { + args = buildArgs[0] + } + + lexer := shell.NewLex('\\') + + // Start with build args provided externally (e.g., from devcontainer.json) + // These have higher precedence than default values in ARG instructions + argsCopy := make([]string, len(args)) + copy(argsCopy, args) + + // Parse build args into a map for easy lookup + buildArgsMap := make(map[string]string) + for _, arg := range args { + if parts := strings.SplitN(arg, "=", 2); len(parts) == 2 { + buildArgsMap[parts[0]] = parts[1] + } + } + var imageRef string lines := strings.Split(dockerfileContent, "\n") - // Iterate over lines in reverse + // Iterate over lines in reverse to find ARG declarations and FROM instruction for i := len(lines) - 1; i >= 0; i-- { line := lines[i] if arg, ok := strings.CutPrefix(line, "ARG "); ok { arg = strings.TrimSpace(arg) if strings.Contains(arg, "=") { parts := strings.SplitN(arg, "=", 2) - key, _, err := lexer.ProcessWord(parts[0], shell.EnvsFromSlice(args)) + key, _, err := lexer.ProcessWord(parts[0], shell.EnvsFromSlice(argsCopy)) if err != nil { return nil, fmt.Errorf("processing %q: %w", line, err) } - val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(args)) - if err != nil { - return nil, fmt.Errorf("processing %q: %w", line, err) + + // Only use the default value if no build arg was provided + if _, exists := buildArgsMap[key]; !exists { + val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(argsCopy)) + if err != nil { + return nil, fmt.Errorf("processing %q: %w", line, err) + } + argsCopy = append(argsCopy, key+"="+val) } - args = append(args, key+"="+val) } continue } @@ -421,7 +495,7 @@ func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) { if imageRef == "" { return nil, fmt.Errorf("no FROM directive found") } - imageRef, _, err := lexer.ProcessWord(imageRef, shell.EnvsFromSlice(args)) + imageRef, _, err := lexer.ProcessWord(imageRef, shell.EnvsFromSlice(argsCopy)) if err != nil { return nil, fmt.Errorf("processing %q: %w", imageRef, err) } diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 9582c99..6f9cd23 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -224,13 +224,76 @@ func TestImageFromDockerfile(t *testing.T) { func TestImageFromDockerfileWithArgs(t *testing.T) { t.Parallel() for _, tc := range []struct { + default_image string + content string + image string + }{{ + default_image: "mcr.microsoft.com/devcontainers/python:1-3-bookworm", + content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }, { + default_image: "mcr.microsoft.com/devcontainers/python:1-3.10", + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }, { + default_image: "mcr.microsoft.com/devcontainers/python:1-3.10", + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT\nUSER app", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }} { + tc := tc + t.Run(tc.image, func(t *testing.T) { + t.Parallel() + dc := &devcontainer.Spec{ + Build: devcontainer.BuildSpec{ + Dockerfile: "Dockerfile", + Context: ".", + Args: map[string]string{ + "VARIANT": "3.11-bookworm", + }, + }, + } + fs := memfs.New() + dcDir := "/workspaces/coder/.devcontainer" + err := fs.MkdirAll(dcDir, 0o755) + require.NoError(t, err) + file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) + require.NoError(t, err) + _, err = io.WriteString(file, tc.content) + require.NoError(t, err) + _ = file.Close() + params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) + require.NoError(t, err) + require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) + require.Equal(t, params.DockerfileContent, tc.content) + ref, err := devcontainer.ImageFromDockerfile(tc.content, params.BuildArgs) + require.NoError(t, err) + require.Equal(t, tc.image, ref.Name()) + // Test without args (using defaults) + fmt.Println("Testing ImageFromDockerfile without args...") + ref1, err := devcontainer.ImageFromDockerfile(tc.content) + require.NoError(t, err) + require.Equal(t, tc.default_image, ref1.Name()) + }) + } +} + +func TestUserFromDockerfileWithArgs(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + user string content string image string }{{ + user: "root", content: "ARG VARIANT=3-bookworm\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}", image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", }, { - content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT ", + user: "root", + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT", + image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", + }, { + user: "app", + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:1-$VARIANT\nUSER app", image: "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", }} { tc := tc @@ -258,9 +321,17 @@ func TestImageFromDockerfileWithArgs(t *testing.T) { require.NoError(t, err) require.Equal(t, "VARIANT=3.11-bookworm", params.BuildArgs[0]) require.Equal(t, params.DockerfileContent, tc.content) - ref, err := devcontainer.ImageFromDockerfile(tc.content) + // Test UserFromDockerfile without args + fmt.Println("\nTesting UserFromDockerfile without args...") + user1, err := devcontainer.UserFromDockerfile(tc.content) require.NoError(t, err) - require.Equal(t, tc.image, ref.Name()) + require.Equal(t, tc.user, user1) + // Test UserFromDockerfile with args + fmt.Println("\nTesting UserFromDockerfile with args...") + user2, err := devcontainer.UserFromDockerfile(tc.content, params.BuildArgs) + require.NoError(t, err) + require.Equal(t, tc.user, user2) + }) } } diff --git a/envbuilder.go b/envbuilder.go index ad2c84f..df90227 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -493,7 +493,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro defer cleanupBuildContext() if runtimeData.Built && opts.SkipRebuild { endStage := startStage("🏗️ Skipping build because of cache...") - imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent) + imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent, buildParams.BuildArgs) if err != nil { return nil, fmt.Errorf("image from dockerfile: %w", err) } From 85a20eb7028631bb4cab43fa1a607272693b8d38 Mon Sep 17 00:00:00 2001 From: twsl <45483159+twsl@users.noreply.github.com> Date: Sat, 24 May 2025 21:38:58 +0000 Subject: [PATCH 8/8] Fix format --- devcontainer/devcontainer_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 6f9cd23..650703c 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -331,7 +331,6 @@ func TestUserFromDockerfileWithArgs(t *testing.T) { user2, err := devcontainer.UserFromDockerfile(tc.content, params.BuildArgs) require.NoError(t, err) require.Equal(t, tc.user, user2) - }) } }