From 6e437b151aea1a0e936732f1612a766e51c337e9 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Fri, 29 Mar 2024 19:46:17 -0700 Subject: [PATCH] Work around caching issue by copying extracted feature to an intermediate stage buildah treats bind mounts from a build context as uncacheable. --- devcontainer/devcontainer.go | 9 ++++++--- devcontainer/features/features.go | 10 ++++++---- devcontainer/features/features_test.go | 13 +++++++------ 3 files changed, 19 insertions(+), 13 deletions(-) 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 ( 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)) }) }