Skip to content

Commit a85f07e

Browse files
Per G. da Silvaclaude
andcommitted
✨ Make RBACPreAuthorizer clusterCollectionVerbs configurable
Decouple the RBACPreAuthorizer from the hardcoded cluster-scoped collection verbs (list, watch) that were tightly coupled to the contentmanager's requirements. The cluster collection verbs are now configurable via the WithClusterCollectionVerbs functional option. The helm applier configures the preauthorizer with list and watch cluster collection verbs (needed by contentmanager), while the boxcutter applier uses no cluster collection verbs. Closes: #1911 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 55473d8 commit a85f07e

3 files changed

Lines changed: 29 additions & 20 deletions

File tree

cmd/operator-controller/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
724724
// determine if PreAuthorizer should be enabled based on feature gate
725725
var preAuth authorization.PreAuthorizer
726726
if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
727-
preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient())
727+
preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient(), authorization.WithClusterCollectionVerbs("list", "watch"))
728728
}
729729

730730
cm := contentmanager.NewManager(clientRestConfigMapper, c.mgr.GetConfig(), c.mgr.GetRESTMapper())

internal/operator-controller/authorization/rbac.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,26 +59,35 @@ type ScopedPolicyRules struct {
5959

6060
var objectVerbs = []string{"get", "patch", "update", "delete"}
6161

62-
// Here we are splitting collection verbs based on required scope
63-
// NB: this split is tightly coupled to the requirements of the contentmanager, specifically
64-
// its need for cluster-scoped list/watch permissions.
65-
// TODO: We are accepting this coupling for now, but plan to decouple
66-
// TODO: link for above https://github.com/operator-framework/operator-controller/issues/1911
6762
var namespacedCollectionVerbs = []string{"create"}
68-
var clusterCollectionVerbs = []string{"list", "watch"}
63+
64+
type RBACPreAuthorizerOption func(*rbacPreAuthorizer)
65+
66+
// WithClusterCollectionVerbs configures cluster-scoped collection verbs (e.g. list, watch)
67+
// that are checked in addition to object and namespaced collection verbs.
68+
func WithClusterCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
69+
return func(a *rbacPreAuthorizer) {
70+
a.clusterCollectionVerbs = verbs
71+
}
72+
}
6973

7074
type rbacPreAuthorizer struct {
71-
authorizer authorizer.Authorizer
72-
ruleResolver validation.AuthorizationRuleResolver
73-
restMapper meta.RESTMapper
75+
authorizer authorizer.Authorizer
76+
ruleResolver validation.AuthorizationRuleResolver
77+
restMapper meta.RESTMapper
78+
clusterCollectionVerbs []string
7479
}
7580

76-
func NewRBACPreAuthorizer(cl client.Client) PreAuthorizer {
77-
return &rbacPreAuthorizer{
81+
func NewRBACPreAuthorizer(cl client.Client, opts ...RBACPreAuthorizerOption) PreAuthorizer {
82+
a := &rbacPreAuthorizer{
7883
authorizer: newRBACAuthorizer(cl),
7984
ruleResolver: newRBACRulesResolver(cl),
8085
restMapper: cl.RESTMapper(),
8186
}
87+
for _, opt := range opts {
88+
opt(a)
89+
}
90+
return a
8291
}
8392

8493
func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, manifestReader io.Reader, additionalRequiredPerms ...UserAuthorizerAttributesFactory) ([]ScopedPolicyRules, error) {
@@ -88,7 +97,7 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, ma
8897
}
8998

9099
// derive manifest related attributes records
91-
attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user)
100+
attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user, a.clusterCollectionVerbs)
92101

93102
// append additional required perms
94103
for _, fn := range additionalRequiredPerms {
@@ -324,7 +333,7 @@ func (dm *decodedManifest) rbacObjects() []client.Object {
324333
return objects
325334
}
326335

327-
func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info) []authorizer.AttributesRecord {
336+
func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info, clusterCollectionVerbs []string) []authorizer.AttributesRecord {
328337
// Calculate initial capacity as an upper-bound estimate:
329338
// - For each key: len(objectVerbs) records (4)
330339
// - For unique namespaces: len(namespacedCollectionVerbs) records (1 per unique namespace across all keys in a GVR)
@@ -369,7 +378,7 @@ func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManag
369378
})
370379
}
371380
}
372-
// generate records for cluster-scoped collection verbs (list, watch) required by contentmanager
381+
// generate records for cluster-scoped collection verbs (e.g. list, watch)
373382
for _, v := range clusterCollectionVerbs {
374383
attributeRecords = append(attributeRecords, authorizer.AttributesRecord{
375384
User: manifestManager,

internal/operator-controller/authorization/rbac_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ func setupFakeClient(role client.Object) client.Client {
396396
func TestPreAuthorize_Success(t *testing.T) {
397397
t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) {
398398
fakeClient := setupFakeClient(privilegedClusterRole)
399-
preAuth := NewRBACPreAuthorizer(fakeClient)
399+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
400400
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
401401
require.NoError(t, err)
402402
require.Equal(t, []ScopedPolicyRules{}, missingRules)
@@ -406,7 +406,7 @@ func TestPreAuthorize_Success(t *testing.T) {
406406
func TestPreAuthorize_MissingRBAC(t *testing.T) {
407407
t.Run("preauthorize fails and finds missing rbac rules", func(t *testing.T) {
408408
fakeClient := setupFakeClient(limitedClusterRole)
409-
preAuth := NewRBACPreAuthorizer(fakeClient)
409+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
410410
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
411411
require.NoError(t, err)
412412
require.Equal(t, expectedSingleNamespaceMissingRules, missingRules)
@@ -416,7 +416,7 @@ func TestPreAuthorize_MissingRBAC(t *testing.T) {
416416
func TestPreAuthorizeMultiNamespace_MissingRBAC(t *testing.T) {
417417
t.Run("preauthorize fails and finds missing rbac rules in multiple namespaces", func(t *testing.T) {
418418
fakeClient := setupFakeClient(limitedClusterRole)
419-
preAuth := NewRBACPreAuthorizer(fakeClient)
419+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
420420
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifestMultiNamespace))
421421
require.NoError(t, err)
422422
require.Equal(t, expectedMultiNamespaceMissingRules, missingRules)
@@ -426,7 +426,7 @@ func TestPreAuthorizeMultiNamespace_MissingRBAC(t *testing.T) {
426426
func TestPreAuthorize_CheckEscalation(t *testing.T) {
427427
t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) {
428428
fakeClient := setupFakeClient(escalatingClusterRole)
429-
preAuth := NewRBACPreAuthorizer(fakeClient)
429+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
430430
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
431431
require.NoError(t, err)
432432
require.Equal(t, []ScopedPolicyRules{}, missingRules)
@@ -436,7 +436,7 @@ func TestPreAuthorize_CheckEscalation(t *testing.T) {
436436
func TestPreAuthorize_AdditionalRequiredPerms_MissingRBAC(t *testing.T) {
437437
t.Run("preauthorize fails and finds missing rbac rules coming from the additional required permissions", func(t *testing.T) {
438438
fakeClient := setupFakeClient(escalatingClusterRole)
439-
preAuth := NewRBACPreAuthorizer(fakeClient)
439+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"))
440440
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest), func(user user.Info) []authorizer.AttributesRecord {
441441
return []authorizer.AttributesRecord{
442442
{

0 commit comments

Comments
 (0)