fix: use correct ref and path for features when useBuildContexts is true (#205)

Author: mafredri

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

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)

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