Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around caching issue by copying extracted feature to an intermediate stage #118

Merged
merged 1 commit into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions devcontainer/devcontainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions devcontainer/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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...)
}
Expand Down Expand Up @@ -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 (
Expand Down
13 changes: 7 additions & 6 deletions devcontainer/features/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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))
})
Expand All @@ -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))
})
Expand All @@ -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))
})
Expand All @@ -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))
})
}
Loading