diff --git a/client/client_test.go b/client/client_test.go index 0cd9c19b..881ee920 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "syscall" "testing" "time" @@ -422,6 +423,49 @@ func TestConnectError(t *testing.T) { }) } +func TestConnectErrorHint(t *testing.T) { + desktop := api.DesktopSocketPath() + standalone := api.StandaloneSocketPath() + denied := func(socket string) string { + return "Your user lacks permission to connect to " + socket + ". Grant it read and write access to the socket and retry." + } + tests := []struct { + name string + reason ConnectReason + socket string + hint string + }{ + {name: "desktop not running", reason: ReasonNotRunning, socket: desktop, hint: desktopNotRunningHint}, + {name: "desktop timeout", reason: ReasonTimeout, socket: desktop, hint: desktopTimeoutHint}, + { + name: "standalone not running", + reason: ReasonNotRunning, + socket: standalone, + hint: "Start the standalone secrets engine and retry: nothing is listening on " + strconv.Quote(standalone) + ".", + }, + { + name: "standalone timeout", + reason: ReasonTimeout, + socket: standalone, + hint: "The standalone secrets engine on " + strconv.Quote(standalone) + " did not respond. Restart it and retry.", + }, + {name: "custom socket not running", reason: ReasonNotRunning, socket: "/s.sock", hint: `Start the secrets engine and retry: nothing is listening on "/s.sock".`}, + {name: "custom socket timeout", reason: ReasonTimeout, socket: "/s.sock", hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`}, + {name: "permission denied", reason: ReasonPermissionDenied, socket: "/s.sock", hint: denied(`"/s.sock"`)}, + {name: "permission denied on desktop socket", reason: ReasonPermissionDenied, socket: desktop, hint: denied(strconv.Quote(desktop))}, + {name: "permission denied without path", reason: ReasonPermissionDenied, hint: denied("the secrets engine socket")}, + {name: "not running without path", reason: ReasonNotRunning, hint: "Start the secrets engine and retry: nothing is listening on the secrets engine socket."}, + {name: "control characters in path are quoted", reason: ReasonNotRunning, socket: "/s\n.sock", hint: `Start the secrets engine and retry: nothing is listening on "/s\n.sock".`}, + {name: "unknown reason has no hint", reason: ReasonUnknown, socket: "/s.sock", hint: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ce := &ConnectError{Reason: tc.reason, SocketPath: tc.socket, Err: errors.New("dial")} + assert.Equal(t, tc.hint, ce.Hint()) + }) + } +} + func TestConnectErrorFromSocket(t *testing.T) { getSecrets := func(t *testing.T, socketPath string) error { t.Helper() diff --git a/client/errors.go b/client/errors.go index 6aac009b..3adf788f 100644 --- a/client/errors.go +++ b/client/errors.go @@ -17,8 +17,12 @@ package client import ( "context" "errors" + "fmt" "io/fs" "net" + "path/filepath" + + "github.com/docker/secrets-engine/x/api" ) var ( @@ -89,6 +93,71 @@ func (e *ConnectError) Is(target error) bool { return s != nil && target == s } +const ( + desktopNotRunningHint = "Start Docker Desktop and retry: the secrets engine runs as part of it." + desktopTimeoutHint = "The secrets engine did not respond. Restart Docker Desktop and retry." + standaloneNotRunningHint = "Start the standalone secrets engine and retry: nothing is listening on %s." + standaloneTimeoutHint = "The standalone secrets engine on %s did not respond. Restart it and retry." + customNotRunningHint = "Start the secrets engine and retry: nothing is listening on %s." + customTimeoutHint = "The secrets engine on %s did not respond. Restart it and retry." + permissionDeniedHint = "Your user lacks permission to connect to %s. Grant it read and write access to the socket and retry." +) + +// Hint returns a user friendly message to help recover from the error. +func (e *ConnectError) Hint() string { + socket := "the secrets engine socket" + if e.SocketPath != "" { + socket = fmt.Sprintf("%q", e.SocketPath) + } + kind := classifySocket(e.SocketPath) + switch e.Reason { + case ReasonNotRunning: + switch kind { + case engineDesktop: + return desktopNotRunningHint + case engineStandalone: + return fmt.Sprintf(standaloneNotRunningHint, socket) + default: + return fmt.Sprintf(customNotRunningHint, socket) + } + case ReasonTimeout: + switch kind { + case engineDesktop: + return desktopTimeoutHint + case engineStandalone: + return fmt.Sprintf(standaloneTimeoutHint, socket) + default: + return fmt.Sprintf(customTimeoutHint, socket) + } + case ReasonPermissionDenied: + return fmt.Sprintf(permissionDeniedHint, socket) + default: + return "" + } +} + +type engineKind int + +const ( + engineCustom engineKind = iota + engineDesktop + engineStandalone +) + +func classifySocket(path string) engineKind { + if path == "" { + return engineCustom + } + switch filepath.Clean(path) { + case filepath.Clean(api.DesktopSocketPath()): + return engineDesktop + case filepath.Clean(api.StandaloneSocketPath()): + return engineStandalone + default: + return engineCustom + } +} + func connectReason(err error) ConnectReason { var ne net.Error switch { diff --git a/plugins/pass/commands/client.go b/plugins/pass/commands/client.go index 4e66918b..2bc7de90 100644 --- a/plugins/pass/commands/client.go +++ b/plugins/pass/commands/client.go @@ -20,6 +20,8 @@ import ( "fmt" "time" + "github.com/spf13/cobra" + "github.com/docker/secrets-engine/client" "github.com/docker/secrets-engine/x/api" "github.com/docker/secrets-engine/x/secrets" @@ -27,6 +29,32 @@ import ( const defaultPreflightPingTimeout = 3 * time.Second +func wrapEngineErrors(cmd *cobra.Command) *cobra.Command { + if pre := cmd.PreRunE; pre != nil { + cmd.PreRunE = func(c *cobra.Command, args []string) error { + return withEngineHint(pre(c, args)) + } + } + if run := cmd.RunE; run != nil { + cmd.RunE = func(c *cobra.Command, args []string) error { + return withEngineHint(run(c, args)) + } + } + return cmd +} + +func withEngineHint(err error) error { + ce, ok := errors.AsType[*client.ConnectError](err) + if !ok { + return err + } + hint := ce.Hint() + if hint == "" { + return err + } + return fmt.Errorf("%w\n\n%s", err, hint) +} + type clientOpts struct { timeout time.Duration responseTimeout time.Duration diff --git a/plugins/pass/commands/command_test.go b/plugins/pass/commands/command_test.go index d35ee86f..a503e839 100644 --- a/plugins/pass/commands/command_test.go +++ b/plugins/pass/commands/command_test.go @@ -17,6 +17,8 @@ package commands import ( "bytes" "errors" + "fmt" + "strconv" "testing" "time" @@ -29,6 +31,7 @@ import ( "github.com/docker/secrets-engine/plugins/pass/teststore" "github.com/docker/secrets-engine/store" "github.com/docker/secrets-engine/store/keychain" + "github.com/docker/secrets-engine/x/api" "github.com/docker/secrets-engine/x/secrets" "github.com/docker/secrets-engine/x/testhelper" ) @@ -210,9 +213,11 @@ func Test_RmCommand(t *testing.T) { }) t.Run("unreachable engine removes nothing", func(t *testing.T) { mock := twoSecrets() - cmd := mustRmCommand(t, WithTimeout(time.Second), WithSocketPath(deadSocket(t))) + socket := deadSocket(t) + cmd := mustRmCommand(t, WithTimeout(time.Second), WithSocketPath(socket)) out, err := execute(t, cmd, mock, "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) + assert.Contains(t, out, "Start the secrets engine and retry: nothing is listening on "+strconv.Quote(socket)+".") assert.NotContains(t, out, "RM:") assert.Equal(t, 2, remaining(t, mock)) }) @@ -342,10 +347,12 @@ func Test_GetCommand(t *testing.T) { assert.NotContains(t, out, "bar") }) t.Run("--reveal pings the engine first when requests are unbounded", func(t *testing.T) { - cmd := mustGetCommand(t, WithSocketPath(deadSocket(t))) + socket := deadSocket(t) + cmd := mustGetCommand(t, WithSocketPath(socket)) out, err := execute(t, cmd, fooStore(), "--reveal", "foo") assert.ErrorIs(t, err, client.ErrSecretsEngineNotRunning) assert.ErrorContains(t, err, "preflight ping") + assert.Contains(t, out, "\n\nStart the secrets engine and retry: nothing is listening on "+strconv.Quote(socket)+".\n") assert.NotContains(t, out, "bar") }) t.Run("rejects an invalid option", func(t *testing.T) { @@ -369,6 +376,42 @@ func mustRmCommand(t *testing.T, options ...ClientOption) *cobra.Command { return cmd } +func TestWithEngineHint(t *testing.T) { + t.Parallel() + connErr := func(reason client.ConnectReason, socket string) error { + return fmt.Errorf("authorizing: %w", &client.ConnectError{Reason: reason, SocketPath: socket, Err: errors.New("dial")}) + } + desktop := api.DesktopSocketPath() + tests := []struct { + name string + err error + hint string + }{ + { + name: "wrapped connect error", + err: connErr(client.ReasonNotRunning, desktop), + hint: "Start Docker Desktop and retry: the secrets engine runs as part of it.", + }, + { + name: "joined with other errors", + err: errors.Join(store.ErrCredentialNotFound, connErr(client.ReasonTimeout, "/s.sock")), + hint: `The secrets engine on "/s.sock" did not respond. Restart it and retry.`, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := withEngineHint(tc.err) + assert.Equal(t, tc.err.Error()+"\n\n"+tc.hint, got.Error()) + assert.ErrorIs(t, got, tc.err) + }) + } + t.Run("unknown reason and other errors pass through", func(t *testing.T) { + for _, err := range []error{nil, connErr(client.ReasonUnknown, "/s.sock"), client.ErrAccessDenied} { + assert.Equal(t, err, withEngineHint(err)) + } + }) +} + func deadSocket(t *testing.T) string { t.Helper() return testhelper.RandomShortSocketName() diff --git a/plugins/pass/commands/get.go b/plugins/pass/commands/get.go index 92e80d70..d9edc3cd 100644 --- a/plugins/pass/commands/get.go +++ b/plugins/pass/commands/get.go @@ -79,7 +79,7 @@ func GetCommand(options ...ClientOption) (*cobra.Command, error) { }, } cmd.Flags().BoolVar(&reveal, "reveal", false, "Show the secret value in plaintext") - return wrapKeychainErrors(cmd), nil + return wrapEngineErrors(wrapKeychainErrors(cmd)), nil } func printSecret(w io.Writer, id store.ID, value []byte) error { diff --git a/plugins/pass/commands/rm.go b/plugins/pass/commands/rm.go index fd91119a..af103815 100644 --- a/plugins/pass/commands/rm.go +++ b/plugins/pass/commands/rm.go @@ -65,7 +65,7 @@ func RmCommand(options ...ClientOption) (*cobra.Command, error) { } flags := cmd.Flags() flags.BoolVar(&opts.all, "all", false, "Remove all secrets") - return wrapKeychainErrors(cmd), nil + return wrapEngineErrors(wrapKeychainErrors(cmd)), nil } func validateArgs(args []string, opts rmOpts) ([]store.ID, error) { diff --git a/plugins/pass/commands/run.go b/plugins/pass/commands/run.go index 23fcefda..85474dfc 100644 --- a/plugins/pass/commands/run.go +++ b/plugins/pass/commands/run.go @@ -61,7 +61,7 @@ func RunCommand(options ...ClientOption) (*cobra.Command, error) { if err != nil { return nil, err } - return newRunCommand(runOpts{clientOpts: copts}), nil + return wrapEngineErrors(newRunCommand(runOpts{clientOpts: copts})), nil } func newRunCommand(opts runOpts) *cobra.Command {