Skip to content

Commit

Permalink
Document GetDefaultAuthFile, and don't call it in CLI defaults
Browse files Browse the repository at this point in the history
We _need_ to know whether the user explicitly set an option
or not, because --authfile and --compat-auth-file conflict.

Also, --authfile= (unset) and --authfile=$the_default_auth_json
have different effects, so setting the default value of the option
is _not_ a practical way to show what the system would do if the
option were not specified.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Nov 13, 2023
1 parent 2e16fde commit de06ef8
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
28 changes: 28 additions & 0 deletions pkg/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,24 @@ func (e ErrNewCredentialsInvalid) Unwrap() error {
// GetDefaultAuthFile returns env value REGISTRY_AUTH_FILE as default
// --authfile path used in multiple --authfile flag definitions
// Will fail over to DOCKER_CONFIG if REGISTRY_AUTH_FILE environment is not set
//
// WARNINGS:
// - In almost all invocations, expect this function to return ""; so it can not be used
// for directly accessing the file.
// - Use this only for commands that _read_ credentials, not write them.
// The path may refer to github.com/containers auth.json, or to Docker config.json,
// and the distinction is lost; writing auth.json data to config.json may not be consumable by Docker,
// or it may overwrite and discard unrelated Docker configuration set by the user.
func GetDefaultAuthFile() string {
// Keep this in sync with the default logic in systemContextWithOptions!

if authfile := os.Getenv("REGISTRY_AUTH_FILE"); authfile != "" {
return authfile
}
// This pre-existing behavior is not conceptually consistent:
// If users have a ~/.docker/config.json in the default path, and no environment variable
// set, we read auth.json first, falling back to config.json;
// but if DOCKER_CONFIG is set, we read only config.json in that path, and we don’t read auth.json at all.
if authEnv := os.Getenv("DOCKER_CONFIG"); authEnv != "" {
return filepath.Join(authEnv, "config.json")
}
Expand Down Expand Up @@ -74,6 +88,20 @@ func systemContextWithOptions(sys *types.SystemContext, authFile, certDir string

if authFile != "" {
sys.AuthFilePath = authFile
} else {
// Keep this in sync with GetDefaultAuthFile()!
//
// Note that c/image does not natively implement the REGISTRY_AUTH_FILE
// variable, so not all callers look for credentials in this location.
if authFileVar := os.Getenv("REGISTRY_AUTH_FILE"); authFileVar != "" {
sys.AuthFilePath = authFileVar
} else if dockerConfig := os.Getenv("DOCKER_CONFIG"); dockerConfig != "" {
// This preserves pre-existing _inconsistent_ behavior:
// If the Docker configuration exists in the default ~/.docker/config.json location,
// we DO NOT write to it; instead, we update auth.json in the default path.
// Only if the user explicitly sets DOCKER_CONFIG, we write to that config.json.
sys.AuthFilePath = filepath.Join(dockerConfig, "config.json")
}
}
if certDir != "" {
sys.DockerCertPath = certDir
Expand Down
4 changes: 2 additions & 2 deletions pkg/auth/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type LogoutOptions struct {
// GetLoginFlags defines and returns login flags for containers tools
func GetLoginFlags(flags *LoginOptions) *pflag.FlagSet {
fs := pflag.FlagSet{}
fs.StringVar(&flags.AuthFile, "authfile", GetDefaultAuthFile(), "path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
fs.StringVar(&flags.AuthFile, "authfile", "", "path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
fs.StringVar(&flags.CertDir, "cert-dir", "", "use certificates at the specified path to access the registry")
fs.StringVarP(&flags.Password, "password", "p", "", "Password for registry")
fs.StringVarP(&flags.Username, "username", "u", "", "Username for registry")
Expand All @@ -68,7 +68,7 @@ func GetLoginFlagsCompletions() completion.FlagCompletions {
// GetLogoutFlags defines and returns logout flags for containers tools
func GetLogoutFlags(flags *LogoutOptions) *pflag.FlagSet {
fs := pflag.FlagSet{}
fs.StringVar(&flags.AuthFile, "authfile", GetDefaultAuthFile(), "path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
fs.StringVar(&flags.AuthFile, "authfile", "", "path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override")
fs.BoolVarP(&flags.All, "all", "a", false, "Remove the cached credentials for all registries in the auth file")
return &fs
}
Expand Down

0 comments on commit de06ef8

Please sign in to comment.