Skip to content

Commit 8bba1b9

Browse files
kuiwang02kuiwang02
committed
add create verb to boxcutter preflight
1 parent 4f50aa7 commit 8bba1b9

7 files changed

Lines changed: 204 additions & 17 deletions

File tree

‎cmd/operator-controller/main.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,6 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
743743
c.mgr.GetClient(),
744744
// Additional verbs / bundle manifest that are expected by the content manager to watch those resources
745745
authorization.WithClusterCollectionVerbs("list", "watch"),
746-
authorization.WithNamespacedCollectionVerbs("create"),
747746
)
748747
}
749748

‎internal/operator-controller/authorization/rbac.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type ScopedPolicyRules struct {
5757
MissingRules []rbacv1.PolicyRule
5858
}
5959

60-
var objectVerbs = []string{"get", "patch", "update", "delete"}
60+
var objectVerbs = []string{"get", "patch", "update", "delete", "create"}
6161

6262
type RBACPreAuthorizerOption func(*rbacPreAuthorizer)
6363

@@ -69,7 +69,7 @@ func WithClusterCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
6969
}
7070
}
7171

72-
// WithNamespacedCollectionVerbs configures namespaced collection verbs (e.g. create)
72+
// WithNamespacedCollectionVerbs configures additional namespaced collection verbs
7373
// that are checked for each unique namespace across all objects in a GVR.
7474
func WithNamespacedCollectionVerbs(verbs ...string) RBACPreAuthorizerOption {
7575
return func(a *rbacPreAuthorizer) {
@@ -342,7 +342,7 @@ func (dm *decodedManifest) rbacObjects() []client.Object {
342342

343343
func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info, clusterCollectionVerbs, namespacedCollectionVerbs []string) []authorizer.AttributesRecord {
344344
// Calculate initial capacity as an upper-bound estimate:
345-
// - For each key: len(objectVerbs) records (4)
345+
// - For each key: len(objectVerbs) records (5)
346346
// - For unique namespaces: len(namespacedCollectionVerbs) records (1 per unique namespace across all keys in a GVR)
347347
// We use totalKeys as upper bound (worst case: each key in different namespace)
348348
// - For each GVR: len(clusterCollectionVerbs) records (2)
@@ -357,7 +357,7 @@ func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManag
357357
namespaces := sets.New[string]()
358358
for _, k := range keys {
359359
namespaces.Insert(k.Namespace)
360-
// generate records for object-specific verbs (get, update, patch, delete) in their respective namespaces
360+
// generate records for object-specific verbs (get, update, patch, delete, create) in their respective namespaces
361361
for _, v := range objectVerbs {
362362
attributeRecords = append(attributeRecords, authorizer.AttributesRecord{
363363
User: manifestManager,
@@ -371,7 +371,7 @@ func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManag
371371
})
372372
}
373373
}
374-
// generate records for namespaced collection verbs (create) for each relevant namespace
374+
// generate records for namespaced collection verbs for each relevant namespace
375375
for _, ns := range sets.List(namespaces) {
376376
for _, v := range namespacedCollectionVerbs {
377377
attributeRecords = append(attributeRecords, authorizer.AttributesRecord{

‎internal/operator-controller/authorization/rbac_test.go‎

Lines changed: 11 additions & 11 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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
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, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create"))
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
{
@@ -468,8 +468,8 @@ func TestPreAuthorize_AdditionalRequiredPerms_MissingRBAC(t *testing.T) {
468468
func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
469469
// expectedNamespacedMissingRules are the missing rules expected in the "test-namespace"
470470
// namespace regardless of cluster collection verb configuration. These come from object
471-
// verbs (get, patch, update, delete), namespaced collection verbs (create), and the
472-
// escalation check for the role/rolebinding in the manifest.
471+
// verbs (get, patch, update, delete, create) and the escalation check for the
472+
// role/rolebinding in the manifest.
473473
expectedNamespacedMissingRules := ScopedPolicyRules{
474474
Namespace: "test-namespace",
475475
MissingRules: []rbacv1.PolicyRule{
@@ -514,7 +514,7 @@ func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
514514

515515
t.Run("no cluster collection verbs option omits cluster-scoped collection rules", func(t *testing.T) {
516516
fakeClient := setupFakeClient(limitedClusterRole)
517-
preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
517+
preAuth := NewRBACPreAuthorizer(fakeClient)
518518
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
519519
require.NoError(t, err)
520520
// With no cluster collection verbs, there should be no cluster-scoped (namespace="") missing rules
@@ -557,7 +557,7 @@ func TestPreAuthorize_WithClusterCollectionVerbs(t *testing.T) {
557557

558558
t.Run("privileged user with no cluster collection verbs succeeds", func(t *testing.T) {
559559
fakeClient := setupFakeClient(privilegedClusterRole)
560-
preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create"))
560+
preAuth := NewRBACPreAuthorizer(fakeClient)
561561
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
562562
require.NoError(t, err)
563563
require.Equal(t, []ScopedPolicyRules{}, missingRules)
@@ -635,7 +635,7 @@ func TestPreAuthorize_WithNamespacedCollectionVerbs(t *testing.T) {
635635

636636
t.Run("namespaced collection verbs option checks those verbs per namespace", func(t *testing.T) {
637637
fakeClient := setupFakeClient(limitedClusterRole)
638-
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("create", "deletecollection"))
638+
preAuth := NewRBACPreAuthorizer(fakeClient, WithClusterCollectionVerbs("list", "watch"), WithNamespacedCollectionVerbs("deletecollection"))
639639
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
640640
require.NoError(t, err)
641641
// Should have cluster-scoped missing rules plus namespaced rules with both create and deletecollection.
@@ -689,7 +689,7 @@ func TestPreAuthorize_WithNamespacedCollectionVerbs(t *testing.T) {
689689

690690
t.Run("privileged user with custom namespaced collection verbs succeeds", func(t *testing.T) {
691691
fakeClient := setupFakeClient(privilegedClusterRole)
692-
preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("create", "deletecollection"))
692+
preAuth := NewRBACPreAuthorizer(fakeClient, WithNamespacedCollectionVerbs("deletecollection"))
693693
missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest))
694694
require.NoError(t, err)
695695
require.Equal(t, []ScopedPolicyRules{}, missingRules)

‎test/e2e/features/install.feature‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,38 @@ Feature: Install ClusterExtension
535535
nodeSelector:
536536
kubernetes.io/os: linux
537537
"""
538+
539+
@BoxcutterRuntime
540+
@PreflightPermissions
541+
Scenario: Boxcutter preflight check detects missing CREATE permissions
542+
Given ServiceAccount "olm-sa" without create permissions is available in ${TEST_NAMESPACE}
543+
And ClusterExtension is applied
544+
"""
545+
apiVersion: olm.operatorframework.io/v1
546+
kind: ClusterExtension
547+
metadata:
548+
name: ${NAME}
549+
spec:
550+
namespace: ${TEST_NAMESPACE}
551+
serviceAccount:
552+
name: olm-sa
553+
source:
554+
sourceType: Catalog
555+
catalog:
556+
packageName: test
557+
selector:
558+
matchLabels:
559+
"olm.operatorframework.io/metadata.name": test-catalog
560+
"""
561+
And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
562+
"""
563+
pre-authorization failed: service account requires the following permissions to manage cluster extension
564+
"""
565+
And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
566+
"""
567+
Verbs:[create]
568+
"""
569+
When ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
570+
Then ClusterExtension is available
571+
And ClusterExtension reports Progressing as True with Reason Succeeded
572+
And ClusterExtension reports Installed as True

‎test/e2e/steps/steps.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
120120
sc.Step(`^(?i)ServiceAccount "([^"]*)" with permissions to install extensions is available in "([^"]*)" namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace)
121121
sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in test namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace)
122122
sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace)
123+
sc.Step(`^(?i)ServiceAccount "([^"]*)" without create permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace)
123124
sc.Step(`^(?i)ServiceAccount "([^"]*)" is available in \${TEST_NAMESPACE}$`, ServiceAccountIsAvailableInNamespace)
124125
sc.Step(`^(?i)ServiceAccount "([^"]*)" in test namespace is cluster admin$`, ServiceAccountWithClusterAdminPermissionsIsAvailableInNamespace)
125126
sc.Step(`^(?i)ServiceAccount "([^"]+)" in test namespace has permissions to fetch "([^"]+)" metrics$`, ServiceAccountWithFetchMetricsPermissions)
@@ -877,6 +878,18 @@ func ServiceAccountWithNeededPermissionsIsAvailableInTestNamespace(ctx context.C
877878
return applyPermissionsToServiceAccount(ctx, serviceAccount, rbacTemplate)
878879
}
879880

881+
// ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace creates a ServiceAccount with permissions that
882+
// intentionally exclude the "create" verb to test preflight permission validation for Boxcutter applier.
883+
// This is used to verify that the preflight check properly detects missing CREATE permissions.
884+
func ServiceAccountWithoutCreatePermissionsIsAvailableInTestNamespace(ctx context.Context, serviceAccount string) error {
885+
kernel := "helm"
886+
if enabled, found := featureGates[features.BoxcutterRuntime]; found && enabled {
887+
kernel = "boxcutter"
888+
}
889+
rbacTemplate := fmt.Sprintf("%s-%s-no-create-rbac-template.yaml", serviceAccount, kernel)
890+
return applyPermissionsToServiceAccount(ctx, serviceAccount, rbacTemplate)
891+
}
892+
880893
// ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace creates a ServiceAccount and enables creation of any cluster extension on behalf of this account.
881894
func ServiceAccountWithNeededPermissionsIsAvailableInGivenNamespace(ctx context.Context, serviceAccount string, ns string) error {
882895
sc := scenarioCtx(ctx)

‎test/e2e/steps/testdata/olm-sa-boxcutter-no-create-rbac-template.yaml‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
apiVersion: v1
3+
kind: Namespace
4+
metadata:
5+
name: ${TEST_NAMESPACE}
6+
---
7+
apiVersion: rbac.authorization.k8s.io/v1
8+
kind: ClusterRole
9+
metadata:
10+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-olm-admin-clusterrole
11+
rules:
12+
# Allow management of ClusterExtensionRevision finalizers (e.g. by the Boxcutter applier)
13+
- apiGroups: [olm.operatorframework.io]
14+
resources: [clusterextensionrevisions/finalizers]
15+
verbs: [update, patch]
16+
# OLMv0 compatibility requirement for AllNamespaces install
17+
# https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/operators/olm/operatorgroup.go#L530
18+
- apiGroups: [ "" ]
19+
resources:
20+
- namespaces
21+
verbs: [ get, list, watch ]
22+
# Bundle resource management RBAC derived from bundle resource and permissions described in the ClusterServiceVersion
23+
# NOTE: Intentionally MISSING "create" verb to test preflight check
24+
- apiGroups: [apiextensions.k8s.io]
25+
resources: [customresourcedefinitions]
26+
verbs: [update, get, delete, patch]
27+
- apiGroups: [""]
28+
resources:
29+
- configmaps
30+
- serviceaccounts
31+
verbs: [update, list, watch, get, delete, patch]
32+
- apiGroups: [ "" ]
33+
resources:
34+
- events
35+
verbs: [ patch ]
36+
- apiGroups: ["apps"]
37+
resources:
38+
- deployments
39+
verbs: [ update, get, delete, patch ]
40+
- apiGroups: ["networking.k8s.io"]
41+
resources:
42+
- networkpolicies
43+
verbs: [update, list, get, delete, patch]
44+
- apiGroups: ["rbac.authorization.k8s.io"]
45+
resources:
46+
- clusterroles
47+
- roles
48+
- clusterrolebindings
49+
- rolebindings
50+
verbs: [ update, get, delete, patch ]
51+
- apiGroups: ["coordination.k8s.io"]
52+
resources: ["leases"]
53+
verbs: [update, list, watch, get, delete, patch]
54+
- apiGroups: ["authorization.k8s.io"]
55+
resources: ["subjectaccessreviews"]
56+
verbs: [create]
57+
- apiGroups: ["authentication.k8s.io"]
58+
resources: ["tokenreviews"]
59+
verbs: [create]
60+
---
61+
apiVersion: rbac.authorization.k8s.io/v1
62+
kind: ClusterRoleBinding
63+
metadata:
64+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-install-binding
65+
roleRef:
66+
apiGroup: rbac.authorization.k8s.io
67+
kind: ClusterRole
68+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-olm-admin-clusterrole
69+
subjects:
70+
- kind: ServiceAccount
71+
name: ${SERVICEACCOUNT_NAME}
72+
namespace: ${TEST_NAMESPACE}

‎test/e2e/steps/testdata/olm-sa-helm-no-create-rbac-template.yaml‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
apiVersion: v1
3+
kind: Namespace
4+
metadata:
5+
name: ${TEST_NAMESPACE}
6+
---
7+
apiVersion: rbac.authorization.k8s.io/v1
8+
kind: ClusterRole
9+
metadata:
10+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-olm-admin-clusterrole
11+
rules:
12+
- apiGroups: [olm.operatorframework.io]
13+
resources: [clusterextensions, clusterextensions/finalizers]
14+
resourceNames: ["${CLUSTEREXTENSION_NAME}"]
15+
verbs: [update]
16+
# Bundle resource management RBAC derived from bundle resource and permissions described in the ClusterServiceVersion
17+
# NOTE: Intentionally MISSING "create" verb to test preflight check
18+
- apiGroups: [apiextensions.k8s.io]
19+
resources: [customresourcedefinitions]
20+
verbs: [update, list, watch, get, delete, patch]
21+
- apiGroups: [""]
22+
resources:
23+
- configmaps
24+
- secrets
25+
- services
26+
- serviceaccounts
27+
- events
28+
- namespaces
29+
- persistentvolumes
30+
- persistentvolumeclaims
31+
verbs: [update, list, watch, get, delete, patch]
32+
- apiGroups: ["apps"]
33+
resources:
34+
- deployments
35+
verbs: [ update, list, watch, get, delete, patch ]
36+
- apiGroups: ["networking.k8s.io"]
37+
resources:
38+
- networkpolicies
39+
verbs: [ update, list, watch, get, delete, patch ]
40+
- apiGroups: ["rbac.authorization.k8s.io"]
41+
resources:
42+
- clusterroles
43+
- roles
44+
- clusterrolebindings
45+
- rolebindings
46+
verbs: [ update, list, watch, get, delete, patch ]
47+
- apiGroups: ["coordination.k8s.io"]
48+
resources: ["leases"]
49+
verbs: [ update, list, watch, get, delete, patch ]
50+
- apiGroups: ["authorization.k8s.io"]
51+
resources: ["subjectaccessreviews"]
52+
verbs: [create]
53+
- apiGroups: ["authentication.k8s.io"]
54+
resources: ["tokenreviews"]
55+
verbs: [create]
56+
---
57+
apiVersion: rbac.authorization.k8s.io/v1
58+
kind: ClusterRoleBinding
59+
metadata:
60+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-install-binding
61+
roleRef:
62+
apiGroup: rbac.authorization.k8s.io
63+
kind: ClusterRole
64+
name: ${TEST_NAMESPACE}-${SERVICEACCOUNT_NAME}-olm-admin-clusterrole
65+
subjects:
66+
- kind: ServiceAccount
67+
name: ${SERVICEACCOUNT_NAME}
68+
namespace: ${TEST_NAMESPACE}

0 commit comments

Comments
 (0)