From 4bea949e42617b4cabb4c605e9d82dff39aa8749 Mon Sep 17 00:00:00 2001
From: Filinto Duran <fudran@gaikai.com>
Date: Fri, 14 Dec 2018 09:40:18 -0600
Subject: [PATCH] fixes #4728 problem that arise because of the zero value of
 unknown is the same as the value of Namespace and if the name of Unknown is
 lexicographically than Namespace then it wins when it shouldn't

Signed-off-by: Filinto Duran <duranto@gmail.com>
---
 pkg/tiller/kind_sorter.go      | 13 ++++++++----
 pkg/tiller/kind_sorter_test.go | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/pkg/tiller/kind_sorter.go b/pkg/tiller/kind_sorter.go
index 8aff4e6c1..7df63c6ac 100644
--- a/pkg/tiller/kind_sorter.go
+++ b/pkg/tiller/kind_sorter.go
@@ -124,12 +124,13 @@ func (k *kindSorter) Less(i, j int) bool {
 	b := k.manifests[j]
 	first, aok := k.ordering[a.Head.Kind]
 	second, bok := k.ordering[b.Head.Kind]
-	// if same kind (including unknown) sub sort alphanumeric
-	if first == second {
-		// if both are unknown and of different kind sort by kind alphabetically
-		if !aok && !bok && a.Head.Kind != b.Head.Kind {
+	// if both are unknown
+	if !aok && !bok {
+		// and of different kind sort by kind alphabetically
+		if a.Head.Kind != b.Head.Kind {
 			return a.Head.Kind < b.Head.Kind
 		}
+		// and of same kind sort by name alphabetically
 		return a.Name < b.Name
 	}
 	// unknown kind is last
@@ -139,6 +140,10 @@ func (k *kindSorter) Less(i, j int) bool {
 	if !bok {
 		return true
 	}
+	// if same kind sort alphanumeric
+	if first == second {
+		return a.Name < b.Name
+	}
 	// sort different kinds
 	return first < second
 }
diff --git a/pkg/tiller/kind_sorter_test.go b/pkg/tiller/kind_sorter_test.go
index 1c187e90d..823d9674e 100644
--- a/pkg/tiller/kind_sorter_test.go
+++ b/pkg/tiller/kind_sorter_test.go
@@ -223,3 +223,39 @@ func TestKindSorterSubSort(t *testing.T) {
 		})
 	}
 }
+
+func TestKindSorterUnknownsWithNamespace(t *testing.T) {
+	// a.Kind=Namespace should win over b.Kind=Unknown even if the a.Name > b.Name
+	manifests := []Manifest{
+		{
+			Name: "a",
+			Head: &util.SimpleHead{Kind: "Namespace"},
+		},
+		{
+			Name: "N",
+			Head: &util.SimpleHead{Kind: "Unknown"},
+		},
+	}
+	for _, test := range []struct {
+		description string
+		order       SortOrder
+		expected    string
+	}{
+		// expectation is sorted by kind (unknown is last) and then sub sorted alphabetically within each group
+		{"Namespace,Unknown", InstallOrder, "aN"},
+	} {
+		var buf bytes.Buffer
+		t.Run(test.description, func(t *testing.T) {
+
+			defer buf.Reset()
+			m := make([]Manifest, len(manifests))
+			copy(m, manifests)
+			for _, r := range sortByKind(m, test.order) {
+				buf.WriteString(r.Name)
+			}
+			if got := buf.String(); got != test.expected {
+				t.Errorf("Expected %q, got %q", test.expected, got)
+			}
+		})
+	}
+}