Add support for ClusterRole AggregationRule during bundle generation

changelog/fragments/01-support-aggregated-rules-of-clusterroles.yaml

@@ -0,0 +1,41 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      The bundle generator now includes policy rules from ClusterRoles originating through AggregationRule.
+      Previously, only policy rules of ClusterRoles explicitly bound to ServiceAccounts via ClusterRoleBindings were included.
+      With this update, the generator also aggregates and applies rules from matching ClusterRoles, 
+      ensuring proper RBAC coverage.
+
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: addition
+
+    # Is this a breaking change?
+    breaking: false
+
+    # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+    # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+    #
+    # The generator auto-detects the PR number from the commit
+    # message in which this file was originally added.
+    #
+    # What is the pull request number (without the "#")?
+    # pull_request_override: 0
+
+
+    # Migration can be defined to automatically add a section to
+    # the migration guide. This is required for breaking changes.
+#    migration:
+#      header: Header text for the migration section
+#      body: |
+#        Body of the migration section. This should be formatted as markdown and can
+#        span multiple lines.
+#
+#        Using the YAML string '|' operator means that newlines in this string will
+#        be honored and interpretted as newlines in the rendered markdown.

hack/generate/samples/internal/go/memcached-with-customization/manifests/aggregation-subrole.clusterole.yaml

@@ -0,0 +1,13 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: aggregation-memcached-subrole
+  labels:
+    rbac.example.memcached.com/aggregate-to-aggregation-memcached: "true"
+rules:
+  - apiGroups:
+      - ""
+    resources:
+      - pods
+    verbs:
+      - update

hack/generate/samples/internal/go/memcached-with-customization/manifests/aggregation.clusterolebinding.yaml

@@ -0,0 +1,12 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: aggregation-memcached
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: aggregation-memcached
+subjects:
+  - kind: ServiceAccount
+    name: controller-manager
+    namespace: system

hack/generate/samples/internal/go/memcached-with-customization/manifests/aggregation.clusterrole.yaml

@@ -0,0 +1,8 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: aggregation-memcached
+aggregationRule:
+  clusterRoleSelectors:
+    - matchLabels:
+        rbac.example.memcached.com/aggregate-to-aggregation-memcached: "true"

hack/generate/samples/internal/go/memcached-with-customization/manifests/aggregation.kustomization.patch.yaml

@@ -0,0 +1,4 @@
+
+- aggregation.clusterrolebinding.yaml
+- aggregation-subrole.clusterrole.yaml
+- aggregation.clusterrole.yaml

hack/generate/samples/internal/go/memcached-with-customization/manifests/registry.go

@@ -0,0 +1,19 @@
+package manifests
+
+import (
+	_ "embed"
+)
+
+var (
+	//go:embed "aggregation.clusterrole.yaml"
+	AggregationClusterRoleString string
+
+	//go:embed "aggregation-subrole.clusterole.yaml"
+	AggregationSubroleClusterRoleString string
+
+	//go:embed "aggregation.clusterolebinding.yaml"
+	AggregationClusterRoleBindingString string
+
+	//go:embed "aggregation.kustomization.patch.yaml"
+	AggregationKustomizationPatchString string
+)

hack/generate/samples/internal/go/memcached-with-customization/memcached_with_customization.go

@@ -21,6 +21,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/operator-framework/operator-sdk/hack/generate/samples/internal/go/memcached-with-customization/manifests"
 	log "github.com/sirupsen/logrus"
 	kbutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util"
 
@@ -139,6 +140,9 @@ func (mh *Memcached) Run() {
 
 	mh.implementingWebhooks()
 
+	log.Infof("implementing the aggregated cluster rule")
+	mh.implementingAggregatedClusterRules()
+
 	mh.uncommentDefaultKustomizationV4()
 	mh.uncommentManifestsKustomizationv4()
 
@@ -780,6 +784,40 @@ func (mh *Memcached) customizingMakefile() {
 	pkg.CheckError("adding metrics documentation", err)
 }
 
+func (mh *Memcached) implementingAggregatedClusterRules() {
+	rbacDirPath := filepath.Join(mh.ctx.Dir, "config", "rbac")
+
+	var filePerm os.FileMode = 0600
+
+	err := os.WriteFile(
+		filepath.Join(rbacDirPath, "aggregation.clusterrole.yaml"),
+		[]byte(manifests.AggregationClusterRoleString),
+		filePerm,
+	)
+	pkg.CheckError("creating aggregation clusterrole", err)
+
+	err = os.WriteFile(
+		filepath.Join(rbacDirPath, "aggregation-subrole.clusterrole.yaml"),
+		[]byte(manifests.AggregationSubroleClusterRoleString),
+		filePerm,
+	)
+	pkg.CheckError("creating aggregation clusterrole subrole", err)
+
+	err = os.WriteFile(
+		filepath.Join(rbacDirPath, "aggregation.clusterrolebinding.yaml"),
+		[]byte(manifests.AggregationClusterRoleBindingString),
+		filePerm,
+	)
+	pkg.CheckError("creating aggregation clusterrolebinding", err)
+
+	kustomizationPath := filepath.Join(rbacDirPath, "kustomization.yaml")
+	err = kbutil.InsertCode(kustomizationPath,
+		`- memcached_viewer_role.yaml`,
+		manifests.AggregationKustomizationPatchString,
+	)
+	pkg.CheckError("adding metrics documentation", err)
+}
+
 const metricsFragment = `
 
 package monitoring

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,48 @@ 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, 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.