From 7ccfc6d7d48c874bb5bd97624372e70bbba656c8 Mon Sep 17 00:00:00 2001
From: "Steven E. Harris" <seharris@us.ibm.com>
Date: Wed, 29 Mar 2017 17:31:21 -0400
Subject: [PATCH] Sort dependent RBAC role and binding kinds

During installation and uninstallation, handle the RBAC-related
ClusterRole, ClusterRoleBinding, Role, and RoleBinding kinds in an
order that respects their potential referential integrity, namely that
ClusterRoleBindings can refer to ClusterRoles and ServiceAccounts, and
RoleBindings can refer to ClusterRoles, Roles, and ServiceAccounts.

Fixes #2199.
---
 pkg/tiller/kind_sorter.go      | 44 +++++++++++++++++--
 pkg/tiller/kind_sorter_test.go | 78 +++++++++++++++++++++++-----------
 2 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/pkg/tiller/kind_sorter.go b/pkg/tiller/kind_sorter.go
index ff991fc4a..bf40e7c6e 100644
--- a/pkg/tiller/kind_sorter.go
+++ b/pkg/tiller/kind_sorter.go
@@ -23,11 +23,47 @@ import (
 // SortOrder is an ordering of Kinds.
 type SortOrder []string
 
-// InstallOrder is the order in which manifests should be installed (by Kind)
-var InstallOrder SortOrder = []string{"Namespace", "Secret", "ConfigMap", "PersistentVolume", "PersistentVolumeClaim", "ServiceAccount", "Service", "Pod", "ReplicationController", "Deployment", "DaemonSet", "Ingress", "Job"}
+// InstallOrder is the order in which manifests should be installed (by Kind).
+var InstallOrder SortOrder = []string{
+	"Namespace",
+	"Secret",
+	"ConfigMap",
+	"PersistentVolume",
+	"PersistentVolumeClaim",
+	"ServiceAccount",
+	"ClusterRole",
+	"ClusterRoleBinding",
+	"Role",
+	"RoleBinding",
+	"Service",
+	"Pod",
+	"ReplicationController",
+	"Deployment",
+	"DaemonSet",
+	"Ingress",
+	"Job",
+}
 
-// UninstallOrder is the order in which manifests should be uninstalled (by Kind)
-var UninstallOrder SortOrder = []string{"Service", "Pod", "ReplicationController", "Deployment", "DaemonSet", "ConfigMap", "Secret", "PersistentVolumeClaim", "PersistentVolume", "ServiceAccount", "Ingress", "Job", "Namespace"}
+// UninstallOrder is the order in which manifests should be uninstalled (by Kind).
+var UninstallOrder SortOrder = []string{
+	"Service",
+	"Pod",
+	"ReplicationController",
+	"Deployment",
+	"DaemonSet",
+	"ConfigMap",
+	"Secret",
+	"PersistentVolumeClaim",
+	"PersistentVolume",
+	"RoleBinding",
+	"Role",
+	"ClusterRoleBinding",
+	"ClusterRole",
+	"ServiceAccount",
+	"Ingress",
+	"Job",
+	"Namespace",
+}
 
 // sortByKind does an in-place sort of manifests by Kind.
 //
diff --git a/pkg/tiller/kind_sorter_test.go b/pkg/tiller/kind_sorter_test.go
index 212b8aefc..55fc10251 100644
--- a/pkg/tiller/kind_sorter_test.go
+++ b/pkg/tiller/kind_sorter_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package tiller
 
 import (
+	"bytes"
 	"testing"
 
 	util "k8s.io/helm/pkg/releaseutil"
@@ -27,48 +28,77 @@ func TestKindSorter(t *testing.T) {
 		{
 			name:    "m",
 			content: "",
-			head:    &util.SimpleHead{Kind: "Deployment"},
+			head:    &util.SimpleHead{Kind: "ClusterRole"},
 		},
 		{
-			name:    "l",
+			name:    " ",
 			content: "",
-			head:    &util.SimpleHead{Kind: "Service"},
+			head:    &util.SimpleHead{Kind: "ClusterRoleBinding"},
+		},
+		{
+			name:    "e",
+			content: "",
+			head:    &util.SimpleHead{Kind: "ConfigMap"},
+		},
+		{
+			name:    "k",
+			content: "",
+			head:    &util.SimpleHead{Kind: "Deployment"},
 		},
 		{
 			name:    "!",
 			content: "",
 			head:    &util.SimpleHead{Kind: "HonkyTonkSet"},
 		},
+		{
+			name:    "s",
+			content: "",
+			head:    &util.SimpleHead{Kind: "Job"},
+		},
 		{
 			name:    "h",
 			content: "",
 			head:    &util.SimpleHead{Kind: "Namespace"},
 		},
 		{
-			name:    "e",
+			name:    "w",
 			content: "",
-			head:    &util.SimpleHead{Kind: "ConfigMap"},
+			head:    &util.SimpleHead{Kind: "Role"},
+		},
+		{
+			name:    "o",
+			content: "",
+			head:    &util.SimpleHead{Kind: "RoleBinding"},
+		},
+		{
+			name:    "r",
+			content: "",
+			head:    &util.SimpleHead{Kind: "Service"},
+		},
+		{
+			name:    "l",
+			content: "",
+			head:    &util.SimpleHead{Kind: "ServiceAccount"},
 		},
 	}
 
-	res := sortByKind(manifests, InstallOrder)
-	got := ""
-	expect := "helm!"
-	for _, r := range res {
-		got += r.name
-	}
-	if got != expect {
-		t.Errorf("Expected %q, got %q", expect, got)
-	}
-
-	expect = "lmeh!"
-	got = ""
-	res = sortByKind(manifests, UninstallOrder)
-	for _, r := range res {
-		got += r.name
+	for _, test := range []struct {
+		description string
+		order       SortOrder
+		expected    string
+	}{
+		{"install", InstallOrder, "helm works!"},
+		{"uninstall", UninstallOrder, "rkeow mlsh!"},
+	} {
+		var buf bytes.Buffer
+		t.Run(test.description, func(t *testing.T) {
+			defer buf.Reset()
+			for _, r := range sortByKind(manifests, test.order) {
+				buf.WriteString(r.name)
+			}
+			if got := buf.String(); got != test.expected {
+				t.Errorf("Expected %q, got %q", test.expected, got)
+			}
+		})
 	}
-	if got != expect {
-		t.Errorf("Expected %q, got %q", expect, got)
-	}
-
 }