From 61d68da91f9276ce8bb3bf0f57ad4e2577b21529 Mon Sep 17 00:00:00 2001
From: Simon Pasquier <spasquie@redhat.com>
Date: Wed, 18 Apr 2018 18:02:47 +0200
Subject: [PATCH] router: assign system:auth-delegator role

Instead of adding more rules to the system:router role, this change
reuses the existing system:auth-delegator role.
---
 pkg/cmd/server/bootstrappolicy/policy.go      |  3 --
 pkg/oc/admin/router/router.go                 | 40 +++++++++++++++----
 .../bootstrap_cluster_roles.yaml              | 12 ------
 .../bootstrap_policy_file.yaml                | 14 -------
 4 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/pkg/cmd/server/bootstrappolicy/policy.go b/pkg/cmd/server/bootstrappolicy/policy.go
index 36c237d04bab..36e46381288b 100644
--- a/pkg/cmd/server/bootstrappolicy/policy.go
+++ b/pkg/cmd/server/bootstrappolicy/policy.go
@@ -564,9 +564,6 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
 				rbac.NewRule("list", "watch").Groups(kapiGroup).Resources("endpoints").RuleOrDie(),
 				rbac.NewRule("list", "watch").Groups(kapiGroup).Resources("services").RuleOrDie(),
 
-				rbac.NewRule("create").Groups(kAuthnGroup).Resources("tokenreviews").RuleOrDie(),
-				rbac.NewRule("create").Groups(kAuthzGroup).Resources("subjectaccessreviews").RuleOrDie(),
-
 				rbac.NewRule("list", "watch").Groups(routeGroup, legacyRouteGroup).Resources("routes").RuleOrDie(),
 				rbac.NewRule("update").Groups(routeGroup, legacyRouteGroup).Resources("routes/status").RuleOrDie(),
 			},
diff --git a/pkg/oc/admin/router/router.go b/pkg/oc/admin/router/router.go
index e375c0a4a527..8c53f34d6660 100644
--- a/pkg/oc/admin/router/router.go
+++ b/pkg/oc/admin/router/router.go
@@ -755,6 +755,20 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write
 				Name: "system:router",
 			},
 		},
+		&authapi.ClusterRoleBinding{
+			ObjectMeta: metav1.ObjectMeta{Name: generateAuthRoleBindingName(cfg.Name)},
+			Subjects: []kapi.ObjectReference{
+				{
+					Kind:      "ServiceAccount",
+					Name:      cfg.ServiceAccount,
+					Namespace: namespace,
+				},
+			},
+			RoleRef: kapi.ObjectReference{
+				Kind: "ClusterRole",
+				Name: "system:auth-delegator",
+			},
+		},
 	)
 
 	objects = append(objects, &appsapi.DeploymentConfig{
@@ -829,7 +843,7 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write
 
 	levelPrefixFilter := func(e error) string {
 		// Avoid failing when service accounts or role bindings already exist.
-		if ignoreError(e, cfg.ServiceAccount, generateRoleBindingName(cfg.Name)) {
+		if ignoreError(e, cfg.ServiceAccount, generateRoleBindingName(cfg.Name), generateAuthRoleBindingName(cfg.Name)) {
 			return "warning"
 		}
 		return "error"
@@ -846,9 +860,9 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write
 }
 
 // ignoreError will return true if the error is an already exists status error and
-// 1. it is for a cluster role binding named roleBindingName, or
-// 2. it is for a service account name saName
-func ignoreError(e error, saName string, roleBindingName string) bool {
+// 1. it is for a cluster role binding matching in roleBindingNames, or
+// 2. it is for a service account named saName
+func ignoreError(e error, saName string, roleBindingNames ...string) bool {
 	if !errors.IsAlreadyExists(e) {
 		return false
 	}
@@ -860,9 +874,17 @@ func ignoreError(e error, saName string, roleBindingName string) bool {
 	if details == nil {
 		return false
 	}
-	return (details.Kind == "serviceaccounts" && details.Name == saName) ||
-		(details.Kind == "clusterrolebinding" /*pre-3.7*/ && details.Name == roleBindingName) ||
-		(details.Kind == "clusterrolebindings" /*3.7+*/ && details.Name == roleBindingName)
+	if details.Kind == "serviceaccounts" {
+		return details.Name == saName
+	}
+	if details.Kind == "clusterrolebinding" /*pre-3.7*/ || details.Kind == "clusterrolebindings" /*3.7+*/ {
+		for _, name := range roleBindingNames {
+			if details.Name == name {
+				return true
+			}
+		}
+	}
+	return false
 }
 
 // generateRoleBindingName generates a name for the rolebinding object if it is
@@ -871,6 +893,10 @@ func generateRoleBindingName(name string) string {
 	return fmt.Sprintf("router-%s-role", name)
 }
 
+func generateAuthRoleBindingName(name string) string {
+	return fmt.Sprintf("router-%s-auth-role", name)
+}
+
 // generateStatsPassword creates a random password.
 func generateStatsPassword() string {
 	rand := rand.New(rand.NewSource(time.Now().UTC().UnixNano()))
diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
index a61f21c9df07..d723d659f6d5 100644
--- a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
+++ b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
@@ -1764,18 +1764,6 @@ items:
     verbs:
     - list
     - watch
-  - apiGroups:
-    - authentication.k8s.io
-    resources:
-    - tokenreviews
-    verbs:
-    - create
-  - apiGroups:
-    - authorization.k8s.io
-    resources:
-    - subjectaccessreviews
-    verbs:
-    - create
   - apiGroups:
     - ""
     - route.openshift.io
diff --git a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml
index a7428dbcec04..030c56510ac4 100644
--- a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml
+++ b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml
@@ -1934,20 +1934,6 @@ items:
     verbs:
     - list
     - watch
-  - apiGroups:
-    - authentication.k8s.io
-    attributeRestrictions: null
-    resources:
-    - tokenreviews
-    verbs:
-    - create
-  - apiGroups:
-    - authorization.k8s.io
-    attributeRestrictions: null
-    resources:
-    - subjectaccessreviews
-    verbs:
-    - create
   - apiGroups:
     - ""
     - route.openshift.io