Integrate CLI plugins into docker info

Fairly straight forward. It became necessary to wrap `Plugin.Err` with a type
which implements `encoding.MarshalText` in order to have that field rendered
properly in the `docker info -f '{{json}}'` output.

Since I changed the type somewhat I also added a unit test for `formatInfo`.

Signed-off-by: Ian Campbell <ijc@docker.com>

cli-plugins/manager/candidate_test.go

@@ -2,11 +2,13 @@ package manager
 
 import (
 	"fmt"
+	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
 	"gotest.tools/assert"
+	"gotest.tools/assert/cmp"
 )
 
 type fakeCandidate struct {
@@ -73,6 +75,7 @@ func TestValidateCandidate(t *testing.T) {
 			assert.ErrorContains(t, err, tc.err)
 		} else if tc.invalid != "" {
 			assert.NilError(t, err)
+			assert.Assert(t, cmp.ErrorType(p.Err, reflect.TypeOf(&pluginError{})))
 			assert.ErrorContains(t, p.Err, tc.invalid)
 		} else {
 			assert.NilError(t, err)

cli-plugins/manager/error.go

@@ -0,0 +1,43 @@
+package manager
+
+import (
+	"github.com/pkg/errors"
+)
+
+// pluginError is set as Plugin.Err by NewPlugin if the plugin
+// candidate fails one of the candidate tests. This exists primarily
+// to implement encoding.TextMarshaller such that rendering a plugin as JSON
+// (e.g. for `docker info -f '{{json .CLIPlugins}}'`) renders the Err
+// field as a useful string and not just `{}`. See
+// https://github.com/golang/go/issues/10748 for some discussion
+// around why the builtin error type doesn't implement this.
+type pluginError struct {
+	cause error
+}
+
+// Error satisfies the core error interface for pluginError.
+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
+}
+
+// MarshalText marshalls the pluginError into a textual form.
+func (e *pluginError) MarshalText() (text []byte, err error) {
+	return []byte(e.cause.Error()), nil
+}
+
+// wrapAsPluginError wraps an error in a pluginError with an
+// additional message, analogous to errors.Wrapf.
+func wrapAsPluginError(err error, msg string) error {
+	return &pluginError{cause: errors.Wrap(err, msg)}
+}
+
+// NewPluginError creates a new pluginError, analogous to
+// errors.Errorf.
+func NewPluginError(msg string, args ...interface{}) error {
+	return &pluginError{cause: errors.Errorf(msg, args...)}
+}

cli-plugins/manager/error_test.go

@@ -0,0 +1,24 @@
+package manager
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/pkg/errors"
+	"gopkg.in/yaml.v2"
+	"gotest.tools/assert"
+)
+
+func TestPluginError(t *testing.T) {
+	err := NewPluginError("new error")
+	assert.Error(t, err, "new error")
+
+	inner := fmt.Errorf("testing")
+	err = wrapAsPluginError(inner, "wrapping")
+	assert.Error(t, err, "wrapping: testing")
+	assert.Equal(t, inner, errors.Cause(err))
+
+	actual, err := yaml.Marshal(err)
+	assert.NilError(t, err)
+	assert.Equal(t, "'wrapping: testing'\n", string(actual))
+}

cli-plugins/manager/metadata.go

@@ -13,13 +13,13 @@ const (
 // Metadata provided by the plugin
 type Metadata struct {
 	// SchemaVersion describes the version of this struct. Mandatory, must be "0.1.0"
-	SchemaVersion string
+	SchemaVersion string `json:",omitempty"`
 	// Vendor is the name of the plugin vendor. Mandatory
-	Vendor string
+	Vendor string `json:",omitempty"`
 	// Version is the optional version of this plugin.
-	Version string
+	Version string `json:",omitempty"`
 	// ShortDescription should be suitable for a single line help message.
-	ShortDescription string
+	ShortDescription string `json:",omitempty"`
 	// URL is a pointer to the plugin's homepage.
-	URL string
+	URL string `json:",omitempty"`
 }

cli-plugins/manager/plugin.go

@@ -19,8 +19,8 @@ var (
 type Plugin struct {
 	Metadata
 
-	Name string
-	Path string
+	Name string `json:",omitempty"`
+	Path string `json:",omitempty"`
 
 	// Err is non-nil if the plugin failed one of the candidate tests.
 	Err error `json:",omitempty"`
@@ -31,8 +31,9 @@ type Plugin struct {
 
 // 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, but the `Plugin` is still returned with no error. An error
-// is only returned due to a non-recoverable error.
+// 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, rootcmd *cobra.Command) (Plugin, error) {
 	path := c.Path()
 	if path == "" {
@@ -63,7 +64,7 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 
 	// Now apply the candidate tests, so these update p.Err.
 	if !pluginNameRe.MatchString(p.Name) {
-		p.Err = errors.Errorf("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
 	}
 
@@ -76,11 +77,11 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 				continue
 			}
 			if cmd.Name() == p.Name {
-				p.Err = errors.Errorf("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 = errors.Errorf("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
 			}
 		}
@@ -89,21 +90,21 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 	// We are supposed to check for relevant execute permissions here. Instead we rely on an attempt to execute.
 	meta, err := c.Metadata()
 	if err != nil {
-		p.Err = errors.Wrap(err, "failed to fetch metadata")
+		p.Err = wrapAsPluginError(err, "failed to fetch metadata")
 		return p, nil
 	}
 
 	if err := json.Unmarshal(meta, &p.Metadata); err != nil {
-		p.Err = errors.Wrap(err, "invalid metadata")
+		p.Err = wrapAsPluginError(err, "invalid metadata")
 		return p, nil
 	}
 
 	if p.Metadata.SchemaVersion != "0.1.0" {
-		p.Err = errors.Errorf("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 = errors.Errorf("plugin metadata does not define a vendor")
+		p.Err = NewPluginError("plugin metadata does not define a vendor")
 		return p, nil
 	}
 	return p, nil