From 0ab8ec0e4c1226cc176cdbcef9a8083d6995dc89 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@docker.com>
Date: Wed, 19 Dec 2018 11:29:01 +0000
Subject: [PATCH] Output broken CLI plugins in `help` output.

Signed-off-by: Ian Campbell <ijc@docker.com>
---
 cli/cobra.go                 | 44 ++++++++++++++++++++++++++++++++++++
 cli/cobra_test.go            | 21 +++++++++++++++++
 e2e/cli-plugins/help_test.go |  3 +++
 3 files changed, 68 insertions(+)

diff --git a/cli/cobra.go b/cli/cobra.go
index 6a75110117..cea1a9be4e 100644
--- a/cli/cobra.go
+++ b/cli/cobra.go
@@ -24,11 +24,14 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 
 	cobra.AddTemplateFunc("hasSubCommands", hasSubCommands)
 	cobra.AddTemplateFunc("hasManagementSubCommands", hasManagementSubCommands)
+	cobra.AddTemplateFunc("hasInvalidPlugins", hasInvalidPlugins)
 	cobra.AddTemplateFunc("operationSubCommands", operationSubCommands)
 	cobra.AddTemplateFunc("managementSubCommands", managementSubCommands)
+	cobra.AddTemplateFunc("invalidPlugins", invalidPlugins)
 	cobra.AddTemplateFunc("wrappedFlagUsages", wrappedFlagUsages)
 	cobra.AddTemplateFunc("commandVendor", commandVendor)
 	cobra.AddTemplateFunc("isFirstLevelCommand", isFirstLevelCommand) // is it an immediate sub-command of the root
+	cobra.AddTemplateFunc("invalidPluginReason", invalidPluginReason)
 
 	rootCmd.SetUsageTemplate(usageTemplate)
 	rootCmd.SetHelpTemplate(helpTemplate)
@@ -115,6 +118,10 @@ var helpCommand = &cobra.Command{
 	},
 }
 
+func isPlugin(cmd *cobra.Command) bool {
+	return cmd.Annotations[pluginmanager.CommandAnnotationPlugin] == "true"
+}
+
 func hasSubCommands(cmd *cobra.Command) bool {
 	return len(operationSubCommands(cmd)) > 0
 }
@@ -123,9 +130,16 @@ func hasManagementSubCommands(cmd *cobra.Command) bool {
 	return len(managementSubCommands(cmd)) > 0
 }
 
+func hasInvalidPlugins(cmd *cobra.Command) bool {
+	return len(invalidPlugins(cmd)) > 0
+}
+
 func operationSubCommands(cmd *cobra.Command) []*cobra.Command {
 	cmds := []*cobra.Command{}
 	for _, sub := range cmd.Commands() {
+		if isPlugin(sub) && invalidPluginReason(sub) != "" {
+			continue
+		}
 		if sub.IsAvailableCommand() && !sub.HasSubCommands() {
 			cmds = append(cmds, sub)
 		}
@@ -159,6 +173,9 @@ func commandVendor(cmd *cobra.Command) string {
 func managementSubCommands(cmd *cobra.Command) []*cobra.Command {
 	cmds := []*cobra.Command{}
 	for _, sub := range cmd.Commands() {
+		if isPlugin(sub) && invalidPluginReason(sub) != "" {
+			continue
+		}
 		if sub.IsAvailableCommand() && sub.HasSubCommands() {
 			cmds = append(cmds, sub)
 		}
@@ -166,6 +183,23 @@ func managementSubCommands(cmd *cobra.Command) []*cobra.Command {
 	return cmds
 }
 
+func invalidPlugins(cmd *cobra.Command) []*cobra.Command {
+	cmds := []*cobra.Command{}
+	for _, sub := range cmd.Commands() {
+		if !isPlugin(sub) {
+			continue
+		}
+		if invalidPluginReason(sub) != "" {
+			cmds = append(cmds, sub)
+		}
+	}
+	return cmds
+}
+
+func invalidPluginReason(cmd *cobra.Command) string {
+	return cmd.Annotations[pluginmanager.CommandAnnotationPluginInvalid]
+}
+
 var usageTemplate = `Usage:
 
 {{- if not .HasSubCommands}}	{{.UseLine}}{{end}}
@@ -209,6 +243,16 @@ Commands:
 {{- end}}
 {{- end}}
 
+{{- if hasInvalidPlugins . }}
+
+Invalid Plugins:
+
+{{- range invalidPlugins . }}
+  {{rpad .Name .NamePadding }} {{invalidPluginReason .}}
+{{- end}}
+
+{{- end}}
+
 {{- if .HasSubCommands }}
 
 Run '{{.CommandPath}} COMMAND --help' for more information on a command.
diff --git a/cli/cobra_test.go b/cli/cobra_test.go
index 99744e0670..a9d943e678 100644
--- a/cli/cobra_test.go
+++ b/cli/cobra_test.go
@@ -4,8 +4,10 @@ import (
 	"testing"
 
 	pluginmanager "github.com/docker/cli/cli-plugins/manager"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/cobra"
 	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
 )
 
 func TestVisitAll(t *testing.T) {
@@ -55,3 +57,22 @@ func TestCommandVendor(t *testing.T) {
 		})
 	}
 }
+
+func TestInvalidPlugin(t *testing.T) {
+	root := &cobra.Command{Use: "root"}
+	sub1 := &cobra.Command{Use: "sub1"}
+	sub1sub1 := &cobra.Command{Use: "sub1sub1"}
+	sub1sub2 := &cobra.Command{Use: "sub1sub2"}
+	sub2 := &cobra.Command{Use: "sub2"}
+
+	assert.Assert(t, is.Len(invalidPlugins(root), 0))
+
+	sub1.Annotations = map[string]string{
+		pluginmanager.CommandAnnotationPlugin:        "true",
+		pluginmanager.CommandAnnotationPluginInvalid: "foo",
+	}
+	root.AddCommand(sub1, sub2)
+	sub1.AddCommand(sub1sub1, sub1sub2)
+
+	assert.DeepEqual(t, invalidPlugins(root), []*cobra.Command{sub1}, cmpopts.IgnoreUnexported(cobra.Command{}))
+}
diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go
index f9690fe503..f82959dbab 100644
--- a/e2e/cli-plugins/help_test.go
+++ b/e2e/cli-plugins/help_test.go
@@ -31,6 +31,7 @@ func TestGlobalHelp(t *testing.T) {
 	//  - Each of the main headings
 	//  - Some builtin commands under the main headings
 	//  - The `helloworld` plugin in the appropriate place
+	//  - The `badmeta` plugin under the "Invalid Plugins" heading.
 	//
 	// Regexps are needed because the width depends on `unix.TIOCGWINSZ` or similar.
 	for _, expected := range []*regexp.Regexp{
@@ -41,6 +42,8 @@ func TestGlobalHelp(t *testing.T) {
 		regexp.MustCompile(`^  create\s+Create a new container$`),
 		regexp.MustCompile(`^  helloworld\s+\(Docker Inc\.\)\s+A basic Hello World plugin for tests$`),
 		regexp.MustCompile(`^  ps\s+List containers$`),
+		regexp.MustCompile(`^Invalid Plugins:$`),
+		regexp.MustCompile(`^  badmeta\s+invalid metadata: invalid character 'i' looking for beginning of object key string$`),
 	} {
 		var found bool
 		for scanner.Scan() {