Merge pull request kubernetes#87633 from brianpursley/kubectl-792

Prevent error message from being displayed during plugin list when path includes empty string

Author: k8s-ci-robot

staging/src/k8s.io/kubectl/pkg/cmd/plugin/plugin.go

@@ -113,10 +113,14 @@ func (o *PluginListOptions) Run() error {
 	pluginWarnings := 0
 
 	for _, dir := range uniquePathsList(o.PluginPaths) {
+		if len(strings.TrimSpace(dir)) == 0 {
+			continue
+		}
+
 		files, err := ioutil.ReadDir(dir)
 		if err != nil {
 			if _, ok := err.(*os.PathError); ok {
-				fmt.Fprintf(o.ErrOut, "Unable read directory %q from your PATH: %v. Skipping...", dir, err)
+				fmt.Fprintf(o.ErrOut, "Unable read directory %q from your PATH: %v. Skipping...\n", dir, err)
 				continue
 			}
 
@@ -133,7 +137,7 @@ func (o *PluginListOptions) Run() error {
 			}
 
 			if isFirstFile {
-				fmt.Fprintf(o.ErrOut, "The following compatible plugins are available:\n\n")
+				fmt.Fprintf(o.Out, "The following compatible plugins are available:\n\n")
 				pluginsFound = true
 				isFirstFile = false
 			}
@@ -165,7 +169,6 @@ func (o *PluginListOptions) Run() error {
 		}
 	}
 	if len(pluginErrors) > 0 {
-		fmt.Fprintln(o.ErrOut)
 		errs := bytes.NewBuffer(nil)
 		for _, e := range pluginErrors {
 			fmt.Fprintln(errs, e)

staging/src/k8s.io/kubectl/pkg/cmd/plugin/plugin_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package plugin
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -98,6 +97,8 @@ func TestPluginPathsAreValid(t *testing.T) {
 		verifier           *fakePluginPathVerifier
 		expectVerifyErrors []error
 		expectErr          string
+		expectErrOut       string
+		expectOut          string
 	}{
 		{
 			name:        "ensure no plugins found if no files begin with kubectl- prefix",
@@ -106,7 +107,7 @@ func TestPluginPathsAreValid(t *testing.T) {
 			pluginFile: func() (*os.File, error) {
 				return ioutil.TempFile(tempDir, "notkubectl-")
 			},
-			expectErr: "unable to find any kubectl plugins in your PATH",
+			expectErr: "error: unable to find any kubectl plugins in your PATH\n",
 		},
 		{
 			name:        "ensure de-duplicated plugin-paths slice",
@@ -115,12 +116,22 @@ func TestPluginPathsAreValid(t *testing.T) {
 			pluginFile: func() (*os.File, error) {
 				return ioutil.TempFile(tempDir, "kubectl-")
 			},
+			expectOut: "The following compatible plugins are available:",
+		},
+		{
+			name:        "ensure no errors when empty string or blank path are specified",
+			pluginPaths: []string{tempDir, "", " "},
+			verifier:    newFakePluginPathVerifier(),
+			pluginFile: func() (*os.File, error) {
+				return ioutil.TempFile(tempDir, "kubectl-")
+			},
+			expectOut: "The following compatible plugins are available:",
 		},
 	}
 
 	for _, test := range tc {
 		t.Run(test.name, func(t *testing.T) {
-			ioStreams, _, _, errOut := genericclioptions.NewTestIOStreams()
+			ioStreams, _, out, errOut := genericclioptions.NewTestIOStreams()
 			o := &PluginListOptions{
 				Verifier:  test.verifier,
 				IOStreams: ioStreams,
@@ -145,23 +156,23 @@ func TestPluginPathsAreValid(t *testing.T) {
 
 			err := o.Run()
 			if err == nil && len(test.expectErr) > 0 {
-				t.Fatalf("unexpected non-error: expecting %v", test.expectErr)
-			}
-			if err != nil && len(test.expectErr) == 0 {
-				t.Fatalf("unexpected error: %v - %v", err, errOut.String())
-			}
-			if err == nil {
-				return
+				t.Fatalf("unexpected non-error: expected %v, but got nothing", test.expectErr)
+			} else if err != nil && len(test.expectErr) == 0 {
+				t.Fatalf("unexpected error: expected nothing, but got %v", err.Error())
+			} else if err != nil && err.Error() != test.expectErr {
+				t.Fatalf("unexpected error: expected %v, but got %v", test.expectErr, err.Error())
 			}
 
-			allErrs := bytes.NewBuffer(errOut.Bytes())
-			if _, err := allErrs.WriteString(err.Error()); err != nil {
-				t.Fatalf("unexpected error: %v", err)
+			if len(test.expectErrOut) == 0 && errOut.Len() > 0 {
+				t.Fatalf("unexpected error output: expected nothing, but got %v", errOut.String())
+			} else if len(test.expectErrOut) > 0 && !strings.Contains(errOut.String(), test.expectErrOut) {
+				t.Fatalf("unexpected error output: expected to contain %v, but got %v", test.expectErrOut, errOut.String())
 			}
-			if len(test.expectErr) > 0 {
-				if !strings.Contains(allErrs.String(), test.expectErr) {
-					t.Fatalf("unexpected error: expected %v, but got %v", test.expectErr, allErrs.String())
-				}
+
+			if len(test.expectOut) == 0 && out.Len() > 0 {
+				t.Fatalf("unexpected output: expected nothing, but got %v", out.String())
+			} else if len(test.expectOut) > 0 && !strings.Contains(out.String(), test.expectOut) {
+				t.Fatalf("unexpected output: expected to contain %v, but got %v", test.expectOut, out.String())
 			}
 		})
 	}