Merge pull request #6269 from thaJeztah/28.x_backport_plugin_manager_unexport

[28.x backport] cli-plugins/manager: various fixes and deprecations

Author: thaJeztah

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
 }

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...)}
 }

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(<nil>)"))
 }

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

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) {

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

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

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))
+		})
+	}
+}

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"),
 	},
 }
 

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 {