Add support for ClusterRole AggregationRule during bundle generation

ClusterRoles that use an AggregationRule often do not have any rules
defined directly. Instead, their rules are aggregated from other
ClusterRoles that match the AggregationRule’s label selector.

The existing generator logic only included rules from ClusterRoles that
were explicitly bound via ClusterRoleBindings to the ServiceAccounts
used by Deployments. Since ClusterRoles with an AggregationRule
typically lack direct rule definitions, the resulting permission bundle
ended up being empty.

This update adds support for handling ClusterRoles that use an
AggregationRule.

Signed-off-by: Maciej Zimnoch <[email protected]>

Author: zimnx

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -31,6 +32,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/operator-framework/operator-sdk/internal/generate/collector"
@@ -57,8 +61,14 @@ func apply(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion,
 	switch strategy.StrategyName {
 	case operatorsv1alpha1.InstallStrategyNameDeployment:
 		inPerms, inCPerms, _ := c.SplitCSVPermissionsObjects(extraSAs)
-		applyRoles(c, inPerms, &strategy.StrategySpec, extraSAs)
-		applyClusterRoles(c, inCPerms, &strategy.StrategySpec, extraSAs)
+		err := applyRoles(c, inPerms, &strategy.StrategySpec, extraSAs)
+		if err != nil {
+			return fmt.Errorf("can't apply roles: %w", err)
+		}
+		err = applyClusterRoles(c, inCPerms, &strategy.StrategySpec, extraSAs)
+		if err != nil {
+			return fmt.Errorf("can't apply cluster roles: %w", err)
+		}
 		applyDeployments(c, &strategy.StrategySpec)
 	}
 	csv.Spec.InstallStrategy = strategy
@@ -88,7 +98,7 @@ const defaultServiceAccountName = "default"
 
 // applyRoles applies Roles to strategy's permissions field by combining Roles bound to ServiceAccounts
 // into one set of permissions.
-func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) { //nolint:dupl
+func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) error { //nolint:dupl
 	roleSet := make(map[string]rbacv1.Role)
 	cRoleSet := make(map[string]rbacv1.ClusterRole)
 	for i := range objs {
@@ -120,8 +130,16 @@ func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operator
 				hasRole = has
 			case "ClusterRole":
 				role, has := cRoleSet[binding.RoleRef.Name]
-				rules = role.Rules
+
 				hasRole = has
+				if has {
+					var err error
+					rules, err = getClusterRoleRules(role, c.ClusterRoles)
+					if err != nil {
+						return fmt.Errorf("can't get ClusterRole rules: %w", err)
+					}
+				}
+
 			default:
 				continue
 			}
@@ -143,11 +161,13 @@ func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operator
 		return perms[i].ServiceAccountName < perms[j].ServiceAccountName
 	})
 	strategy.Permissions = perms
+
+	return nil
 }
 
 // applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field by combining ClusterRoles
 // bound to ServiceAccounts into one set of clusterPermissions.
-func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) { //nolint:dupl
+func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) error { //nolint:dupl
 	roleSet := make(map[string]rbacv1.ClusterRole)
 	for i := range objs {
 		switch t := objs[i].(type) {
@@ -166,7 +186,12 @@ func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *o
 				continue
 			}
 			if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole {
-				perm.Rules = append(perm.Rules, role.Rules...)
+				rules, err := getClusterRoleRules(role, c.ClusterRoles)
+				if err != nil {
+					return fmt.Errorf("can't get ClusterRole rules: %w", err)
+				}
+
+				perm.Rules = append(perm.Rules, rules...)
 				saToPermissions[subject.Name] = perm
 			}
 		}
@@ -183,6 +208,49 @@ func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *o
 		return perms[i].ServiceAccountName < perms[j].ServiceAccountName
 	})
 	strategy.ClusterPermissions = perms
+
+	return nil
+}
+
+// getClusterRoleRules returns all PolicyRules for a given ClusterRole, including rules from aggregated ClusterRoles
+// as specified by the AggregationRule. It recursively collects rules from other ClusterRoles that match the label selectors
+// in the AggregationRule, ensuring no duplicate rules are added.
+func getClusterRoleRules(clusterRole rbacv1.ClusterRole, clusterRoles []rbacv1.ClusterRole) ([]rbacv1.PolicyRule, error) {
+	rules := make([]rbacv1.PolicyRule, 0, len(clusterRole.Rules))
+	rules = append(rules, clusterRole.Rules...)
+
+	if clusterRole.AggregationRule == nil {
+		return rules, nil
+	}
+
+	for _, crSelector := range clusterRole.AggregationRule.ClusterRoleSelectors {
+		labelSelector, err := metav1.LabelSelectorAsSelector(&crSelector)
+		if err != nil {
+			return nil, fmt.Errorf("can't create label selector from ClusterRole %q AggregationRule: %w", clusterRole.Name, err)
+		}
+
+		for _, cr := range clusterRoles {
+			if cr.Name == clusterRole.Name {
+				continue
+			}
+
+			if !labelSelector.Matches(labels.Set(cr.Labels)) {
+				continue
+			}
+
+			for _, rule := range cr.Rules {
+				ruleExists := slices.ContainsFunc(rules, func(r rbacv1.PolicyRule) bool {
+					return equality.Semantic.DeepEqual(rule, r)
+				})
+
+				if !ruleExists {
+					rules = append(rules, rule)
+				}
+			}
+		}
+	}
+
+	return rules, nil
 }
 
 // initPermissionSet initializes a map of ServiceAccount name to permissions, which are empty.

internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go

@@ -24,6 +24,7 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -65,6 +66,9 @@ var _ = Describe("apply functions", func() {
 			saName1    = "service-account-1"
 			roleName1  = "role-1"
 			cRoleName1 = "cluster-role-1"
+			cRoleName2 = "cluster-role-2"
+			cRoleName3 = "cluster-role-3"
+			cRoleName4 = "cluster-role-4"
 		)
 
 		BeforeEach(func() {
@@ -79,7 +83,8 @@ var _ = Describe("apply functions", func() {
 				rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}}
 				perms := []client.Object{newRole(roleName1, rules...)}
 				c.RoleBindings = []rbacv1.RoleBinding{newRoleBinding("role-binding", newRoleRef(roleName1), newServiceAccountSubject(saName1))}
-				applyRoles(c, perms, strategy, nil)
+				err := applyRoles(c, perms, strategy, nil)
+				Expect(err).NotTo(HaveOccurred())
 				Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{
 					{ServiceAccountName: saName1, Rules: rules},
 				}))
@@ -90,7 +95,52 @@ var _ = Describe("apply functions", func() {
 				rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}}
 				perms := []client.Object{newClusterRole(cRoleName1, rules...)}
 				c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))}
-				applyClusterRoles(c, perms, strategy, nil)
+				err := applyClusterRoles(c, perms, strategy, nil)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{
+					{ServiceAccountName: saName1, Rules: rules},
+				}))
+			})
+			It("adds rules from aggregated ClusterRoles eliminating duplicates to the CSV deployment strategy", func() {
+				c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)}
+				c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)}
+				rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}, {Verbs: []string{"update"}}}
+				var emptyRules []rbacv1.PolicyRule
+				perms := []client.Object{
+					func() *rbacv1.ClusterRole {
+						cr := newClusterRole(cRoleName1, emptyRules...)
+						cr.AggregationRule = &rbacv1.AggregationRule{
+							ClusterRoleSelectors: []metav1.LabelSelector{
+								{
+									MatchLabels: map[string]string{
+										"aggregate-to-cluster-role-1": "true",
+									},
+								},
+							},
+						}
+						return cr
+					}(),
+					func() *rbacv1.ClusterRole {
+						cr := newClusterRole(cRoleName2, rules...)
+						cr.Labels = map[string]string{
+							"aggregate-to-cluster-role-1": "true",
+						}
+						return cr
+					}(),
+					func() *rbacv1.ClusterRole {
+						cr := newClusterRole(cRoleName3, rules...)
+						cr.Labels = map[string]string{
+							"aggregate-to-cluster-role-1": "true",
+						}
+						return cr
+					}(),
+					// ClusterRole not bound to any ServiceAccount, nor matching any ClusterRule AggregationRule,
+					// it shouldn't land in strategy ClusterPermissions.
+					newClusterRole(cRoleName4, []rbacv1.PolicyRule{{Verbs: []string{"delete"}}}...),
+				}
+				c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))}
+				err := applyClusterRoles(c, perms, strategy, nil)
+				Expect(err).NotTo(HaveOccurred())
 				Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{
 					{ServiceAccountName: saName1, Rules: rules},
 				}))
@@ -128,8 +178,10 @@ var _ = Describe("apply functions", func() {
 					newClusterRoleBinding("cluster-role-binding-2", newClusterRoleRef(cRoleName2), newServiceAccountSubject(extraSAName)),
 					newClusterRoleBinding("cluster-role-binding-3", newClusterRoleRef(cRoleName3), newServiceAccountSubject(extraSAName)),
 				}
-				applyRoles(c, perms, strategy, []string{extraSAName})
-				applyClusterRoles(c, cperms, strategy, []string{extraSAName})
+				err := applyRoles(c, perms, strategy, []string{extraSAName})
+				Expect(err).NotTo(HaveOccurred())
+				err = applyClusterRoles(c, cperms, strategy, []string{extraSAName})
+				Expect(err).NotTo(HaveOccurred())
 				Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{
 					{ServiceAccountName: saName1, Rules: rules},
 					{ServiceAccountName: extraSAName, Rules: rules},
@@ -146,14 +198,16 @@ var _ = Describe("apply functions", func() {
 				c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)}
 				c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)}
 				c.RoleBindings = []rbacv1.RoleBinding{newRoleBinding("role-binding", newRoleRef(roleName1), newServiceAccountSubject(saName1))}
-				applyRoles(c, nil, strategy, nil)
+				err := applyRoles(c, nil, strategy, nil)
+				Expect(err).NotTo(HaveOccurred())
 				Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{}))
 			})
 			It("adds no ClusterPermissions to the CSV deployment strategy", func() {
 				c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)}
 				c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)}
 				c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))}
-				applyClusterRoles(c, nil, strategy, nil)
+				err := applyClusterRoles(c, nil, strategy, nil)
+				Expect(err).NotTo(HaveOccurred())
 				Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{}))
 			})
 		})