✨ Make RBACPreAuthorizer collection verbs configurable (operator-framework#2539)

Make both clusterCollectionVerbs and namespacedCollectionVerbs on
RBACPreAuthorizer configurable via functional options, decoupling them
from the hardcoded verbs that were tightly coupled to the
contentmanager's requirements.

Both the helm and boxcutter appliers explicitly configure
namespacedCollectionVerbs with "create". The helm applier additionally
configures clusterCollectionVerbs with "list" and "watch" (needed by
contentmanager), while the boxcutter applier uses no cluster collection
verbs.

Also updates e2e tests to select the appropriate RBAC template based
on the BoxcutterRuntime feature gate, using a narrower template
without list/watch when BoxcutterRuntime is enabled.

Closes: operator-framework#1911

Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
Co-authored-by: Per G. da Silva <pegoncal@redhat.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 3 people

cmd/operator-controller/main.go

@@ -724,7 +724,12 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
 	// determine if PreAuthorizer should be enabled based on feature gate
 	var preAuth authorization.PreAuthorizer
 	if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
-		preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient())
+		preAuth = authorization.NewRBACPreAuthorizer(
+			c.mgr.GetClient(),
+			// Additional verbs / bundle manifest that are expected by the content manager to watch those resources
+			authorization.WithClusterCollectionVerbs("list", "watch"),
+			authorization.WithNamespacedCollectionVerbs("create"),
+		)
 	}
 
 	cm := contentmanager.NewManager(clientRestConfigMapper, c.mgr.GetConfig(), c.mgr.GetRESTMapper())

internal/operator-controller/authorization/rbac.go

@@ -29,7 +29,7 @@ import (
 	rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1"
 	rbacregistry "k8s.io/kubernetes/pkg/registry/rbac"
 	"k8s.io/kubernetes/pkg/registry/rbac/validation"
-	rbac "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
+	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -59,26 +59,42 @@ type ScopedPolicyRules struct {
 
 var objectVerbs = []string{"get", "patch", "update", "delete"}
 
-// Here we are splitting collection verbs based on required scope
-// NB: this split is tightly coupled to the requirements of the contentmanager, specifically
-// its need for cluster-scoped list/watch permissions.
-// TODO: We are accepting this coupling for now, but plan to decouple
-// TODO: link for above https://github.com/operator-framework/operator-controller/issues/1911
-var namespacedCollectionVerbs = []string{"create"}
-var clusterCollectionVerbs = []string{"list", "watch"}
+type RBACPreAuthorizerOption func(*rbacPreAuthorizer)
+
+// WithClusterCollectionVerbs configures cluster-scoped collection verbs (e.g. list, watch)
+// that are checked in addition to object and namespaced collection verbs.
+func WithClusterCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
+	return func(a *rbacPreAuthorizer) {
+		a.clusterCollectionVerbs = slices.Clone(verbs)
+	}
+}
+
+// WithNamespacedCollectionVerbs configures namespaced collection verbs (e.g. create)
+// that are checked for each unique namespace across all objects in a GVR.
+func WithNamespacedCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
+	return func(a *rbacPreAuthorizer) {
+		a.namespacedCollectionVerbs = slices.Clone(verbs)
+	}
+}
 
 type rbacPreAuthorizer struct {
-	authorizer   authorizer.Authorizer
-	ruleResolver validation.AuthorizationRuleResolver
-	restMapper   meta.RESTMapper
+	authorizer                authorizer.Authorizer
+	ruleResolver              validation.AuthorizationRuleResolver
+	restMapper                meta.RESTMapper
+	clusterCollectionVerbs    []string
+	namespacedCollectionVerbs []string
 }
 
-func NewRBACPreAuthorizer(cl client.Client) PreAuthorizer {
-	return &rbacPreAuthorizer{
+func NewRBACPreAuthorizer(cl client.Client, opts ...RBACPreAuthorizerOption) PreAuthorizer {
+	a := &rbacPreAuthorizer{
 		authorizer:   newRBACAuthorizer(cl),
 		ruleResolver: newRBACRulesResolver(cl),
 		restMapper:   cl.RESTMapper(),
 	}
+	for _, opt := range opts {
+		opt(a)
+	}
+	return a
 }
 
 func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, manifestReader io.Reader, additionalRequiredPerms ...UserAuthorizerAttributesFactory) ([]ScopedPolicyRules, error) {
@@ -88,7 +104,7 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, ma
 	}
 
 	// derive manifest related attributes records
-	attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user)
+	attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user, a.clusterCollectionVerbs, a.namespacedCollectionVerbs)
 
 	// append additional required perms
 	for _, fn := range additionalRequiredPerms {
@@ -324,7 +340,7 @@ func (dm *decodedManifest) rbacObjects() []client.Object {
 	return objects
 }
 
-func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info) []authorizer.AttributesRecord {
+func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info, clusterCollectionVerbs, namespacedCollectionVerbs []string) []authorizer.AttributesRecord {
 	// Calculate initial capacity as an upper-bound estimate:
 	// - For each key: len(objectVerbs) records (4)
 	// - For unique namespaces: len(namespacedCollectionVerbs) records (1 per unique namespace across all keys in a GVR)
@@ -369,7 +385,7 @@ func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManag
 				})
 			}
 		}
-		// generate records for cluster-scoped collection verbs (list, watch) required by contentmanager
+		// generate records for cluster-scoped collection verbs (e.g. list, watch)
 		for _, v := range clusterCollectionVerbs {
 			attributeRecords = append(attributeRecords, authorizer.AttributesRecord{
 				User:            manifestManager,

internal/operator-controller/authorization/rbac_test.go

@@ -396,7 +396,7 @@ func setupFakeClient(role client.Object) client.Client {
 func TestPreAuthorize_Success(t *testing.T) {
 	t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) {
 		fakeClient := setupFakeClient(privilegedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
 		require.NoError(t, err)
 		require.Equal(t, []ScopedPolicyRules{}, missingRules)
@@ -406,7 +406,7 @@ func TestPreAuthorize_Success(t *testing.T) {
 func TestPreAuthorize_MissingRBAC(t *testing.T) {
 	t.Run("preauthorize fails and finds missing rbac rules", func(t *testing.T) {
 		fakeClient := setupFakeClient(limitedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
 		require.NoError(t, err)
 		require.Equal(t, expectedSingleNamespaceMissingRules, missingRules)
@@ -416,7 +416,7 @@ func TestPreAuthorize_MissingRBAC(t *testing.T) {
 func TestPreAuthorizeMultiNamespace_MissingRBAC(t *testing.T) {
 	t.Run("preauthorize fails and finds missing rbac rules in multiple namespaces", func(t *testing.T) {
 		fakeClient := setupFakeClient(limitedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifestMultiNamespace))
 		require.NoError(t, err)
 		require.Equal(t, expectedMultiNamespaceMissingRules, missingRules)
@@ -426,7 +426,7 @@ func TestPreAuthorizeMultiNamespace_MissingRBAC(t *testing.T) {
 func TestPreAuthorize_CheckEscalation(t *testing.T) {
 	t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) {
 		fakeClient := setupFakeClient(escalatingClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
 		require.NoError(t, err)
 		require.Equal(t, []ScopedPolicyRules{}, missingRules)
@@ -436,7 +436,7 @@ func TestPreAuthorize_CheckEscalation(t *testing.T) {
 func TestPreAuthorize_AdditionalRequiredPerms_MissingRBAC(t *testing.T) {
 	t.Run("preauthorize fails and finds missing rbac rules coming from the additional required permissions", func(t *testing.T) {
 		fakeClient := setupFakeClient(escalatingClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest), func(user user.Info) []authorizer.AttributesRecord {
 			return []authorizer.AttributesRecord{
 				{
@@ -465,6 +465,237 @@ func TestPreAuthorize_AdditionalRequiredPerms_MissingRBAC(t *testing.T) {
 	})
 }
 
+func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
+	// expectedNamespacedMissingRules are the missing rules expected in the "test-namespace"
+	// namespace regardless of cluster collection verb configuration. These come from object
+	// verbs (get, patch, update, delete), namespaced collection verbs (create), and the
+	// escalation check for the role/rolebinding in the manifest.
+	expectedNamespacedMissingRules := ScopedPolicyRules{
+		Namespace: "test-namespace",
+		MissingRules: []rbacv1.PolicyRule{
+			{
+				Verbs:     []string{"create"},
+				APIGroups: []string{"*"},
+				Resources: []string{"certificates"}},
+			{
+				Verbs:     []string{"create"},
+				APIGroups: []string{""},
+				Resources: []string{"services"}},
+			{
+				Verbs:     []string{"create"},
+				APIGroups: []string{"rbac.authorization.k8s.io"},
+				Resources: []string{"rolebindings"}},
+			{
+				Verbs:     []string{"create"},
+				APIGroups: []string{"rbac.authorization.k8s.io"},
+				Resources: []string{"roles"}},
+			{
+				Verbs:         []string{"delete", "get", "patch", "update"},
+				APIGroups:     []string{""},
+				Resources:     []string{"services"},
+				ResourceNames: []string{"test-service"}},
+			{
+				Verbs:         []string{"delete", "get", "patch", "update"},
+				APIGroups:     []string{"rbac.authorization.k8s.io"},
+				Resources:     []string{"rolebindings"},
+				ResourceNames: []string{"test-extension-binding"}},
+			{
+				Verbs:         []string{"delete", "get", "patch", "update"},
+				APIGroups:     []string{"rbac.authorization.k8s.io"},
+				Resources:     []string{"roles"},
+				ResourceNames: []string{"test-extension-role"}},
+			{
+				Verbs:     []string{"watch"},
+				APIGroups: []string{"*"},
+				Resources: []string{"serviceaccounts"},
+			},
+		},
+	}
+
+	t.Run("no cluster collection verbs option omits cluster-scoped collection rules", func(t *testing.T) {
+		fakeClient := setupFakeClient(limitedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		// With no cluster collection verbs, there should be no cluster-scoped (namespace="") missing rules
+		require.Equal(t, []ScopedPolicyRules{expectedNamespacedMissingRules}, missingRules)
+	})
+
+	t.Run("cluster verbs option only checks those verbs at cluster scope", func(t *testing.T) {
+		fakeClient := setupFakeClient(limitedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("get", "patch", "update"), WithNamespacedCollectionVerbs("create"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		require.Equal(t, []ScopedPolicyRules{
+			{
+				Namespace: "",
+				MissingRules: []rbacv1.PolicyRule{
+					{
+						Verbs:           []string{"get", "patch", "update"},
+						APIGroups:       []string{""},
+						Resources:       []string{"services"},
+						ResourceNames:   []string(nil),
+						NonResourceURLs: []string(nil)},
+					{
+						Verbs:           []string{"get", "patch", "update"},
+						APIGroups:       []string{"rbac.authorization.k8s.io"},
+						Resources:       []string{"rolebindings"},
+						ResourceNames:   []string(nil),
+						NonResourceURLs: []string(nil)},
+					{
+						Verbs:           []string{"get", "patch", "update"},
+						APIGroups:       []string{"rbac.authorization.k8s.io"},
+						Resources:       []string{"roles"},
+						ResourceNames:   []string(nil),
+						NonResourceURLs: []string(nil),
+					},
+				},
+			},
+			expectedNamespacedMissingRules,
+		}, missingRules)
+	})
+
+	t.Run("privileged user with no cluster collection verbs succeeds", func(t *testing.T) {
+		fakeClient := setupFakeClient(privilegedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		require.Equal(t, []ScopedPolicyRules{}, missingRules)
+	})
+}
+
+func TestPreAuthorize_WithNamespacedCollectionVerbs(t *testing.T) {
+	// expectedClusterMissingRules are the missing rules expected at cluster scope
+	// when cluster collection verbs are configured as "list", "watch".
+	expectedClusterMissingRules := ScopedPolicyRules{
+		Namespace: "",
+		MissingRules: []rbacv1.PolicyRule{
+			{
+				Verbs:           []string{"list", "watch"},
+				APIGroups:       []string{""},
+				Resources:       []string{"services"},
+				ResourceNames:   []string(nil),
+				NonResourceURLs: []string(nil)},
+			{
+				Verbs:           []string{"list", "watch"},
+				APIGroups:       []string{"rbac.authorization.k8s.io"},
+				Resources:       []string{"rolebindings"},
+				ResourceNames:   []string(nil),
+				NonResourceURLs: []string(nil)},
+			{
+				Verbs:           []string{"list", "watch"},
+				APIGroups:       []string{"rbac.authorization.k8s.io"},
+				Resources:       []string{"roles"},
+				ResourceNames:   []string(nil),
+				NonResourceURLs: []string(nil),
+			},
+		},
+	}
+
+	t.Run("no namespaced collection verbs option omits namespaced collection rules", func(t *testing.T) {
+		fakeClient := setupFakeClient(limitedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		// Without namespaced collection verbs, no "create" rules from collection verbs should appear,
+		// but object verbs (get, patch, update, delete) and escalation checks still apply
+		require.Equal(t, []ScopedPolicyRules{
+			expectedClusterMissingRules,
+			{
+				Namespace: "test-namespace",
+				MissingRules: []rbacv1.PolicyRule{
+					{
+						Verbs:     []string{"create"},
+						APIGroups: []string{"*"},
+						Resources: []string{"certificates"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{""},
+						Resources:     []string{"services"},
+						ResourceNames: []string{"test-service"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{"rbac.authorization.k8s.io"},
+						Resources:     []string{"rolebindings"},
+						ResourceNames: []string{"test-extension-binding"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{"rbac.authorization.k8s.io"},
+						Resources:     []string{"roles"},
+						ResourceNames: []string{"test-extension-role"}},
+					{
+						Verbs:     []string{"watch"},
+						APIGroups: []string{"*"},
+						Resources: []string{"serviceaccounts"},
+					},
+				},
+			},
+		}, missingRules)
+	})
+
+	t.Run("namespaced collection verbs option checks those verbs per namespace", func(t *testing.T) {
+		fakeClient := setupFakeClient(limitedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create", "deletecollection"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		// Should have cluster-scoped missing rules plus namespaced rules with both create and deletecollection.
+		// Note: "certificates" with apiGroup "*" comes from the escalation check on the Role, not
+		// from namespaced collection verbs, so it only has "create".
+		require.Equal(t, []ScopedPolicyRules{
+			expectedClusterMissingRules,
+			{
+				Namespace: "test-namespace",
+				MissingRules: []rbacv1.PolicyRule{
+					{
+						Verbs:     []string{"create", "deletecollection"},
+						APIGroups: []string{""},
+						Resources: []string{"services"}},
+					{
+						Verbs:     []string{"create", "deletecollection"},
+						APIGroups: []string{"rbac.authorization.k8s.io"},
+						Resources: []string{"rolebindings"}},
+					{
+						Verbs:     []string{"create", "deletecollection"},
+						APIGroups: []string{"rbac.authorization.k8s.io"},
+						Resources: []string{"roles"}},
+					{
+						Verbs:     []string{"create"},
+						APIGroups: []string{"*"},
+						Resources: []string{"certificates"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{""},
+						Resources:     []string{"services"},
+						ResourceNames: []string{"test-service"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{"rbac.authorization.k8s.io"},
+						Resources:     []string{"rolebindings"},
+						ResourceNames: []string{"test-extension-binding"}},
+					{
+						Verbs:         []string{"delete", "get", "patch", "update"},
+						APIGroups:     []string{"rbac.authorization.k8s.io"},
+						Resources:     []string{"roles"},
+						ResourceNames: []string{"test-extension-role"}},
+					{
+						Verbs:     []string{"watch"},
+						APIGroups: []string{"*"},
+						Resources: []string{"serviceaccounts"},
+					},
+				},
+			},
+		}, missingRules)
+	})
+
+	t.Run("privileged user with custom namespaced collection verbs succeeds", func(t *testing.T) {
+		fakeClient := setupFakeClient(privilegedClusterRole)
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create", "deletecollection"))
+		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
+		require.NoError(t, err)
+		require.Equal(t, []ScopedPolicyRules{}, missingRules)
+	})
+}
+
 // TestParseEscalationErrorForMissingRules Are tests with respect to https://github.com/kubernetes/api/blob/e8d4d542f6a9a16a694bfc8e3b8cd1557eecfc9d/rbac/v1/types.go#L49-L74
 // Goal is: prove the regex works as planned AND that if the error messages ever change we'll learn about it with these tests
 func TestParseEscalationErrorForMissingRules_ParsingLogic(t *testing.T) {