From d8783fe25d718f794ab6d64c71f6c3008a88262c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 17 Jan 2024 17:33:52 +0800 Subject: [PATCH] feat: error type for not finding basic auth creds and export config path (#674) Two changes proposed in this PR 1) When failed to fetch credentials for basic auth, return a new error 2) Add a new function to DynamicStore An example usage of the proposed change: oras-project/oras#1235, resolves #676 Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 6 +++++- registry/remote/auth/client_test.go | 12 ++++++++++++ .../remote/credentials/internal/config/config.go | 5 +++++ .../credentials/internal/config/config_test.go | 10 ++++++++++ registry/remote/credentials/store.go | 5 +++++ registry/remote/credentials/store_test.go | 13 +++++++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index 58355161..8d9685a2 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -31,6 +31,10 @@ import ( "oras.land/oras-go/v2/registry/remote/retry" ) +// ErrBasicCredentialNotFound is returned when the credential is not found for +// basic auth. +var ErrBasicCredentialNotFound = errors.New("basic credential not found") + // DefaultClient is the default auth-decorated client. var DefaultClient = &Client{ Client: retry.DefaultClient, @@ -280,7 +284,7 @@ func (c *Client) fetchBasicAuth(ctx context.Context, registry string) (string, e return "", fmt.Errorf("failed to resolve credential: %w", err) } if cred == EmptyCredential { - return "", errors.New("credential required for basic auth") + return "", ErrBasicCredentialNotFound } if cred.Username == "" || cred.Password == "" { return "", errors.New("missing username or password for basic auth") diff --git a/registry/remote/auth/client_test.go b/registry/remote/auth/client_test.go index de879863..9f2b473c 100644 --- a/registry/remote/auth/client_test.go +++ b/registry/remote/auth/client_test.go @@ -3964,3 +3964,15 @@ func TestClient_StaticCredential_withRefreshToken(t *testing.T) { t.Errorf("incorrect error: %v, expected %v", err, expectedError) } } + +func TestClient_fetchBasicAuth(t *testing.T) { + c := &Client{ + Credential: func(ctx context.Context, registry string) (Credential, error) { + return EmptyCredential, nil + }, + } + _, err := c.fetchBasicAuth(context.Background(), "") + if err != ErrBasicCredentialNotFound { + t.Errorf("incorrect error: %v, expected %v", err, ErrBasicCredentialNotFound) + } +} diff --git a/registry/remote/credentials/internal/config/config.go b/registry/remote/credentials/internal/config/config.go index 5bb66a0e..20ee0743 100644 --- a/registry/remote/credentials/internal/config/config.go +++ b/registry/remote/credentials/internal/config/config.go @@ -224,6 +224,11 @@ func (cfg *Config) CredentialsStore() string { return cfg.credentialsStore } +// Path returns the path to the config file. +func (cfg *Config) Path() string { + return cfg.path +} + // SetCredentialsStore puts the configured credentials store. func (cfg *Config) SetCredentialsStore(credsStore string) error { cfg.rwLock.Lock() diff --git a/registry/remote/credentials/internal/config/config_test.go b/registry/remote/credentials/internal/config/config_test.go index 6622108b..a18eccac 100644 --- a/registry/remote/credentials/internal/config/config_test.go +++ b/registry/remote/credentials/internal/config/config_test.go @@ -1450,3 +1450,13 @@ func Test_toHostname(t *testing.T) { }) } } + +func TestConfig_Path(t *testing.T) { + mockedPath := "/path/to/config.json" + config := Config{ + path: mockedPath, + } + if got := config.Path(); got != mockedPath { + t.Errorf("Config.Path() = %v, want %v", got, mockedPath) + } +} diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index 973e0e67..ae8ce5be 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -167,6 +167,11 @@ func (ds *DynamicStore) IsAuthConfigured() bool { return ds.config.IsAuthConfigured() } +// ConfigPath returns the path to the config file. +func (ds *DynamicStore) ConfigPath() string { + return ds.config.Path() +} + // getHelperSuffix returns the credential helper suffix for the given server // address. func (ds *DynamicStore) getHelperSuffix(serverAddress string) string { diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index 285e2796..919f8757 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -612,6 +612,19 @@ func Test_DynamicStore_getHelperSuffix(t *testing.T) { } } +func Test_DynamicStore_ConfigPath(t *testing.T) { + path := "../../testdata/credsStore_config.json" + var err error + store, err := NewStore(path, StoreOptions{}) + if err != nil { + t.Fatal("NewFileStore() error =", err) + } + got := store.ConfigPath() + if got != path { + t.Errorf("Config.GetPath() = %v, want %v", got, path) + } +} + func Test_DynamicStore_getStore_nativeStore(t *testing.T) { tests := []struct { name string