Repository: coder/envbuilder
Pull request #118 (2024-03-30 03:53:31)

Work around caching issue by copying extracted feature to an intermediate stage
buildah treats bind mounts from a build context as uncacheable.


Patch:
diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go
index b7d4ad5..19642e3 100644
--- a/devcontainer/devcontainer.go
+++ b/devcontainer/devcontainer.go
@@ -245,6 +245,7 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
 	// is deterministic which allows for caching.
 	sort.Strings(featureOrder)
 
+	var lines []string
 	for _, featureRefRaw := range featureOrder {
 		var (
 			featureRef string
@@ -284,24 +285,26 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir
 		if err != nil {
 			return "", nil, fmt.Errorf("extract feature %s: %w", featureRefRaw, err)
 		}
-		directive, err := spec.Compile(featureName, containerUser, remoteUser, useBuildContexts, featureOpts)
+		fromDirective, directive, err := spec.Compile(featureName, containerUser, remoteUser, useBuildContexts, featureOpts)
 		if err != nil {
 			return "", nil, fmt.Errorf("compile feature %s: %w", featureRefRaw, err)
 		}
 		featureDirectives = append(featureDirectives, directive)
 		if useBuildContexts {
 			featureContexts[featureName] = featureDir
+			lines = append(lines, fromDirective)
 		}
 	}
 
-	lines := []string{"\nUSER root"}
+	lines = append(lines, dockerfileContent)
+	lines = append(lines, "\nUSER root")
 	lines = append(lines, featureDirectives...)
 	if remoteUser != "" {
 		// TODO: We should warn that because we were unable to find the remote user,
 		// we're going to run as root.
 		lines = append(lines, fmt.Sprintf("USER %s", remoteUser))
 	}
-	return strings.Join(append([]string{dockerfileContent}, lines...), "\n"), featureContexts, err
+	return strings.Join(lines, "\n"), featureContexts, err
 }
 
 // UserFromDockerfile inspects the contents of a provided Dockerfile
diff --git a/devcontainer/features/features.go b/devcontainer/features/features.go
index d3a4184..739211e 100644
--- a/devcontainer/features/features.go
+++ b/devcontainer/features/features.go
@@ -194,10 +194,11 @@ type Spec struct {
 
 // 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, error) {
+func (s *Spec) Compile(featureName, 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
+	var fromDirective string
 	runDirective := []string{
 		"_CONTAINER_USER=" + strconv.Quote(containerUser),
 		"_REMOTE_USER=" + strconv.Quote(remoteUser),
@@ -213,14 +214,15 @@ func (s *Spec) Compile(featureName, containerUser, remoteUser string, useBuildCo
 		runDirective = append(runDirective, fmt.Sprintf(`%s=%q`, convertOptionNameToEnv(key), strValue))
 	}
 	if len(options) > 0 {
-		return "", fmt.Errorf("unknown option: %v", options)
+		return "", "", fmt.Errorf("unknown option: %v", options)
 	}
 	// It's critical that the Dockerfile produced is deterministic,
 	// regardless of map iteration order.
 	sort.Strings(runDirective)
 	// See https://containers.dev/implementors/features/#invoking-installsh
 	if useBuildContexts {
-		runDirective = append([]string{"RUN", "--mount=type=bind,from=" + featureName + ",target=/envbuilder-features/" + featureName + ",rw"}, runDirective...)
+		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...)
 	} else {
 		runDirective = append([]string{"RUN"}, runDirective...)
 	}
@@ -257,7 +259,7 @@ func (s *Spec) Compile(featureName, containerUser, remoteUser string, useBuildCo
 	}
 	lines = append(lines, strings.Join(runDirective, " "))
 
-	return strings.Join(lines, "\n"), nil
+	return fromDirective, strings.Join(lines, "\n"), nil
 }
 
 var (


Test patch:
diff --git a/devcontainer/features/features_test.go b/devcontainer/features/features_test.go
index 03f8783..03a073b 100644
--- a/devcontainer/features/features_test.go
+++ b/devcontainer/features/features_test.go
@@ -73,7 +73,7 @@ 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("test", "containerUser", "remoteUser", false, map[string]any{
 			"unknown": "value",
 		})
 		require.ErrorContains(t, err, "unknown option")
@@ -83,7 +83,7 @@ func TestCompile(t *testing.T) {
 		spec := &features.Spec{
 			Directory: "/",
 		}
-		directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		_, directive, err := spec.Compile("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))
 	})
@@ -95,7 +95,7 @@ func TestCompile(t *testing.T) {
 				"FOO": "bar",
 			},
 		}
-		directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		_, directive, err := spec.Compile("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))
 	})
@@ -109,7 +109,7 @@ func TestCompile(t *testing.T) {
 				},
 			},
 		}
-		directive, err := spec.Compile("test", "containerUser", "remoteUser", false, nil)
+		_, directive, err := spec.Compile("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))
 	})
@@ -118,8 +118,9 @@ func TestCompile(t *testing.T) {
 		spec := &features.Spec{
 			Directory: "/",
 		}
-		directive, err := spec.Compile("test", "containerUser", "remoteUser", true, nil)
+		fromDirective, runDirective, err := spec.Compile("test", "containerUser", "remoteUser", true, nil)
 		require.NoError(t, err)
-		require.Equal(t, "WORKDIR /envbuilder-features/test\nRUN --mount=type=bind,from=test,target=/envbuilder-features/test,rw _CONTAINER_USER=\"containerUser\" _REMOTE_USER=\"remoteUser\" ./install.sh", strings.TrimSpace(directive))
+		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))
 	})
 }