-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: search $DOCKER_CONFIG
if no base64 config is provided
#398
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8c0198f
fix: always set DOCKER_CONFIG unless manually set and no base64 confi…
mafredri 5fbcf2d
simplify if statement
mafredri d559c1d
simplify return
mafredri 5da2aaa
copy config to protect against remount, fix docker perms
mafredri 6459b39
rewrite config detection and env override
mafredri 3f6830d
update tests
mafredri d463de4
remote shadow
mafredri 86db5a5
add a bit of extra validation
mafredri 296ab91
remove log
mafredri 167af18
update docs, add make gen
mafredri 8ad40eb
Merge branch 'main' into mafredri/fix-docker-config-path
mafredri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -154,13 +155,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.Filesystem, 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 | ||
|
||
|
@@ -717,6 +718,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) | ||
|
@@ -776,11 +782,6 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro | |
exportEnvFile.Close() | ||
} | ||
|
||
// Remove the Docker config secret file! | ||
if err := cleanupDockerConfigJSON(); err != nil { | ||
return err | ||
} | ||
|
||
if runtimeData.ContainerUser == "" { | ||
opts.Logger(log.LevelWarn, "#%d: no user specified, using root", stageNumber) | ||
} | ||
|
@@ -984,13 +985,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.Filesystem, 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 | ||
|
||
|
@@ -1321,7 +1322,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 | ||
} | ||
|
||
|
@@ -1573,8 +1574,22 @@ 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) { | ||
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 { | ||
|
@@ -1601,6 +1616,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 { | ||
|
@@ -1633,55 +1663,161 @@ 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 } | ||
if dockerConfigBase64 == "" { | ||
return noop, 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) { | ||
// 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) | ||
} | ||
} | ||
|
||
oldDockerConfig := os.Getenv(dockerConfigEnvKey) | ||
var oldDockerConfigFile string | ||
if oldDockerConfig != "" { | ||
oldDockerConfigFile = filepath.Join(oldDockerConfig, dockerConfigFile) | ||
} | ||
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.LevelWarn, "Found Docker config at %s, this file will remain after the build", path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
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) | ||
} | ||
break | ||
} | ||
|
||
if dockerConfigJSON == nil { | ||
// No user-provided config available. | ||
return func() error { return nil }, nil | ||
} | ||
|
||
dockerConfigJSON, err = hujson.Standardize(dockerConfigJSON) | ||
if err != nil { | ||
return nil, fmt.Errorf("humanize json for docker config: %w", err) | ||
} | ||
cfgPath := workingDir.Join("config.json") | ||
decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64) | ||
|
||
if err = logDockerAuthConfigs(logf, dockerConfigJSON); err != nil { | ||
return nil, fmt.Errorf("log docker auth configs: %w", err) | ||
} | ||
|
||
// 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 nil, fmt.Errorf("create docker config dir: %w", err) | ||
} | ||
|
||
if fileExists(bfs, newDockerConfigFile) { | ||
return nil, fmt.Errorf("unable to write Docker config file, file already exists: %s", newDockerConfigFile) | ||
} | ||
|
||
restoreEnv, err := setAndRestoreEnv(logf, dockerConfigEnvKey, newDockerConfig) | ||
if err != nil { | ||
return noop, fmt.Errorf("decode docker config: %w", err) | ||
return nil, fmt.Errorf("set docker config override: %w", err) | ||
} | ||
var configFile DockerConfig | ||
decoded, err = hujson.Standardize(decoded) | ||
|
||
err = writeFile(bfs, newDockerConfigFile, dockerConfigJSON, 0o600) | ||
if err != nil { | ||
return noop, fmt.Errorf("humanize json for docker config: %w", err) | ||
_ = restoreEnv() // Best effort. | ||
return nil, fmt.Errorf("write docker config: %w", err) | ||
} | ||
err = json.Unmarshal(decoded, &configFile) | ||
logf(log.LevelInfo, "Wrote Docker config JSON to %s", newDockerConfigFile) | ||
|
||
cleanupFile := onceErrFunc(func() error { | ||
// Remove the Docker config secret file! | ||
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 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 noop, fmt.Errorf("parse docker config: %w", err) | ||
return fmt.Errorf("load docker config: %w", err) | ||
} | ||
for k := range configFile.AuthConfigs { | ||
for k := range dc.AuthConfigs { | ||
logf(log.LevelInfo, "Docker config contains auth for registry %q", k) | ||
} | ||
err = os.WriteFile(cfgPath, decoded, 0o644) | ||
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 { | ||
return noop, 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 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) | ||
// Remove the Docker config secret file! | ||
if cleanupErr = os.Remove(cfgPath); err != nil { | ||
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 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 { | ||
var once sync.Once | ||
return func() error { | ||
var err error | ||
once.Do(func() { | ||
err = f() | ||
}) | ||
return cleanupErr | ||
return err | ||
} | ||
return cleanup, err | ||
} | ||
|
||
// Allows quick testing of layer caching using a local directory! | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review: Moved this higher up so that when we reset DOCKER_CONFIG, we don't interfere with envbuilder trying to set envs from the build/devcontainer.