Skip to content
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

Revert "authn: Add NewConfigKeychain to load a config from explicit path" #1664

Merged
merged 1 commit into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
109 changes: 27 additions & 82 deletions pkg/authn/keychain.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,11 @@ type Keychain interface {
// defaultKeychain implements Keychain with the semantics of the standard Docker
// credential keychain.
type defaultKeychain struct {
once sync.Once
cfg types.AuthConfig

configFilePath string
mu sync.Mutex
}

var (
// DefaultKeychain implements Keychain by interpreting the Docker config file.
// This matches the behavior of tools like `docker` and `podman`.
//
// This keychain looks for credentials configured in a few places, in order:
//
// 1. $HOME/.docker/config.json
// 2. $DOCKER_CONFIG/config.json
// 3. $XDG_RUNTIME_DIR/containers/auth.json (for compatibility with Podman)
//
// If a config file is found and can be parsed, Resolve will return credentials
// configured by the file for the given registry.
//
// If no config file is found, Resolve returns Anonymous.
// If a config file is found but can't be parsed, Resolve returns an error.
// DefaultKeychain implements Keychain by interpreting the docker config file.
DefaultKeychain = RefreshingKeychain(&defaultKeychain{}, 5*time.Minute)
)

Expand All @@ -78,16 +62,11 @@ const (
DefaultAuthKey = "https://" + name.DefaultRegistry + "/v1/"
)

// NewConfigKeychain implements Keychain by interpreting the Docker config file
// at the specified file path.
//
// It acts like DefaultKeychain except that the exact path of the file can be specified,
// instead of being dependent on environment variables and conventional file names.
func NewConfigKeychain(filename string) Keychain {
return &defaultKeychain{configFilePath: filename}
}
// Resolve implements Keychain.
func (dk *defaultKeychain) Resolve(target Resource) (Authenticator, error) {
dk.mu.Lock()
defer dk.mu.Unlock()

func getDefaultConfigFile() (*configfile.ConfigFile, error) {
// Podman users may have their container registry auth configured in a
// different location, that Docker packages aren't aware of.
// If the Docker config file isn't found, we'll fallback to look where
Expand Down Expand Up @@ -120,73 +99,39 @@ func getDefaultConfigFile() (*configfile.ConfigFile, error) {
} else {
f, err := os.Open(filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "containers/auth.json"))
if err != nil {
return nil, nil
return Anonymous, nil
}
defer f.Close()
cf, err = config.LoadFromReader(f)
if err != nil {
return nil, err
}
}
return cf, nil
}

func (dk *defaultKeychain) Resolve(target Resource) (Authenticator, error) {
var err error
var empty types.AuthConfig
dk.once.Do(func() {
var cf *configfile.ConfigFile
if dk.configFilePath == "" {
cf, err = getDefaultConfigFile()
if err != nil {
return
}
if cf == nil {
dk.cfg = empty
return
}
} else {
var f *os.File
f, err = os.Open(dk.configFilePath)
if err != nil {
return
}
defer f.Close()
cf, err = config.LoadFromReader(f)
if err != nil {
return
}
// See:
// https://github.com/google/ko/issues/90
// https://github.com/moby/moby/blob/fc01c2b481097a6057bec3cd1ab2d7b4488c50c4/registry/config.go#L397-L404
var cfg, empty types.AuthConfig
for _, key := range []string{
target.String(),
target.RegistryStr(),
} {
if key == name.DefaultRegistry {
key = DefaultAuthKey
}

// See:
// https://github.com/google/ko/issues/90
// https://github.com/moby/moby/blob/fc01c2b481097a6057bec3cd1ab2d7b4488c50c4/registry/config.go#L397-L404
for _, key := range []string{
target.String(),
target.RegistryStr(),
} {
if key == name.DefaultRegistry {
key = DefaultAuthKey
}

dk.cfg, err = cf.GetAuthConfig(key)
if err != nil {
return
}
// cf.GetAuthConfig automatically sets the ServerAddress attribute. Since
// we don't make use of it, clear the value for a proper "is-empty" test.
// See: https://github.com/google/go-containerregistry/issues/1510
dk.cfg.ServerAddress = ""
if dk.cfg != empty {
break
}
cfg, err = cf.GetAuthConfig(key)
if err != nil {
return nil, err
}
// cf.GetAuthConfig automatically sets the ServerAddress attribute. Since
// we don't make use of it, clear the value for a proper "is-empty" test.
// See: https://github.com/google/go-containerregistry/issues/1510
cfg.ServerAddress = ""
if cfg != empty {
break
}
})
if err != nil {
return nil, err
}

cfg := dk.cfg
if cfg == empty {
return Anonymous, nil
}
Expand Down
41 changes: 4 additions & 37 deletions pkg/authn/keychain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestPodmanConfig(t *testing.T) {
// At first, $DOCKER_CONFIG is unset and $HOME/.docker/config.json isn't
// found, but Podman auth is configured. This should return Podman's
// auth.
auth, err := NewConfigKeychain("").Resolve(testRegistry)
auth, err := DefaultKeychain.Resolve(testRegistry)
if err != nil {
t.Fatalf("Resolve() = %v", err)
}
Expand All @@ -140,7 +140,7 @@ func TestPodmanConfig(t *testing.T) {
t.Fatalf("write %q: %v", cfg, err)
}
defer func() { os.Remove(cfg) }()
auth, err = NewConfigKeychain("").Resolve(testRegistry)
auth, err = DefaultKeychain.Resolve(testRegistry)
if err != nil {
t.Fatalf("Resolve() = %v", err)
}
Expand All @@ -164,7 +164,7 @@ func TestPodmanConfig(t *testing.T) {
cd := setupConfigFile(t, content)
defer os.RemoveAll(filepath.Dir(cd))

auth, err = NewConfigKeychain("").Resolve(testRegistry)
auth, err = DefaultKeychain.Resolve(testRegistry)
if err != nil {
t.Fatalf("Resolve() = %v", err)
}
Expand All @@ -181,39 +181,6 @@ func TestPodmanConfig(t *testing.T) {
}
}

func TestAuthConfigPath(t *testing.T) {
tmpdir := os.Getenv("TEST_TMPDIR")
if tmpdir == "" {
tmpdir = t.TempDir()
}
fresh++
p := filepath.Join(tmpdir, fmt.Sprintf("%d", fresh))
if err := os.MkdirAll(filepath.Join(p, "custom"), 0777); err != nil {
t.Fatalf("mkdir %s/custom: %v", p, err)
}
cfg := filepath.Join(p, "cfg.xml")
content := fmt.Sprintf(`{"auths": {"test.io": {"auth": %q}}}`, encode("foo", "bar"))
if err := os.WriteFile(cfg, []byte(content), 0600); err != nil {
t.Fatalf("write %q: %v", cfg, err)
}

auth, err := NewConfigKeychain(cfg).Resolve(testRegistry)
if err != nil {
t.Fatalf("Resolve() = %v", err)
}
got, err := auth.Authorization()
if err != nil {
t.Fatal(err)
}
want := &AuthConfig{
Username: "foo",
Password: "bar",
}
if !reflect.DeepEqual(got, want) {
t.Errorf("got %+v, want %+v", got, want)
}
}

func encode(user, pass string) string {
delimited := fmt.Sprintf("%s:%s", user, pass)
return base64.StdEncoding.EncodeToString([]byte(delimited))
Expand Down Expand Up @@ -310,7 +277,7 @@ func TestVariousPaths(t *testing.T) {
// For some reason, these tempdirs don't get cleaned up.
defer os.RemoveAll(filepath.Dir(cd))

auth, err := NewConfigKeychain("").Resolve(test.target)
auth, err := DefaultKeychain.Resolve(test.target)
if test.wantErr {
if err == nil {
t.Fatal("wanted err, got nil")
Expand Down