From 8c0198f30b97937d3782b67b2091c9828e575f31 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 28 Oct 2024 16:03:06 +0000 Subject: [PATCH 01/10] fix: always set DOCKER_CONFIG unless manually set and no base64 config provided Fixes #392 --- envbuilder.go | 86 ++++++++++++++++++++++----------- integration/integration_test.go | 68 ++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 34 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 295ef3f3..eb7a4652 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -154,13 +154,13 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro 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, workingDir, opts.DockerConfigBase64) + cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64) if err != nil { return err } defer func() { - if err := cleanupDockerConfigJSON(); err != nil { - opts.Logger(log.LevelError, "failed to cleanup docker config JSON: %w", err) + if err := cleanupDockerConfigOverride(); err != nil { + opts.Logger(log.LevelError, "failed to cleanup docker config override: %w", err) } }() // best effort @@ -771,7 +771,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro } // Remove the Docker config secret file! - if err := cleanupDockerConfigJSON(); err != nil { + if err := cleanupDockerConfigOverride(); err != nil { return err } @@ -978,13 +978,13 @@ 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, workingDir, opts.DockerConfigBase64) + cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64) if err != nil { return nil, err } defer func() { - if err := cleanupDockerConfigJSON(); err != nil { - opts.Logger(log.LevelError, "failed to cleanup docker config JSON: %w", err) + if err := cleanupDockerConfigOverride(); err != nil { + opts.Logger(log.LevelError, "failed to cleanup docker config override: %w", err) } }() // best effort @@ -1315,7 +1315,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error) options.UnsetEnv() // Remove the Docker config secret file! - if err := cleanupDockerConfigJSON(); err != nil { + if err := cleanupDockerConfigOverride(); err != nil { return nil, err } @@ -1627,55 +1627,83 @@ func parseMagicImageFile(fs billy.Filesystem, path string, v any) error { return nil } -func initDockerConfigJSON(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) { - var cleanupOnce sync.Once - noop := func() error { return nil } +func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) { + var ( + oldDockerConfig = os.Getenv("DOCKER_CONFIG") + newDockerConfig = workingDir.Path() + cfgPath = workingDir.Join("config.json") + restoreEnv = func() error { return nil } // noop. + ) + if dockerConfigBase64 != "" || (dockerConfigBase64 == "" && oldDockerConfig == "") { + err := os.Setenv("DOCKER_CONFIG", newDockerConfig) + if err != nil { + logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err) + return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err) + } + logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig) + + restoreEnv = func() error { + // Restore the old DOCKER_CONFIG value. + if oldDockerConfig == "" { + err := os.Unsetenv("DOCKER_CONFIG") + if err != nil { + err = fmt.Errorf("unset DOCKER_CONFIG: %w", err) + } + return err + } + err := os.Setenv("DOCKER_CONFIG", oldDockerConfig) + if err != nil { + return fmt.Errorf("restore DOCKER_CONFIG: %w", err) + } + logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig) + return nil + } + } else { + logf(log.LevelInfo, "Using existing DOCKER_CONFIG set to %s", oldDockerConfig) + } + if dockerConfigBase64 == "" { - return noop, nil + return restoreEnv, nil } - cfgPath := workingDir.Join("config.json") + decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64) if err != nil { - return noop, fmt.Errorf("decode docker config: %w", err) + return restoreEnv, fmt.Errorf("decode docker config: %w", err) } var configFile DockerConfig decoded, err = hujson.Standardize(decoded) if err != nil { - return noop, fmt.Errorf("humanize json for docker config: %w", err) + return restoreEnv, fmt.Errorf("humanize json for docker config: %w", err) } err = json.Unmarshal(decoded, &configFile) if err != nil { - return noop, fmt.Errorf("parse docker config: %w", err) + return restoreEnv, 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) + return restoreEnv, fmt.Errorf("write docker config: %w", err) } logf(log.LevelInfo, "Wrote Docker config JSON to %s", cfgPath) - oldDockerConfig := os.Getenv("DOCKER_CONFIG") - _ = os.Setenv("DOCKER_CONFIG", workingDir.Path()) - newDockerConfig := os.Getenv("DOCKER_CONFIG") - logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig) - cleanup := func() error { + + var cleanupOnce sync.Once + return func() error { var cleanupErr error cleanupOnce.Do(func() { - // Restore the old DOCKER_CONFIG value. - os.Setenv("DOCKER_CONFIG", oldDockerConfig) - logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig) + cleanupErr = restoreEnv() // Remove the Docker config secret file! - if cleanupErr = os.Remove(cfgPath); err != nil { + if err := os.Remove(cfgPath); err != nil { if !errors.Is(err, fs.ErrNotExist) { - cleanupErr = fmt.Errorf("remove docker config: %w", cleanupErr) + err = errors.Join(err, fmt.Errorf("remove docker config: %w", err)) } logf(log.LevelError, "Failed to remove the Docker config secret file: %s", cleanupErr) + cleanupErr = errors.Join(cleanupErr, err) } }) return cleanupErr - } - return cleanup, err + }, nil } // Allows quick testing of layer caching using a local directory! diff --git a/integration/integration_test.go b/integration/integration_test.go index 9413c349..5099fd0b 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -552,13 +552,20 @@ func TestBuildFromDockerfile(t *testing.T) { "Dockerfile": "FROM " + testImageAlpine, }, }) - ctr, err := runEnvbuilder(t, runOpts{env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString([]byte(`{"experimental": "enabled"}`))), - }}) + logbuf := new(bytes.Buffer) + ctr, err := runEnvbuilder(t, runOpts{ + env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString([]byte(`{"experimental": "enabled"}`))), + "DOCKER_CONFIG=/config", // Ignored, because we're setting DOCKER_CONFIG_BASE64. + }, + logbuf: logbuf, + }) require.NoError(t, err) + require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") + output := execContainer(t, ctr, "echo hello") require.Equal(t, "hello", strings.TrimSpace(output)) @@ -568,6 +575,52 @@ func TestBuildFromDockerfile(t *testing.T) { require.Contains(t, output, "No such file or directory") } +func TestBuildDockerConfigPathFromEnv(t *testing.T) { + // Ensures that a Git repository with a Dockerfile is cloned and built. + srv := gittest.CreateGitServer(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": "FROM " + testImageAlpine, + }, + }) + dir := t.TempDir() + err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"experimental": "enabled"}`), 0o644) + require.NoError(t, err) + + logbuf := new(bytes.Buffer) + _, err = runEnvbuilder(t, runOpts{ + env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + "DOCKER_CONFIG=/config", + }, + binds: []string{fmt.Sprintf("%s:/config:ro", dir)}, + logbuf: logbuf, + }) + require.NoError(t, err) + + require.Contains(t, logbuf.String(), "Using existing DOCKER_CONFIG set to /config") +} + +func TestBuildDockerConfigDefaultPath(t *testing.T) { + // Ensures that a Git repository with a Dockerfile is cloned and built. + srv := gittest.CreateGitServer(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": "FROM " + testImageAlpine, + }, + }) + logbuf := new(bytes.Buffer) + _, err := runEnvbuilder(t, runOpts{ + env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + }, + logbuf: logbuf, + }) + require.NoError(t, err) + + require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") +} + func TestBuildPrintBuildOutput(t *testing.T) { // Ensures that a Git repository with a Dockerfile is cloned and built. srv := gittest.CreateGitServer(t, gittest.Options{ @@ -2296,6 +2349,7 @@ type runOpts struct { binds []string env []string volumes map[string]string + logbuf *bytes.Buffer } // runEnvbuilder starts the envbuilder container with the given environment @@ -2340,6 +2394,7 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) { testContainerLabel: "true", }, }, &container.HostConfig{ + CapAdd: []string{"SYS_ADMIN"}, // For remounting. NetworkMode: container.NetworkMode("host"), Binds: opts.binds, Mounts: mounts, @@ -2357,6 +2412,9 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) { logChan, errChan := streamContainerLogs(t, cli, ctr.ID) go func() { for log := range logChan { + if opts.logbuf != nil { + opts.logbuf.WriteString(log) + } if strings.HasPrefix(log, "=== Running init command") { errChan <- nil return From 5fbcf2dfd0f37cfffdfbafbcbf4f02db1bf7c399 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 28 Oct 2024 16:06:56 +0000 Subject: [PATCH 02/10] simplify if statement --- envbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envbuilder.go b/envbuilder.go index eb7a4652..b766c6d8 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1634,7 +1634,7 @@ func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, d cfgPath = workingDir.Join("config.json") restoreEnv = func() error { return nil } // noop. ) - if dockerConfigBase64 != "" || (dockerConfigBase64 == "" && oldDockerConfig == "") { + if dockerConfigBase64 != "" || oldDockerConfig == "" { err := os.Setenv("DOCKER_CONFIG", newDockerConfig) if err != nil { logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err) From d559c1d3eb4e526ce233ac6dfa449b257cdb8f6d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 28 Oct 2024 16:14:24 +0000 Subject: [PATCH 03/10] simplify return --- envbuilder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index b766c6d8..b518576e 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1647,9 +1647,9 @@ func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, d if oldDockerConfig == "" { err := os.Unsetenv("DOCKER_CONFIG") if err != nil { - err = fmt.Errorf("unset DOCKER_CONFIG: %w", err) + return fmt.Errorf("unset DOCKER_CONFIG: %w", err) } - return err + return nil } err := os.Setenv("DOCKER_CONFIG", oldDockerConfig) if err != nil { From 5da2aaa6c3400f7822cd1869b90fbc3e0dbc9998 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 28 Oct 2024 19:17:22 +0000 Subject: [PATCH 04/10] copy config to protect against remount, fix docker perms --- envbuilder.go | 155 ++++++++++++++++++++------------ integration/integration_test.go | 52 +++++++---- 2 files changed, 136 insertions(+), 71 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index b518576e..270366d1 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -154,7 +154,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version()) - cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64) + cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Filesystem, opts.Logger, workingDir, opts.DockerConfigBase64) if err != nil { return err } @@ -711,6 +711,11 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro // Sanitize the environment of any opts! options.UnsetEnv() + // Remove the Docker config secret file! + if err := cleanupDockerConfigOverride(); err != nil { + return err + } + // Set the environment from /etc/environment first, so it can be // overridden by the image and devcontainer settings. err = setEnvFromEtcEnvironment(opts.Logger) @@ -770,11 +775,6 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro exportEnvFile.Close() } - // Remove the Docker config secret file! - if err := cleanupDockerConfigOverride(); err != nil { - return err - } - if runtimeData.ContainerUser == "" { opts.Logger(log.LevelWarn, "#%d: no user specified, using root", stageNumber) } @@ -978,7 +978,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()) - cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64) + cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Filesystem, opts.Logger, workingDir, opts.DockerConfigBase64) if err != nil { return nil, err } @@ -1571,6 +1571,20 @@ func fileExists(fs billy.Filesystem, path string) bool { return err == nil } +func readFile(fs billy.Filesystem, name string) ([]byte, error) { + f, err := fs.Open(name) + if err != nil { + return nil, fmt.Errorf("open file: %w", err) + } + defer f.Close() + + b, err := io.ReadAll(f) + if err != nil { + return nil, fmt.Errorf("read file: %w", err) + } + return b, nil +} + func copyFile(fs billy.Filesystem, src, dst string, mode fs.FileMode) error { srcF, err := fs.Open(src) if err != nil { @@ -1595,6 +1609,21 @@ func copyFile(fs billy.Filesystem, src, dst string, mode fs.FileMode) error { return nil } +func writeFile(fs billy.Filesystem, name string, data []byte, perm fs.FileMode) error { + f, err := fs.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, perm) + if err != nil { + return fmt.Errorf("create file: %w", err) + } + _, err = f.Write(data) + if err != nil { + err = fmt.Errorf("write file: %w", err) + } + if err2 := f.Close(); err2 != nil && err == nil { + err = fmt.Errorf("close file: %w", err2) + } + return err +} + func writeMagicImageFile(fs billy.Filesystem, path string, v any) error { file, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644) if err != nil { @@ -1627,39 +1656,45 @@ func parseMagicImageFile(fs billy.Filesystem, path string, v any) error { return nil } -func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) { - var ( - oldDockerConfig = os.Getenv("DOCKER_CONFIG") - newDockerConfig = workingDir.Path() - cfgPath = workingDir.Join("config.json") - restoreEnv = func() error { return nil } // noop. - ) - if dockerConfigBase64 != "" || oldDockerConfig == "" { - err := os.Setenv("DOCKER_CONFIG", newDockerConfig) - if err != nil { - logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err) - return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err) - } - logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig) +func initDockerConfigOverride(bfs billy.Filesystem, logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) { + configFile := "config.json" + oldDockerConfig := os.Getenv("DOCKER_CONFIG") + newDockerConfig := workingDir.Path() - restoreEnv = func() error { - // Restore the old DOCKER_CONFIG value. + err := os.Setenv("DOCKER_CONFIG", newDockerConfig) + if err != nil { + logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err) + return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err) + } + logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig) + restoreEnv := onceErrFunc(func() error { + // Restore the old DOCKER_CONFIG value. + if err := func() error { if oldDockerConfig == "" { - err := os.Unsetenv("DOCKER_CONFIG") - if err != nil { - return fmt.Errorf("unset DOCKER_CONFIG: %w", err) - } - return nil - } - err := os.Setenv("DOCKER_CONFIG", oldDockerConfig) - if err != nil { - return fmt.Errorf("restore DOCKER_CONFIG: %w", err) + return os.Unsetenv("DOCKER_CONFIG") } - logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig) - return nil + return os.Setenv("DOCKER_CONFIG", oldDockerConfig) + }(); err != nil { + return fmt.Errorf("restore DOCKER_CONFIG: %w", err) } - } else { - logf(log.LevelInfo, "Using existing DOCKER_CONFIG set to %s", oldDockerConfig) + logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig) + return nil + }) + + // If the user hasn't set the BASE64 encoded Docker config, we + // should respect the DOCKER_CONFIG environment variable. + oldDockerConfigFile := filepath.Join(oldDockerConfig, configFile) + if dockerConfigBase64 == "" && fileExists(bfs, oldDockerConfigFile) { + // It's possible that the target file is mounted and needs to + // be remounted later, so we should copy the file instead of + // hoping that the file is still there when we build. + logf(log.LevelInfo, "Using DOCKER_CONFIG at %s", oldDockerConfig) + b, err := readFile(bfs, oldDockerConfigFile) + if err != nil { + return nil, fmt.Errorf("read existing DOCKER_CONFIG: %w", err) + } + // Read into dockerConfigBase64 so we can keep the same logic. + dockerConfigBase64 = base64.StdEncoding.EncodeToString(b) } if dockerConfigBase64 == "" { @@ -1670,40 +1705,50 @@ func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, d if err != nil { return restoreEnv, fmt.Errorf("decode docker config: %w", err) } - var configFile DockerConfig decoded, err = hujson.Standardize(decoded) if err != nil { return restoreEnv, fmt.Errorf("humanize json for docker config: %w", err) } - err = json.Unmarshal(decoded, &configFile) + var dockerConfig DockerConfig + err = json.Unmarshal(decoded, &dockerConfig) if err != nil { return restoreEnv, fmt.Errorf("parse docker config: %w", err) } - for k := range configFile.AuthConfigs { + for k := range dockerConfig.AuthConfigs { logf(log.LevelInfo, "Docker config contains auth for registry %q", k) } - err = os.WriteFile(cfgPath, decoded, 0o644) + + newDockerConfigFile := filepath.Join(newDockerConfig, configFile) + err = writeFile(bfs, newDockerConfigFile, decoded, 0o644) if err != nil { return restoreEnv, fmt.Errorf("write docker config: %w", err) } - logf(log.LevelInfo, "Wrote Docker config JSON to %s", cfgPath) + logf(log.LevelInfo, "Wrote Docker config JSON to %s", newDockerConfigFile) - var cleanupOnce sync.Once - return func() error { - var cleanupErr error - cleanupOnce.Do(func() { - cleanupErr = restoreEnv() - // Remove the Docker config secret file! - if err := os.Remove(cfgPath); err != nil { - if !errors.Is(err, fs.ErrNotExist) { - err = errors.Join(err, fmt.Errorf("remove docker config: %w", err)) - } - logf(log.LevelError, "Failed to remove the Docker config secret file: %s", cleanupErr) - cleanupErr = errors.Join(cleanupErr, err) + cleanup := onceErrFunc(func() error { + err := restoreEnv() + // Remove the Docker config secret file! + if err2 := os.Remove(newDockerConfigFile); err2 != nil { + if !errors.Is(err2, fs.ErrNotExist) { + err2 = fmt.Errorf("remove docker config: %w", err2) } + logf(log.LevelError, "Failed to remove the Docker config secret file: %s", err2) + err = errors.Join(err, err2) + } + return err + }) + return cleanup, nil +} + +func onceErrFunc(f func() error) func() error { + var once sync.Once + return func() error { + var err error + once.Do(func() { + err = f() }) - return cleanupErr - }, nil + return err + } } // Allows quick testing of layer caching using a local directory! diff --git a/integration/integration_test.go b/integration/integration_test.go index 5099fd0b..3778cddf 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -565,6 +565,7 @@ func TestBuildFromDockerfile(t *testing.T) { require.NoError(t, err) require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") + require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ") output := execContainer(t, ctr, "echo hello") require.Equal(t, "hello", strings.TrimSpace(output)) @@ -582,8 +583,17 @@ func TestBuildDockerConfigPathFromEnv(t *testing.T) { "Dockerfile": "FROM " + testImageAlpine, }, }) + config, err := json.Marshal(envbuilder.DockerConfig{ + AuthConfigs: map[string]clitypes.AuthConfig{ + "mytestimage": { + Username: "user", + Password: "test", + }, + }, + }) + require.NoError(t, err) dir := t.TempDir() - err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"experimental": "enabled"}`), 0o644) + err = os.WriteFile(filepath.Join(dir, "config.json"), config, 0o644) require.NoError(t, err) logbuf := new(bytes.Buffer) @@ -593,12 +603,16 @@ func TestBuildDockerConfigPathFromEnv(t *testing.T) { envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), "DOCKER_CONFIG=/config", }, - binds: []string{fmt.Sprintf("%s:/config:ro", dir)}, - logbuf: logbuf, + privileged: true, + binds: []string{fmt.Sprintf("%s:/config:ro", dir)}, + logbuf: logbuf, }) require.NoError(t, err) - require.Contains(t, logbuf.String(), "Using existing DOCKER_CONFIG set to /config") + // Logs that the DOCKER_CONFIG is used. + require.Contains(t, logbuf.String(), "Using DOCKER_CONFIG at /config") + // Logs registry auth info from existing file. + require.Contains(t, logbuf.String(), "mytestimage") } func TestBuildDockerConfigDefaultPath(t *testing.T) { @@ -619,6 +633,7 @@ func TestBuildDockerConfigDefaultPath(t *testing.T) { require.NoError(t, err) require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") + require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ") } func TestBuildPrintBuildOutput(t *testing.T) { @@ -2345,11 +2360,12 @@ func startContainerFromRef(ctx context.Context, t *testing.T, cli *client.Client } type runOpts struct { - image string - binds []string - env []string - volumes map[string]string - logbuf *bytes.Buffer + image string + privileged bool // Required for remounting. + binds []string + env []string + volumes map[string]string + logbuf *bytes.Buffer } // runEnvbuilder starts the envbuilder container with the given environment @@ -2387,18 +2403,22 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) { require.NoError(t, err, "failed to read image pull response") img = opts.image } + hostConfig := &container.HostConfig{ + NetworkMode: container.NetworkMode("host"), + Binds: opts.binds, + Mounts: mounts, + } + if opts.privileged { + hostConfig.CapAdd = append(hostConfig.CapAdd, "SYS_ADMIN") + hostConfig.Privileged = true + } ctr, err := cli.ContainerCreate(ctx, &container.Config{ Image: img, Env: opts.env, Labels: map[string]string{ testContainerLabel: "true", }, - }, &container.HostConfig{ - CapAdd: []string{"SYS_ADMIN"}, // For remounting. - NetworkMode: container.NetworkMode("host"), - Binds: opts.binds, - Mounts: mounts, - }, nil, nil, "") + }, hostConfig, nil, nil, "") require.NoError(t, err) t.Cleanup(func() { _ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{ @@ -2413,7 +2433,7 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) { go func() { for log := range logChan { if opts.logbuf != nil { - opts.logbuf.WriteString(log) + opts.logbuf.WriteString(log + "\n") } if strings.HasPrefix(log, "=== Running init command") { errChan <- nil From 6459b39c9244e9c9a1040246bbc28ec233b1ba52 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 29 Oct 2024 17:31:10 +0000 Subject: [PATCH 05/10] rewrite config detection and env override --- envbuilder.go | 185 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 124 insertions(+), 61 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 270366d1..e4b857cd 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -41,6 +41,7 @@ import ( "github.com/distribution/distribution/v3/configuration" "github.com/distribution/distribution/v3/registry/handlers" _ "github.com/distribution/distribution/v3/registry/storage/driver/filesystem" + dockerconfig "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" "github.com/fatih/color" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -56,7 +57,7 @@ import ( var ErrNoFallbackImage = errors.New("no fallback image has been specified") // DockerConfig represents the Docker configuration file. -type DockerConfig configfile.ConfigFile +type DockerConfig = configfile.ConfigFile type runtimeDataStore struct { // Runtime data. @@ -1567,8 +1568,8 @@ func maybeDeleteFilesystem(logger log.Func, force bool) error { } func fileExists(fs billy.Filesystem, path string) bool { - _, err := fs.Stat(path) - return err == nil + fi, err := fs.Stat(path) + return err == nil && !fi.IsDir() } func readFile(fs billy.Filesystem, name string) ([]byte, error) { @@ -1656,88 +1657,150 @@ func parseMagicImageFile(fs billy.Filesystem, path string, v any) error { return nil } +const ( + dockerConfigFile = dockerconfig.ConfigFileName + dockerConfigEnvKey = dockerconfig.EnvOverrideConfigDir +) + +// initDockerConfigOverride sets the DOCKER_CONFIG environment variable +// to a path within the working directory. If a base64 encoded Docker +// config is provided, it is written to the path/config.json and the +// DOCKER_CONFIG environment variable is set to the path. If no base64 +// encoded Docker config is provided, the following paths are checked in +// order: +// +// 1. $DOCKER_CONFIG/config.json +// 2. $DOCKER_CONFIG +// 3. /.envbuilder/config.json +// +// If a Docker config file is found, its path is set as DOCKER_CONFIG. func initDockerConfigOverride(bfs billy.Filesystem, logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) { - configFile := "config.json" - oldDockerConfig := os.Getenv("DOCKER_CONFIG") - newDockerConfig := workingDir.Path() + // If dockerConfigBase64 is set, it will have priority over file + // detection. + var dockerConfigJSON []byte + var err error + if dockerConfigBase64 != "" { + logf(log.LevelInfo, "Using base64 encoded Docker config") + + dockerConfigJSON, err = base64.StdEncoding.DecodeString(dockerConfigBase64) + if err != nil { + return nil, fmt.Errorf("decode docker config: %w", err) + } + } - err := os.Setenv("DOCKER_CONFIG", newDockerConfig) - if err != nil { - logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err) - return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err) + oldDockerConfig := os.Getenv(dockerConfigEnvKey) + var oldDockerConfigFile string + if oldDockerConfig != "" { + oldDockerConfigFile = filepath.Join(oldDockerConfig, dockerConfigFile) } - logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig) - restoreEnv := onceErrFunc(func() error { - // Restore the old DOCKER_CONFIG value. - if err := func() error { - if oldDockerConfig == "" { - return os.Unsetenv("DOCKER_CONFIG") - } - return os.Setenv("DOCKER_CONFIG", oldDockerConfig) - }(); err != nil { - return fmt.Errorf("restore DOCKER_CONFIG: %w", err) + for _, path := range []string{ + oldDockerConfigFile, // $DOCKER_CONFIG/config.json + oldDockerConfig, // $DOCKER_CONFIG + workingDir.Join(dockerConfigFile), // /.envbuilder/config.json + } { + if path == "" || !fileExists(bfs, path) { + continue } - logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig) - return nil - }) - // If the user hasn't set the BASE64 encoded Docker config, we - // should respect the DOCKER_CONFIG environment variable. - oldDockerConfigFile := filepath.Join(oldDockerConfig, configFile) - if dockerConfigBase64 == "" && fileExists(bfs, oldDockerConfigFile) { - // It's possible that the target file is mounted and needs to - // be remounted later, so we should copy the file instead of - // hoping that the file is still there when we build. - logf(log.LevelInfo, "Using DOCKER_CONFIG at %s", oldDockerConfig) - b, err := readFile(bfs, oldDockerConfigFile) - if err != nil { - return nil, fmt.Errorf("read existing DOCKER_CONFIG: %w", err) + logf(log.LevelWarn, "Found Docker config at %s, this file will remain after the build", path) + + if dockerConfigJSON == nil { + logf(log.LevelInfo, "Using Docker config at %s", path) + + dockerConfigJSON, err = readFile(bfs, path) + if err != nil { + return nil, fmt.Errorf("read docker config: %w", err) + } + } else { + logf(log.LevelWarn, "Ignoring Docker config at %s, using base64 encoded Docker config instead", path) } - // Read into dockerConfigBase64 so we can keep the same logic. - dockerConfigBase64 = base64.StdEncoding.EncodeToString(b) + break } - if dockerConfigBase64 == "" { - return restoreEnv, nil + if dockerConfigJSON == nil { + // No user-provided config available. + return func() error { return nil }, nil } - decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64) + dockerConfigJSON, err = hujson.Standardize(dockerConfigJSON) if err != nil { - return restoreEnv, fmt.Errorf("decode docker config: %w", err) + return nil, fmt.Errorf("humanize json for docker config: %w", err) } - decoded, err = hujson.Standardize(decoded) - if err != nil { - return restoreEnv, fmt.Errorf("humanize json for docker config: %w", err) + + if err = logDockerAuthConfigs(logf, dockerConfigJSON); err != nil { + return nil, fmt.Errorf("log docker auth configs: %w", err) } - var dockerConfig DockerConfig - err = json.Unmarshal(decoded, &dockerConfig) + + // We're going to set the DOCKER_CONFIG environment variable to a + // path within the working directory so that Kaniko can pick it up. + // A user should not mount a file directly to this path as we will + // write to the file. + newDockerConfig := workingDir.Join(".docker") + newDockerConfigFile := filepath.Join(newDockerConfig, dockerConfigFile) + err = bfs.MkdirAll(newDockerConfig, 0o700) if err != nil { - return restoreEnv, fmt.Errorf("parse docker config: %w", err) + return nil, fmt.Errorf("create docker config dir: %w", err) } - for k := range dockerConfig.AuthConfigs { - logf(log.LevelInfo, "Docker config contains auth for registry %q", k) + + if fileExists(bfs, newDockerConfigFile) { + return nil, fmt.Errorf("unable to write Docker config file, file already exists: %s", newDockerConfigFile) } - newDockerConfigFile := filepath.Join(newDockerConfig, configFile) - err = writeFile(bfs, newDockerConfigFile, decoded, 0o644) + restoreEnv, err := setAndRestoreEnv(logf, dockerConfigEnvKey, newDockerConfig) if err != nil { - return restoreEnv, fmt.Errorf("write docker config: %w", err) + return nil, fmt.Errorf("set docker config override: %w", err) + } + + err = writeFile(bfs, newDockerConfigFile, dockerConfigJSON, 0o600) + if err != nil { + _ = restoreEnv() // Best effort. + return nil, fmt.Errorf("write docker config: %w", err) } logf(log.LevelInfo, "Wrote Docker config JSON to %s", newDockerConfigFile) - cleanup := onceErrFunc(func() error { - err := restoreEnv() + cleanupFile := onceErrFunc(func() error { // Remove the Docker config secret file! - if err2 := os.Remove(newDockerConfigFile); err2 != nil { - if !errors.Is(err2, fs.ErrNotExist) { - err2 = fmt.Errorf("remove docker config: %w", err2) - } - logf(log.LevelError, "Failed to remove the Docker config secret file: %s", err2) - err = errors.Join(err, err2) + if err := bfs.Remove(newDockerConfigFile); err != nil { + logf(log.LevelError, "Failed to remove the Docker config secret file: %s", err) + return fmt.Errorf("remove docker config: %w", err) } - return err + return nil }) - return cleanup, nil + return func() error { return errors.Join(cleanupFile(), restoreEnv()) }, nil +} + +func logDockerAuthConfigs(logf log.Func, dockerConfigJSON []byte) error { + dc := new(DockerConfig) + err := dc.LoadFromReader(bytes.NewReader(dockerConfigJSON)) + if err != nil { + return fmt.Errorf("load docker config: %w", err) + } + for k := range dc.AuthConfigs { + logf(log.LevelInfo, "Docker config contains auth for registry %q", k) + } + return nil +} + +func setAndRestoreEnv(logf log.Func, key, value string) (restore func() error, err error) { + old := os.Getenv(key) + err = os.Setenv(key, value) + if err != nil { + logf(log.LevelError, "Failed to set %s: %s", key, err) + return nil, fmt.Errorf("set %s: %w", key, err) + } + logf(log.LevelInfo, "Set %s to %s", key, value) + return onceErrFunc(func() error { + if err := func() error { + if old == "" { + return os.Unsetenv(key) + } + return os.Setenv(key, old) + }(); err != nil { + return fmt.Errorf("restore %s: %w", key, err) + } + logf(log.LevelInfo, "Restored %s to %s", key, old) + return nil + }), nil } func onceErrFunc(f func() error) func() error { From 3f6830db790c72267227a3f41992817470c5bfae Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 29 Oct 2024 17:31:41 +0000 Subject: [PATCH 06/10] update tests --- integration/integration_test.go | 195 +++++++++++++++++++------------- 1 file changed, 116 insertions(+), 79 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 3778cddf..c0fb8246 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -545,95 +545,132 @@ func TestBuildFromDevcontainerWithFeatures(t *testing.T) { require.Equal(t, "hello from test 3!", strings.TrimSpace(test3Output)) } -func TestBuildFromDockerfile(t *testing.T) { - // Ensures that a Git repository with a Dockerfile is cloned and built. - srv := gittest.CreateGitServer(t, gittest.Options{ - Files: map[string]string{ - "Dockerfile": "FROM " + testImageAlpine, - }, - }) - logbuf := new(bytes.Buffer) - ctr, err := runEnvbuilder(t, runOpts{ - env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString([]byte(`{"experimental": "enabled"}`))), - "DOCKER_CONFIG=/config", // Ignored, because we're setting DOCKER_CONFIG_BASE64. - }, - logbuf: logbuf, - }) - require.NoError(t, err) +func TestBuildFromDockerfileAndConfig(t *testing.T) { + t.Parallel() - require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") - require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ") + type configFile struct { + name string + data string + } + type testCase struct { + name string + env []string + configFile configFile + configBase64 string + validate func(t *testing.T, tc testCase, ctrID, logs string) + } - output := execContainer(t, ctr, "echo hello") - require.Equal(t, "hello", strings.TrimSpace(output)) + validateDockerConfig := func(t *testing.T, tc testCase, ctrID, logs string) { + t.Helper() + got := execContainer(t, ctrID, "cat /docker_config_json") + got = strings.TrimSpace(got) + want := tc.configBase64 + if want == "" { + want = tc.configFile.data + } + if want != "" { + require.Contains(t, logs, "Set DOCKER_CONFIG to /.envbuilder/.docker") + require.Equal(t, want, got) + } + } - // Verify that the Docker configuration secret file is removed - configJSONContainerPath := workingdir.Default.Join("config.json") - output = execContainer(t, ctr, "stat "+configJSONContainerPath) - require.Contains(t, output, "No such file or directory") -} + configJSONContainerPath := workingdir.Default.Join(".docker", "config.json") + defaultConfigJSON := `{"experimental": "enabled"}` -func TestBuildDockerConfigPathFromEnv(t *testing.T) { - // Ensures that a Git repository with a Dockerfile is cloned and built. - srv := gittest.CreateGitServer(t, gittest.Options{ - Files: map[string]string{ - "Dockerfile": "FROM " + testImageAlpine, - }, - }) - config, err := json.Marshal(envbuilder.DockerConfig{ - AuthConfigs: map[string]clitypes.AuthConfig{ - "mytestimage": { - Username: "user", - Password: "test", + tests := []testCase{ + { + name: "Plain", + validate: func(t *testing.T, tc testCase, ctrID, logs string) { + output := execContainer(t, ctrID, "echo hello") + require.Equal(t, "hello", strings.TrimSpace(output)) }, }, - }) - require.NoError(t, err) - dir := t.TempDir() - err = os.WriteFile(filepath.Join(dir, "config.json"), config, 0o644) - require.NoError(t, err) - - logbuf := new(bytes.Buffer) - _, err = runEnvbuilder(t, runOpts{ - env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), - "DOCKER_CONFIG=/config", + { + name: "ConfigBase64", + configBase64: defaultConfigJSON, + validate: validateDockerConfig, }, - privileged: true, - binds: []string{fmt.Sprintf("%s:/config:ro", dir)}, - logbuf: logbuf, - }) - require.NoError(t, err) - - // Logs that the DOCKER_CONFIG is used. - require.Contains(t, logbuf.String(), "Using DOCKER_CONFIG at /config") - // Logs registry auth info from existing file. - require.Contains(t, logbuf.String(), "mytestimage") -} - -func TestBuildDockerConfigDefaultPath(t *testing.T) { - // Ensures that a Git repository with a Dockerfile is cloned and built. - srv := gittest.CreateGitServer(t, gittest.Options{ - Files: map[string]string{ - "Dockerfile": "FROM " + testImageAlpine, + { + name: "BindConfigToKnownLocation", + configFile: configFile{"/.envbuilder/config.json", defaultConfigJSON}, + validate: validateDockerConfig, }, - }) - logbuf := new(bytes.Buffer) - _, err := runEnvbuilder(t, runOpts{ - env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + { + name: "BindConfigToPath", + env: []string{"DOCKER_CONFIG=/secret"}, + configFile: configFile{"/secret/config.json", defaultConfigJSON}, + validate: validateDockerConfig, }, - logbuf: logbuf, - }) - require.NoError(t, err) + { + name: "BindConfigToCustomFile", + env: []string{"DOCKER_CONFIG=/secret/my.json"}, + configFile: configFile{"/secret/my.json", defaultConfigJSON}, + validate: validateDockerConfig, + }, + { + name: "ConfigBase64AndBindUsesBase64", + configFile: configFile{"/.envbuilder/config.json", `{"experimental": "disabled"}`}, + configBase64: defaultConfigJSON, + validate: validateDockerConfig, + }, + { + name: "ConfigBase64AndCustomConfigPath", + env: []string{"DOCKER_CONFIG=/secret"}, + configBase64: defaultConfigJSON, + validate: validateDockerConfig, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Ensures that a Git repository with a Dockerfile is cloned and built. + srv := gittest.CreateGitServer(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": fmt.Sprintf(` + FROM %[1]s + RUN if [ -f %[2]q ]; then cat %[2]q > /docker_config_json; fi + `, testImageAlpine, configJSONContainerPath), + }, + }) + + logbuf := new(bytes.Buffer) + opts := runOpts{ + env: []string{ + envbuilderEnv("GIT_URL", srv.URL), + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + }, + logbuf: logbuf, + } - require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder") - require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ") + if tt.configFile.name != "" { + dir := t.TempDir() + configFile := filepath.Join(dir, filepath.Base(tt.configFile.name)) + err := os.WriteFile(configFile, []byte(tt.configFile.data), 0o600) + require.NoError(t, err, "failed to write config") + + opts.privileged = true + opts.binds = []string{fmt.Sprintf("%s:%s:rw", configFile, tt.configFile.name)} + } + t.Log(opts.binds) + if tt.configBase64 != "" { + enc := base64.StdEncoding.EncodeToString([]byte(tt.configBase64)) + tt.env = append(tt.env, envbuilderEnv("DOCKER_CONFIG_BASE64", enc)) + } + + opts.env = append(opts.env, tt.env...) + + ctrID, err := runEnvbuilder(t, opts) + require.NoError(t, err) + + tt.validate(t, tt, ctrID, logbuf.String()) + + // Always verify that the Docker configuration secret file is removed. + configJSONContainerPath := workingdir.Default.Join(".docker", "config.json") + output := execContainer(t, ctrID, "stat "+configJSONContainerPath) + require.Contains(t, output, "No such file or directory") + }) + } } func TestBuildPrintBuildOutput(t *testing.T) { From d463de44e52b3f39b7c1dab9e77a33ec609fdafb Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 29 Oct 2024 17:39:31 +0000 Subject: [PATCH 07/10] remote shadow --- integration/integration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index c0fb8246..425436c0 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -666,7 +666,6 @@ func TestBuildFromDockerfileAndConfig(t *testing.T) { tt.validate(t, tt, ctrID, logbuf.String()) // Always verify that the Docker configuration secret file is removed. - configJSONContainerPath := workingdir.Default.Join(".docker", "config.json") output := execContainer(t, ctrID, "stat "+configJSONContainerPath) require.Contains(t, output, "No such file or directory") }) From 86db5a5b8090ced926068fa302fa108d4adabb2c Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 29 Oct 2024 17:45:10 +0000 Subject: [PATCH 08/10] add a bit of extra validation --- integration/integration_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/integration/integration_test.go b/integration/integration_test.go index 425436c0..a940597b 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -562,6 +562,9 @@ func TestBuildFromDockerfileAndConfig(t *testing.T) { validateDockerConfig := func(t *testing.T, tc testCase, ctrID, logs string) { t.Helper() + + // Ensure that the config matches the expected value, base64 is + // always prioritized over a file. got := execContainer(t, ctrID, "cat /docker_config_json") got = strings.TrimSpace(got) want := tc.configBase64 @@ -572,6 +575,15 @@ func TestBuildFromDockerfileAndConfig(t *testing.T) { require.Contains(t, logs, "Set DOCKER_CONFIG to /.envbuilder/.docker") require.Equal(t, want, got) } + + // Ensure that a warning message is printed if config secrets + // will remain in the container after build. + warningMessage := "this file will remain after the build" + if tc.configFile.name != "" { + require.Contains(t, logs, warningMessage) + } else { + require.NotContains(t, logs, warningMessage) + } } configJSONContainerPath := workingdir.Default.Join(".docker", "config.json") From 296ab910ff821ba703c4d454f44fb4bd6aa0b917 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 29 Oct 2024 17:49:42 +0000 Subject: [PATCH 09/10] remove log --- integration/integration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index a940597b..cc94ed78 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -664,7 +664,6 @@ func TestBuildFromDockerfileAndConfig(t *testing.T) { opts.privileged = true opts.binds = []string{fmt.Sprintf("%s:%s:rw", configFile, tt.configFile.name)} } - t.Log(opts.binds) if tt.configBase64 != "" { enc := base64.StdEncoding.EncodeToString([]byte(tt.configBase64)) tt.env = append(tt.env, envbuilderEnv("DOCKER_CONFIG_BASE64", enc)) From 167af1854b9e001264f6733e2887eb06a5edcf49 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 30 Oct 2024 13:58:28 +0000 Subject: [PATCH 10/10] update docs, add make gen --- Makefile | 5 ++++- docs/container-registry-auth.md | 14 ++++++++++++-- docs/env-variables.md | 2 +- options/options.go | 4 +++- options/testdata/options.golden | 4 +++- scripts/docsgen/main.go | 2 +- 6 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index ca4c0e6d..ce079015 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,9 @@ develop: build: scripts/envbuilder-$(GOARCH) ./scripts/build.sh +.PHONY: gen +gen: docs/env-variables.md update-golden-files + .PHONY: update-golden-files update-golden-files: .gen-golden @@ -85,4 +88,4 @@ test-images-pull: docker push localhost:5000/envbuilder-test-ubuntu:latest .registry-cache/docker/registry/v2/repositories/envbuilder-test-codercom-code-server: - docker push localhost:5000/envbuilder-test-codercom-code-server:latest \ No newline at end of file + docker push localhost:5000/envbuilder-test-codercom-code-server:latest diff --git a/docs/container-registry-auth.md b/docs/container-registry-auth.md index e0d7663e..f14a66d4 100644 --- a/docs/container-registry-auth.md +++ b/docs/container-registry-auth.md @@ -14,9 +14,19 @@ After you have a configuration that resembles the following: } ``` -`base64` encode the JSON and provide it to envbuilder as the `ENVBUILDER_DOCKER_CONFIG_BASE64` environment variable. +`base64` encode the JSON and provide it to envbuilder as the +`ENVBUILDER_DOCKER_CONFIG_BASE64` environment variable. -Alternatively, if running `envbuilder` in Kubernetes, you can create an `ImagePullSecret` and +Alternatively, the configuration file can be placed in `/.envbuilder/config.json`. +The `DOCKER_CONFIG` environment variable can be used to define a custom path. The +path must either be the path to a directory containing `config.json` or the full +path to the JSON file itself. + +> [!NOTE] Providing the docker configuration through other means than the +> `ENVBUILDER_DOCKER_CONFIG_BASE64` environment variable will leave the +> configuration file in the container filesystem. This may be a security risk. + +When running `envbuilder` in Kubernetes, you can create an `ImagePullSecret` and pass it into the pod as a volume mount. This example will work for all registries. ```shell diff --git a/docs/env-variables.md b/docs/env-variables.md index 5513fd55..cb7054f3 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -15,7 +15,7 @@ | `--dockerfile-path` | `ENVBUILDER_DOCKERFILE_PATH` | | The relative path to the Dockerfile that will be used to build the workspace. This is an alternative to using a devcontainer that some might find simpler. | | `--build-context-path` | `ENVBUILDER_BUILD_CONTEXT_PATH` | | Can be specified when a DockerfilePath is specified outside the base WorkspaceFolder. This path MUST be relative to the WorkspaceFolder path into which the repo is cloned. | | `--cache-ttl-days` | `ENVBUILDER_CACHE_TTL_DAYS` | | The number of days to use cached layers before expiring them. Defaults to 7 days. | -| `--docker-config-base64` | `ENVBUILDER_DOCKER_CONFIG_BASE64` | | The base64 encoded Docker config file that will be used to pull images from private container registries. | +| `--docker-config-base64` | `ENVBUILDER_DOCKER_CONFIG_BASE64` | | The base64 encoded Docker config file that will be used to pull images from private container registries. When this is set, Docker configuration set via the DOCKER_CONFIG environment variable is ignored. | | `--fallback-image` | `ENVBUILDER_FALLBACK_IMAGE` | | Specifies an alternative image to use when neither an image is declared in the devcontainer.json file nor a Dockerfile is present. If there's a build failure (from a faulty Dockerfile) or a misconfiguration, this image will be the substitute. Set ExitOnBuildFailure to true to halt the container if the build faces an issue. | | `--exit-on-build-failure` | `ENVBUILDER_EXIT_ON_BUILD_FAILURE` | | Terminates the container upon a build failure. This is handy when preferring the FALLBACK_IMAGE in cases where no devcontainer.json or image is provided. However, it ensures that the container stops if the build process encounters an error. | | `--force-safe` | `ENVBUILDER_FORCE_SAFE` | | Ignores any filesystem safety checks. This could cause serious harm to your system! This is used in cases where bypass is needed to unblock customers. | diff --git a/options/options.go b/options/options.go index ebd5b69c..efa61be7 100644 --- a/options/options.go +++ b/options/options.go @@ -277,7 +277,9 @@ func (o *Options) CLI() serpent.OptionSet { Env: WithEnvPrefix("DOCKER_CONFIG_BASE64"), Value: serpent.StringOf(&o.DockerConfigBase64), Description: "The base64 encoded Docker config file that " + - "will be used to pull images from private container registries.", + "will be used to pull images from private container registries. " + + "When this is set, Docker configuration set via the DOCKER_CONFIG " + + "environment variable is ignored.", }, { Flag: "fallback-image", diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 518f7741..397d5e0b 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -47,7 +47,9 @@ OPTIONS: --docker-config-base64 string, $ENVBUILDER_DOCKER_CONFIG_BASE64 The base64 encoded Docker config file that will be used to pull images - from private container registries. + from private container registries. When this is set, Docker + configuration set via the DOCKER_CONFIG environment variable is + ignored. --dockerfile-path string, $ENVBUILDER_DOCKERFILE_PATH The relative path to the Dockerfile that will be used to build the diff --git a/scripts/docsgen/main.go b/scripts/docsgen/main.go index b61de096..deb308f8 100644 --- a/scripts/docsgen/main.go +++ b/scripts/docsgen/main.go @@ -16,5 +16,5 @@ func main() { if err != nil { panic(err) } - fmt.Printf("%s updated successfully with the latest flags!", path) + fmt.Printf("%s updated successfully with the latest flags!\n", path) }