add create verb to boxcutter preflight (#2587)

Author: kuiwang02

cmd/operator-controller/main.go

@@ -744,7 +744,6 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
 			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"),
 		)
 	}
 

internal/operator-controller/authorization/rbac.go

@@ -57,8 +57,16 @@ type ScopedPolicyRules struct {
 	MissingRules []rbacv1.PolicyRule
 }
 
+// objectVerbs are the verbs checked for each specific resource (by name) in the manifest.
+// These verbs operate on existing resource instances and can be restricted by resourceNames in RBAC rules.
 var objectVerbs = []string{"get", "patch", "update", "delete"}
 
+// namespacedCollectionVerbs are the verbs checked once per unique namespace across all resources in a GVR.
+// These verbs operate at the resource type level (collection) and cannot be restricted by resourceNames.
+// The "create" verb is included here because it operates on the collection level - you can't specify
+// resourceNames for create operations since the resource doesn't exist yet.
+var namespacedCollectionVerbs = []string{"create"}
+
 type RBACPreAuthorizerOption func(*rbacPreAuthorizer)
 
 // WithClusterCollectionVerbs configures cluster-scoped collection verbs (e.g. list, watch)
@@ -69,20 +77,11 @@ func WithClusterCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
 	}
 }
 
-// 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
-	clusterCollectionVerbs    []string
-	namespacedCollectionVerbs []string
+	authorizer             authorizer.Authorizer
+	ruleResolver           validation.AuthorizationRuleResolver
+	restMapper             meta.RESTMapper
+	clusterCollectionVerbs []string
 }
 
 func NewRBACPreAuthorizer(cl client.Client, opts ...RBACPreAuthorizerOption) PreAuthorizer {
@@ -104,7 +103,7 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, ma
 	}
 
 	// derive manifest related attributes records
-	attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user, a.clusterCollectionVerbs, a.namespacedCollectionVerbs)
+	attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user, a.clusterCollectionVerbs, namespacedCollectionVerbs)
 
 	// append additional required perms
 	for _, fn := range additionalRequiredPerms {

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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
 		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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
 		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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
 		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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
 		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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest), func(user user.Info) []authorizer.AttributesRecord {
 			return []authorizer.AttributesRecord{
 				{
@@ -514,7 +514,7 @@ func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
 
 	t.Run("no cluster collection verbs option omits cluster-scoped collection rules", func(t *testing.T) {
 		fakeClient := setupFakeClient(limitedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient)
 		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
@@ -523,7 +523,7 @@ func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
 
 	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"))
+		preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("get", "patch", "update"))
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
 		require.NoError(t, err)
 		require.Equal(t, []ScopedPolicyRules{
@@ -557,139 +557,31 @@ func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
 
 	t.Run("privileged user with no cluster collection verbs succeeds", func(t *testing.T) {
 		fakeClient := setupFakeClient(privilegedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
+		preAuth := NewRBACPreAuthorizer(fakeClient)
 		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),
-			},
-		},
-	}
+func TestPreAuthorize_NamespacedCollectionVerbs(t *testing.T) {
+	// With namespacedCollectionVerbs now being a fixed variable (containing "create"),
+	// this test verifies that "create" permissions are always checked at the namespace level.
 
-	t.Run("no namespaced collection verbs option omits namespaced collection rules", func(t *testing.T) {
+	t.Run("create verb is always checked as namespaced collection verb", 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)
+		// The "create" verb should always appear in missing rules because it's part of the
+		// namespacedCollectionVerbs. This test verifies expectedSingleNamespaceMissingRules
+		// which includes "create" verbs for namespace-scoped resources.
+		require.Equal(t, expectedSingleNamespaceMissingRules, missingRules)
 	})
 
-	t.Run("privileged user with custom namespaced collection verbs succeeds", func(t *testing.T) {
+	t.Run("privileged user with namespaced collection verbs succeeds", func(t *testing.T) {
 		fakeClient := setupFakeClient(privilegedClusterRole)
-		preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create", "deletecollection"))
+		preAuth := NewRBACPreAuthorizer(fakeClient)
 		missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
 		require.NoError(t, err)
 		require.Equal(t, []ScopedPolicyRules{}, missingRules)

test/e2e/features/install.feature

@@ -564,3 +564,38 @@ Feature: Install ClusterExtension
             nodeSelector:
               kubernetes.io/os: linux
       """
+
+  @BoxcutterRuntime
+  @PreflightPermissions
+  Scenario: Boxcutter preflight check detects missing CREATE permissions
+    Given ServiceAccount "olm-sa" without create permissions is available in ${TEST_NAMESPACE}
+    And ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
+      """
+      pre-authorization failed: service account requires the following permissions to manage cluster extension
+      """
+    And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
+      """
+      Verbs:[create]
+      """
+    When ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+    Then ClusterExtension is available
+    And ClusterExtension reports Progressing as True with Reason Succeeded
+    And ClusterExtension reports Installed as True

test/e2e/steps/steps.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/sets"
 	k8sresource "k8s.io/cli-runtime/pkg/resource"
+	"k8s.io/component-base/featuregate"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
@@ -125,6 +126,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" with permissions to install extensions is available in "([^"]*)" namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in test namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace)
+	sc.Step(`^(?i)ServiceAccount "([^"]*)" without create permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" is available in \${TEST_NAMESPACE}$`, ServiceAccountIsAvailableInNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" in test namespace is cluster admin$`, ServiceAccountWithClusterAdminPermissionsIsAvailableInNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]+)" in test namespace has permissions to fetch "([^"]+)" metrics$`, ServiceAccountWithFetchMetricsPermissions)
@@ -436,6 +438,11 @@ type msgMatchFn func(string) bool
 
 func alwaysMatch(_ string) bool { return true }
 
+func isFeatureGateEnabled(feature featuregate.Feature) bool {
+	enabled, found := featureGates[feature]
+	return enabled && found
+}
+
 func messageComparison(ctx context.Context, msg *godog.DocString) msgMatchFn {
 	msgCmp := alwaysMatch
 	if msg != nil {
@@ -1119,6 +1126,23 @@ func ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace(ctx context.C
 	return applyPermissionsToServiceAccount(ctx, serviceAccount, rbacTemplate)
 }
 
+// ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace creates a ServiceAccount with permissions that
+// intentionally exclude the "create" verb to test preflight permission validation for Boxcutter applier.
+// This is used to verify that the preflight check properly detects missing CREATE permissions.
+// Note: This function requires both @BoxcutterRuntime and @PreflightPermissions tags.
+func ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace(ctx context.Context, serviceAccount string) error {
+	// This test is only valid with Boxcutter runtime enabled
+	if !isFeatureGateEnabled(features.BoxcutterRuntime) {
+		return fmt.Errorf("this step requires BoxcutterRuntime feature gate to be enabled")
+	}
+	// It also requires preflight permissions checks to be enabled
+	if !isFeatureGateEnabled(features.PreflightPermissions) {
+		return fmt.Errorf("this step requires PreflightPermissions feature gate to be enabled")
+	}
+	rbacTemplate := fmt.Sprintf("%s-boxcutter-no-create-rbac-template.yaml", serviceAccount)
+	return applyPermissionsToServiceAccount(ctx, serviceAccount, rbacTemplate)
+}
+
 // ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace creates a ServiceAccount and enables creation of any cluster extension on behalf of this account.
 func ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace(ctx context.Context, serviceAccount string, ns string) error {
 	sc := scenarioCtx(ctx)