From e72603c2f2e78faa5336678dd511024da68e49cb Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Tue, 18 Apr 2023 11:59:51 -0400 Subject: [PATCH] use sync.Once instead of mutex --- pkg/authn/keychain.go | 100 ++++++++++++++++++++----------------- pkg/authn/keychain_test.go | 8 +-- 2 files changed, 58 insertions(+), 50 deletions(-) diff --git a/pkg/authn/keychain.go b/pkg/authn/keychain.go index d5a4fc863..ac40ea423 100644 --- a/pkg/authn/keychain.go +++ b/pkg/authn/keychain.go @@ -50,6 +50,9 @@ type Keychain interface { type defaultKeychain struct { mu sync.Mutex + once sync.Once + cfg types.AuthConfig + configFilePath string } @@ -131,56 +134,61 @@ func getDefaultConfigFile() (*configfile.ConfigFile, error) { } func (dk *defaultKeychain) Resolve(target Resource) (Authenticator, error) { - dk.mu.Lock() - defer dk.mu.Unlock() - - var cf *configfile.ConfigFile - if dk.configFilePath == "" { - var err error - cf, err = getDefaultConfigFile() - if err != nil { - return nil, err - } - if cf == nil { - return Anonymous, nil - } - } else { - f, err := os.Open(dk.configFilePath) - if err != nil { - return Anonymous, nil - } - defer f.Close() - cf, err = config.LoadFromReader(f) - if err != nil { - return nil, err + 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 - } - - var err error - 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 + // 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 + } } + }) + if err != nil { + return nil, err } + + cfg := dk.cfg if cfg == empty { return Anonymous, nil } diff --git a/pkg/authn/keychain_test.go b/pkg/authn/keychain_test.go index 5a2796b61..ec2c3b745 100644 --- a/pkg/authn/keychain_test.go +++ b/pkg/authn/keychain_test.go @@ -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 := DefaultKeychain.Resolve(testRegistry) + auth, err := NewConfigKeychain("").Resolve(testRegistry) if err != nil { t.Fatalf("Resolve() = %v", err) } @@ -140,7 +140,7 @@ func TestPodmanConfig(t *testing.T) { t.Fatalf("write %q: %v", cfg, err) } defer func() { os.Remove(cfg) }() - auth, err = DefaultKeychain.Resolve(testRegistry) + auth, err = NewConfigKeychain("").Resolve(testRegistry) if err != nil { t.Fatalf("Resolve() = %v", err) } @@ -164,7 +164,7 @@ func TestPodmanConfig(t *testing.T) { cd := setupConfigFile(t, content) defer os.RemoveAll(filepath.Dir(cd)) - auth, err = DefaultKeychain.Resolve(testRegistry) + auth, err = NewConfigKeychain("").Resolve(testRegistry) if err != nil { t.Fatalf("Resolve() = %v", err) } @@ -310,7 +310,7 @@ func TestVariousPaths(t *testing.T) { // For some reason, these tempdirs don't get cleaned up. defer os.RemoveAll(filepath.Dir(cd)) - auth, err := DefaultKeychain.Resolve(test.target) + auth, err := NewConfigKeychain("").Resolve(test.target) if test.wantErr { if err == nil { t.Fatal("wanted err, got nil")