Merge pull request #32028 from ryansann/fp-rbac

[master] Implement deterministic naming for Bindings to mitigate duplicate issues

pkg/controllers/management/auth/indexes.go

@@ -23,7 +23,6 @@ func getRBOwnerKey(rb *v1.RoleBinding) []string {
 
 func rbRoleSubjectKey(roleName string, subject v1.Subject) string {
 	return roleName + "." + subject.Kind + "." + subject.Name
-
 }
 
 func rbRoleSubjectKeys(roleName string, subjects []v1.Subject) []string {

pkg/controllers/management/auth/manager.go

@@ -146,20 +146,31 @@ func (m *manager) ensureClusterMembershipBinding(roleName, rtbNsAndName string,
 
 	if len(objs) == 0 {
 		logrus.Infof("[%v] Creating clusterRoleBinding for membership in cluster %v for subject %v", m.controller, cluster.Name, subject.Name)
-		_, err := m.mgmt.RBAC.ClusterRoleBindings("").Create(&v1.ClusterRoleBinding{
+		roleRef := v1.RoleRef{
+			Kind: "ClusterRole",
+			Name: roleName,
+		}
+		crbName := pkgrbac.NameForClusterRoleBinding(roleRef, subject) // use deterministic name for crb
+		_, err = m.mgmt.RBAC.ClusterRoleBindings("").Create(&v1.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
-				GenerateName: "clusterrolebinding-",
+				Name: crbName,
 				Labels: map[string]string{
 					rtbNsAndName: MembershipBindingOwner,
 				},
 			},
 			Subjects: []v1.Subject{subject},
-			RoleRef: v1.RoleRef{
-				Kind: "ClusterRole",
-				Name: roleName,
-			},
+			RoleRef:  roleRef,
 		})
-		return err
+		if !apierrors.IsAlreadyExists(err) {
+			return err
+		}
+
+		// if the binding exists but was not found in the index, manually retrieve it so that we can add appropriate labels
+		crb, err := m.mgmt.RBAC.ClusterRoleBindings("").Get(crbName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		objs = append(objs, crb)
 	}
 
 	crb, _ = objs[0].(*v1.ClusterRoleBinding)
@@ -219,20 +230,32 @@ func (m *manager) ensureProjectMembershipBinding(roleName, rtbNsAndName, namespa
 
 	if len(objs) == 0 {
 		logrus.Infof("[%v] Creating roleBinding for membership in project %v for subject %v", m.controller, project.Name, subject.Name)
-		_, err := m.mgmt.RBAC.RoleBindings(namespace).Create(&v1.RoleBinding{
+		roleRef := v1.RoleRef{
+			Kind: "Role",
+			Name: roleName,
+		}
+		// use deterministic name for rb
+		rbName := pkgrbac.NameForRoleBinding(namespace, roleRef, subject)
+		_, err = m.mgmt.RBAC.RoleBindings(namespace).Create(&v1.RoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
-				GenerateName: "rolebinding-",
+				Name: rbName,
 				Labels: map[string]string{
 					rtbNsAndName: MembershipBindingOwner,
 				},
 			},
 			Subjects: []v1.Subject{subject},
-			RoleRef: v1.RoleRef{
-				Kind: "Role",
-				Name: roleName,
-			},
+			RoleRef:  roleRef,
 		})
-		return err
+		if !apierrors.IsAlreadyExists(err) {
+			return err
+		}
+
+		// if the binding already exists but was not found in the index, manually retrieve it so that we can add appropriate labels
+		rb, err := m.mgmt.RBAC.RoleBindings(namespace).Get(rbName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		objs = append(objs, rb)
 	}
 
 	rb, _ = objs[0].(*v1.RoleBinding)

pkg/controllers/managementuser/rbac/handler_base.go

@@ -435,7 +435,7 @@ func (m *manager) ensureBindings(ns string, roles map[string]*v3.RoleTemplate, b
 		return err
 	}
 	for roleName := range roles {
-		rbKey, objectMeta, subjects, roleRef := bindingParts(roleName, meta.GetNamespace()+"_"+meta.GetName(), subject)
+		rbKey, objectMeta, subjects, roleRef := bindingParts(ns, roleName, meta.GetNamespace()+"_"+meta.GetName(), subject)
 		desiredRBs[rbKey] = create(objectMeta, subjects, roleRef)
 	}
 
@@ -468,16 +468,17 @@ func (m *manager) ensureBindings(ns string, roles map[string]*v3.RoleTemplate, b
 	}
 
 	for key, rb := range desiredRBs {
-		logrus.Infof("Creating roleBinding %v", key)
 		switch roleBinding := rb.(type) {
 		case *rbacv1.RoleBinding:
+			logrus.Infof("Creating roleBinding %v", key)
 			_, err := m.workload.RBAC.RoleBindings(ns).Create(roleBinding)
-			if err != nil {
+			if err != nil && !apierrors.IsAlreadyExists(err) {
 				return err
 			}
 		case *rbacv1.ClusterRoleBinding:
+			logrus.Infof("Creating clusterRoleBinding %v", key)
 			_, err := m.workload.RBAC.ClusterRoleBindings("").Create(roleBinding)
-			if err != nil {
+			if err != nil && !apierrors.IsAlreadyExists(err) {
 				return err
 			}
 		}
@@ -492,18 +493,28 @@ func (m *manager) ensureBindings(ns string, roles map[string]*v3.RoleTemplate, b
 	return nil
 }
 
-func bindingParts(roleName, parentNsAndName string, subject rbacv1.Subject) (string, metav1.ObjectMeta, []rbacv1.Subject, rbacv1.RoleRef) {
-	crbKey := rbRoleSubjectKey(roleName, subject)
-	return crbKey,
+func bindingParts(namespace, roleName, parentNsAndName string, subject rbacv1.Subject) (string, metav1.ObjectMeta, []rbacv1.Subject, rbacv1.RoleRef) {
+	key := rbRoleSubjectKey(roleName, subject)
+
+	roleRef := rbacv1.RoleRef{
+		Kind: "ClusterRole",
+		Name: roleName,
+	}
+
+	var name string
+	if namespace == "" { // if namespace is empty, binding will be ClusterRoleBinding, so name accordingly
+		name = pkgrbac.NameForClusterRoleBinding(roleRef, subject)
+	} else {
+		name = pkgrbac.NameForRoleBinding(namespace, roleRef, subject)
+	}
+
+	return key,
 		metav1.ObjectMeta{
-			GenerateName: "clusterrolebinding-",
-			Labels:       map[string]string{rtbOwnerLabel: parentNsAndName},
+			Name:   name,
+			Labels: map[string]string{rtbOwnerLabel: parentNsAndName},
 		},
 		[]rbacv1.Subject{subject},
-		rbacv1.RoleRef{
-			Kind: "ClusterRole",
-			Name: roleName,
-		}
+		roleRef
 }
 
 func prtbByProjectName(obj interface{}) ([]string, error) {
@@ -511,7 +522,6 @@ func prtbByProjectName(obj interface{}) ([]string, error) {
 	if !ok {
 		return []string{}, nil
 	}
-
 	return []string{prtb.ProjectName}, nil
 }
 
@@ -536,7 +546,6 @@ func prtbByProjectAndSubject(obj interface{}) ([]string, error) {
 	if !ok {
 		return []string{}, nil
 	}
-
 	return []string{getPRTBProjectAndSubjectKey(prtb)}, nil
 }
 
@@ -566,7 +575,6 @@ func crbRoleSubjectKeys(roleName string, subjects []rbacv1.Subject) []string {
 
 func rbRoleSubjectKey(roleName string, subject rbacv1.Subject) string {
 	return subject.Kind + " " + subject.Name + " Role " + roleName
-
 }
 
 func crbByRoleAndSubject(obj interface{}) ([]string, error) {
@@ -574,7 +582,6 @@ func crbByRoleAndSubject(obj interface{}) ([]string, error) {
 	if !ok {
 		return []string{}, nil
 	}
-
 	return crbRoleSubjectKeys(crb.RoleRef.Name, crb.Subjects), nil
 }
 

pkg/controllers/managementuser/rbac/prtb_handler.go

@@ -247,7 +247,7 @@ func (m *manager) checkForGlobalResourceRules(role *v3.RoleTemplate, resource st
 	return verbs, nil
 }
 
-// Ensure the clusterRole used to grant access of global resources to users/groups in projects has appropriate rulesfor the give resource and verbs
+// Ensure the clusterRole used to grant access of global resources to users/groups in projects has appropriate rules for the given resource and verbs
 func (m *manager) reconcileRoleForProjectAccessToGlobalResource(resource string, rt *v3.RoleTemplate, newVerbs map[string]bool) (string, error) {
 	clusterRoles := m.workload.RBAC.ClusterRoles("")
 	roleName := rt.Name + "-promoted"

pkg/controllers/managementuser/rbac/reconcile_roletemplate.go

@@ -86,50 +86,64 @@ func (m *manager) reconcileProjectAccessToGlobalResources(binding *v3.ProjectRol
 		crbs, _ := m.crbIndexer.ByIndex(crbByRoleAndSubjectIndex, crbKey)
 		if len(crbs) == 0 {
 			logrus.Infof("Creating clusterRoleBinding for project access to global resource for subject %v role %v.", subject.Name, role)
+			roleRef := rbacv1.RoleRef{
+				Kind: "ClusterRole",
+				Name: role,
+			}
+			crbName := pkgrbac.NameForClusterRoleBinding(roleRef, subject)
 			createdCRB, err := bindingCli.Create(&rbacv1.ClusterRoleBinding{
 				ObjectMeta: metav1.ObjectMeta{
-					GenerateName: "clusterrolebinding-",
+					Name: crbName,
 					Labels: map[string]string{
 						rtbUID: owner,
 					},
 				},
 				Subjects: []rbacv1.Subject{subject},
-				RoleRef: rbacv1.RoleRef{
-					Kind: "ClusterRole",
-					Name: role,
-				},
+				RoleRef:  roleRef,
 			})
+			if err == nil {
+				crbsToKeep[createdCRB.Name] = true
+				continue
+			}
+			if !apierrors.IsAlreadyExists(err) {
+				return nil, err
+			}
+
+			// the binding exists but was not found in the index, manually retrieve it so that we can add appropriate labels
+			crb, err := bindingCli.Get(crbName, metav1.GetOptions{})
 			if err != nil {
 				return nil, err
 			}
-			crbsToKeep[createdCRB.Name] = true
-		} else {
-		CRBs:
-			for _, obj := range crbs {
-				crb, ok := obj.(*rbacv1.ClusterRoleBinding)
-				if !ok {
-					continue
-				}
-				crbsToKeep[crb.Name] = true
-				for owner := range crb.Labels {
-					if rtbUID == owner {
-						continue CRBs
-					}
-				}
 
-				crb = crb.DeepCopy()
-				if crb.Labels == nil {
-					crb.Labels = map[string]string{}
-				}
-				crb.Labels[rtbUID] = owner
-				logrus.Infof("Updating clusterRoleBinding %v for project access to global resource for subject %v role %v.", crb.Name, subject.Name, role)
-				_, err := bindingCli.Update(crb)
-				if err != nil {
-					return nil, err
+			crbs = append(crbs, crb)
+		}
+
+	CRBs:
+		for _, obj := range crbs {
+			crb, ok := obj.(*rbacv1.ClusterRoleBinding)
+			if !ok {
+				continue
+			}
+
+			crbsToKeep[crb.Name] = true
+			for owner := range crb.Labels {
+				if rtbUID == owner {
+					continue CRBs
 				}
 			}
-		}
 
+			crb = crb.DeepCopy()
+			if crb.Labels == nil {
+				crb.Labels = map[string]string{}
+			}
+			crb.Labels[rtbUID] = owner
+			logrus.Infof("Updating clusterRoleBinding %v for project access to global resource for subject %v role %v.", crb.Name, subject.Name, role)
+			_, err := bindingCli.Update(crb)
+			if err != nil {
+				return nil, err
+			}
+		}
 	}
+
 	return crbsToKeep, nil
 }

pkg/rbac/common.go

@@ -9,6 +9,7 @@
 	"github.com/rancher/norman/types"
 	v3 "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3"
 	"github.com/rancher/rancher/pkg/ref"
+	"github.com/sirupsen/logrus"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -161,3 +162,40 @@ func TypeFromContext(apiContext *types.APIContext, resource *types.RawResource)
 func GetRTBLabel(objMeta metav1.ObjectMeta) string {
 	return objMeta.Namespace + "_" + objMeta.Name
 }
+
+// NameForRoleBinding returns a deterministic name for a RoleBinding with the provided namespace, roleName, and subject
+func NameForRoleBinding(namespace string, role rbacv1.RoleRef, subject rbacv1.Subject) string {
+	var name strings.Builder
+	name.WriteString("rb-")
+	name.WriteString(getBindingHash(namespace, role, subject))
+	nm := name.String()
+	logrus.Debugf("RoleBinding with namespace=%s role.kind=%s role.name=%s subject.kind=%s subject.name=%s has name: %s", namespace, role.Kind, role.Name, subject.Kind, subject.Name, nm)
+	return nm
+}
+
+// NameForClusterRoleBinding returns a deterministic name for a ClusterRoleBinding with the provided roleName and subject
+func NameForClusterRoleBinding(role rbacv1.RoleRef, subject rbacv1.Subject) string {
+	var name strings.Builder
+	name.WriteString("crb-")
+	name.WriteString(getBindingHash("", role, subject))
+	nm := name.String()
+	logrus.Debugf("ClusterRoleBinding with role.kind=%s role.name=%s subject.kind=%s subject.name=%s has name: %s", role.Kind, role.Name, subject.Kind, subject.Name, nm)
+	return nm
+}
+
+// getBindingHash returns a hash created from the passed in arguments
+// uses base32 encoding for hash, since all characters in encoding scheme are valid in k8s resource names
+// probability of collision is: 1/32^10 == 1/(2^5)^10 == 1/2^50 (sufficiently low)
+func getBindingHash(namespace string, role rbacv1.RoleRef, subject rbacv1.Subject) string {
+	var input strings.Builder
+	input.WriteString(namespace)
+	input.WriteString(role.Kind)
+	input.WriteString(role.Name)
+	input.WriteString(subject.Kind)
+	input.WriteString(subject.Name)
+
+	hasher := sha256.New()
+	hasher.Write([]byte(input.String()))
+	digest := base32.StdEncoding.WithPadding(-1).EncodeToString(hasher.Sum(nil))
+	return strings.ToLower(digest[:10])
+}