Skip to content

fix: allow setting MagicDir in Options #337

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.

Sign up for GitHub

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 14 commits into from
Sep 9, 2024
Merged

fix: allow setting MagicDir in Options #337

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
56be370
fix: allow setting MagicDir in Options
johnstcn Sep 6, 2024
61029cf
rm unnecessary test
johnstcn Sep 6, 2024
4b3c649
capitalize log line
johnstcn Sep 9, 2024
406bf81
update comment on MagicDir
johnstcn Sep 9, 2024
4d2833f
rename String() to Path()
johnstcn Sep 9, 2024
0f0adc3
use defaultMagicDir for MagicDirectives
johnstcn Sep 9, 2024
f7f7049
rm comment
johnstcn Sep 9, 2024
88051f8
make MagicDir un-stringable directly
johnstcn Sep 9, 2024
fe05a77
options.MagicDir -> options.MagicDirBase (string)
johnstcn Sep 9, 2024
0b0cdcd
move EmptyWorkspaceDir to options
johnstcn Sep 9, 2024
590155d
move ErrNoFallbackImage to envbuilder package
johnstcn Sep 9, 2024
b8eb67e
move MagicDir to internal/magicdir
johnstcn Sep 9, 2024
5726246
correctly set option defaults for magicdir and remote repo dir
johnstcn Sep 9, 2024
552252f
Merge branch 'main' into cj/less-magic
johnstcn Sep 9, 2024
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
80 changes: 44 additions & 36 deletions constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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))
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking this abstraction could be a source of bugs. One might not always think to call String and instead do string(MagicDir) which would result in an empty string or potentially lack of /.

I'd rather see this as a validation step in options so that the input is verified there, and an error can be returned if it's not absolute. The options struct could have the MagicImage and MagicBuilt functions if that's preferable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think a simpler option here it to simply change the type to a struct with a single unexported field. I'm less worried about leaking the implementation details; we can simply move MagicDir to an internal package.


// 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")
}
99 changes: 48 additions & 51 deletions envbuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
Expand All @@ -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...)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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",
Expand All @@ -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!
Expand Down Expand Up @@ -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)
}
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: inverted the logic here for readability.

}

return util.DeleteFilesystem()
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Comment on lines +1492 to +1494
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: I found this useful when troubleshooting!

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() {
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down
Loading