diff --git a/cli-plugins/manager/candidate.go b/cli-plugins/manager/candidate.go index d809926cf6..a4253968dc 100644 --- a/cli-plugins/manager/candidate.go +++ b/cli-plugins/manager/candidate.go @@ -6,12 +6,6 @@ import ( "github.com/docker/cli/cli-plugins/metadata" ) -// Candidate represents a possible plugin candidate, for mocking purposes -type Candidate interface { - Path() string - Metadata() ([]byte, error) -} - type candidate struct { path string } diff --git a/cli-plugins/manager/error.go b/cli-plugins/manager/error.go index 1b5f4f60e5..aaedae14fa 100644 --- a/cli-plugins/manager/error.go +++ b/cli-plugins/manager/error.go @@ -23,11 +23,6 @@ func (e *pluginError) Error() string { return e.cause.Error() } -// Cause satisfies the errors.causer interface for pluginError. -func (e *pluginError) Cause() error { - return e.cause -} - // Unwrap provides compatibility for Go 1.13 error chains. func (e *pluginError) Unwrap() error { return e.cause @@ -41,14 +36,11 @@ func (e *pluginError) MarshalText() (text []byte, err error) { // wrapAsPluginError wraps an error in a pluginError with an // additional message. func wrapAsPluginError(err error, msg string) error { - if err == nil { - return nil - } return &pluginError{cause: fmt.Errorf("%s: %w", msg, err)} } -// NewPluginError creates a new pluginError, analogous to +// newPluginError creates a new pluginError, analogous to // errors.Errorf. -func NewPluginError(msg string, args ...any) error { +func newPluginError(msg string, args ...any) error { return &pluginError{cause: fmt.Errorf(msg, args...)} } diff --git a/cli-plugins/manager/error_test.go b/cli-plugins/manager/error_test.go index c4cb19bd55..2e92001c20 100644 --- a/cli-plugins/manager/error_test.go +++ b/cli-plugins/manager/error_test.go @@ -10,7 +10,7 @@ import ( ) func TestPluginError(t *testing.T) { - err := NewPluginError("new error") + err := newPluginError("new error") assert.Check(t, is.Error(err, "new error")) inner := errors.New("testing") @@ -21,4 +21,7 @@ func TestPluginError(t *testing.T) { actual, err := json.Marshal(err) assert.Check(t, err) assert.Check(t, is.Equal(`"wrapping: testing"`, string(actual))) + + err = wrapAsPluginError(nil, "wrapping") + assert.Check(t, is.Error(err, "wrapping: %!w()")) } diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go index 25515bccb8..b76dbbe376 100644 --- a/cli-plugins/manager/manager.go +++ b/cli-plugins/manager/manager.go @@ -9,6 +9,7 @@ import ( "strings" "sync" + "github.com/containerd/errdefs" "github.com/docker/cli/cli-plugins/metadata" "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" @@ -23,12 +24,6 @@ const ( // plugin. Assuming $PATH and $CWD remain unchanged this should allow // the plugin to re-execute the original CLI. ReexecEnvvar = metadata.ReexecEnvvar - - // ResourceAttributesEnvvar is the name of the envvar that includes additional - // resource attributes for OTEL. - // - // Deprecated: The "OTEL_RESOURCE_ATTRIBUTES" env-var is part of the OpenTelemetry specification; users should define their own const for this. This const will be removed in the next release. - ResourceAttributesEnvvar = "OTEL_RESOURCE_ATTRIBUTES" ) // errPluginNotFound is the error returned when a plugin could not be found. @@ -40,15 +35,11 @@ func (e errPluginNotFound) Error() string { return "Error: No such CLI plugin: " + string(e) } -type notFound interface{ NotFound() } - // IsNotFound is true if the given error is due to a plugin not being found. +// +// Deprecated: use [errdefs.IsNotFound]. func IsNotFound(err error) bool { - if e, ok := err.(*pluginError); ok { - err = e.Cause() - } - _, ok := err.(notFound) - return ok + return errdefs.IsNotFound(err) } // getPluginDirs returns the platform-specific locations to search for plugins @@ -127,7 +118,7 @@ func getPlugin(name string, pluginDirs []string, rootcmd *cobra.Command) (*Plugi if err != nil { return nil, err } - if !IsNotFound(p.Err) { + if !errdefs.IsNotFound(p.Err) { p.ShadowedPaths = paths[1:] } return &p, nil @@ -164,7 +155,7 @@ func ListPlugins(dockerCli config.Provider, rootcmd *cobra.Command) ([]Plugin, e if err != nil { return err } - if !IsNotFound(p.Err) { + if !errdefs.IsNotFound(p.Err) { p.ShadowedPaths = paths[1:] mu.Lock() defer mu.Unlock() @@ -185,9 +176,9 @@ func ListPlugins(dockerCli config.Provider, rootcmd *cobra.Command) ([]Plugin, e return plugins, nil } -// PluginRunCommand returns an "os/exec".Cmd which when .Run() will execute the named plugin. +// PluginRunCommand returns an [os/exec.Cmd] which when [os/exec.Cmd.Run] will execute the named plugin. // The rootcmd argument is referenced to determine the set of builtin commands in order to detect conficts. -// The error returned satisfies the IsNotFound() predicate if no plugin was found or if the first candidate plugin was invalid somehow. +// The error returned satisfies the [errdefs.IsNotFound] predicate if no plugin was found or if the first candidate plugin was invalid somehow. func PluginRunCommand(dockerCli config.Provider, name string, rootcmd *cobra.Command) (*exec.Cmd, error) { // This uses the full original args, not the args which may // have been provided by cobra to our caller. This is because diff --git a/cli-plugins/manager/manager_test.go b/cli-plugins/manager/manager_test.go index 64609fbbad..d1223ed4ec 100644 --- a/cli-plugins/manager/manager_test.go +++ b/cli-plugins/manager/manager_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/containerd/errdefs" "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/internal/test" @@ -131,7 +132,7 @@ echo '{"SchemaVersion":"0.1.0"}'`, fs.WithMode(0o777)), _, err = GetPlugin("ccc", cli, &cobra.Command{}) assert.Error(t, err, "Error: No such CLI plugin: ccc") - assert.Assert(t, IsNotFound(err)) + assert.Assert(t, errdefs.IsNotFound(err)) } func TestListPluginsIsSorted(t *testing.T) { @@ -166,8 +167,8 @@ func TestErrPluginNotFound(t *testing.T) { var err error = errPluginNotFound("test") err.(errPluginNotFound).NotFound() assert.Error(t, err, "Error: No such CLI plugin: test") - assert.Assert(t, IsNotFound(err)) - assert.Assert(t, !IsNotFound(nil)) + assert.Assert(t, errdefs.IsNotFound(err)) + assert.Assert(t, !errdefs.IsNotFound(nil)) } func TestGetPluginDirs(t *testing.T) { diff --git a/cli-plugins/manager/metadata.go b/cli-plugins/manager/metadata.go index 9bddb12142..6fda7fa1af 100644 --- a/cli-plugins/manager/metadata.go +++ b/cli-plugins/manager/metadata.go @@ -6,18 +6,26 @@ import ( const ( // NamePrefix is the prefix required on all plugin binary names + // + // Deprecated: use [metadata.NamePrefix]. This alias will be removed in a future release. NamePrefix = metadata.NamePrefix // MetadataSubcommandName is the name of the plugin subcommand // which must be supported by every plugin and returns the // plugin metadata. + // + // Deprecated: use [metadata.MetadataSubcommandName]. This alias will be removed in a future release. MetadataSubcommandName = metadata.MetadataSubcommandName // HookSubcommandName is the name of the plugin subcommand // which must be implemented by plugins declaring support // for hooks in their metadata. + // + // Deprecated: use [metadata.HookSubcommandName]. This alias will be removed in a future release. HookSubcommandName = metadata.HookSubcommandName ) // Metadata provided by the plugin. +// +// Deprecated: use [metadata.Metadata]. This alias will be removed in a future release. type Metadata = metadata.Metadata diff --git a/cli-plugins/manager/plugin.go b/cli-plugins/manager/plugin.go index fa846452b5..2caae39b02 100644 --- a/cli-plugins/manager/plugin.go +++ b/cli-plugins/manager/plugin.go @@ -2,6 +2,7 @@ package manager import ( "context" + "encoding" "encoding/json" "errors" "fmt" @@ -31,12 +32,34 @@ type Plugin struct { ShadowedPaths []string `json:",omitempty"` } +// MarshalJSON implements [json.Marshaler] to handle marshaling the +// [Plugin.Err] field (Go doesn't marshal errors by default). +func (p *Plugin) MarshalJSON() ([]byte, error) { + type Alias Plugin // avoid recursion + + cp := *p // shallow copy to avoid mutating original + + if cp.Err != nil { + if _, ok := cp.Err.(encoding.TextMarshaler); !ok { + cp.Err = &pluginError{cp.Err} + } + } + + return json.Marshal((*Alias)(&cp)) +} + +// pluginCandidate represents a possible plugin candidate, for mocking purposes. +type pluginCandidate interface { + Path() string + Metadata() ([]byte, error) +} + // newPlugin determines if the given candidate is valid and returns a // Plugin. If the candidate fails one of the tests then `Plugin.Err` // is set, and is always a `pluginError`, but the `Plugin` is still // returned with no error. An error is only returned due to a // non-recoverable error. -func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { +func newPlugin(c pluginCandidate, cmds []*cobra.Command) (Plugin, error) { path := c.Path() if path == "" { return Plugin{}, errors.New("plugin candidate path cannot be empty") @@ -63,7 +86,7 @@ func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { // Now apply the candidate tests, so these update p.Err. if !pluginNameRe.MatchString(p.Name) { - p.Err = NewPluginError("plugin candidate %q did not match %q", p.Name, pluginNameRe.String()) + p.Err = newPluginError("plugin candidate %q did not match %q", p.Name, pluginNameRe.String()) return p, nil } @@ -75,11 +98,11 @@ func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { continue } if cmd.Name() == p.Name { - p.Err = NewPluginError("plugin %q duplicates builtin command", p.Name) + p.Err = newPluginError("plugin %q duplicates builtin command", p.Name) return p, nil } if cmd.HasAlias(p.Name) { - p.Err = NewPluginError("plugin %q duplicates an alias of builtin command %q", p.Name, cmd.Name()) + p.Err = newPluginError("plugin %q duplicates an alias of builtin command %q", p.Name, cmd.Name()) return p, nil } } @@ -96,11 +119,11 @@ func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) { return p, nil } if p.Metadata.SchemaVersion != "0.1.0" { - p.Err = NewPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion) + p.Err = newPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion) return p, nil } if p.Metadata.Vendor == "" { - p.Err = NewPluginError("plugin metadata does not define a vendor") + p.Err = newPluginError("plugin metadata does not define a vendor") return p, nil } return p, nil diff --git a/cli-plugins/manager/plugin_test.go b/cli-plugins/manager/plugin_test.go new file mode 100644 index 0000000000..ace51c12c6 --- /dev/null +++ b/cli-plugins/manager/plugin_test.go @@ -0,0 +1,43 @@ +package manager + +import ( + "encoding/json" + "errors" + "testing" + + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestPluginMarshal(t *testing.T) { + const jsonWithError = `{"Name":"some-plugin","Err":"something went wrong"}` + const jsonNoError = `{"Name":"some-plugin"}` + + tests := []struct { + doc string + error error + expected string + }{ + { + doc: "no error", + expected: jsonNoError, + }, + { + doc: "regular error", + error: errors.New("something went wrong"), + expected: jsonWithError, + }, + { + doc: "custom error", + error: newPluginError("something went wrong"), + expected: jsonWithError, + }, + } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + actual, err := json.Marshal(&Plugin{Name: "some-plugin", Err: tc.error}) + assert.NilError(t, err) + assert.Check(t, is.Equal(string(actual), tc.expected)) + }) + } +} diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index 726d1ea5a2..6b193b7fe9 100644 --- a/cli/command/system/info_test.go +++ b/cli/command/system/info_test.go @@ -2,6 +2,7 @@ package system import ( "encoding/base64" + "errors" "net" "testing" "time" @@ -220,7 +221,7 @@ var samplePluginsInfo = []pluginmanager.Plugin{ { Name: "badplugin", Path: "/path/to/docker-badplugin", - Err: pluginmanager.NewPluginError("something wrong"), + Err: errors.New("something wrong"), }, } diff --git a/cmd/docker/builder.go b/cmd/docker/builder.go index cccae304fe..00fc1b40f1 100644 --- a/cmd/docker/builder.go +++ b/cmd/docker/builder.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + "github.com/containerd/errdefs" pluginmanager "github.com/docker/cli/cli-plugins/manager" "github.com/docker/cli/cli-plugins/metadata" "github.com/docker/cli/cli/command" @@ -36,7 +37,7 @@ const ( ) func newBuilderError(errorMsg string, pluginLoadErr error) error { - if pluginmanager.IsNotFound(pluginLoadErr) { + if errdefs.IsNotFound(pluginLoadErr) { return errors.New(errorMsg) } if pluginLoadErr != nil { diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 3209110631..b8e57add4a 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -201,7 +201,7 @@ func setupHelpCommand(dockerCli command.Cli, rootCmd, helpCmd *cobra.Command) { if err == nil { return helpcmd.Run() } - if !pluginmanager.IsNotFound(err) { + if !errdefs.IsNotFound(err) { return fmt.Errorf("unknown help topic: %v", strings.Join(args, " ")) } } @@ -240,7 +240,7 @@ func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) { if err == nil { return } - if !pluginmanager.IsNotFound(err) { + if !errdefs.IsNotFound(err) { ccmd.Println(err) return } @@ -473,7 +473,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { } return nil } - if !pluginmanager.IsNotFound(err) { + if !errdefs.IsNotFound(err) { // For plugin not found we fall through to // cmd.Execute() which deals with reporting // "command not found" in a consistent way.