Merge pull request #15364 from deads2k/auth-22-fix-controller

remove old controller roles

Author: liggitt

pkg/cmd/server/bootstrappolicy/controller_policy.go

@@ -9,10 +9,47 @@ import (
 	rbac "k8s.io/kubernetes/pkg/apis/rbac"
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
+
+	// we need the conversions registered for our init block
+	_ "github.com/openshift/origin/pkg/authorization/apis/authorization/install"
 )
 
 const saRolePrefix = "system:openshift:controller:"
 
+const (
+	InfraOriginNamespaceServiceAccountName                      = "origin-namespace-controller"
+	InfraServiceAccountControllerServiceAccountName             = "serviceaccount-controller"
+	InfraServiceAccountPullSecretsControllerServiceAccountName  = "serviceaccount-pull-secrets-controller"
+	InfraServiceAccountTokensControllerServiceAccountName       = "serviceaccount-tokens-controller"
+	InfraServiceServingCertServiceAccountName                   = "service-serving-cert-controller"
+	InfraBuildControllerServiceAccountName                      = "build-controller"
+	InfraBuildConfigChangeControllerServiceAccountName          = "build-config-change-controller"
+	InfraDeploymentConfigControllerServiceAccountName           = "deploymentconfig-controller"
+	InfraDeploymentTriggerControllerServiceAccountName          = "deployment-trigger-controller"
+	InfraDeployerControllerServiceAccountName                   = "deployer-controller"
+	InfraImageTriggerControllerServiceAccountName               = "image-trigger-controller"
+	InfraImageImportControllerServiceAccountName                = "image-import-controller"
+	InfraSDNControllerServiceAccountName                        = "sdn-controller"
+	InfraClusterQuotaReconciliationControllerServiceAccountName = "cluster-quota-reconciliation-controller"
+	InfraUnidlingControllerServiceAccountName                   = "unidling-controller"
+	InfraServiceIngressIPControllerServiceAccountName           = "service-ingress-ip-controller"
+	InfraPersistentVolumeRecyclerControllerServiceAccountName   = "pv-recycler-controller"
+	InfraResourceQuotaControllerServiceAccountName              = "resourcequota-controller"
+
+	// template instance controller watches for TemplateInstance object creation
+	// and instantiates templates as a result.
+	InfraTemplateInstanceControllerServiceAccountName = "template-instance-controller"
+
+	// template service broker is an open service broker-compliant API
+	// implementation which serves up OpenShift templates.  It uses the
+	// TemplateInstance backend for most of the heavy lifting.
+	InfraTemplateServiceBrokerServiceAccountName = "template-service-broker"
+
+	// This is a special constant which maps to the service account name used by the underlying
+	// Kubernetes code, so that we can build out the extra policy required to scale OpenShift resources.
+	InfraHorizontalPodAutoscalerControllerServiceAccountName = "horizontal-pod-autoscaler"
+)
+
 var (
 	// controllerRoles is a slice of roles used for controllers
 	controllerRoles = []rbac.ClusterRole{}
@@ -296,6 +333,19 @@ func init() {
 		},
 	})
 
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraTemplateServiceBrokerServiceAccountName},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("create").Groups(kAuthzGroup).Resources("subjectaccessreviews").RuleOrDie(),
+			rbac.NewRule("create").Groups(authzGroup).Resources("subjectaccessreviews").RuleOrDie(),
+			rbac.NewRule("get", "create", "update", "delete").Groups(templateGroup).Resources("brokertemplateinstances").RuleOrDie(),
+			rbac.NewRule("get", "create", "delete", "assign").Groups(templateGroup).Resources("templateinstances").RuleOrDie(),
+			rbac.NewRule("get", "list", "create", "delete").Groups(kapiGroup).Resources("secrets").RuleOrDie(),
+			rbac.NewRule("list").Groups(kapiGroup).Resources("services", "configmaps").RuleOrDie(),
+			rbac.NewRule("list").Groups(routeGroup).Resources("routes").RuleOrDie(),
+			eventsRule(),
+		},
+	})
 }
 
 // ControllerRoles returns the cluster roles used by controllers

pkg/cmd/server/bootstrappolicy/dead.go

@@ -56,5 +56,4 @@ func init() {
 	addDeadClusterRole("system:build-controller")
 	addDeadClusterRole("system:deploymentconfig-controller")
 	addDeadClusterRole("system:deployment-controller")
-
 }

pkg/cmd/server/bootstrappolicy/infra_sa_policy.go

@@ -959,7 +959,6 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 	// dead cluster roles need to be checked for conflicts (in case something new comes up)
 	// so add them to this list.
 	openshiftClusterRoles = append(openshiftClusterRoles, GetDeadClusterRoles()...)
-	openshiftSAClusterRoles := InfraSAs.AllRoles()
 	kubeClusterRoles, err := GetKubeBootstrapClusterRoles()
 	// coder error
 	if err != nil {
@@ -984,9 +983,6 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 	for _, clusterRole := range openshiftClusterRoles {
 		openshiftClusterRoleNames.Insert(clusterRole.Name)
 	}
-	for _, clusterRole := range openshiftSAClusterRoles {
-		openshiftClusterRoleNames.Insert(clusterRole.Name)
-	}
 	for _, clusterRole := range kubeClusterRoles {
 		kubeClusterRoleNames.Insert(clusterRole.Name)
 	}
@@ -1005,7 +1001,6 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 
 	finalClusterRoles := []authorizationapi.ClusterRole{}
 	finalClusterRoles = append(finalClusterRoles, openshiftClusterRoles...)
-	finalClusterRoles = append(finalClusterRoles, openshiftSAClusterRoles...)
 	finalClusterRoles = append(finalClusterRoles, openshiftControllerRoles...)
 	finalClusterRoles = append(finalClusterRoles, kubeSAClusterRoles...)
 	for i := range kubeClusterRoles {
@@ -1264,9 +1259,6 @@ var clusterRoleConflicts = sets.NewString(
 	// TODO this should probably be re-swizzled to be the delta on top of the kube role
 	"system:discovery",
 
-	// TODO deconflict this
-	"system:node-bootstrapper",
-
 	// TODO these should be reconsidered
 	"cluster-admin",
 	"system:node",

pkg/cmd/server/bootstrappolicy/policy.go

@@ -46,7 +46,6 @@ var rolesToHide = sets.NewString(
 	"system:node-proxier",
 	"system:node-reader",
 	"system:oauth-token-deleter",
-	"system:openshift:template-service-broker",
 	"system:openshift:templateservicebroker-client",
 	"system:persistent-volume-provisioner",
 	"system:registry",
@@ -75,15 +74,11 @@ func TestSystemOnlyRoles(t *testing.T) {
 	}
 
 	if !show.Equal(rolesToShow) || !hide.Equal(rolesToHide) {
-		shouldNotShow := show.Difference(rolesToShow).List()
-		shouldNotHide := hide.Difference(rolesToHide).List()
 		t.Error("The list of expected end user roles has been changed.  Please discuss with the web console team to update role annotations.")
-		if len(shouldNotShow) > 0 {
-			t.Errorf("These roles are visible but not in rolesToShow: %v", shouldNotShow)
-		}
-		if len(shouldNotHide) > 0 {
-			t.Errorf("These roles are hidden but not in rolesToHide: %v", shouldNotHide)
-		}
+		t.Logf("These roles are visible but not in rolesToShow: %v", show.Difference(rolesToShow).List())
+		t.Logf("These roles are hidden but not in rolesToHide: %v", hide.Difference(rolesToHide).List())
+		t.Logf("These roles are in rolesToShow but are missing from the visible list: %v", rolesToShow.Difference(show).List())
+		t.Logf("These roles are in rolesToHide but are missing from the hidden list: %v", rolesToHide.Difference(hide).List())
 	}
 }
 

pkg/cmd/server/bootstrappolicy/web_console_role_test.go

@@ -54,15 +54,7 @@ func (c *MasterConfig) ensureOpenShiftInfraNamespace() {
 		return
 	}
 
-	roleAccessor := policy.NewClusterRoleBindingAccessor(c.ServiceAccountRoleBindingClient())
-	for _, saName := range bootstrappolicy.InfraSAs.GetServiceAccounts() {
-		_, err := c.KubeClientsetInternal().Core().ServiceAccounts(ns).Create(&kapi.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: saName}})
-		if err != nil && !kapierror.IsAlreadyExists(err) {
-			glog.Errorf("Error creating service account %s/%s: %v", ns, saName, err)
-		}
-
-		role, _ := bootstrappolicy.InfraSAs.RoleFor(saName)
-
+	for _, role := range bootstrappolicy.ControllerRoles() {
 		reconcileRole := &policy.ReconcileClusterRolesOptions{
 			RolesToReconcile: []string{role.Name},
 			Confirmed:        true,
@@ -73,16 +65,17 @@ func (c *MasterConfig) ensureOpenShiftInfraNamespace() {
 		if err := reconcileRole.RunReconcileClusterRoles(nil, nil); err != nil {
 			glog.Errorf("Could not reconcile %v: %v\n", role.Name, err)
 		}
-
-		addRole := &policy.RoleModificationOptions{
-			RoleName:            role.Name,
-			RoleBindingAccessor: roleAccessor,
-			Subjects:            []kapi.ObjectReference{{Namespace: ns, Name: saName, Kind: "ServiceAccount"}},
+	}
+	for _, roleBinding := range bootstrappolicy.ControllerRoleBindings() {
+		reconcileRoleBinding := &policy.ReconcileClusterRoleBindingsOptions{
+			RolesToReconcile:  []string{roleBinding.RoleRef.Name},
+			Confirmed:         true,
+			Union:             true,
+			Out:               ioutil.Discard,
+			RoleBindingClient: c.PrivilegedLoopbackOpenShiftClient.ClusterRoleBindings(),
 		}
-		if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { return addRole.AddRole() }); err != nil {
-			glog.Errorf("Could not add %v service accounts to the %v cluster role: %v\n", saName, role.Name, err)
-		} else {
-			glog.V(2).Infof("Added %v service accounts to the %v cluster role: %v\n", saName, role.Name, err)
+		if err := reconcileRoleBinding.RunReconcileClusterRoleBindings(nil, nil); err != nil {
+			glog.Errorf("Could not reconcile %v: %v\n", roleBinding.Name, err)
 		}
 	}