Initial clusterRoleBindings feature

Signed-off-by: fxiang1 <fxiang@redhat.com>

Author: fxiang1

api/v1alpha1/clusterpermission_types.go

@@ -37,6 +37,11 @@ type ClusterPermissionSpec struct {
 	// +kubebuilder:validation:XValidation:rule="has(self.subject) || has(self.subjects)",message="Either subject or subjects has to exist in clusterRoleBinding"
 	ClusterRoleBinding *ClusterRoleBinding `json:"clusterRoleBinding,omitempty"`
 
+	// ClusterRoleBindings represents multiple ClusterRoleBindings that are being created on the managed cluster
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="self.all(i, has(i.subject) || has(i.subjects))",message="Either subject or subjects has to exist in every clusterRoleBinding"
+	ClusterRoleBindings *[]ClusterRoleBinding `json:"clusterRoleBindings,omitempty"`
+
 	// Roles represents roles that are being created on the managed cluster
 	// +optional
 	Roles *[]Role `json:"roles,omitempty"`

api/v1alpha1/zz_generated.deepcopy.go

@@ -194,6 +194,107 @@ spec:
                 x-kubernetes-validations:
                 - message: Either subject or subjects has to exist in clusterRoleBinding
                   rule: has(self.subject) || has(self.subjects)
+              clusterRoleBindings:
+                description: ClusterRoleBindings represents multiple ClusterRoleBindings
+                  that are being created on the managed cluster
+                items:
+                  description: ClusterRoleBinding represents the ClusterRoleBinding
+                    that is being created on the managed cluster
+                  properties:
+                    name:
+                      description: Name of the ClusterRoleBinding if a name different
+                        than the ClusterPermission name is used
+                      type: string
+                    roleRef:
+                      description: RoleRef contains information that points to the
+                        ClusterRole being used
+                      properties:
+                        apiGroup:
+                          description: APIGroup is the group for the resource being
+                            referenced
+                          type: string
+                        kind:
+                          description: Kind is the type of resource being referenced
+                          type: string
+                        name:
+                          description: Name is the name of resource being referenced
+                          type: string
+                      required:
+                      - apiGroup
+                      - kind
+                      - name
+                      type: object
+                      x-kubernetes-map-type: atomic
+                    subject:
+                      description: |-
+                        Subject contains a reference to the object or user identities a ClusterPermission binding applies to.
+                        Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
+                        If both subject and subjects exist then only subjects will be used.
+                      properties:
+                        apiGroup:
+                          description: |-
+                            APIGroup holds the API group of the referenced subject.
+                            Defaults to "" for ServiceAccount subjects.
+                            Defaults to "rbac.authorization.k8s.io" for User and Group subjects.
+                          type: string
+                        kind:
+                          description: |-
+                            Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount".
+                            If the Authorizer does not recognized the kind value, the Authorizer should report an error.
+                          type: string
+                        name:
+                          description: Name of the object being referenced.
+                          type: string
+                        namespace:
+                          description: |-
+                            Namespace of the referenced object.  If the object kind is non-namespace, such as "User" or "Group", and this value is not empty
+                            the Authorizer should report an error.
+                          type: string
+                      required:
+                      - kind
+                      - name
+                      type: object
+                      x-kubernetes-map-type: atomic
+                    subjects:
+                      description: |-
+                        Subjects contains an array of references to objects or user identities a ClusterPermission binding applies to.
+                        Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
+                        If both subject and subjects exist then only subjects will be used.
+                      items:
+                        description: |-
+                          Subject contains a reference to the object or user identities a role binding applies to.  This can either hold a direct API object reference,
+                          or a value for non-objects such as user and group names.
+                        properties:
+                          apiGroup:
+                            description: |-
+                              APIGroup holds the API group of the referenced subject.
+                              Defaults to "" for ServiceAccount subjects.
+                              Defaults to "rbac.authorization.k8s.io" for User and Group subjects.
+                            type: string
+                          kind:
+                            description: |-
+                              Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount".
+                              If the Authorizer does not recognized the kind value, the Authorizer should report an error.
+                            type: string
+                          name:
+                            description: Name of the object being referenced.
+                            type: string
+                          namespace:
+                            description: |-
+                              Namespace of the referenced object.  If the object kind is non-namespace, such as "User" or "Group", and this value is not empty
+                              the Authorizer should report an error.
+                            type: string
+                        required:
+                        - kind
+                        - name
+                        type: object
+                        x-kubernetes-map-type: atomic
+                      type: array
+                  type: object
+                type: array
+                x-kubernetes-validations:
+                - message: Either subject or subjects has to exist in every clusterRoleBinding
+                  rule: self.all(i, has(i.subject) || has(i.subjects))
               roleBindings:
                 description: RoleBindings represents RoleBindings that are being created
                   on the managed cluster

config/crds/rbac.open-cluster-management.io_clusterpermissions.yaml

@@ -0,0 +1,24 @@
+apiVersion: rbac.open-cluster-management.io/v1alpha1
+kind: ClusterPermission
+metadata:
+  name: clusterpermission-multiple-clusterrolebindings
+spec:
+  clusterRoleBindings:
+    - name: multi-crb-binding1
+      roleRef:
+        apiGroup: rbac.authorization.k8s.io
+        kind: ClusterRole
+        name: argocd-application-controller-1
+      subject:
+        kind: User
+        name: user1
+    - name: multi-crb-binding2
+      roleRef:
+        apiGroup: rbac.authorization.k8s.io
+        kind: ClusterRole
+        name: argocd-application-controller-3
+      subjects:
+        - kind: User
+          name: user2
+        - kind: Group
+          name: group1

config/samples/clusterpermission_multiple_clusterrolebindings.yaml

@@ -82,7 +82,9 @@ func (r *ClusterPermissionReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	log.Info("validating ClusterPermission")
 
 	/* Validations */
-	if clusterPermission.Spec.ClusterRoleBinding == nil && clusterPermission.Spec.RoleBindings == nil {
+	if clusterPermission.Spec.ClusterRoleBinding == nil &&
+		(clusterPermission.Spec.ClusterRoleBindings == nil || len(*clusterPermission.Spec.ClusterRoleBindings) == 0) &&
+		(clusterPermission.Spec.RoleBindings == nil || len(*clusterPermission.Spec.RoleBindings) == 0) {
 		log.Info("no bindings defined for ClusterPermission")
 
 		err := r.updateStatus(ctx, &clusterPermission, &metav1.Condition{
@@ -117,7 +119,7 @@ func (r *ClusterPermissionReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	log.Info("preparing ManifestWork payload")
 
-	clusterRole, clusterRoleBinding, roles, roleBindings, err := r.generateManifestWorkPayload(
+	clusterRole, clusterRoleBindings, roles, roleBindings, err := r.generateManifestWorkPayload(
 		ctx, &clusterPermission)
 	if err != nil {
 		log.Error(err, "failed to generate payload")
@@ -134,7 +136,7 @@ func (r *ClusterPermissionReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	mwName := generateManifestWorkName(clusterPermission)
 	manifestWork := buildManifestWork(clusterPermission, mwName,
-		clusterRole, clusterRoleBinding, roles, roleBindings)
+		clusterRole, clusterRoleBindings, roles, roleBindings)
 
 	var mw workv1.ManifestWork
 	err = r.Get(ctx, types.NamespacedName{Name: mwName, Namespace: clusterPermission.Namespace}, &mw)
@@ -255,9 +257,9 @@ func (r *ClusterPermissionReconciler) generateSubjects(ctx context.Context,
 
 // generateManifestWorkPayload creates the payload for the ManifestWork based on the ClusterPermission spec
 func (r *ClusterPermissionReconciler) generateManifestWorkPayload(ctx context.Context, clusterPermission *cpv1alpha1.ClusterPermission) (
-	*rbacv1.ClusterRole, *rbacv1.ClusterRoleBinding, []rbacv1.Role, []rbacv1.RoleBinding, error) {
+	*rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding, []rbacv1.Role, []rbacv1.RoleBinding, error) {
 	var clusterRole *rbacv1.ClusterRole
-	var clusterRoleBinding *rbacv1.ClusterRoleBinding
+	var clusterRoleBindings []rbacv1.ClusterRoleBinding
 	var roles []rbacv1.Role
 	var roleBindings []rbacv1.RoleBinding
 
@@ -304,16 +306,63 @@ func (r *ClusterPermissionReconciler) generateManifestWorkPayload(ctx context.Co
 			clusterRoleBindingRoleRef = *clusterPermission.Spec.ClusterRoleBinding.RoleRef
 		}
 
-		clusterRoleBinding = &rbacv1.ClusterRoleBinding{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: rbacv1.SchemeGroupVersion.String(),
-				Kind:       "ClusterRoleBinding",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name: clusterRoleBindingName,
-			},
-			RoleRef:  clusterRoleBindingRoleRef,
-			Subjects: subjects,
+		// We make the behaviour consistent with how subject and subjects are handled in ClusterRoleBinding
+		// subjects will just override subject if both are provided
+		if clusterPermission.Spec.ClusterRoleBindings == nil || len(*clusterPermission.Spec.ClusterRoleBindings) == 0 {
+			clusterRoleBindings = append(clusterRoleBindings, rbacv1.ClusterRoleBinding{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: rbacv1.SchemeGroupVersion.String(),
+					Kind:       "ClusterRoleBinding",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterRoleBindingName,
+				},
+				RoleRef:  clusterRoleBindingRoleRef,
+				Subjects: subjects,
+			})
+		}
+	}
+
+	// ClusterRoleBindings payload (plural)
+	if clusterPermission.Spec.ClusterRoleBindings != nil && len(*clusterPermission.Spec.ClusterRoleBindings) > 0 {
+		for _, clusterRoleBinding := range *clusterPermission.Spec.ClusterRoleBindings {
+			crbSubjects := getSubjects(clusterRoleBinding.Subject, clusterRoleBinding.Subjects)
+			if err := r.validateSubject(ctx, crbSubjects, clusterPermission.Namespace); err != nil {
+				return nil, nil, nil, nil, err
+			}
+
+			subjects, err := r.generateSubjects(ctx, crbSubjects, clusterPermission.Namespace)
+			if err != nil {
+				return nil, nil, nil, nil, err
+			}
+
+			// default to ClusterPermission name unless using custom name
+			clusterRoleBindingName := clusterPermission.Name
+			if clusterRoleBinding.Name != "" {
+				clusterRoleBindingName = clusterRoleBinding.Name
+			}
+
+			// default to creating a ClusterRole unless using existing ClusterRole
+			clusterRoleBindingRoleRef := rbacv1.RoleRef{
+				APIGroup: rbacv1.GroupName,
+				Kind:     "ClusterRole",
+				Name:     clusterPermission.Name,
+			}
+			if clusterRoleBinding.RoleRef != nil {
+				clusterRoleBindingRoleRef = *clusterRoleBinding.RoleRef
+			}
+
+			clusterRoleBindings = append(clusterRoleBindings, rbacv1.ClusterRoleBinding{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: rbacv1.SchemeGroupVersion.String(),
+					Kind:       "ClusterRoleBinding",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterRoleBindingName,
+				},
+				RoleRef:  clusterRoleBindingRoleRef,
+				Subjects: subjects,
+			})
 		}
 	}
 
@@ -479,5 +528,5 @@ func (r *ClusterPermissionReconciler) generateManifestWorkPayload(ctx context.Co
 		}
 	}
 
-	return clusterRole, clusterRoleBinding, roles, roleBindings, nil
+	return clusterRole, clusterRoleBindings, roles, roleBindings, nil
 }

controllers/clusterpermission_controller.go

@@ -371,6 +371,58 @@ var _ = Describe("ClusterPermission controller", func() {
 				}
 				return true
 			}).Should(BeTrue())
+
+			By("Creating a ClusterPermission that has multiple ClusterRoleBindings")
+			clusterPermissionMultipleCRBs := cpv1alpha1.ClusterPermission{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "clusterpermission-multiple-crbs",
+					Namespace: clusterName,
+				},
+				Spec: cpv1alpha1.ClusterPermissionSpec{
+					ClusterRoleBindings: &[]cpv1alpha1.ClusterRoleBinding{
+						{
+							Name: "crb-1",
+							RoleRef: &rbacv1.RoleRef{
+								APIGroup: "rbac.authorization.k8s.io",
+								Kind:     "ClusterRole",
+								Name:     "argocd-application-controller-1",
+							},
+							Subject: rbacv1.Subject{
+								Kind: "User",
+								Name: "user1",
+							},
+						},
+						{
+							Name: "crb-2",
+							RoleRef: &rbacv1.RoleRef{
+								APIGroup: "rbac.authorization.k8s.io",
+								Kind:     "ClusterRole",
+								Name:     "argocd-application-controller-2",
+							},
+							Subjects: []rbacv1.Subject{
+								{
+									Kind: "User",
+									Name: "user2",
+								},
+								{
+									Kind: "Group",
+									Name: "group1",
+								},
+							},
+						},
+					},
+				},
+			}
+
+			Expect(k8sClient.Create(ctx, &clusterPermissionMultipleCRBs)).Should(Succeed())
+			mwKeyMultipleCRBs := types.NamespacedName{Name: generateManifestWorkName(clusterPermissionMultipleCRBs), Namespace: clusterName}
+			mwMultipleCRBs := workv1.ManifestWork{}
+			Eventually(func() bool {
+				if err := k8sClient.Get(ctx, mwKeyMultipleCRBs, &mwMultipleCRBs); err != nil {
+					return false
+				}
+				return true
+			}).Should(BeTrue())
 		})
 	})
 

controllers/clusterpermission_controller_test.go

@@ -33,7 +33,7 @@ func generateManifestWorkName(clusterPermission cpv1alpha1.ClusterPermission) st
 // buildManifestWork wraps the payloads in a ManifestWork
 func buildManifestWork(clusterPermission cpv1alpha1.ClusterPermission, manifestWorkName string,
 	clusterRole *rbacv1.ClusterRole,
-	clusterRoleBinding *rbacv1.ClusterRoleBinding,
+	clusterRoleBindings []rbacv1.ClusterRoleBinding,
 	roles []rbacv1.Role,
 	roleBindings []rbacv1.RoleBinding) *workv1.ManifestWork {
 	var manifests []workv1.Manifest
@@ -42,8 +42,10 @@ func buildManifestWork(clusterPermission cpv1alpha1.ClusterPermission, manifestW
 		manifests = append(manifests, workv1.Manifest{RawExtension: runtime.RawExtension{Object: clusterRole}})
 	}
 
-	if clusterRoleBinding != nil {
-		manifests = append(manifests, workv1.Manifest{RawExtension: runtime.RawExtension{Object: clusterRoleBinding}})
+	if len(clusterRoleBindings) > 0 {
+		for i := range clusterRoleBindings {
+			manifests = append(manifests, workv1.Manifest{RawExtension: runtime.RawExtension{Object: &clusterRoleBindings[i]}})
+		}
 	}
 
 	if len(roles) > 0 {

controllers/helper.go

@@ -128,7 +128,7 @@ func Test_buildManifestWork(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := buildManifestWork(tt.args.clusterPermission, tt.args.manifestWorkName, tt.args.clusterRole, tt.args.clusterRoleBinding,
+			got := buildManifestWork(tt.args.clusterPermission, tt.args.manifestWorkName, tt.args.clusterRole, []rbacv1.ClusterRoleBinding{*tt.args.clusterRoleBinding},
 				tt.args.roles, tt.args.roleBindings)
 			// check work name
 			if got.Name != tt.wants.manifestWorkName {