From 3484c6f67637792a3a91a5c7ff04cb5b28e56c23 Mon Sep 17 00:00:00 2001
From: juanvallejo <jvallejo@redhat.com>
Date: Tue, 29 Nov 2016 12:25:10 -0500
Subject: [PATCH] add resource prefix to multiple items w/ same kind

This patch ensures that a resource prefix is added to multiple items of
the same kind, when using `oc get all`. Before, a prefix was added only
when a single item was returned on `oc get all`, but ignored if only a
single resource kind existed but multiple items for that kind were
returned.
---
 pkg/kubectl/cmd/get.go               | 11 +-----
 pkg/kubectl/cmd/util/helpers.go      |  6 +---
 pkg/kubectl/resource/builder.go      | 33 +++++++++++++++++
 pkg/kubectl/resource/builder_test.go | 54 ++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 15 deletions(-)

diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go
index 7fab4c5d47..76a328e020 100644
--- a/pkg/kubectl/cmd/get.go
+++ b/pkg/kubectl/cmd/get.go
@@ -153,7 +153,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [
 	if err != nil {
 		return err
 	}
-	printAll := false
 	filterFuncs := f.DefaultResourceFilterFunc()
 	filterOpts := f.DefaultResourceFilterOptions(cmd, allNamespaces)
 
@@ -178,14 +177,6 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [
 		return cmdutil.UsageError(cmd, usageString)
 	}
 
-	// determine if args contains "all"
-	for _, a := range args {
-		if a == "all" {
-			printAll = true
-			break
-		}
-	}
-
 	// always show resources when getting by name or filename
 	argsHasNames, err := resource.HasNames(args)
 	if err != nil {
@@ -423,7 +414,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [
 	w := kubectl.GetNewTabWriter(out)
 	filteredResourceCount := 0
 
-	if cmdutil.MustPrintWithKinds(objs, infos, sorter, printAll) {
+	if resource.MultipleTypesRequested(args) || cmdutil.MustPrintWithKinds(objs, infos, sorter) {
 		showKind = true
 	}
 
diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go
index 5d07e062e5..4e0e6ef068 100644
--- a/pkg/kubectl/cmd/util/helpers.go
+++ b/pkg/kubectl/cmd/util/helpers.go
@@ -629,13 +629,9 @@ func MaybeConvertObject(obj runtime.Object, gv schema.GroupVersion, converter ru
 // with multiple resource kinds, in which case it will
 // return true, indicating resource kind will be
 // included as part of printer output
-func MustPrintWithKinds(objs []runtime.Object, infos []*resource.Info, sorter *kubectl.RuntimeSort, printAll bool) bool {
+func MustPrintWithKinds(objs []runtime.Object, infos []*resource.Info, sorter *kubectl.RuntimeSort) bool {
 	var lastMap *meta.RESTMapping
 
-	if len(infos) == 1 && printAll {
-		return true
-	}
-
 	for ix := range objs {
 		var mapping *meta.RESTMapping
 		if sorter != nil {
diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go
index e7fc926d60..48992b5162 100644
--- a/pkg/kubectl/resource/builder.go
+++ b/pkg/kubectl/resource/builder.go
@@ -803,3 +803,36 @@ func HasNames(args []string) (bool, error) {
 	}
 	return hasCombinedTypes || len(args) > 1, nil
 }
+
+// MultipleTypesRequested returns true if the provided args contain multiple resource kinds
+func MultipleTypesRequested(args []string) bool {
+	args = normalizeMultipleResourcesArgs(args)
+	rKinds := sets.NewString()
+	for _, arg := range args {
+		if arg == "all" {
+			return true
+		}
+		rTuple, found, err := splitResourceTypeName(arg)
+		if err != nil {
+			continue
+		}
+
+		// if tuple not found, assume arg is of the form "type1,type2,...".
+		// Since SplitResourceArgument returns a unique list of kinds,
+		// return true here if len(uniqueList) > 1
+		if !found {
+			if strings.Contains(arg, ",") {
+				splitArgs := SplitResourceArgument(arg)
+				if len(splitArgs) > 1 {
+					return true
+				}
+			}
+			continue
+		}
+		if rKinds.Has(rTuple.Resource) {
+			continue
+		}
+		rKinds.Insert(rTuple.Resource)
+	}
+	return (rKinds.Len() > 1)
+}
diff --git a/pkg/kubectl/resource/builder_test.go b/pkg/kubectl/resource/builder_test.go
index 043140ca90..bd62f85f7c 100644
--- a/pkg/kubectl/resource/builder_test.go
+++ b/pkg/kubectl/resource/builder_test.go
@@ -1235,3 +1235,57 @@ func TestHasNames(t *testing.T) {
 		}
 	}
 }
+
+func TestMultipleTypesRequested(t *testing.T) {
+	tests := []struct {
+		args                  []string
+		expectedMultipleTypes bool
+	}{
+		{
+			args: []string{""},
+			expectedMultipleTypes: false,
+		},
+		{
+			args: []string{"all"},
+			expectedMultipleTypes: true,
+		},
+		{
+			args: []string{"rc"},
+			expectedMultipleTypes: false,
+		},
+		{
+			args: []string{"rc,pod,svc"},
+			expectedMultipleTypes: true,
+		},
+		{
+			args: []string{"rc/foo"},
+			expectedMultipleTypes: false,
+		},
+		{
+			args: []string{"rc/foo", "rc/bar"},
+			expectedMultipleTypes: false,
+		},
+		{
+			args: []string{"rc", "foo"},
+			expectedMultipleTypes: false,
+		},
+		{
+			args: []string{"rc,pod,svc", "foo"},
+			expectedMultipleTypes: true,
+		},
+		{
+			args: []string{"rc,secrets"},
+			expectedMultipleTypes: true,
+		},
+		{
+			args: []string{"rc/foo", "rc/bar", "svc/svc"},
+			expectedMultipleTypes: true,
+		},
+	}
+	for _, test := range tests {
+		hasMultipleTypes := MultipleTypesRequested(test.args)
+		if hasMultipleTypes != test.expectedMultipleTypes {
+			t.Errorf("expected HasName to return %v for %s", test.expectedMultipleTypes, test.args)
+		}
+	}
+}