Merge branch 'main' into 74-git-submodule-support

Author: bjornrobertsson

README.md

@@ -101,20 +101,5 @@ Issue: [#113](https://github.com/coder/envbuilder/issues/113)
 
 ## Contributing
 
-Building `envbuilder` currently **requires** a Linux system.
-
-On macOS or Windows systems, we recommend using a VM or the provided `.devcontainer` for development.
-
-**Additional Requirements:**
-
-- `go 1.22`
-- `make`
-- Docker daemon (for running tests)
-
-**Makefile targets:**
-
-- `build`: Builds and tags `envbuilder:latest` for your current architecture.
-- `develop`: Runs `envbuilder:latest` against a sample Git repository.
-- `test`: Runs tests.
-- `test-registry`: Stands up a local registry for caching images used in tests.
-- `docs/env-variables.md`: Updated the [environment variables documentation](./docs/env-variables.md).
+See [Development](./docs/development.md) for build instructions, dependency
+management notes, and common troubleshooting.

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, BuildArgsMap(params.BuildArgs))
 		if err != nil {
 			return nil, fmt.Errorf("user from dockerfile: %w", err)
 		}
@@ -306,14 +306,46 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
 	return strings.Join(lines, "\n"), featureContexts, err
 }
 
+// BuildArgsMap converts a slice of "KEY=VALUE" strings to a map.
+func BuildArgsMap(buildArgs []string) map[string]string {
+	m := make(map[string]string, len(buildArgs))
+	for _, arg := range buildArgs {
+		if key, val, ok := strings.Cut(arg, "="); ok {
+			m[key] = val
+		}
+	}
+	return m
+}
+
 // 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) {
+func UserFromDockerfile(dockerfileContent string, buildArgs map[string]string) (user string, err error) {
 	res, err := parser.Parse(strings.NewReader(dockerfileContent))
 	if err != nil {
 		return "", fmt.Errorf("parse dockerfile: %w", err)
 	}
 
+	// Collect ARG values (defaults + overrides from buildArgs) for
+	// substitution into FROM image refs.
+	lexer := shell.NewLex('\\')
+	var argEnvs []string
+	for _, child := range res.AST.Children {
+		if !strings.EqualFold(child.Value, "arg") || child.Next == nil {
+			continue
+		}
+		if key, val, ok := strings.Cut(child.Next.Value, "="); ok {
+			if override, has := buildArgs[key]; has {
+				val = override
+			}
+			argEnvs = append(argEnvs, key+"="+val)
+		} else {
+			arg := child.Next.Value
+			if val, has := buildArgs[arg]; has {
+				argEnvs = append(argEnvs, arg+"="+val)
+			}
+		}
+	}
+
 	// Parse stages and user commands to determine the relevant user
 	// from the final stage.
 	var (
@@ -330,6 +362,12 @@ func UserFromDockerfile(dockerfileContent string) (user string, err error) {
 
 		switch i := inst.(type) {
 		case *instructions.Stage:
+			// Substitute ARG values in the base image name.
+			baseName, _, err := lexer.ProcessWord(i.BaseName, shell.EnvsFromSlice(argEnvs))
+			if err != nil {
+				return "", fmt.Errorf("processing ARG substitution in FROM %q: %w", i.BaseName, err)
+			}
+			i.BaseName = baseName
 			stages = append(stages, i)
 			if i.Name != "" {
 				stageNames[i.Name] = i
@@ -388,7 +426,7 @@ 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) {
+func ImageFromDockerfile(dockerfileContent string, buildArgs map[string]string) (name.Reference, error) {
 	lexer := shell.NewLex('\\')
 	var args []string
 	var imageRef string
@@ -398,17 +436,25 @@ func ImageFromDockerfile(dockerfileContent string) (name.Reference, error) {
 		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))
+			if key, val, ok := strings.Cut(arg, "="); ok {
+				key, _, err := lexer.ProcessWord(key, shell.EnvsFromSlice(args))
 				if err != nil {
 					return nil, fmt.Errorf("processing %q: %w", line, err)
 				}
-				val, _, err := lexer.ProcessWord(parts[1], shell.EnvsFromSlice(args))
+				val, _, err := lexer.ProcessWord(val, shell.EnvsFromSlice(args))
 				if err != nil {
 					return nil, fmt.Errorf("processing %q: %w", line, err)
 				}
+				// Allow buildArgs to override Dockerfile ARG defaults.
+				if override, has := buildArgs[key]; has {
+					val = override
+				}
 				args = append(args, key+"="+val)
+			} else {
+				// ARG without a default — look up in buildArgs.
+				if val, has := buildArgs[arg]; has {
+					args = append(args, arg+"="+val)
+				}
 			}
 			continue
 		}

devcontainer/devcontainer_test.go

@@ -213,13 +213,61 @@ func TestImageFromDockerfile(t *testing.T) {
 		tc := tc
 		t.Run(tc.image, func(t *testing.T) {
 			t.Parallel()
-			ref, err := devcontainer.ImageFromDockerfile(tc.content)
+			ref, err := devcontainer.ImageFromDockerfile(tc.content, nil)
 			require.NoError(t, err)
 			require.Equal(t, tc.image, ref.Name())
 		})
 	}
 }
 
+func TestImageFromDockerfile_BuildArgs(t *testing.T) {
+	t.Parallel()
+
+	// Test that build args override ARG defaults.
+	t.Run("OverridesDefault", func(t *testing.T) {
+		t.Parallel()
+		content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}"
+		ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"})
+		require.NoError(t, err)
+		require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name())
+	})
+
+	// Test that build args supply values for ARGs without defaults.
+	t.Run("SuppliesArgWithoutDefault", func(t *testing.T) {
+		t.Parallel()
+		content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}"
+		ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"})
+		require.NoError(t, err)
+		require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name())
+	})
+}
+
+func TestUserFromDockerfile_BuildArgs(t *testing.T) {
+	t.Parallel()
+
+	t.Run("SubstitutesARGInFROM", func(t *testing.T) {
+		t.Parallel()
+		registry := registrytest.New(t)
+		image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{
+			Config: v1.Config{
+				User: "testuser",
+			},
+		}})
+		require.NoError(t, err)
+		ref := strings.TrimPrefix(registry, "http://") + "/coder/test:latest"
+		parsed, err := name.ParseReference(ref)
+		require.NoError(t, err)
+		err = remote.Write(parsed, image)
+		require.NoError(t, err)
+
+		// Dockerfile uses ARG without default for the image ref.
+		content := fmt.Sprintf("ARG TAG\nFROM %s/coder/test:${TAG}", strings.TrimPrefix(registry, "http://"))
+		user, err := devcontainer.UserFromDockerfile(content, map[string]string{"TAG": "latest"})
+		require.NoError(t, err)
+		require.Equal(t, "testuser", user)
+	})
+}
+
 func TestUserFrom(t *testing.T) {
 	t.Parallel()
 
@@ -287,7 +335,7 @@ func TestUserFrom(t *testing.T) {
 		for _, tt := range tests {
 			t.Run(tt.name, func(t *testing.T) {
 				t.Parallel()
-				user, err := devcontainer.UserFromDockerfile(tt.content)
+				user, err := devcontainer.UserFromDockerfile(tt.content, nil)
 				require.NoError(t, err)
 				require.Equal(t, tt.user, user)
 			})
@@ -364,7 +412,7 @@ FROM a`,
 
 				content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test")
 
-				user, err := devcontainer.UserFromDockerfile(content)
+				user, err := devcontainer.UserFromDockerfile(content, nil)
 				require.NoError(t, err)
 				require.Equal(t, tt.user, user)
 			})

docs/development.md

@@ -0,0 +1,120 @@
+# Development
+
+Building envbuilder currently **requires** a Linux system.
+
+On macOS or Windows systems, we recommend using a VM or the provided
+`.devcontainer` for development.
+
+## Requirements
+
+- Go (see the version in `go.mod`)
+- `make`
+- Docker daemon (for running tests)
+
+## Makefile targets
+
+- `build`: Builds and tags `envbuilder:latest` for your current architecture.
+- `develop`: Runs `envbuilder:latest` against a sample Git repository.
+- `test`: Runs tests.
+- `test-registry`: Stands up a local registry for caching images used in tests.
+- `docs/env-variables.md`: Updates the environment variables documentation.
+
+## Dependency management
+
+Envbuilder has several forked and interrelated dependencies that require care
+when upgrading. This section documents known constraints and pitfalls.
+
+### Kaniko
+
+Envbuilder uses a [fork of Kaniko](https://github.com/coder/kaniko) for
+container image building. The replace directive in `go.mod` points to this fork:
+
+```
+replace github.com/GoogleContainerTools/kaniko => github.com/coder/kaniko <version>
+```
+
+The Kaniko fork pins its own versions of `docker/docker`,
+`containerd/containerd`, and related packages. When upgrading deps, be aware
+that the versions resolved by Go's MVS (minimum version selection) may be higher
+than what the Kaniko fork expects if other dependencies (like `coder/coder/v2`)
+require newer versions.
+
+### Tailscale
+
+Envbuilder uses a [Coder fork of Tailscale](https://github.com/coder/tailscale)
+via a replace directive:
+
+```
+replace tailscale.com => github.com/coder/tailscale <version>
+```
+
+The `coder/coder/v2` module depends on symbols that only exist in this fork
+(e.g. `netns.SetCoderSoftIsolation`, `tsaddr.CoderServiceIPv6`). When upgrading
+`coder/coder/v2`, you **must** also update the Tailscale replace to match the
+version used in `coder/coder/v2`'s own `go.mod`. You can find it with:
+
+```bash
+grep 'replace tailscale.com' /path/to/coder/coder/go.mod
+```
+
+### vishvananda/netlink and tailscale/netlink
+
+`tailscale/netlink` is a fork of `vishvananda/netlink` from 2021. Starting from
+`vishvananda/netlink` v1.3.0, the `XfrmAddress.ToIPNet` method gained an
+additional `family uint16` parameter, which breaks `tailscale/netlink`.
+
+If a transitive dependency (e.g. `containerd/containerd/v2`) pulls in
+`vishvananda/netlink` >= v1.3.0, you will see build errors like:
+
+```
+not enough arguments in call to msg.Sel.Daddr.ToIPNet
+    have (uint8)
+    want (uint8, uint16)
+```
+
+The fix is to add `exclude` directives in `go.mod` for the incompatible
+versions, forcing Go to select v1.2.x:
+
+```
+exclude (
+    github.com/vishvananda/netlink v1.3.0
+    github.com/vishvananda/netlink v1.3.1-0.20250303224720-0e7078ed04c8
+)
+```
+
+You may need to add additional exclude directives if new versions are released.
+
+### moby/go-archive
+
+`docker/docker` (the `+incompatible` module) imports `github.com/moby/go-archive`
+and expects certain symbols (`archive.Uncompressed`, `archive.Compression`) at the
+package root. In `moby/go-archive` v0.2.0, these were moved to a `compression`
+subpackage and the top-level aliases were removed.
+
+If you see errors like:
+
+```
+undefined: archive.Uncompressed
+undefined: archive.Compression
+```
+
+Pin `moby/go-archive` to v0.1.0 in `go.mod`:
+
+```bash
+go mod edit -require 'github.com/moby/go-archive@v0.1.0'
+go mod tidy
+```
+
+### General upgrade workflow
+
+When upgrading `coder/coder/v2` or other major dependencies:
+
+1. Update the dependency version in `go.mod`.
+2. Compare replace directives (especially `tailscale.com`) against the
+   upstream module's `go.mod` and update to match.
+3. Run `go mod tidy`.
+4. Run `go build ./...` and check for compilation errors.
+5. If you see errors from transitive dependencies, check whether version
+   conflicts can be resolved with `exclude` directives or by pinning
+   specific versions with `go mod edit -require`.
+6. Run `make test` to verify everything works end-to-end.

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, devcontainer.BuildArgsMap(buildParams.BuildArgs))
 				if err != nil {
 					return nil, fmt.Errorf("image from dockerfile: %w", err)
 				}
@@ -812,10 +812,16 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
 		// We need to change the ownership of the files to the user that will
 		// be running the init script.
 		if chownErr := filepath.Walk(opts.WorkspaceFolder, func(path string, _ os.FileInfo, err error) error {
+			if errors.Is(err, fs.ErrNotExist) {
+				return nil
+			}
 			if err != nil {
 				return err
 			}
-			return os.Lchown(path, execArgs.UserInfo.uid, execArgs.UserInfo.gid)
+			if err := os.Lchown(path, execArgs.UserInfo.uid, execArgs.UserInfo.gid); err != nil && !errors.Is(err, fs.ErrNotExist) {
+				return err
+			}
+			return nil
 		}); chownErr != nil {
 			opts.Logger(log.LevelError, "chown %q: %s", execArgs.UserInfo.user.HomeDir, chownErr.Error())
 			endStage("⚠️ Failed to the ownership of the workspace, you may need to fix this manually!")
@@ -829,10 +835,16 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
 	if execArgs.UserInfo.uid != 0 {
 		endStage := startStage("🔄 Updating ownership of %s...", execArgs.UserInfo.user.HomeDir)
 		if chownErr := filepath.Walk(execArgs.UserInfo.user.HomeDir, func(path string, _ fs.FileInfo, err error) error {
+			if errors.Is(err, fs.ErrNotExist) {
+				return nil
+			}
 			if err != nil {
 				return err
 			}
-			return os.Lchown(path, execArgs.UserInfo.uid, execArgs.UserInfo.gid)
+			if err := os.Lchown(path, execArgs.UserInfo.uid, execArgs.UserInfo.gid); err != nil && !errors.Is(err, fs.ErrNotExist) {
+				return err
+			}
+			return nil
 		}); chownErr != nil {
 			opts.Logger(log.LevelError, "chown %q: %s", execArgs.UserInfo.user.HomeDir, chownErr.Error())
 			endStage("⚠️ Failed to update ownership of %s, you may need to fix this manually!", execArgs.UserInfo.user.HomeDir)