use sync.Once instead of mutex

google · Apr 18, 2023 · e72603c · e72603c
1 parent 5080696
commit e72603c
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 50 deletions.
diff --git 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
@@ -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")