From 827fcb147a802cf4629bd74a7daf19bc50460e71 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 8 Jan 2024 09:18:42 +0000 Subject: [PATCH 01/14] feat: new error type of empty creds for basic auth Signed-off-by: Billy Zha --- errdef/errors.go | 1 + registry/remote/auth/client.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/errdef/errors.go b/errdef/errors.go index 7adb44b1..07855337 100644 --- a/errdef/errors.go +++ b/errdef/errors.go @@ -28,4 +28,5 @@ var ( ErrSizeExceedsLimit = errors.New("size exceeds limit") ErrUnsupported = errors.New("unsupported") ErrUnsupportedVersion = errors.New("unsupported version") + ErrBasicCredNotFound = errors.New("basic credential not found") ) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index 58355161..adc9fcb1 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -27,6 +27,7 @@ import ( "net/url" "strings" + "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry/remote/internal/errutil" "oras.land/oras-go/v2/registry/remote/retry" ) @@ -280,7 +281,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 "", errdef.ErrBasicCredNotFound } if cred.Username == "" || cred.Password == "" { return "", errors.New("missing username or password for basic auth") From e70a1f8ee572101e471b61ba6c3f866e1386bd87 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 8 Jan 2024 14:30:05 +0000 Subject: [PATCH 02/14] export func to get docker config path Signed-off-by: Billy Zha --- registry/remote/credentials/store.go | 6 +++--- registry/remote/credentials/store_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index 973e0e67..21e246b0 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -121,7 +121,7 @@ func NewStore(configPath string, opts StoreOptions) (*DynamicStore, error) { // - https://docs.docker.com/engine/reference/commandline/cli/#configuration-files // - https://docs.docker.com/engine/reference/commandline/cli/#change-the-docker-directory func NewStoreFromDocker(opt StoreOptions) (*DynamicStore, error) { - configPath, err := getDockerConfigPath() + configPath, err := GetDockerConfigPath() if err != nil { return nil, err } @@ -193,8 +193,8 @@ func (ds *DynamicStore) getStore(serverAddress string) Store { return fs } -// getDockerConfigPath returns the path to the default docker config file. -func getDockerConfigPath() (string, error) { +// GetDockerConfigPath returns the path to the default docker config file. +func GetDockerConfigPath() (string, error) { // first try the environment variable configDir := os.Getenv(dockerConfigDirEnv) if configDir == "" { diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index 285e2796..59773083 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -878,7 +878,7 @@ func Test_getDockerConfigPath_env(t *testing.T) { } t.Setenv("DOCKER_CONFIG", dir) - got, err := getDockerConfigPath() + got, err := GetDockerConfigPath() if err != nil { t.Fatal("getDockerConfigPath() error =", err) } @@ -890,7 +890,7 @@ func Test_getDockerConfigPath_env(t *testing.T) { func Test_getDockerConfigPath_homeDir(t *testing.T) { t.Setenv("DOCKER_CONFIG", "") - got, err := getDockerConfigPath() + got, err := GetDockerConfigPath() if err != nil { t.Fatal("getDockerConfigPath() error =", err) } From a6479aa1cba029edc3f29ce5d7f31e6210f6fbf4 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 9 Jan 2024 01:49:55 +0000 Subject: [PATCH 03/14] remove URL in returned error Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index adc9fcb1..e6b567fb 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -216,7 +216,7 @@ func (c *Client) Do(originalReq *http.Request) (*http.Response, error) { return c.fetchBasicAuth(ctx, host) }) if err != nil { - return nil, fmt.Errorf("%s %q: %w", resp.Request.Method, resp.Request.URL, err) + return nil, err } req = originalReq.Clone(ctx) From 8abeeaba18f7f1f38001c616b87095d201d95da4 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 10 Jan 2024 06:13:42 +0000 Subject: [PATCH 04/14] resolve comment Signed-off-by: Billy Zha --- errdef/errors.go | 1 - registry/remote/auth/client.go | 3 +-- registry/remote/auth/errors.go | 23 +++++++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 registry/remote/auth/errors.go diff --git a/errdef/errors.go b/errdef/errors.go index 07855337..7adb44b1 100644 --- a/errdef/errors.go +++ b/errdef/errors.go @@ -28,5 +28,4 @@ var ( ErrSizeExceedsLimit = errors.New("size exceeds limit") ErrUnsupported = errors.New("unsupported") ErrUnsupportedVersion = errors.New("unsupported version") - ErrBasicCredNotFound = errors.New("basic credential not found") ) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index e6b567fb..f97ba677 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -27,7 +27,6 @@ import ( "net/url" "strings" - "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry/remote/internal/errutil" "oras.land/oras-go/v2/registry/remote/retry" ) @@ -281,7 +280,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 "", errdef.ErrBasicCredNotFound + return "", ErrBasicCredNotFound } if cred.Username == "" || cred.Password == "" { return "", errors.New("missing username or password for basic auth") diff --git a/registry/remote/auth/errors.go b/registry/remote/auth/errors.go new file mode 100644 index 00000000..33a3f5cd --- /dev/null +++ b/registry/remote/auth/errors.go @@ -0,0 +1,23 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import "errors" + +// Common errors used in ORAS +var ( + ErrBasicCredNotFound = errors.New("basic credential not found") +) From e197c00aebb13736cec6ffe80b673f2e606cdc7f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 11 Jan 2024 08:59:18 +0000 Subject: [PATCH 05/14] add path function for dynamic Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 8 +++++-- registry/remote/auth/errors.go | 23 ------------------- .../credentials/internal/config/config.go | 8 +++++++ registry/remote/credentials/store.go | 11 ++++++--- registry/remote/credentials/store_test.go | 4 ++-- 5 files changed, 24 insertions(+), 30 deletions(-) delete mode 100644 registry/remote/auth/errors.go diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index f97ba677..16029947 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" ) +var ( + ErrBasicCredentialNotFound = errors.New("basic credential not found") +) + // DefaultClient is the default auth-decorated client. var DefaultClient = &Client{ Client: retry.DefaultClient, @@ -215,7 +219,7 @@ func (c *Client) Do(originalReq *http.Request) (*http.Response, error) { return c.fetchBasicAuth(ctx, host) }) if err != nil { - return nil, err + return nil, fmt.Errorf("%s %q: %w", resp.Request.Method, resp.Request.URL, err) } req = originalReq.Clone(ctx) @@ -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 "", ErrBasicCredNotFound + return "", ErrBasicCredentialNotFound } if cred.Username == "" || cred.Password == "" { return "", errors.New("missing username or password for basic auth") diff --git a/registry/remote/auth/errors.go b/registry/remote/auth/errors.go deleted file mode 100644 index 33a3f5cd..00000000 --- a/registry/remote/auth/errors.go +++ /dev/null @@ -1,23 +0,0 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package auth - -import "errors" - -// Common errors used in ORAS -var ( - ErrBasicCredNotFound = errors.New("basic credential not found") -) diff --git a/registry/remote/credentials/internal/config/config.go b/registry/remote/credentials/internal/config/config.go index 5bb66a0e..43cb8c0c 100644 --- a/registry/remote/credentials/internal/config/config.go +++ b/registry/remote/credentials/internal/config/config.go @@ -155,6 +155,14 @@ func Load(configPath string) (*Config, error) { return cfg, nil } +// Path returns the path to the config file. +func (cfg *Config) Path() (string, error) { + if cfg == nil { + errors.New("config file is nil") + } + return cfg.path, nil +} + // GetAuthConfig returns an auth.Credential for serverAddress. func (cfg *Config) GetCredential(serverAddress string) (auth.Credential, error) { cfg.rwLock.RLock() diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index 21e246b0..bbbbb893 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -121,7 +121,7 @@ func NewStore(configPath string, opts StoreOptions) (*DynamicStore, error) { // - https://docs.docker.com/engine/reference/commandline/cli/#configuration-files // - https://docs.docker.com/engine/reference/commandline/cli/#change-the-docker-directory func NewStoreFromDocker(opt StoreOptions) (*DynamicStore, error) { - configPath, err := GetDockerConfigPath() + configPath, err := getDockerConfigPath() if err != nil { return nil, err } @@ -167,6 +167,11 @@ func (ds *DynamicStore) IsAuthConfigured() bool { return ds.config.IsAuthConfigured() } +// GetConfigPath returns the path to the config file. +func (ds *DynamicStore) GetConfigPath() (string, error) { + return ds.config.Path() +} + // getHelperSuffix returns the credential helper suffix for the given server // address. func (ds *DynamicStore) getHelperSuffix(serverAddress string) string { @@ -193,8 +198,8 @@ func (ds *DynamicStore) getStore(serverAddress string) Store { return fs } -// GetDockerConfigPath returns the path to the default docker config file. -func GetDockerConfigPath() (string, error) { +// getDockerConfigPath returns the path to the default docker config file. +func getDockerConfigPath() (string, error) { // first try the environment variable configDir := os.Getenv(dockerConfigDirEnv) if configDir == "" { diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index 59773083..285e2796 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -878,7 +878,7 @@ func Test_getDockerConfigPath_env(t *testing.T) { } t.Setenv("DOCKER_CONFIG", dir) - got, err := GetDockerConfigPath() + got, err := getDockerConfigPath() if err != nil { t.Fatal("getDockerConfigPath() error =", err) } @@ -890,7 +890,7 @@ func Test_getDockerConfigPath_env(t *testing.T) { func Test_getDockerConfigPath_homeDir(t *testing.T) { t.Setenv("DOCKER_CONFIG", "") - got, err := GetDockerConfigPath() + got, err := getDockerConfigPath() if err != nil { t.Fatal("getDockerConfigPath() error =", err) } From df42924238fde5a778438e808475381aa7131ab5 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 11 Jan 2024 11:08:55 +0000 Subject: [PATCH 06/14] add unit test Signed-off-by: Billy Zha --- .../credentials/internal/config/config.go | 2 +- .../internal/config/config_test.go | 22 ++++++++++++++++ registry/remote/credentials/store_test.go | 25 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/registry/remote/credentials/internal/config/config.go b/registry/remote/credentials/internal/config/config.go index 43cb8c0c..bd00a010 100644 --- a/registry/remote/credentials/internal/config/config.go +++ b/registry/remote/credentials/internal/config/config.go @@ -158,7 +158,7 @@ func Load(configPath string) (*Config, error) { // Path returns the path to the config file. func (cfg *Config) Path() (string, error) { if cfg == nil { - errors.New("config file is nil") + return "", errors.New("config file is nil") } return cfg.path, nil } diff --git a/registry/remote/credentials/internal/config/config_test.go b/registry/remote/credentials/internal/config/config_test.go index 6622108b..c673f1a9 100644 --- a/registry/remote/credentials/internal/config/config_test.go +++ b/registry/remote/credentials/internal/config/config_test.go @@ -925,6 +925,28 @@ func TestConfig_GetCredentialHelper(t *testing.T) { } } +func TestConfig_Path(t *testing.T) { + cfg := Config{ + path: "/path/to/config.json", + } + got, err := cfg.Path() + + if err != nil { + t.Errorf("Config.GetPath() error = %v", err) + } + if got != cfg.path { + t.Errorf("Config.GetPath() = %v, want %v", got, cfg.path) + } +} + +func TestConfig_Path_nil(t *testing.T) { + var cfg *Config + _, err := cfg.Path() + if err == nil { + t.Error("expecting error, got nil") + } +} + func TestConfig_CredentialsStore(t *testing.T) { tests := []struct { name string diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index 285e2796..cf9f30cc 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -25,6 +25,7 @@ import ( "testing" "oras.land/oras-go/v2/registry/remote/auth" + "oras.land/oras-go/v2/registry/remote/credentials/internal/config" "oras.land/oras-go/v2/registry/remote/credentials/internal/config/configtest" ) @@ -611,6 +612,30 @@ func Test_DynamicStore_getHelperSuffix(t *testing.T) { }) } } +func Test_DynamicStore_GetConfigPath(t *testing.T) { + var store DynamicStore + var err error + path := "../../testdata/credsStore_config.json" + store.config, err = config.Load(path) + if err != nil { + t.Fatal("config.Load() error =", err) + } + got, err := store.GetConfigPath() + if err != nil { + t.Errorf("Config.GetPath() error = %v", err) + } + if got != path { + t.Errorf("Config.GetPath() = %v, want %v", got, path) + } +} + +func Test_DynamicStore_Path_nil(t *testing.T) { + var store DynamicStore + _, err := store.GetConfigPath() + if err == nil { + t.Error("expecting error, got nil") + } +} func Test_DynamicStore_getStore_nativeStore(t *testing.T) { tests := []struct { From 02575df6b1e72ec799e2652f3fd95e8320c17836 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 12 Jan 2024 06:16:09 +0000 Subject: [PATCH 07/14] resolve comments Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 4 +--- .../credentials/internal/config/config.go | 8 ------- .../internal/config/config_test.go | 22 ------------------- registry/remote/credentials/store.go | 15 ++++++++----- registry/remote/credentials/store_test.go | 12 +++++----- 5 files changed, 16 insertions(+), 45 deletions(-) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index 16029947..0c8012d8 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -31,9 +31,7 @@ import ( "oras.land/oras-go/v2/registry/remote/retry" ) -var ( - ErrBasicCredentialNotFound = errors.New("basic credential not found") -) +var ErrBasicCredentialNotFound = errors.New("basic credential not found") // DefaultClient is the default auth-decorated client. var DefaultClient = &Client{ diff --git a/registry/remote/credentials/internal/config/config.go b/registry/remote/credentials/internal/config/config.go index bd00a010..5bb66a0e 100644 --- a/registry/remote/credentials/internal/config/config.go +++ b/registry/remote/credentials/internal/config/config.go @@ -155,14 +155,6 @@ func Load(configPath string) (*Config, error) { return cfg, nil } -// Path returns the path to the config file. -func (cfg *Config) Path() (string, error) { - if cfg == nil { - return "", errors.New("config file is nil") - } - return cfg.path, nil -} - // GetAuthConfig returns an auth.Credential for serverAddress. func (cfg *Config) GetCredential(serverAddress string) (auth.Credential, error) { cfg.rwLock.RLock() diff --git a/registry/remote/credentials/internal/config/config_test.go b/registry/remote/credentials/internal/config/config_test.go index c673f1a9..6622108b 100644 --- a/registry/remote/credentials/internal/config/config_test.go +++ b/registry/remote/credentials/internal/config/config_test.go @@ -925,28 +925,6 @@ func TestConfig_GetCredentialHelper(t *testing.T) { } } -func TestConfig_Path(t *testing.T) { - cfg := Config{ - path: "/path/to/config.json", - } - got, err := cfg.Path() - - if err != nil { - t.Errorf("Config.GetPath() error = %v", err) - } - if got != cfg.path { - t.Errorf("Config.GetPath() = %v, want %v", got, cfg.path) - } -} - -func TestConfig_Path_nil(t *testing.T) { - var cfg *Config - _, err := cfg.Path() - if err == nil { - t.Error("expecting error, got nil") - } -} - func TestConfig_CredentialsStore(t *testing.T) { tests := []struct { name string diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index bbbbb893..0a282054 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -51,6 +51,7 @@ type Store interface { // in the config file. type DynamicStore struct { config *config.Config + configPath string options StoreOptions detectedCredsStore string setCredsStoreOnce sync.Once @@ -100,8 +101,9 @@ func NewStore(configPath string, opts StoreOptions) (*DynamicStore, error) { return nil, err } ds := &DynamicStore{ - config: cfg, - options: opts, + config: cfg, + configPath: configPath, + options: opts, } if opts.DetectDefaultNativeStore && !cfg.IsAuthConfigured() { // no authentication configured, detect the default credentials store @@ -167,9 +169,12 @@ func (ds *DynamicStore) IsAuthConfigured() bool { return ds.config.IsAuthConfigured() } -// GetConfigPath returns the path to the config file. -func (ds *DynamicStore) GetConfigPath() (string, error) { - return ds.config.Path() +// ConfigPath returns the path to the config file. +func (ds *DynamicStore) ConfigPath() (string, error) { + if ds.config == nil { + return "", fmt.Errorf("config is not loaded") + } + return ds.configPath, nil } // getHelperSuffix returns the credential helper suffix for the given server diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index cf9f30cc..c9098493 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -25,7 +25,6 @@ import ( "testing" "oras.land/oras-go/v2/registry/remote/auth" - "oras.land/oras-go/v2/registry/remote/credentials/internal/config" "oras.land/oras-go/v2/registry/remote/credentials/internal/config/configtest" ) @@ -613,14 +612,13 @@ func Test_DynamicStore_getHelperSuffix(t *testing.T) { } } func Test_DynamicStore_GetConfigPath(t *testing.T) { - var store DynamicStore - var err error path := "../../testdata/credsStore_config.json" - store.config, err = config.Load(path) + var err error + store, err := NewStore(path, StoreOptions{}) if err != nil { - t.Fatal("config.Load() error =", err) + t.Fatal("NewFileStore() error =", err) } - got, err := store.GetConfigPath() + got, err := store.ConfigPath() if err != nil { t.Errorf("Config.GetPath() error = %v", err) } @@ -631,7 +629,7 @@ func Test_DynamicStore_GetConfigPath(t *testing.T) { func Test_DynamicStore_Path_nil(t *testing.T) { var store DynamicStore - _, err := store.GetConfigPath() + _, err := store.ConfigPath() if err == nil { t.Error("expecting error, got nil") } From ea7cc52abc22f3342918bee5732ab717401e66a1 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 12 Jan 2024 06:36:58 +0000 Subject: [PATCH 08/14] resolve comment Signed-off-by: Billy Zha --- registry/remote/credentials/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index 0a282054..d41d8676 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -171,7 +171,7 @@ func (ds *DynamicStore) IsAuthConfigured() bool { // ConfigPath returns the path to the config file. func (ds *DynamicStore) ConfigPath() (string, error) { - if ds.config == nil { + if ds == nil { return "", fmt.Errorf("config is not loaded") } return ds.configPath, nil From 850cb336331ddcc8f1e399ca5b09d8559ef4ec0e Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 12 Jan 2024 06:38:00 +0000 Subject: [PATCH 09/14] resolve comment Signed-off-by: Billy Zha --- registry/remote/credentials/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index d41d8676..054d9ec3 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -171,7 +171,7 @@ func (ds *DynamicStore) IsAuthConfigured() bool { // ConfigPath returns the path to the config file. func (ds *DynamicStore) ConfigPath() (string, error) { - if ds == nil { + if ds == nil || ds.config == nil { return "", fmt.Errorf("config is not loaded") } return ds.configPath, nil From b178cc7a6d894ccaa269db8d0e4551ac18ee0cf6 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 12 Jan 2024 07:03:46 +0000 Subject: [PATCH 10/14] resolve comments Signed-off-by: Billy Zha --- registry/remote/credentials/store.go | 7 ++----- registry/remote/credentials/store_test.go | 15 ++------------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/registry/remote/credentials/store.go b/registry/remote/credentials/store.go index 054d9ec3..467ace66 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -170,11 +170,8 @@ func (ds *DynamicStore) IsAuthConfigured() bool { } // ConfigPath returns the path to the config file. -func (ds *DynamicStore) ConfigPath() (string, error) { - if ds == nil || ds.config == nil { - return "", fmt.Errorf("config is not loaded") - } - return ds.configPath, nil +func (ds *DynamicStore) ConfigPath() string { + return ds.configPath } // getHelperSuffix returns the credential helper suffix for the given server diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index c9098493..159b9b14 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -611,30 +611,19 @@ func Test_DynamicStore_getHelperSuffix(t *testing.T) { }) } } -func Test_DynamicStore_GetConfigPath(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, err := store.ConfigPath() - if err != nil { - t.Errorf("Config.GetPath() error = %v", err) - } + got := store.ConfigPath() if got != path { t.Errorf("Config.GetPath() = %v, want %v", got, path) } } -func Test_DynamicStore_Path_nil(t *testing.T) { - var store DynamicStore - _, err := store.ConfigPath() - if err == nil { - t.Error("expecting error, got nil") - } -} - func Test_DynamicStore_getStore_nativeStore(t *testing.T) { tests := []struct { name string From 704e303c9d8715072f14ccfe5da23411e891f932 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 16 Jan 2024 09:06:19 +0000 Subject: [PATCH 11/14] refactor Signed-off-by: Billy Zha --- registry/remote/credentials/internal/config/config.go | 5 +++++ .../remote/credentials/internal/config/config_test.go | 10 ++++++++++ registry/remote/credentials/store.go | 8 +++----- registry/remote/credentials/store_test.go | 1 + 4 files changed, 19 insertions(+), 5 deletions(-) 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 467ace66..ae8ce5be 100644 --- a/registry/remote/credentials/store.go +++ b/registry/remote/credentials/store.go @@ -51,7 +51,6 @@ type Store interface { // in the config file. type DynamicStore struct { config *config.Config - configPath string options StoreOptions detectedCredsStore string setCredsStoreOnce sync.Once @@ -101,9 +100,8 @@ func NewStore(configPath string, opts StoreOptions) (*DynamicStore, error) { return nil, err } ds := &DynamicStore{ - config: cfg, - configPath: configPath, - options: opts, + config: cfg, + options: opts, } if opts.DetectDefaultNativeStore && !cfg.IsAuthConfigured() { // no authentication configured, detect the default credentials store @@ -171,7 +169,7 @@ func (ds *DynamicStore) IsAuthConfigured() bool { // ConfigPath returns the path to the config file. func (ds *DynamicStore) ConfigPath() string { - return ds.configPath + return ds.config.Path() } // getHelperSuffix returns the credential helper suffix for the given server diff --git a/registry/remote/credentials/store_test.go b/registry/remote/credentials/store_test.go index 159b9b14..919f8757 100644 --- a/registry/remote/credentials/store_test.go +++ b/registry/remote/credentials/store_test.go @@ -611,6 +611,7 @@ func Test_DynamicStore_getHelperSuffix(t *testing.T) { }) } } + func Test_DynamicStore_ConfigPath(t *testing.T) { path := "../../testdata/credsStore_config.json" var err error From a60fe73494f14bbcd84c65df6c0008751fdb2adb Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 16 Jan 2024 09:10:00 +0000 Subject: [PATCH 12/14] add unit test Signed-off-by: Billy Zha --- registry/remote/auth/client_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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) + } +} From 4a4b3faf8c5e62186c7aad47d79ba8f09f34c41c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 17 Jan 2024 04:48:43 +0000 Subject: [PATCH 13/14] add doc Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index 0c8012d8..efb981e0 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -31,6 +31,8 @@ import ( "oras.land/oras-go/v2/registry/remote/retry" ) +// ErrBasicCredentialNotFound is returned when the basic credential is not +// found. var ErrBasicCredentialNotFound = errors.New("basic credential not found") // DefaultClient is the default auth-decorated client. From f1d1a3dd077d51d74cc4472a8242005363864f07 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 17 Jan 2024 07:40:12 +0000 Subject: [PATCH 14/14] resolve comments Signed-off-by: Billy Zha --- registry/remote/auth/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registry/remote/auth/client.go b/registry/remote/auth/client.go index efb981e0..8d9685a2 100644 --- a/registry/remote/auth/client.go +++ b/registry/remote/auth/client.go @@ -31,8 +31,8 @@ import ( "oras.land/oras-go/v2/registry/remote/retry" ) -// ErrBasicCredentialNotFound is returned when the basic credential is not -// found. +// 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.