Skip to content

fix: always add COPY directives in DoCacheProbe #293

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

Merged
merged 2 commits into from
Aug 2, 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
20 changes: 20 additions & 0 deletions envbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,26 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
})
}

// We expect an image built and pushed by envbuilder to have the envbuilder
// binary present at a predefined path. In order to correctly replicate the
// build via executor.RunCacheProbe we need to have the *exact* copy of the
// envbuilder binary available used to build the image.
exePath := opts.BinaryPath
// Add an exception for the current running binary in kaniko ignore list
if err := util.AddAllowedPathToDefaultIgnoreList(exePath); err != nil {
return nil, xerrors.Errorf("add exe path to ignore list: %w", err)
}
// Copy the envbuilder binary into the build context.
buildParams.DockerfileContent += fmt.Sprintf(`
COPY --chmod=0755 %s %s
USER root
WORKDIR /
ENTRYPOINT [%q]`, exePath, exePath, exePath)
dst := filepath.Join(buildParams.BuildContext, exePath)
if err := copyFile(exePath, dst); err != nil {
return nil, xerrors.Errorf("copy envbuilder binary to build context: %w", err)
}

stdoutWriter, closeStdout := log.Writer(opts.Logger)
defer closeStdout()
stderrWriter, closeStderr := log.Writer(opts.Logger)
Expand Down
5 changes: 3 additions & 2 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1114,13 +1114,14 @@ RUN date --utc > /root/date.txt`, testImageAlpine),
_, err = remote.Image(ref)
require.ErrorContains(t, err, "MANIFEST_UNKNOWN", "expected image to not be present before build + push")

// Then: re-running envbuilder with GET_CACHED_IMAGE should succeed
// Then: re-running envbuilder with GET_CACHED_IMAGE should not succeed, as
// the envbuilder binary is not present in the pushed image.
_, err = runEnvbuilder(t, runOpts{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("GET_CACHED_IMAGE", "1"),
}})
require.NoError(t, err)
require.ErrorContains(t, err, "uncached COPY command is not supported in cache probe mode")
})

t.Run("CacheAndPush", func(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions options/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,7 @@ func (o *Options) SetDefaults() {
if o.WorkspaceFolder == "" {
o.WorkspaceFolder = DefaultWorkspaceFolder(o.GitURL)
}
if o.BinaryPath == "" {
o.BinaryPath = "/.envbuilder/bin/envbuilder"
}
}
1 change: 1 addition & 0 deletions options/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func TestOptions_SetDefaults(t *testing.T) {
Filesystem: chmodfs.New(osfs.New("/")),
GitURL: "",
WorkspaceFolder: constants.EmptyWorkspaceDir,
BinaryPath: "/.envbuilder/bin/envbuilder",
}

var actual options.Options
Expand Down
15 changes: 15 additions & 0 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ type Options struct {
// used to improving cache utilization when multiple users are building
// working on the same repository.
RemoteRepoBuildMode bool

// BinaryPath is the path to the local envbuilder binary when
// attempting to probe the build cache. This is only relevant when
// GetCachedImage is true.
BinaryPath string
}

const envPrefix = "ENVBUILDER_"
Expand Down Expand Up @@ -427,6 +432,13 @@ func (o *Options) CLI() serpent.OptionSet {
Description: "Print the digest of the cached image, if available. " +
"Exits with an error if not found.",
},
{
Flag: "binary-path",
Env: WithEnvPrefix("BINARY_PATH"),
Value: serpent.StringOf(&o.BinaryPath),
Hidden: true,
Description: "Specify the path to an Envbuilder binary for use when probing the build cache.",
},
{
Flag: "remote-repo-build-mode",
Env: WithEnvPrefix("REMOTE_REPO_BUILD_MODE"),
Expand Down Expand Up @@ -485,6 +497,9 @@ func (o *Options) Markdown() string {
_, _ = sb.WriteString("| - | - | - | - |\n")

for _, opt := range cliOptions {
if opt.Hidden {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: drive-by; I don't think hidden options should be documented.

continue
}
d := opt.Default
if d != "" {
d = "`" + d + "`"
Expand Down