Repository: coder/envbuilder
Pull request #205 (2024-05-22 11:15:40)

fix: use correct ref and path for features when useBuildContexts is true
This PR fixes an unused functionality for using build context for dev container features that was behind a boolean flag.

The functionality can be enabled like this:

```diff
diff --git envbuilder.go envbuilder.go
index 7ea7791..b8c847a 100644
--- envbuilder.go
+++ envbuilder.go
@@ -290,7 +290,7 @@ func Run(ctx context.Context, options Options) error {
 					options.Logger(notcodersdk.LogLevelInfo, "No Dockerfile or image specified; falling back to the default image...")
 					fallbackDockerfile = defaultParams.DockerfilePath
 				}
-				buildParams, err = devContainer.Compile(options.Filesystem, devcontainerDir, MagicDir, fallbackDockerfile, options.WorkspaceFolder, false)
+				buildParams, err = devContainer.Compile(options.Filesystem, devcontainerDir, MagicDir, fallbackDockerfile, options.WorkspaceFolder, true)
 				if err != nil {
 					return fmt.Errorf("compile devcontainer.json: %w", err)
 				}
```

It's a bit unclear to me whether or not we will need to use this code-path. It depends if feature files should be available in the image or not, in which case the `RUN --mount` would need to become a `COPY`. Since I investigated this though, I decided to push the fix as well.



Patch:
diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go
index 9fd2740..0454440 100644
--- a/devcontainer/devcontainer.go
+++ b/devcontainer/devcontainer.go
@@ -289,7 +289,7 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
 		if err != nil {
 			return "", nil, fmt.Errorf("extract feature %s: %w", featureRefRaw, err)
 		}
-		fromDirective, directive, err := spec.Compile(featureName, containerUser, remoteUser, useBuildContexts, featureOpts)
+		fromDirective, directive, err := spec.Compile(featureRef, featureName, featureDir, containerUser, remoteUser, useBuildContexts, featureOpts)
 		if err != nil {
 			return "", nil, fmt.Errorf("compile feature %s: %w", featureRefRaw, err)
 		}
diff --git a/devcontainer/features/features.go b/devcontainer/features/features.go
index 07f346e..bbea072 100644
--- a/devcontainer/features/features.go
+++ b/devcontainer/features/features.go
@@ -162,7 +162,6 @@ func Extract(fs billy.Filesystem, devcontainerDir, directory, reference string)
 		return nil, errors.New(`devcontainer-feature.json: name is required`)
 	}
 
-	spec.Directory = directory
 	return spec, nil
 }
 
@@ -188,13 +187,11 @@ type Spec struct {
 	Keywords         []string          `json:"keywords"`
 	Options          map[string]Option `json:"options"`
 	ContainerEnv     map[string]string `json:"containerEnv"`
-
-	Directory string `json:"-"`
 }
 
 // Extract unpacks the feature from the image and returns a set of lines
 // that should be appended to a Dockerfile to install the feature.
-func (s *Spec) Compile(featureName, containerUser, remoteUser string, useBuildContexts bool, options map[string]any) (string, string, error) {
+func (s *Spec) Compile(featureRef, featureName, featureDir, containerUser, remoteUser string, useBuildContexts bool, options map[string]any) (string, string, error) {
 	// TODO not sure how we figure out _(REMOTE|CONTAINER)_USER_HOME
 	// as per the feature spec.
 	// See https://containers.dev/implementors/features/#user-env-var
@@ -221,8 +218,8 @@ func (s *Spec) Compile(featureName, containerUser, remoteUser string, useBuildCo
 	sort.Strings(runDirective)
 	// See https://containers.dev/implementors/features/#invoking-installsh
 	if useBuildContexts {
-		fromDirective = "FROM scratch AS envbuilder_feature_" + featureName + "\nCOPY --from=" + featureName + " / /\n"
-		runDirective = append([]string{"RUN", "--mount=type=bind,from=envbuilder_feature_" + featureName + ",target=/envbuilder-features/" + featureName + ",rw"}, runDirective...)
+		fromDirective = "FROM scratch AS envbuilder_feature_" + featureName + "\nCOPY --from=" + featureRef + " / /\n"
+		runDirective = append([]string{"RUN", "--mount=type=bind,from=envbuilder_feature_" + featureName + ",target=" + featureDir + ",rw"}, runDirective...)
 	} else {
 		runDirective = append([]string{"RUN"}, runDirective...)
 	}
@@ -242,11 +239,7 @@ func (s *Spec) Compile(featureName, containerUser, remoteUser string, useBuildCo
 	if comment != "" {
 		lines = append(lines, comment)
 	}
-	if useBuildContexts {
-		lines = append(lines, "WORKDIR /envbuilder-features/"+featureName)
-	} else {
-		lines = append(lines, "WORKDIR "+s.Directory)
-	}
+	lines = append(lines, "WORKDIR "+featureDir)
 	envKeys := make([]string, 0, len(s.ContainerEnv))
 	for key := range s.ContainerEnv {
 		envKeys = append(envKeys, key)


Test patch:
diff --git a/devcontainer/features/features_test.go b/devcontainer/features/features_test.go
index b4d2fe8..0bef70b 100644
--- a/devcontainer/features/features_test.go
+++ b/devcontainer/features/features_test.go
@@ -73,54 +73,48 @@ func TestCompile(t *testing.T) {
 	t.Run("UnknownOption", func(t *testing.T) {
 		t.Parallel()
 		spec := &features.Spec{}
-		_, _, err := spec.Compile("test", "containerUser", "remoteUser", false, map[string]any{
+		_, _, err := spec.Compile("coder/test:latest", "test", "", "containerUser", "remoteUser", false, map[string]any{
 			"unknown": "value",
 		})
 		require.ErrorContains(t, err, "unknown option")
 	})
 	t.Run("Basic", func(t *testing.T) {
 		t.Parallel()
-		spec := &features.Spec{
-			Directory: "/",
-		}
-		_, directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		spec := &features.Spec{}
+		_, directive, err := spec.Compile("coder/test:latest", "test", "/", "containerUser", "remoteUser", false, nil)
 		require.NoError(t, err)
 		require.Equal(t, "WORKDIR /\nRUN _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(directive))
 	})
 	t.Run("ContainerEnv", func(t *testing.T) {
 		t.Parallel()
 		spec := &features.Spec{
-			Directory: "/",
 			ContainerEnv: map[string]string{
 				"FOO": "bar",
 			},
 		}
-		_, directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		_, directive, err := spec.Compile("coder/test:latest", "test", "/", "containerUser", "remoteUser", false, nil)
 		require.NoError(t, err)
 		require.Equal(t, "WORKDIR /\nENV FOO=bar\nRUN _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(directive))
 	})
 	t.Run("OptionsEnv", func(t *testing.T) {
 		t.Parallel()
 		spec := &features.Spec{
-			Directory: "/",
 			Options: map[string]features.Option{
 				"foo": {
 					Default: "bar",
 				},
 			},
 		}
-		_, directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		_, directive, err := spec.Compile("coder/test:latest", "test", "/", "containerUser", "remoteUser", false, nil)
 		require.NoError(t, err)
 		require.Equal(t, "WORKDIR /\nRUN FOO=\"bar\" _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(directive))
 	})
 	t.Run("BuildContext", func(t *testing.T) {
 		t.Parallel()
-		spec := &features.Spec{
-			Directory: "/",
-		}
-		fromDirective, runDirective, err := spec.Compile("test", "containerUser", "remoteUser", true, nil)
+		spec := &features.Spec{}
+		fromDirective, runDirective, err := spec.Compile("coder/test:latest", "test", "/.envbuilder/feature/test-d8e8fc", "containerUser", "remoteUser", true, nil)
 		require.NoError(t, err)
-		require.Equal(t, "FROM scratch AS envbuilder_feature_test\nCOPY --from=test / /", strings.TrimSpace(fromDirective))
-		require.Equal(t, "WORKDIR /envbuilder-features/test\nRUN --mount=type=bind,from=envbuilder_feature_test,target=/envbuilder-features/test,rw _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(runDirective))
+		require.Equal(t, "FROM scratch AS envbuilder_feature_test\nCOPY --from=coder/test:latest / /", strings.TrimSpace(fromDirective))
+		require.Equal(t, "WORKDIR /.envbuilder/feature/test-d8e8fc\nRUN --mount=type=bind,from=envbuilder_feature_test,target=/.envbuilder/feature/test-d8e8fc,rw _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(runDirective))
 	})
 }