From 56be370ade59ada36d2f9ed2a9cbeec245e32339 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 6 Sep 2024 17:04:11 +0100 Subject: [PATCH 01/13] fix: allow setting MagicDir in Options --- constants/constants.go | 80 ++++++++++++----------- envbuilder.go | 99 ++++++++++++++-------------- integration/integration_test.go | 110 +++++++++++++++++++++++++++++++- options/options.go | 8 ++- 4 files changed, 208 insertions(+), 89 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index fefa1394..e8df15fa 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -15,34 +15,10 @@ const ( // nothing going on... it's empty! EmptyWorkspaceDir = WorkspacesDir + "/empty" - // MagicDir is where all envbuilder related files are stored. - // This is a special directory that must not be modified - // by the user or images. - MagicDir = "/.envbuilder" -) - -var ( - ErrNoFallbackImage = errors.New("no fallback image has been specified") - - // MagicFile is a file that is created in the workspace - // when envbuilder has already been run. This is used - // to skip building when a container is restarting. - // e.g. docker stop -> docker start - MagicFile = filepath.Join(MagicDir, "built") - - // MagicFile is the location of the build context when - // using remote build mode. - MagicRemoteRepoDir = filepath.Join(MagicDir, "repo") - - // MagicBinaryLocation is the expected location of the envbuilder binary - // inside a builder image. - MagicBinaryLocation = filepath.Join(MagicDir, "bin", "envbuilder") - - // MagicImage is a file that is created in the image when - // envbuilder has already been run. This is used to skip - // the destructive initial build step when 'resuming' envbuilder - // from a previously built image. - MagicImage = filepath.Join(MagicDir, "image") + // defaultMagicDir is the default working location for envbuilder. + // This is a special directory that must not be modified by the user + // or images. This is intentionally unexported. + defaultMagicDir = "/.envbuilder" // MagicTempDir is a directory inside the build context inside which // we place files referenced by MagicDirectives. @@ -51,14 +27,46 @@ var ( // MagicDirectives are directives automatically appended to Dockerfiles // when pushing the image. These directives allow the built image to be // 're-used'. - MagicDirectives = fmt.Sprintf(` -COPY --chmod=0755 %[1]s %[2]s -COPY --chmod=0644 %[3]s %[4]s + MagicDirectives = ` +COPY --chmod=0755 .envbuilder.tmp/envbuilder /.envbuilder/bin/envbuilder +COPY --chmod=0644 .envbuilder.tmp/image /.envbuilder/image USER root WORKDIR / -ENTRYPOINT [%[2]q] -`, - ".envbuilder.tmp/envbuilder", MagicBinaryLocation, - ".envbuilder.tmp/image", MagicImage, - ) +ENTRYPOINT ["/.envbuilder/bin/envbuilder"] +` ) + +// ErrNoFallbackImage is returned when no fallback image has been specified. +var ErrNoFallbackImage = errors.New("no fallback image has been specified") + +// MagicDir is a working directory for envbuilder. We use this to +// store files that are used when building images. +type MagicDir string + +// String returns the string representation of the MagicDir. +func (m MagicDir) String() string { + if m == "" { + // Instead of the zero value, use defaultMagicDir. + return defaultMagicDir + } + return filepath.Join("/", string(m)) +} + +// MagicDir implements fmt.Stringer. +var _ fmt.Stringer = MagicDir("") + +// MagicFile is a file that is created in the workspace +// when envbuilder has already been run. This is used +// to skip building when a container is restarting. +// e.g. docker stop -> docker start +func (m MagicDir) Built() string { + return filepath.Join(m.String(), "built") +} + +// MagicImage is a file that is created in the image when +// envbuilder has already been run. This is used to skip +// the destructive initial build step when 'resuming' envbuilder +// from a previously built image. +func (m MagicDir) Image() string { + return filepath.Join(m.String(), "image") +} diff --git a/envbuilder.go b/envbuilder.go index 7f3c983a..109ee2f6 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -92,7 +92,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version()) - cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.DockerConfigBase64) + cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, opts.MagicDir, opts.DockerConfigBase64) if err != nil { return err } @@ -168,7 +168,7 @@ func Run(ctx context.Context, opts options.Options) error { } defaultBuildParams := func() (*devcontainer.Compiled, error) { - dockerfile := filepath.Join(constants.MagicDir, "Dockerfile") + dockerfile := filepath.Join(opts.MagicDir.String(), "Dockerfile") file, err := opts.Filesystem.OpenFile(dockerfile, os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { return nil, err @@ -190,7 +190,7 @@ func Run(ctx context.Context, opts options.Options) error { return &devcontainer.Compiled{ DockerfilePath: dockerfile, DockerfileContent: content, - BuildContext: constants.MagicDir, + BuildContext: opts.MagicDir.String(), }, nil } @@ -232,7 +232,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "No Dockerfile or image specified; falling back to the default image...") fallbackDockerfile = defaultParams.DockerfilePath } - buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, constants.MagicDir, fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) + buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, opts.MagicDir.String(), fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) if err != nil { return fmt.Errorf("compile devcontainer.json: %w", err) } @@ -304,7 +304,7 @@ func Run(ctx context.Context, opts options.Options) error { // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - constants.MagicDir, + opts.MagicDir.String(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", @@ -332,31 +332,26 @@ func Run(ctx context.Context, opts options.Options) error { if err := util.AddAllowedPathToDefaultIgnoreList(opts.BinaryPath); err != nil { return fmt.Errorf("add envbuilder binary to ignore list: %w", err) } - if err := util.AddAllowedPathToDefaultIgnoreList(constants.MagicImage); err != nil { + if err := util.AddAllowedPathToDefaultIgnoreList(opts.MagicDir.Image()); err != nil { return fmt.Errorf("add magic image file to ignore list: %w", err) } - magicTempDir := filepath.Join(buildParams.BuildContext, constants.MagicTempDir) - if err := opts.Filesystem.MkdirAll(magicTempDir, 0o755); err != nil { + magicTempDir := constants.MagicDir(filepath.Join(buildParams.BuildContext, constants.MagicTempDir)) + if err := opts.Filesystem.MkdirAll(magicTempDir.String(), 0o755); err != nil { return fmt.Errorf("create magic temp dir in build context: %w", err) } // Add the magic directives that embed the binary into the built image. buildParams.DockerfileContent += constants.MagicDirectives // Copy the envbuilder binary into the build context. // External callers will need to specify the path to the desired envbuilder binary. - envbuilderBinDest := filepath.Join( - magicTempDir, - filepath.Base(constants.MagicBinaryLocation), - ) + envbuilderBinDest := filepath.Join(magicTempDir.String(), "envbuilder") // Also touch the magic file that signifies the image has been built! - magicImageDest := filepath.Join( - magicTempDir, - filepath.Base(constants.MagicImage), - ) + // magicImageDest := filepath.Join(magicTempDir, "image") + magicImageDest := magicTempDir.Image() // Clean up after build! var cleanupOnce sync.Once cleanupBuildContext = func() { cleanupOnce.Do(func() { - for _, path := range []string{magicImageDest, envbuilderBinDest, magicTempDir} { + for _, path := range []string{magicImageDest, envbuilderBinDest, magicTempDir.String()} { if err := opts.Filesystem.Remove(path); err != nil { opts.Logger(log.LevelWarn, "failed to clean up magic temp dir from build context: %w", err) } @@ -370,15 +365,14 @@ func Run(ctx context.Context, opts options.Options) error { return fmt.Errorf("copy envbuilder binary to build context: %w", err) } - opts.Logger(log.LevelDebug, "touching magic image file at %q in build context %q", magicImageDest, buildParams.BuildContext) + opts.Logger(log.LevelDebug, "touching magic image file at %q in build context %q", magicImageDest, magicTempDir) if err := touchFile(opts.Filesystem, magicImageDest, 0o755); err != nil { return fmt.Errorf("touch magic image file in build context: %w", err) } - } // temp move of all ro mounts - tempRemountDest := filepath.Join("/", constants.MagicDir, "mnt") + tempRemountDest := filepath.Join(opts.MagicDir.String(), "mnt") // ignorePrefixes is a superset of ignorePaths that we pass to kaniko's // IgnoreList. ignorePrefixes := append([]string{"/dev", "/proc", "/sys"}, ignorePaths...) @@ -399,8 +393,8 @@ func Run(ctx context.Context, opts options.Options) error { defer closeStderr() build := func() (v1.Image, error) { defer cleanupBuildContext() - _, alreadyBuiltErr := opts.Filesystem.Stat(constants.MagicFile) - _, isImageErr := opts.Filesystem.Stat(constants.MagicImage) + _, alreadyBuiltErr := opts.Filesystem.Stat(opts.MagicDir.Built()) + _, isImageErr := opts.Filesystem.Stat(opts.MagicDir.Image()) if (alreadyBuiltErr == nil && opts.SkipRebuild) || isImageErr == nil { endStage := startStage("🏗️ Skipping build because of cache...") imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent) @@ -545,7 +539,7 @@ func Run(ctx context.Context, opts options.Options) error { // Create the magic file to indicate that this build // has already been ran before! - file, err := opts.Filesystem.Create(constants.MagicFile) + file, err := opts.Filesystem.Create(opts.MagicDir.Built()) if err != nil { return fmt.Errorf("create magic file: %w", err) } @@ -752,7 +746,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "=== Running the setup command %q as the root user...", opts.SetupScript) envKey := "ENVBUILDER_ENV" - envFile := filepath.Join("/", constants.MagicDir, "environ") + envFile := filepath.Join(opts.MagicDir.String(), "environ") file, err := os.Create(envFile) if err != nil { return fmt.Errorf("create environ file: %w", err) @@ -876,7 +870,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version()) - cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.DockerConfigBase64) + cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, opts.MagicDir, opts.DockerConfigBase64) if err != nil { return nil, err } @@ -1080,7 +1074,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - constants.MagicDir, + opts.MagicDir.String(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", @@ -1103,29 +1097,25 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) // build via executor.RunCacheProbe we need to have the *exact* copy of the // envbuilder binary available used to build the image and we also need to // add the magic directives to the Dockerfile content. + // MAGICDIR buildParams.DockerfileContent += constants.MagicDirectives magicTempDir := filepath.Join(buildParams.BuildContext, constants.MagicTempDir) if err := opts.Filesystem.MkdirAll(magicTempDir, 0o755); err != nil { return nil, fmt.Errorf("create magic temp dir in build context: %w", err) } - envbuilderBinDest := filepath.Join( - magicTempDir, - filepath.Base(constants.MagicBinaryLocation), - ) + envbuilderBinDest := filepath.Join(magicTempDir, "envbuilder") // Copy the envbuilder binary into the build context. - opts.Logger(log.LevelDebug, "copying envbuilder binary at %q to build context %q", opts.BinaryPath, buildParams.BuildContext) + opts.Logger(log.LevelDebug, "copying envbuilder binary at %q to build context %q", opts.BinaryPath, envbuilderBinDest) if err := copyFile(opts.Filesystem, opts.BinaryPath, envbuilderBinDest, 0o755); err != nil { return nil, xerrors.Errorf("copy envbuilder binary to build context: %w", err) } - // Also touch the magic file that signifies the image has been built! - magicImageDest := filepath.Join( - magicTempDir, - filepath.Base(constants.MagicImage), - ) + // Also touch the magic file that signifies the image has been built!A + magicImageDest := filepath.Join(magicTempDir, "image") + opts.Logger(log.LevelDebug, "touching magic image file at %q in build context %q", magicImageDest, magicTempDir) if err := touchFile(opts.Filesystem, magicImageDest, 0o755); err != nil { - return nil, fmt.Errorf("touch magic image file in build context: %w", err) + return nil, fmt.Errorf("touch magic image file at %q: %w", magicImageDest, err) } defer func() { // Clean up after we're done! @@ -1417,21 +1407,24 @@ func findDevcontainerJSON(workspaceFolder string, options options.Options) (stri // maybeDeleteFilesystem wraps util.DeleteFilesystem with a guard to hopefully stop // folks from unwittingly deleting their entire root directory. func maybeDeleteFilesystem(logger log.Func, force bool) error { + // We always expect the magic directory to be set to the default, signifying that + // the user is running envbuilder in a container. + // If this is set to anything else we should bail out to prevent accidental data loss. + defaultMagicDir := constants.MagicDir("") kanikoDir, ok := os.LookupEnv("KANIKO_DIR") - if !ok || strings.TrimSpace(kanikoDir) != constants.MagicDir { - if force { - bailoutSecs := 10 - logger(log.LevelWarn, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE YOUR ROOT FILESYSTEM!") - logger(log.LevelWarn, "You have %d seconds to bail out!", bailoutSecs) - for i := bailoutSecs; i > 0; i-- { - logger(log.LevelWarn, "%d...", i) - <-time.After(time.Second) - } - } else { - logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", constants.MagicDir) + if !ok || strings.TrimSpace(kanikoDir) != defaultMagicDir.String() { + if !force { + logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", defaultMagicDir.String()) logger(log.LevelError, "To bypass this check, set FORCE_SAFE=true.") return errors.New("safety check failed") } + bailoutSecs := 10 + logger(log.LevelWarn, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE YOUR ROOT FILESYSTEM!") + logger(log.LevelWarn, "You have %d seconds to bail out!", bailoutSecs) + for i := bailoutSecs; i > 0; i-- { + logger(log.LevelWarn, "%d...", i) + <-time.After(time.Second) + } } return util.DeleteFilesystem() @@ -1469,13 +1462,13 @@ func touchFile(fs billy.Filesystem, dst string, mode fs.FileMode) error { return f.Close() } -func initDockerConfigJSON(dockerConfigBase64 string) (func() error, error) { +func initDockerConfigJSON(logf log.Func, magicDir constants.MagicDir, dockerConfigBase64 string) (func() error, error) { var cleanupOnce sync.Once noop := func() error { return nil } if dockerConfigBase64 == "" { return noop, nil } - cfgPath := filepath.Join(constants.MagicDir, "config.json") + cfgPath := filepath.Join(magicDir.String(), "config.json") decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64) if err != nil { return noop, fmt.Errorf("decode docker config: %w", err) @@ -1489,10 +1482,14 @@ func initDockerConfigJSON(dockerConfigBase64 string) (func() error, error) { if err != nil { return noop, fmt.Errorf("parse docker config: %w", err) } + for k := range configFile.AuthConfigs { + logf(log.LevelInfo, "Docker config contains auth for registry %q", k) + } err = os.WriteFile(cfgPath, decoded, 0o644) if err != nil { return noop, fmt.Errorf("write docker config: %w", err) } + logf(log.LevelInfo, "Wrote Docker config JSON to %s", cfgPath) cleanup := func() error { var cleanupErr error cleanupOnce.Do(func() { @@ -1501,7 +1498,7 @@ func initDockerConfigJSON(dockerConfigBase64 string) (func() error, error) { if !errors.Is(err, fs.ErrNotExist) { cleanupErr = fmt.Errorf("remove docker config: %w", cleanupErr) } - _, _ = fmt.Fprintf(os.Stderr, "failed to remove the Docker config secret file: %s\n", cleanupErr) + logf(log.LevelError, "failed to remove the Docker config secret file: %s", cleanupErr) } }) return cleanupErr diff --git a/integration/integration_test.go b/integration/integration_test.go index 43d728c2..fa0ccae1 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -365,7 +365,8 @@ func TestBuildFromDockerfile(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) // Verify that the Docker configuration secret file is removed - output = execContainer(t, ctr, "stat "+filepath.Join(constants.MagicDir, "config.json")) + configJSONContainerPath := filepath.Join(constants.MagicDir("").String(), "config.json") + output = execContainer(t, ctr, "stat "+configJSONContainerPath) require.Contains(t, output, "No such file or directory") } @@ -1157,6 +1158,7 @@ RUN date --utc > /root/date.txt`, testImageAlpine), envbuilderEnv("GIT_URL", srv.URL), envbuilderEnv("CACHE_REPO", testRepo), envbuilderEnv("GET_CACHED_IMAGE", "1"), + envbuilderEnv("VERBOSE", "1"), }}) require.ErrorContains(t, err, "error probing build cache: uncached RUN command") // Then: it should fail to build the image and nothing should be pushed @@ -1168,6 +1170,7 @@ RUN date --utc > /root/date.txt`, testImageAlpine), envbuilderEnv("GIT_URL", srv.URL), envbuilderEnv("CACHE_REPO", testRepo), envbuilderEnv("PUSH_IMAGE", "1"), + envbuilderEnv("VERBOSE", "1"), }}) require.NoError(t, err) @@ -1596,6 +1599,111 @@ COPY --from=prebuild /the-future/hello.txt /the-future/hello.txt require.Equal(t, "hello from the past\nhello from the future\n/the-past/hello.txt", strings.TrimSpace(out)) }) + t.Run("MultistageRunCopyMinimal", func(t *testing.T) { + t.Parallel() + + srv := gittest.CreateGitServer(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": fmt.Sprintf(` +FROM %[1]s AS a +RUN date > /date.txt +FROM %[1]s +COPY --from=a /date.txt /date.txt +`, testImageAlpine), + }, + }) + + // Given: an empty registry + testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{}) + testRepo := testReg + "/test" + ref, err := name.ParseReference(testRepo + ":latest") + require.NoError(t, err) + _, err = remote.Image(ref) + require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push") + + // When: we run envbuilder with GET_CACHED_IMAGE + _, err = runEnvbuilder(t, runOpts{env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("CACHE_REPO", testRepo), + envbuilderEnv("GET_CACHED_IMAGE", "1"), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("VERBOSE", "1"), + }}) + require.ErrorContains(t, err, "error probing build cache: uncached RUN command") + // Then: it should fail to build the image and nothing should be pushed + _, err = remote.Image(ref) + require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push") + + // When: we run envbuilder with PUSH_IMAGE set + _, err = runEnvbuilder(t, runOpts{env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("CACHE_REPO", testRepo), + envbuilderEnv("PUSH_IMAGE", "1"), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("VERBOSE", "1"), + }}) + require.NoError(t, err) + + // Then: the image should be pushed + _, err = remote.Image(ref) + require.NoError(t, err, "expected image to be present after build + push") + + // Then: re-running envbuilder with GET_CACHED_IMAGE should succeed + ctrID, err := runEnvbuilder(t, runOpts{env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("CACHE_REPO", testRepo), + envbuilderEnv("GET_CACHED_IMAGE", "1"), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("VERBOSE", "1"), + }}) + require.NoError(t, err) + + // Then: the cached image ref should be emitted in the container logs + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) + require.NoError(t, err) + defer cli.Close() + logs, err := cli.ContainerLogs(ctx, ctrID, container.LogsOptions{ + ShowStdout: true, + ShowStderr: true, + }) + require.NoError(t, err) + defer logs.Close() + logBytes, err := io.ReadAll(logs) + require.NoError(t, err) + require.Regexp(t, `ENVBUILDER_CACHED_IMAGE=(\S+)`, string(logBytes)) + + // When: we pull the image we just built + rc, err := cli.ImagePull(ctx, ref.String(), image.PullOptions{}) + require.NoError(t, err) + t.Cleanup(func() { _ = rc.Close() }) + _, err = io.ReadAll(rc) + require.NoError(t, err) + + // When: we run the image we just built + ctr, err := cli.ContainerCreate(ctx, &container.Config{ + Image: ref.String(), + Entrypoint: []string{"sleep", "infinity"}, + Labels: map[string]string{ + testContainerLabel: "true", + }, + }, nil, nil, nil, "") + require.NoError(t, err) + t.Cleanup(func() { + _ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{ + RemoveVolumes: true, + Force: true, + }) + }) + err = cli.ContainerStart(ctx, ctr.ID, container.StartOptions{}) + require.NoError(t, err) + + // Then: The files from the image are present. + out := execContainer(t, ctr.ID, "/bin/sh -c 'cat /date.txt'") + require.NotEmpty(t, strings.TrimSpace(out)) + }) + t.Run("MultistgeCacheMissAfterChange", func(t *testing.T) { t.Parallel() dockerfilePrebuildContents := fmt.Sprintf(` diff --git a/options/options.go b/options/options.go index d7bd66b3..9c84be80 100644 --- a/options/options.go +++ b/options/options.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "os" + "path/filepath" "strings" "github.com/coder/envbuilder/constants" @@ -166,6 +167,11 @@ type Options struct { // attempting to probe the build cache. This is only relevant when // GetCachedImage is true. BinaryPath string + + // MagicDir is the path to the directory where all envbuilder files should be + // stored. By default, this is set to `/.envbuilder`. This is intentionally + // excluded from the CLI options. + MagicDir constants.MagicDir } const envPrefix = "ENVBUILDER_" @@ -459,7 +465,7 @@ func (o *Options) CLI() serpent.OptionSet { Flag: "remote-repo-dir", Env: WithEnvPrefix("REMOTE_REPO_DIR"), Value: serpent.StringOf(&o.RemoteRepoDir), - Default: constants.MagicRemoteRepoDir, + Default: filepath.Join(constants.MagicDir("").String(), "repo"), Hidden: true, Description: "Specify the destination directory for the cloned repo when using remote repo build mode.", }, From 61029cf2106b1bb22ce87919c5863f443e167e27 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 6 Sep 2024 20:32:57 +0100 Subject: [PATCH 02/13] rm unnecessary test --- integration/integration_test.go | 105 -------------------------------- 1 file changed, 105 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index fa0ccae1..5433364b 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -1599,111 +1599,6 @@ COPY --from=prebuild /the-future/hello.txt /the-future/hello.txt require.Equal(t, "hello from the past\nhello from the future\n/the-past/hello.txt", strings.TrimSpace(out)) }) - t.Run("MultistageRunCopyMinimal", func(t *testing.T) { - t.Parallel() - - srv := gittest.CreateGitServer(t, gittest.Options{ - Files: map[string]string{ - "Dockerfile": fmt.Sprintf(` -FROM %[1]s AS a -RUN date > /date.txt -FROM %[1]s -COPY --from=a /date.txt /date.txt -`, testImageAlpine), - }, - }) - - // Given: an empty registry - testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{}) - testRepo := testReg + "/test" - ref, err := name.ParseReference(testRepo + ":latest") - require.NoError(t, err) - _, err = remote.Image(ref) - require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push") - - // When: we run envbuilder with GET_CACHED_IMAGE - _, err = runEnvbuilder(t, runOpts{env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("CACHE_REPO", testRepo), - envbuilderEnv("GET_CACHED_IMAGE", "1"), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - envbuilderEnv("VERBOSE", "1"), - }}) - require.ErrorContains(t, err, "error probing build cache: uncached RUN command") - // Then: it should fail to build the image and nothing should be pushed - _, err = remote.Image(ref) - require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push") - - // When: we run envbuilder with PUSH_IMAGE set - _, err = runEnvbuilder(t, runOpts{env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("CACHE_REPO", testRepo), - envbuilderEnv("PUSH_IMAGE", "1"), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - envbuilderEnv("VERBOSE", "1"), - }}) - require.NoError(t, err) - - // Then: the image should be pushed - _, err = remote.Image(ref) - require.NoError(t, err, "expected image to be present after build + push") - - // Then: re-running envbuilder with GET_CACHED_IMAGE should succeed - ctrID, err := runEnvbuilder(t, runOpts{env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("CACHE_REPO", testRepo), - envbuilderEnv("GET_CACHED_IMAGE", "1"), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - envbuilderEnv("VERBOSE", "1"), - }}) - require.NoError(t, err) - - // Then: the cached image ref should be emitted in the container logs - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) - require.NoError(t, err) - defer cli.Close() - logs, err := cli.ContainerLogs(ctx, ctrID, container.LogsOptions{ - ShowStdout: true, - ShowStderr: true, - }) - require.NoError(t, err) - defer logs.Close() - logBytes, err := io.ReadAll(logs) - require.NoError(t, err) - require.Regexp(t, `ENVBUILDER_CACHED_IMAGE=(\S+)`, string(logBytes)) - - // When: we pull the image we just built - rc, err := cli.ImagePull(ctx, ref.String(), image.PullOptions{}) - require.NoError(t, err) - t.Cleanup(func() { _ = rc.Close() }) - _, err = io.ReadAll(rc) - require.NoError(t, err) - - // When: we run the image we just built - ctr, err := cli.ContainerCreate(ctx, &container.Config{ - Image: ref.String(), - Entrypoint: []string{"sleep", "infinity"}, - Labels: map[string]string{ - testContainerLabel: "true", - }, - }, nil, nil, nil, "") - require.NoError(t, err) - t.Cleanup(func() { - _ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{ - RemoveVolumes: true, - Force: true, - }) - }) - err = cli.ContainerStart(ctx, ctr.ID, container.StartOptions{}) - require.NoError(t, err) - - // Then: The files from the image are present. - out := execContainer(t, ctr.ID, "/bin/sh -c 'cat /date.txt'") - require.NotEmpty(t, strings.TrimSpace(out)) - }) - t.Run("MultistgeCacheMissAfterChange", func(t *testing.T) { t.Parallel() dockerfilePrebuildContents := fmt.Sprintf(` From 4b3c6496019fc039540fe63a2d466cfff8175cbd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 15:34:50 +0100 Subject: [PATCH 03/13] capitalize log line Co-authored-by: Danny Kopping --- envbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envbuilder.go b/envbuilder.go index 109ee2f6..7e17d670 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1498,7 +1498,7 @@ func initDockerConfigJSON(logf log.Func, magicDir constants.MagicDir, dockerConf if !errors.Is(err, fs.ErrNotExist) { cleanupErr = fmt.Errorf("remove docker config: %w", cleanupErr) } - logf(log.LevelError, "failed to remove the Docker config secret file: %s", cleanupErr) + logf(log.LevelError, "Failed to remove the Docker config secret file: %s", cleanupErr) } }) return cleanupErr From 406bf81956d2d14eb65695f29be204eb1594e87d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:20:18 +0100 Subject: [PATCH 04/13] update comment on MagicDir Co-authored-by: Mathias Fredriksson --- constants/constants.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index e8df15fa..144cbf06 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -39,8 +39,8 @@ ENTRYPOINT ["/.envbuilder/bin/envbuilder"] // ErrNoFallbackImage is returned when no fallback image has been specified. var ErrNoFallbackImage = errors.New("no fallback image has been specified") -// MagicDir is a working directory for envbuilder. We use this to -// store files that are used when building images. +// MagicDir is a working directory for envbuilder. It +// will also be present in images built by envbuilder. type MagicDir string // String returns the string representation of the MagicDir. From 4d2833fabffa5dfc50bb6e576b9724742f77e5da Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 14:36:38 +0100 Subject: [PATCH 05/13] rename String() to Path() --- constants/constants.go | 9 +++------ envbuilder.go | 26 +++++++++++++------------- integration/integration_test.go | 2 +- options/options.go | 2 +- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index 144cbf06..98a7f41c 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -44,7 +44,7 @@ var ErrNoFallbackImage = errors.New("no fallback image has been specified") type MagicDir string // String returns the string representation of the MagicDir. -func (m MagicDir) String() string { +func (m MagicDir) Path() string { if m == "" { // Instead of the zero value, use defaultMagicDir. return defaultMagicDir @@ -52,15 +52,12 @@ func (m MagicDir) String() string { return filepath.Join("/", string(m)) } -// MagicDir implements fmt.Stringer. -var _ fmt.Stringer = MagicDir("") - // MagicFile is a file that is created in the workspace // when envbuilder has already been run. This is used // to skip building when a container is restarting. // e.g. docker stop -> docker start func (m MagicDir) Built() string { - return filepath.Join(m.String(), "built") + return filepath.Join(m.Path(), "built") } // MagicImage is a file that is created in the image when @@ -68,5 +65,5 @@ func (m MagicDir) Built() string { // the destructive initial build step when 'resuming' envbuilder // from a previously built image. func (m MagicDir) Image() string { - return filepath.Join(m.String(), "image") + return filepath.Join(m.Path(), "image") } diff --git a/envbuilder.go b/envbuilder.go index 7e17d670..625b63a1 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -168,7 +168,7 @@ func Run(ctx context.Context, opts options.Options) error { } defaultBuildParams := func() (*devcontainer.Compiled, error) { - dockerfile := filepath.Join(opts.MagicDir.String(), "Dockerfile") + dockerfile := filepath.Join(opts.MagicDir.Path(), "Dockerfile") file, err := opts.Filesystem.OpenFile(dockerfile, os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { return nil, err @@ -190,7 +190,7 @@ func Run(ctx context.Context, opts options.Options) error { return &devcontainer.Compiled{ DockerfilePath: dockerfile, DockerfileContent: content, - BuildContext: opts.MagicDir.String(), + BuildContext: opts.MagicDir.Path(), }, nil } @@ -232,7 +232,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "No Dockerfile or image specified; falling back to the default image...") fallbackDockerfile = defaultParams.DockerfilePath } - buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, opts.MagicDir.String(), fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) + buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, opts.MagicDir.Path(), fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) if err != nil { return fmt.Errorf("compile devcontainer.json: %w", err) } @@ -304,7 +304,7 @@ func Run(ctx context.Context, opts options.Options) error { // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - opts.MagicDir.String(), + opts.MagicDir.Path(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", @@ -336,14 +336,14 @@ func Run(ctx context.Context, opts options.Options) error { return fmt.Errorf("add magic image file to ignore list: %w", err) } magicTempDir := constants.MagicDir(filepath.Join(buildParams.BuildContext, constants.MagicTempDir)) - if err := opts.Filesystem.MkdirAll(magicTempDir.String(), 0o755); err != nil { + if err := opts.Filesystem.MkdirAll(magicTempDir.Path(), 0o755); err != nil { return fmt.Errorf("create magic temp dir in build context: %w", err) } // Add the magic directives that embed the binary into the built image. buildParams.DockerfileContent += constants.MagicDirectives // Copy the envbuilder binary into the build context. // External callers will need to specify the path to the desired envbuilder binary. - envbuilderBinDest := filepath.Join(magicTempDir.String(), "envbuilder") + envbuilderBinDest := filepath.Join(magicTempDir.Path(), "envbuilder") // Also touch the magic file that signifies the image has been built! // magicImageDest := filepath.Join(magicTempDir, "image") magicImageDest := magicTempDir.Image() @@ -351,7 +351,7 @@ func Run(ctx context.Context, opts options.Options) error { var cleanupOnce sync.Once cleanupBuildContext = func() { cleanupOnce.Do(func() { - for _, path := range []string{magicImageDest, envbuilderBinDest, magicTempDir.String()} { + for _, path := range []string{magicImageDest, envbuilderBinDest, magicTempDir.Path()} { if err := opts.Filesystem.Remove(path); err != nil { opts.Logger(log.LevelWarn, "failed to clean up magic temp dir from build context: %w", err) } @@ -372,7 +372,7 @@ func Run(ctx context.Context, opts options.Options) error { } // temp move of all ro mounts - tempRemountDest := filepath.Join(opts.MagicDir.String(), "mnt") + tempRemountDest := filepath.Join(opts.MagicDir.Path(), "mnt") // ignorePrefixes is a superset of ignorePaths that we pass to kaniko's // IgnoreList. ignorePrefixes := append([]string{"/dev", "/proc", "/sys"}, ignorePaths...) @@ -746,7 +746,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "=== Running the setup command %q as the root user...", opts.SetupScript) envKey := "ENVBUILDER_ENV" - envFile := filepath.Join(opts.MagicDir.String(), "environ") + envFile := filepath.Join(opts.MagicDir.Path(), "environ") file, err := os.Create(envFile) if err != nil { return fmt.Errorf("create environ file: %w", err) @@ -1074,7 +1074,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - opts.MagicDir.String(), + opts.MagicDir.Path(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", @@ -1412,9 +1412,9 @@ func maybeDeleteFilesystem(logger log.Func, force bool) error { // If this is set to anything else we should bail out to prevent accidental data loss. defaultMagicDir := constants.MagicDir("") kanikoDir, ok := os.LookupEnv("KANIKO_DIR") - if !ok || strings.TrimSpace(kanikoDir) != defaultMagicDir.String() { + if !ok || strings.TrimSpace(kanikoDir) != defaultMagicDir.Path() { if !force { - logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", defaultMagicDir.String()) + logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", defaultMagicDir.Path()) logger(log.LevelError, "To bypass this check, set FORCE_SAFE=true.") return errors.New("safety check failed") } @@ -1468,7 +1468,7 @@ func initDockerConfigJSON(logf log.Func, magicDir constants.MagicDir, dockerConf if dockerConfigBase64 == "" { return noop, nil } - cfgPath := filepath.Join(magicDir.String(), "config.json") + cfgPath := filepath.Join(magicDir.Path(), "config.json") decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64) if err != nil { return noop, fmt.Errorf("decode docker config: %w", err) diff --git a/integration/integration_test.go b/integration/integration_test.go index 5433364b..bd18b88c 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -365,7 +365,7 @@ func TestBuildFromDockerfile(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) // Verify that the Docker configuration secret file is removed - configJSONContainerPath := filepath.Join(constants.MagicDir("").String(), "config.json") + configJSONContainerPath := filepath.Join(constants.MagicDir("").Path(), "config.json") output = execContainer(t, ctr, "stat "+configJSONContainerPath) require.Contains(t, output, "No such file or directory") } diff --git a/options/options.go b/options/options.go index 9c84be80..64b5e0fb 100644 --- a/options/options.go +++ b/options/options.go @@ -465,7 +465,7 @@ func (o *Options) CLI() serpent.OptionSet { Flag: "remote-repo-dir", Env: WithEnvPrefix("REMOTE_REPO_DIR"), Value: serpent.StringOf(&o.RemoteRepoDir), - Default: filepath.Join(constants.MagicDir("").String(), "repo"), + Default: filepath.Join(constants.MagicDir("").Path(), "repo"), Hidden: true, Description: "Specify the destination directory for the cloned repo when using remote repo build mode.", }, From 0f0adc321eb5da41413ad1e19ef9b502d4b41322 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 14:36:51 +0100 Subject: [PATCH 06/13] use defaultMagicDir for MagicDirectives --- constants/constants.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index 98a7f41c..65ce17b3 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -23,22 +23,23 @@ const ( // MagicTempDir is a directory inside the build context inside which // we place files referenced by MagicDirectives. MagicTempDir = ".envbuilder.tmp" +) +var ( + // ErrNoFallbackImage is returned when no fallback image has been specified. + ErrNoFallbackImage = errors.New("no fallback image has been specified") // MagicDirectives are directives automatically appended to Dockerfiles // when pushing the image. These directives allow the built image to be // 're-used'. - MagicDirectives = ` -COPY --chmod=0755 .envbuilder.tmp/envbuilder /.envbuilder/bin/envbuilder -COPY --chmod=0644 .envbuilder.tmp/image /.envbuilder/image + MagicDirectives = fmt.Sprintf(` +COPY --chmod=0755 %[1]s/envbuilder %[2]s/bin/envbuilder +COPY --chmod=0644 %[1]s/image %[2]s/image USER root WORKDIR / -ENTRYPOINT ["/.envbuilder/bin/envbuilder"] -` +ENTRYPOINT ["%[2]s/bin/envbuilder"] +`, MagicTempDir, defaultMagicDir) ) -// ErrNoFallbackImage is returned when no fallback image has been specified. -var ErrNoFallbackImage = errors.New("no fallback image has been specified") - // MagicDir is a working directory for envbuilder. It // will also be present in images built by envbuilder. type MagicDir string From f7f70498cdf05d0f7ee5c2d6215a0bd7a7f6ab96 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 14:39:36 +0100 Subject: [PATCH 07/13] rm comment --- envbuilder.go | 1 - 1 file changed, 1 deletion(-) diff --git a/envbuilder.go b/envbuilder.go index 625b63a1..337fc1ae 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -345,7 +345,6 @@ func Run(ctx context.Context, opts options.Options) error { // External callers will need to specify the path to the desired envbuilder binary. envbuilderBinDest := filepath.Join(magicTempDir.Path(), "envbuilder") // Also touch the magic file that signifies the image has been built! - // magicImageDest := filepath.Join(magicTempDir, "image") magicImageDest := magicTempDir.Image() // Clean up after build! var cleanupOnce sync.Once From 88051f87c3c6c532975bd37c95d7ee7b2167776e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 21:56:41 +0100 Subject: [PATCH 08/13] make MagicDir un-stringable directly --- constants/constants.go | 40 +++++++++++++++++++++++---------- envbuilder.go | 8 +++---- integration/integration_test.go | 2 +- options/options.go | 3 +-- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index 65ce17b3..e8591e82 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -15,10 +15,10 @@ const ( // nothing going on... it's empty! EmptyWorkspaceDir = WorkspacesDir + "/empty" - // defaultMagicDir is the default working location for envbuilder. + // defaultMagicDirBase is the default working location for envbuilder. // This is a special directory that must not be modified by the user // or images. This is intentionally unexported. - defaultMagicDir = "/.envbuilder" + defaultMagicDirBase = "/.envbuilder" // MagicTempDir is a directory inside the build context inside which // we place files referenced by MagicDirectives. @@ -26,6 +26,10 @@ const ( ) var ( + // DefaultMagicDir is the default working directory for Envbuilder. + // This defaults to /.envbuilder. It should only be used when Envbuilder + // is known to be running as root inside a container. + DefaultMagicDir MagicDir // ErrNoFallbackImage is returned when no fallback image has been specified. ErrNoFallbackImage = errors.New("no fallback image has been specified") // MagicDirectives are directives automatically appended to Dockerfiles @@ -37,34 +41,46 @@ COPY --chmod=0644 %[1]s/image %[2]s/image USER root WORKDIR / ENTRYPOINT ["%[2]s/bin/envbuilder"] -`, MagicTempDir, defaultMagicDir) +`, MagicTempDir, defaultMagicDirBase) ) // MagicDir is a working directory for envbuilder. It // will also be present in images built by envbuilder. -type MagicDir string +type MagicDir struct { + base string +} + +// MagicDirAt returns a MagicDir rooted at filepath.Join(paths...) +func MagicDirAt(paths ...string) MagicDir { + return MagicDir{base: filepath.Join(paths...)} +} + +// Join returns the result of filepath.Join([m.Path, paths...]). +func (m MagicDir) Join(paths ...string) string { + return filepath.Join(append([]string{m.Path()}, paths...)...) +} // String returns the string representation of the MagicDir. func (m MagicDir) Path() string { - if m == "" { - // Instead of the zero value, use defaultMagicDir. - return defaultMagicDir + // Instead of the zero value, use defaultMagicDir. + if m.base == "" { + return defaultMagicDirBase } - return filepath.Join("/", string(m)) + return m.base } -// MagicFile is a file that is created in the workspace +// Built is a file that is created in the workspace // when envbuilder has already been run. This is used // to skip building when a container is restarting. // e.g. docker stop -> docker start func (m MagicDir) Built() string { - return filepath.Join(m.Path(), "built") + return m.Join("built") } -// MagicImage is a file that is created in the image when +// Image is a file that is created in the image when // envbuilder has already been run. This is used to skip // the destructive initial build step when 'resuming' envbuilder // from a previously built image. func (m MagicDir) Image() string { - return filepath.Join(m.Path(), "image") + return m.Join("image") } diff --git a/envbuilder.go b/envbuilder.go index 337fc1ae..0a7072d2 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -335,7 +335,7 @@ func Run(ctx context.Context, opts options.Options) error { if err := util.AddAllowedPathToDefaultIgnoreList(opts.MagicDir.Image()); err != nil { return fmt.Errorf("add magic image file to ignore list: %w", err) } - magicTempDir := constants.MagicDir(filepath.Join(buildParams.BuildContext, constants.MagicTempDir)) + magicTempDir := constants.MagicDirAt(buildParams.BuildContext, constants.MagicTempDir) if err := opts.Filesystem.MkdirAll(magicTempDir.Path(), 0o755); err != nil { return fmt.Errorf("create magic temp dir in build context: %w", err) } @@ -1409,11 +1409,11 @@ func maybeDeleteFilesystem(logger log.Func, force bool) error { // We always expect the magic directory to be set to the default, signifying that // the user is running envbuilder in a container. // If this is set to anything else we should bail out to prevent accidental data loss. - defaultMagicDir := constants.MagicDir("") + // defaultMagicDir := constants.MagicDir("") kanikoDir, ok := os.LookupEnv("KANIKO_DIR") - if !ok || strings.TrimSpace(kanikoDir) != defaultMagicDir.Path() { + if !ok || strings.TrimSpace(kanikoDir) != constants.DefaultMagicDir.Path() { if !force { - logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", defaultMagicDir.Path()) + logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", constants.DefaultMagicDir.Path()) logger(log.LevelError, "To bypass this check, set FORCE_SAFE=true.") return errors.New("safety check failed") } diff --git a/integration/integration_test.go b/integration/integration_test.go index bd18b88c..c4b29a5f 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -365,7 +365,7 @@ func TestBuildFromDockerfile(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) // Verify that the Docker configuration secret file is removed - configJSONContainerPath := filepath.Join(constants.MagicDir("").Path(), "config.json") + configJSONContainerPath := constants.DefaultMagicDir.Join("config.json") output = execContainer(t, ctr, "stat "+configJSONContainerPath) require.Contains(t, output, "No such file or directory") } diff --git a/options/options.go b/options/options.go index 64b5e0fb..def94cef 100644 --- a/options/options.go +++ b/options/options.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "fmt" "os" - "path/filepath" "strings" "github.com/coder/envbuilder/constants" @@ -465,7 +464,7 @@ func (o *Options) CLI() serpent.OptionSet { Flag: "remote-repo-dir", Env: WithEnvPrefix("REMOTE_REPO_DIR"), Value: serpent.StringOf(&o.RemoteRepoDir), - Default: filepath.Join(constants.MagicDir("").Path(), "repo"), + Default: constants.DefaultMagicDir.Join("repo"), Hidden: true, Description: "Specify the destination directory for the cloned repo when using remote repo build mode.", }, From fe05a7720ac21a0462b00a2c082b9c6e633d6dbf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:09:02 +0100 Subject: [PATCH 09/13] options.MagicDir -> options.MagicDirBase (string) --- constants/constants.go | 3 +++ envbuilder.go | 31 ++++++++++++++++++------------- options/options.go | 4 ++-- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index e8591e82..96a0967d 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -52,6 +52,9 @@ type MagicDir struct { // MagicDirAt returns a MagicDir rooted at filepath.Join(paths...) func MagicDirAt(paths ...string) MagicDir { + if len(paths) == 0 { + return MagicDir{} + } return MagicDir{base: filepath.Join(paths...)} } diff --git a/envbuilder.go b/envbuilder.go index 0a7072d2..bdda4cf9 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -68,6 +68,9 @@ func Run(ctx context.Context, opts options.Options) error { if opts.CacheRepo == "" && opts.PushImage { return fmt.Errorf("--cache-repo must be set when using --push-image") } + + magicDir := constants.MagicDirAt(opts.MagicDirBase) + // Default to the shell! initArgs := []string{"-c", opts.InitScript} if opts.InitArgs != "" { @@ -92,7 +95,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version()) - cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, opts.MagicDir, opts.DockerConfigBase64) + cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, magicDir, opts.DockerConfigBase64) if err != nil { return err } @@ -168,7 +171,7 @@ func Run(ctx context.Context, opts options.Options) error { } defaultBuildParams := func() (*devcontainer.Compiled, error) { - dockerfile := filepath.Join(opts.MagicDir.Path(), "Dockerfile") + dockerfile := magicDir.Join("Dockerfile") file, err := opts.Filesystem.OpenFile(dockerfile, os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { return nil, err @@ -190,7 +193,7 @@ func Run(ctx context.Context, opts options.Options) error { return &devcontainer.Compiled{ DockerfilePath: dockerfile, DockerfileContent: content, - BuildContext: opts.MagicDir.Path(), + BuildContext: magicDir.Path(), }, nil } @@ -232,7 +235,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "No Dockerfile or image specified; falling back to the default image...") fallbackDockerfile = defaultParams.DockerfilePath } - buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, opts.MagicDir.Path(), fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) + buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, magicDir.Path(), fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv) if err != nil { return fmt.Errorf("compile devcontainer.json: %w", err) } @@ -304,7 +307,7 @@ func Run(ctx context.Context, opts options.Options) error { // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - opts.MagicDir.Path(), + magicDir.Path(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", @@ -332,7 +335,7 @@ func Run(ctx context.Context, opts options.Options) error { if err := util.AddAllowedPathToDefaultIgnoreList(opts.BinaryPath); err != nil { return fmt.Errorf("add envbuilder binary to ignore list: %w", err) } - if err := util.AddAllowedPathToDefaultIgnoreList(opts.MagicDir.Image()); err != nil { + if err := util.AddAllowedPathToDefaultIgnoreList(magicDir.Image()); err != nil { return fmt.Errorf("add magic image file to ignore list: %w", err) } magicTempDir := constants.MagicDirAt(buildParams.BuildContext, constants.MagicTempDir) @@ -371,7 +374,7 @@ func Run(ctx context.Context, opts options.Options) error { } // temp move of all ro mounts - tempRemountDest := filepath.Join(opts.MagicDir.Path(), "mnt") + tempRemountDest := magicDir.Join("mnt") // ignorePrefixes is a superset of ignorePaths that we pass to kaniko's // IgnoreList. ignorePrefixes := append([]string{"/dev", "/proc", "/sys"}, ignorePaths...) @@ -392,8 +395,8 @@ func Run(ctx context.Context, opts options.Options) error { defer closeStderr() build := func() (v1.Image, error) { defer cleanupBuildContext() - _, alreadyBuiltErr := opts.Filesystem.Stat(opts.MagicDir.Built()) - _, isImageErr := opts.Filesystem.Stat(opts.MagicDir.Image()) + _, alreadyBuiltErr := opts.Filesystem.Stat(magicDir.Built()) + _, isImageErr := opts.Filesystem.Stat(magicDir.Image()) if (alreadyBuiltErr == nil && opts.SkipRebuild) || isImageErr == nil { endStage := startStage("🏗️ Skipping build because of cache...") imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent) @@ -538,7 +541,7 @@ func Run(ctx context.Context, opts options.Options) error { // Create the magic file to indicate that this build // has already been ran before! - file, err := opts.Filesystem.Create(opts.MagicDir.Built()) + file, err := opts.Filesystem.Create(magicDir.Built()) if err != nil { return fmt.Errorf("create magic file: %w", err) } @@ -745,7 +748,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Logger(log.LevelInfo, "=== Running the setup command %q as the root user...", opts.SetupScript) envKey := "ENVBUILDER_ENV" - envFile := filepath.Join(opts.MagicDir.Path(), "environ") + envFile := magicDir.Join("environ") file, err := os.Create(envFile) if err != nil { return fmt.Errorf("create environ file: %w", err) @@ -855,6 +858,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) return nil, fmt.Errorf("--cache-repo must be set when using --get-cached-image") } + magicDir := constants.MagicDirAt(opts.MagicDirBase) + stageNumber := 0 startStage := func(format string, args ...any) func(format string, args ...any) { now := time.Now() @@ -869,7 +874,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version()) - cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, opts.MagicDir, opts.DockerConfigBase64) + cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, magicDir, opts.DockerConfigBase64) if err != nil { return nil, err } @@ -1073,7 +1078,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) // So we add them to the default ignore list. See: // https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136 ignorePaths := append([]string{ - opts.MagicDir.Path(), + magicDir.Path(), opts.WorkspaceFolder, // See: https://github.com/coder/envbuilder/issues/37 "/etc/resolv.conf", diff --git a/options/options.go b/options/options.go index def94cef..87ac5b0f 100644 --- a/options/options.go +++ b/options/options.go @@ -167,10 +167,10 @@ type Options struct { // GetCachedImage is true. BinaryPath string - // MagicDir is the path to the directory where all envbuilder files should be + // MagicDirBase is the path to the directory where all envbuilder files should be // stored. By default, this is set to `/.envbuilder`. This is intentionally // excluded from the CLI options. - MagicDir constants.MagicDir + MagicDirBase string } const envPrefix = "ENVBUILDER_" From 0b0cdcdead5ab390c453d7340e6674ac0ce79b4c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:13:18 +0100 Subject: [PATCH 10/13] move EmptyWorkspaceDir to options --- constants/constants.go | 8 -------- options/defaults.go | 11 +++++++---- options/defaults_test.go | 7 +++---- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index 96a0967d..3868362d 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -7,14 +7,6 @@ import ( ) const ( - // WorkspacesDir is the path to the directory where - // all workspaces are stored by default. - WorkspacesDir = "/workspaces" - - // EmptyWorkspaceDir is the path to a workspace that has - // nothing going on... it's empty! - EmptyWorkspaceDir = WorkspacesDir + "/empty" - // defaultMagicDirBase is the default working location for envbuilder. // This is a special directory that must not be modified by the user // or images. This is intentionally unexported. diff --git a/options/defaults.go b/options/defaults.go index 42e48063..67709d25 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -7,24 +7,27 @@ import ( "github.com/go-git/go-billy/v5/osfs" giturls "github.com/chainguard-dev/git-urls" - "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/internal/chmodfs" ) +// EmptyWorkspaceDir is the path to a workspace that has +// nothing going on... it's empty! +var EmptyWorkspaceDir = "/workspaces/empty" + // DefaultWorkspaceFolder returns the default workspace folder // for a given repository URL. func DefaultWorkspaceFolder(repoURL string) string { if repoURL == "" { - return constants.EmptyWorkspaceDir + return EmptyWorkspaceDir } parsed, err := giturls.Parse(repoURL) if err != nil { - return constants.EmptyWorkspaceDir + return EmptyWorkspaceDir } name := strings.Split(parsed.Path, "/") hasOwnerAndRepo := len(name) >= 2 if !hasOwnerAndRepo { - return constants.EmptyWorkspaceDir + return EmptyWorkspaceDir } repo := strings.TrimSuffix(name[len(name)-1], ".git") return fmt.Sprintf("/workspaces/%s", repo) diff --git a/options/defaults_test.go b/options/defaults_test.go index 48783585..83854ebb 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" - "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/options" "github.com/stretchr/testify/require" ) @@ -44,7 +43,7 @@ func TestDefaultWorkspaceFolder(t *testing.T) { { name: "empty", gitURL: "", - expected: constants.EmptyWorkspaceDir, + expected: options.EmptyWorkspaceDir, }, } for _, tt := range successTests { @@ -70,7 +69,7 @@ func TestDefaultWorkspaceFolder(t *testing.T) { for _, tt := range invalidTests { t.Run(tt.name, func(t *testing.T) { dir := options.DefaultWorkspaceFolder(tt.invalidURL) - require.Equal(t, constants.EmptyWorkspaceDir, dir) + require.Equal(t, options.EmptyWorkspaceDir, dir) }) } } @@ -84,7 +83,7 @@ func TestOptions_SetDefaults(t *testing.T) { IgnorePaths: []string{"/var/run", "/product_uuid", "/product_name"}, Filesystem: chmodfs.New(osfs.New("/")), GitURL: "", - WorkspaceFolder: constants.EmptyWorkspaceDir, + WorkspaceFolder: options.EmptyWorkspaceDir, BinaryPath: "/.envbuilder/bin/envbuilder", } From 590155db37b47b4ba54514e7247de9df541f6dba Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:23:56 +0100 Subject: [PATCH 11/13] move ErrNoFallbackImage to envbuilder package --- constants/constants.go | 3 --- envbuilder.go | 11 +++++++---- integration/integration_test.go | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index 3868362d..4f368f6a 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -1,7 +1,6 @@ package constants import ( - "errors" "fmt" "path/filepath" ) @@ -22,8 +21,6 @@ var ( // This defaults to /.envbuilder. It should only be used when Envbuilder // is known to be running as root inside a container. DefaultMagicDir MagicDir - // ErrNoFallbackImage is returned when no fallback image has been specified. - ErrNoFallbackImage = errors.New("no fallback image has been specified") // MagicDirectives are directives automatically appended to Dockerfiles // when pushing the image. These directives allow the built image to be // 're-used'. diff --git a/envbuilder.go b/envbuilder.go index bdda4cf9..075d1f3c 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -52,6 +52,9 @@ import ( "golang.org/x/xerrors" ) +// ErrNoFallbackImage is returned when no fallback image has been specified. +var ErrNoFallbackImage = errors.New("no fallback image has been specified") + // DockerConfig represents the Docker configuration file. type DockerConfig configfile.ConfigFile @@ -179,11 +182,11 @@ func Run(ctx context.Context, opts options.Options) error { defer file.Close() if opts.FallbackImage == "" { if fallbackErr != nil { - return nil, xerrors.Errorf("%s: %w", fallbackErr.Error(), constants.ErrNoFallbackImage) + return nil, xerrors.Errorf("%s: %w", fallbackErr.Error(), ErrNoFallbackImage) } // We can't use errors.Join here because our tests // don't support parsing a multiline error. - return nil, constants.ErrNoFallbackImage + return nil, ErrNoFallbackImage } content := "FROM " + opts.FallbackImage _, err = file.Write([]byte(content)) @@ -958,11 +961,11 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) defer file.Close() if opts.FallbackImage == "" { if fallbackErr != nil { - return nil, fmt.Errorf("%s: %w", fallbackErr.Error(), constants.ErrNoFallbackImage) + return nil, fmt.Errorf("%s: %w", fallbackErr.Error(), ErrNoFallbackImage) } // We can't use errors.Join here because our tests // don't support parsing a multiline error. - return nil, constants.ErrNoFallbackImage + return nil, ErrNoFallbackImage } content := "FROM " + opts.FallbackImage _, err = file.Write([]byte(content)) diff --git a/integration/integration_test.go b/integration/integration_test.go index c4b29a5f..ab7b39ff 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -592,7 +592,7 @@ func TestCloneFailsFallback(t *testing.T) { _, err := runEnvbuilder(t, runOpts{env: []string{ envbuilderEnv("GIT_URL", "bad-value"), }}) - require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error()) + require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error()) }) } @@ -610,7 +610,7 @@ func TestBuildFailsFallback(t *testing.T) { envbuilderEnv("GIT_URL", srv.URL), envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), }}) - require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error()) + require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error()) require.ErrorContains(t, err, "dockerfile parse error") }) t.Run("FailsBuild", func(t *testing.T) { @@ -626,7 +626,7 @@ RUN exit 1`, envbuilderEnv("GIT_URL", srv.URL), envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), }}) - require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error()) + require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error()) }) t.Run("BadDevcontainer", func(t *testing.T) { t.Parallel() @@ -639,7 +639,7 @@ RUN exit 1`, _, err := runEnvbuilder(t, runOpts{env: []string{ envbuilderEnv("GIT_URL", srv.URL), }}) - require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error()) + require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error()) }) t.Run("NoImageOrDockerfile", func(t *testing.T) { t.Parallel() @@ -972,7 +972,7 @@ func setupPassthroughRegistry(t *testing.T, image string, opts *setupPassthrough func TestNoMethodFails(t *testing.T) { _, err := runEnvbuilder(t, runOpts{env: []string{}}) - require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error()) + require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error()) } func TestDockerfileBuildContext(t *testing.T) { From b8eb67e51be7d7e46405464733bd618e687aa4ef Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:43:57 +0100 Subject: [PATCH 12/13] move MagicDir to internal/magicdir --- envbuilder.go | 22 +++++------ integration/integration_test.go | 4 +- .../magicdir/magicdir.go | 20 +++++----- internal/magicdir/magicdir_internal_test.go | 38 +++++++++++++++++++ options/options.go | 4 +- 5 files changed, 63 insertions(+), 25 deletions(-) rename constants/constants.go => internal/magicdir/magicdir.go (79%) create mode 100644 internal/magicdir/magicdir_internal_test.go diff --git a/envbuilder.go b/envbuilder.go index 075d1f3c..ac64beea 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -25,7 +25,6 @@ import ( "time" "github.com/coder/envbuilder/buildinfo" - "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/git" "github.com/coder/envbuilder/options" "github.com/go-git/go-billy/v5" @@ -36,6 +35,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/coder/envbuilder/devcontainer" "github.com/coder/envbuilder/internal/ebutil" + "github.com/coder/envbuilder/internal/magicdir" "github.com/coder/envbuilder/log" "github.com/containerd/platforms" "github.com/distribution/distribution/v3/configuration" @@ -72,7 +72,7 @@ func Run(ctx context.Context, opts options.Options) error { return fmt.Errorf("--cache-repo must be set when using --push-image") } - magicDir := constants.MagicDirAt(opts.MagicDirBase) + magicDir := magicdir.At(opts.MagicDirBase) // Default to the shell! initArgs := []string{"-c", opts.InitScript} @@ -341,12 +341,12 @@ func Run(ctx context.Context, opts options.Options) error { if err := util.AddAllowedPathToDefaultIgnoreList(magicDir.Image()); err != nil { return fmt.Errorf("add magic image file to ignore list: %w", err) } - magicTempDir := constants.MagicDirAt(buildParams.BuildContext, constants.MagicTempDir) + magicTempDir := magicdir.At(buildParams.BuildContext, magicdir.TempDir) if err := opts.Filesystem.MkdirAll(magicTempDir.Path(), 0o755); err != nil { return fmt.Errorf("create magic temp dir in build context: %w", err) } // Add the magic directives that embed the binary into the built image. - buildParams.DockerfileContent += constants.MagicDirectives + buildParams.DockerfileContent += magicdir.Directives // Copy the envbuilder binary into the build context. // External callers will need to specify the path to the desired envbuilder binary. envbuilderBinDest := filepath.Join(magicTempDir.Path(), "envbuilder") @@ -861,7 +861,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) return nil, fmt.Errorf("--cache-repo must be set when using --get-cached-image") } - magicDir := constants.MagicDirAt(opts.MagicDirBase) + magicDir := magicdir.At(opts.MagicDirBase) stageNumber := 0 startStage := func(format string, args ...any) func(format string, args ...any) { @@ -1105,8 +1105,8 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) // envbuilder binary available used to build the image and we also need to // add the magic directives to the Dockerfile content. // MAGICDIR - buildParams.DockerfileContent += constants.MagicDirectives - magicTempDir := filepath.Join(buildParams.BuildContext, constants.MagicTempDir) + buildParams.DockerfileContent += magicdir.Directives + magicTempDir := filepath.Join(buildParams.BuildContext, magicdir.TempDir) if err := opts.Filesystem.MkdirAll(magicTempDir, 0o755); err != nil { return nil, fmt.Errorf("create magic temp dir in build context: %w", err) } @@ -1417,11 +1417,11 @@ func maybeDeleteFilesystem(logger log.Func, force bool) error { // We always expect the magic directory to be set to the default, signifying that // the user is running envbuilder in a container. // If this is set to anything else we should bail out to prevent accidental data loss. - // defaultMagicDir := constants.MagicDir("") + // defaultMagicDir := magicdir.MagicDir("") kanikoDir, ok := os.LookupEnv("KANIKO_DIR") - if !ok || strings.TrimSpace(kanikoDir) != constants.DefaultMagicDir.Path() { + if !ok || strings.TrimSpace(kanikoDir) != magicdir.Default.Path() { if !force { - logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", constants.DefaultMagicDir.Path()) + logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", magicdir.Default.Path()) logger(log.LevelError, "To bypass this check, set FORCE_SAFE=true.") return errors.New("safety check failed") } @@ -1469,7 +1469,7 @@ func touchFile(fs billy.Filesystem, dst string, mode fs.FileMode) error { return f.Close() } -func initDockerConfigJSON(logf log.Func, magicDir constants.MagicDir, dockerConfigBase64 string) (func() error, error) { +func initDockerConfigJSON(logf log.Func, magicDir magicdir.MagicDir, dockerConfigBase64 string) (func() error, error) { var cleanupOnce sync.Once noop := func() error { return nil } if dockerConfigBase64 == "" { diff --git a/integration/integration_test.go b/integration/integration_test.go index ab7b39ff..e0e012ba 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -22,8 +22,8 @@ import ( "time" "github.com/coder/envbuilder" - "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/devcontainer/features" + "github.com/coder/envbuilder/internal/magicdir" "github.com/coder/envbuilder/options" "github.com/coder/envbuilder/testutil/gittest" "github.com/coder/envbuilder/testutil/mwtest" @@ -365,7 +365,7 @@ func TestBuildFromDockerfile(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) // Verify that the Docker configuration secret file is removed - configJSONContainerPath := constants.DefaultMagicDir.Join("config.json") + configJSONContainerPath := magicdir.Default.Join("config.json") output = execContainer(t, ctr, "stat "+configJSONContainerPath) require.Contains(t, output, "No such file or directory") } diff --git a/constants/constants.go b/internal/magicdir/magicdir.go similarity index 79% rename from constants/constants.go rename to internal/magicdir/magicdir.go index 4f368f6a..31bcd7c9 100644 --- a/constants/constants.go +++ b/internal/magicdir/magicdir.go @@ -1,4 +1,4 @@ -package constants +package magicdir import ( "fmt" @@ -11,26 +11,26 @@ const ( // or images. This is intentionally unexported. defaultMagicDirBase = "/.envbuilder" - // MagicTempDir is a directory inside the build context inside which + // TempDir is a directory inside the build context inside which // we place files referenced by MagicDirectives. - MagicTempDir = ".envbuilder.tmp" + TempDir = ".envbuilder.tmp" ) var ( - // DefaultMagicDir is the default working directory for Envbuilder. + // Default is the default working directory for Envbuilder. // This defaults to /.envbuilder. It should only be used when Envbuilder // is known to be running as root inside a container. - DefaultMagicDir MagicDir - // MagicDirectives are directives automatically appended to Dockerfiles + Default MagicDir + // Directives are directives automatically appended to Dockerfiles // when pushing the image. These directives allow the built image to be // 're-used'. - MagicDirectives = fmt.Sprintf(` + Directives = fmt.Sprintf(` COPY --chmod=0755 %[1]s/envbuilder %[2]s/bin/envbuilder COPY --chmod=0644 %[1]s/image %[2]s/image USER root WORKDIR / ENTRYPOINT ["%[2]s/bin/envbuilder"] -`, MagicTempDir, defaultMagicDirBase) +`, TempDir, defaultMagicDirBase) ) // MagicDir is a working directory for envbuilder. It @@ -39,8 +39,8 @@ type MagicDir struct { base string } -// MagicDirAt returns a MagicDir rooted at filepath.Join(paths...) -func MagicDirAt(paths ...string) MagicDir { +// At returns a MagicDir rooted at filepath.Join(paths...) +func At(paths ...string) MagicDir { if len(paths) == 0 { return MagicDir{} } diff --git a/internal/magicdir/magicdir_internal_test.go b/internal/magicdir/magicdir_internal_test.go new file mode 100644 index 00000000..43b66ba0 --- /dev/null +++ b/internal/magicdir/magicdir_internal_test.go @@ -0,0 +1,38 @@ +package magicdir + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_MagicDir(t *testing.T) { + t.Parallel() + + t.Run("Default", func(t *testing.T) { + t.Parallel() + require.Equal(t, defaultMagicDirBase+"/foo", Default.Join("foo")) + require.Equal(t, defaultMagicDirBase, Default.Path()) + require.Equal(t, defaultMagicDirBase+"/built", Default.Built()) + require.Equal(t, defaultMagicDirBase+"/image", Default.Image()) + }) + + t.Run("ZeroValue", func(t *testing.T) { + t.Parallel() + var md MagicDir + require.Equal(t, defaultMagicDirBase+"/foo", md.Join("foo")) + require.Equal(t, defaultMagicDirBase, md.Path()) + require.Equal(t, defaultMagicDirBase+"/built", md.Built()) + require.Equal(t, defaultMagicDirBase+"/image", md.Image()) + }) + + t.Run("At", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + md := At(tmpDir) + require.Equal(t, tmpDir+"/foo", md.Join("foo")) + require.Equal(t, tmpDir, md.Path()) + require.Equal(t, tmpDir+"/built", md.Built()) + require.Equal(t, tmpDir+"/image", md.Image()) + }) +} diff --git a/options/options.go b/options/options.go index 87ac5b0f..a98b46d4 100644 --- a/options/options.go +++ b/options/options.go @@ -7,7 +7,7 @@ import ( "os" "strings" - "github.com/coder/envbuilder/constants" + "github.com/coder/envbuilder/internal/magicdir" "github.com/coder/envbuilder/log" "github.com/coder/serpent" "github.com/go-git/go-billy/v5" @@ -464,7 +464,7 @@ func (o *Options) CLI() serpent.OptionSet { Flag: "remote-repo-dir", Env: WithEnvPrefix("REMOTE_REPO_DIR"), Value: serpent.StringOf(&o.RemoteRepoDir), - Default: constants.DefaultMagicDir.Join("repo"), + Default: magicdir.Default.Join("repo"), Hidden: true, Description: "Specify the destination directory for the cloned repo when using remote repo build mode.", }, From 5726246acfc52d5adc5c60e93fa53d52e081f923 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 9 Sep 2024 22:53:13 +0100 Subject: [PATCH 13/13] correctly set option defaults for magicdir and remote repo dir --- options/defaults.go | 7 +++++++ options/defaults_test.go | 2 ++ options/options.go | 9 ++++----- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/options/defaults.go b/options/defaults.go index 67709d25..df3d436c 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -8,6 +8,7 @@ import ( giturls "github.com/chainguard-dev/git-urls" "github.com/coder/envbuilder/internal/chmodfs" + "github.com/coder/envbuilder/internal/magicdir" ) // EmptyWorkspaceDir is the path to a workspace that has @@ -58,7 +59,13 @@ func (o *Options) SetDefaults() { if o.WorkspaceFolder == "" { o.WorkspaceFolder = DefaultWorkspaceFolder(o.GitURL) } + if o.RemoteRepoDir == "" { + o.RemoteRepoDir = magicdir.Default.Join("repo") + } if o.BinaryPath == "" { o.BinaryPath = "/.envbuilder/bin/envbuilder" } + if o.MagicDirBase == "" { + o.MagicDirBase = magicdir.Default.Path() + } } diff --git a/options/defaults_test.go b/options/defaults_test.go index 83854ebb..8c9946f6 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -84,6 +84,8 @@ func TestOptions_SetDefaults(t *testing.T) { Filesystem: chmodfs.New(osfs.New("/")), GitURL: "", WorkspaceFolder: options.EmptyWorkspaceDir, + MagicDirBase: "/.envbuilder", + RemoteRepoDir: "/.envbuilder/repo", BinaryPath: "/.envbuilder/bin/envbuilder", } diff --git a/options/options.go b/options/options.go index a98b46d4..a0058cd3 100644 --- a/options/options.go +++ b/options/options.go @@ -7,7 +7,6 @@ import ( "os" "strings" - "github.com/coder/envbuilder/internal/magicdir" "github.com/coder/envbuilder/log" "github.com/coder/serpent" "github.com/go-git/go-billy/v5" @@ -461,10 +460,10 @@ func (o *Options) CLI() serpent.OptionSet { "working on the same repository.", }, { - Flag: "remote-repo-dir", - Env: WithEnvPrefix("REMOTE_REPO_DIR"), - Value: serpent.StringOf(&o.RemoteRepoDir), - Default: magicdir.Default.Join("repo"), + Flag: "remote-repo-dir", + Env: WithEnvPrefix("REMOTE_REPO_DIR"), + Value: serpent.StringOf(&o.RemoteRepoDir), + // Default: magicdir.Default.Join("repo"), // TODO: reinstate once legacy opts are removed. Hidden: true, Description: "Specify the destination directory for the cloned repo when using remote repo build mode.", },