From 7873dde7fdbd2b197a52f27812db2c13abb57d8f Mon Sep 17 00:00:00 2001 From: Ogulcan Aydogan Date: Wed, 10 Jun 2026 23:12:33 +0100 Subject: [PATCH 1/2] fix: use %w instead of %v for error wrapping in login and logout Switch return fmt.Errorf calls in cmd/notation/login.go and cmd/notation/logout.go from %v to %w so callers can inspect the underlying error via errors.Is/errors.As. The house style across the rest of the codebase already uses %w for these patterns. Signed-off-by: Ogulcan Aydogan --- cmd/notation/login.go | 6 +++--- cmd/notation/logout.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/notation/login.go b/cmd/notation/login.go index d7d4309e6..711d5d20d 100644 --- a/cmd/notation/login.go +++ b/cmd/notation/login.go @@ -112,16 +112,16 @@ func runLogin(ctx context.Context, opts *loginOpts) error { cred := opts.Credential() credsStore, err := auth.NewCredentialsStore() if err != nil { - return fmt.Errorf("failed to get credentials store: %v", err) + return fmt.Errorf("failed to get credentials store: %w", err) } registry, err := getRegistryLoginClient(ctx, &opts.SecureFlagOpts, serverAddress) if err != nil { - return fmt.Errorf("failed to get registry client: %v", err) + return fmt.Errorf("failed to get registry client: %w", err) } if err := credentials.Login(ctx, credsStore, registry, cred); err != nil { registryName := registry.Reference.Registry if !errors.Is(err, credentials.ErrPlaintextPutDisabled) { - return fmt.Errorf("failed to log in to %s: %v", registryName, err) + return fmt.Errorf("failed to log in to %s: %w", registryName, err) } // ErrPlaintextPutDisabled returned by Login() indicates that the diff --git a/cmd/notation/logout.go b/cmd/notation/logout.go index ef89c2b31..f9678bb23 100644 --- a/cmd/notation/logout.go +++ b/cmd/notation/logout.go @@ -56,10 +56,10 @@ func runLogout(ctx context.Context, opts *logoutOpts) error { ctx = opts.LoggingFlagOpts.InitializeLogger(ctx) credsStore, err := auth.NewCredentialsStore() if err != nil { - return fmt.Errorf("failed to get credentials store: %v", err) + return fmt.Errorf("failed to get credentials store: %w", err) } if err := credentials.Logout(ctx, credsStore, opts.server); err != nil { - return fmt.Errorf("failed to log out of %s: %v", opts.server, err) + return fmt.Errorf("failed to log out of %s: %w", opts.server, err) } fmt.Println("Logout Succeeded") From 1d3d08c3d372f67c776fea4296aa25d7818cb8ff Mon Sep 17 00:00:00 2001 From: Ogulcan Aydogan Date: Mon, 19 Oct 2026 23:06:36 +0100 Subject: [PATCH 2/2] test: cover wrapped errors in login and logout Check that runLogin and runLogout return errors that wrap the underlying error, for an invalid credentials store config, an invalid registry, an unreachable registry and a failed logout. Signed-off-by: Ogulcan Aydogan --- cmd/notation/login_test.go | 59 +++++++++++++++++++++++++++++++++++++ cmd/notation/logout_test.go | 34 ++++++++++++++++++++- 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/cmd/notation/login_test.go b/cmd/notation/login_test.go index f16497b40..056f98eb2 100644 --- a/cmd/notation/login_test.go +++ b/cmd/notation/login_test.go @@ -14,9 +14,13 @@ package main import ( + "context" + "errors" "os" + "path/filepath" "testing" + "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation/v2/cmd/notation/internal/flag" ) @@ -102,3 +106,58 @@ func TestLoginCommand_MissingArgs(t *testing.T) { t.Fatal("Parse Args expected error, but ok") } } + +// setTestConfigDir points the notation config directory to a temporary directory +// with the given config.json content. +func setTestConfigDir(t *testing.T, config string) { + t.Helper() + oldConfigDir := dir.UserConfigDir + t.Cleanup(func() { dir.UserConfigDir = oldConfigDir }) + dir.UserConfigDir = t.TempDir() + if err := os.WriteFile(filepath.Join(dir.UserConfigDir, dir.PathConfigFile), []byte(config), 0o600); err != nil { + t.Fatalf("failed to write config file: %v", err) + } +} + +// The errors returned by runLogin have to wrap the underlying error so that +// callers can inspect it with errors.Is and errors.As. +func TestRunLogin_WrapsErrors(t *testing.T) { + tests := []struct { + name string + config string + server string + }{ + { + name: "invalid credentials store config", + config: "{", + server: "registry.example.com", + }, + { + name: "invalid registry", + config: "{}", + server: "invalid host name", + }, + { + // nothing listens on port 1, so the connection is refused + name: "registry not reachable", + config: "{}", + server: "localhost:1", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setTestConfigDir(t, tt.config) + opts := &loginOpts{server: tt.server} + opts.Username = "user" + opts.Password = "password" + + err := runLogin(context.Background(), opts) + if err == nil { + t.Fatal("runLogin() expected an error, but got nil") + } + if errors.Unwrap(err) == nil { + t.Fatalf("runLogin() error does not wrap the underlying error: %v", err) + } + }) + } +} diff --git a/cmd/notation/logout_test.go b/cmd/notation/logout_test.go index 5c154f2a0..d82ba9ef1 100644 --- a/cmd/notation/logout_test.go +++ b/cmd/notation/logout_test.go @@ -13,7 +13,11 @@ package main -import "testing" +import ( + "context" + "errors" + "testing" +) func TestLogoutCommand_BasicArgs(t *testing.T) { opts := &logoutOpts{} @@ -43,3 +47,31 @@ func TestLogOutCommand_MissingArgs(t *testing.T) { t.Fatal("Parse Args expected error, but ok") } } + +// The errors returned by runLogout have to wrap the underlying error so that +// callers can inspect it with errors.Is and errors.As. +func TestRunLogout_WrapsErrors(t *testing.T) { + t.Run("invalid credentials store config", func(t *testing.T) { + setTestConfigDir(t, "{") + + err := runLogout(context.Background(), &logoutOpts{server: "registry.example.com"}) + if err == nil { + t.Fatal("runLogout() expected an error, but got nil") + } + if errors.Unwrap(err) == nil { + t.Fatalf("runLogout() error does not wrap the underlying error: %v", err) + } + }) + + t.Run("credentials not found", func(t *testing.T) { + setTestConfigDir(t, "{}") + + err := runLogout(context.Background(), &logoutOpts{server: "registry.example.com"}) + if err == nil { + t.Skip("logging out of an unknown registry does not fail on this platform") + } + if errors.Unwrap(err) == nil { + t.Fatalf("runLogout() error does not wrap the underlying error: %v", err) + } + }) +}