✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations #2429

camilamacedo86 · 2026-01-06T17:22:01Z

What's New

When BoxcutterRuntime is enabled, OLM v1 now correctly uses the ServiceAccount you specify in your ClusterExtension to install and manage extensions. This ensures your extensions are installed with only the permissions you grant, not admin privileges.

What This Means for You

More Control:

Extensions can only create resources that the ServiceAccount is allowed to create
If an extension tries to create something without permission, it fails immediately with a clear error
You see exactly which ServiceAccount performed each action in audit logs

Better Security:

Extensions no longer bypass RBAC during installation
Each extension limited to its ServiceAccount's permissions
No more admin-level access by default

Example

apiVersion: v1
kind: ServiceAccount
metadata:
  name: my-installer
  namespace: my-app
---
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
  name: my-extension
spec:
  namespace: my-app
  serviceAccount:
    name: my-installer  # ← Your extension uses this SA's permissions
  source:
    sourceType: Catalog
    catalog:
      packageName: my-package

What happens: OLM installs the extension using my-installer ServiceAccount. The extension can only do what you've granted that ServiceAccount permission to do.

netlify · 2026-01-06T17:22:07Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`092099c`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/69651f8d74497700087332a1
😎 Deploy Preview	https://deploy-preview-2429--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/revision_engine_factory_test.go

camilamacedo86 · 2026-01-07T18:30:04Z

test/e2e/steps/testdata/rbac-template.yaml

+    # for roles it doesn't have and grant permissions beyond its own. This is required
+    # because extension bundles contain their own RBAC that must be created.
+    # See docs/concepts/permission-model.md for details on these requirements.
+    verbs: [ update, create, list, watch, get, delete, patch, bind, escalate ]


@perdasilva that proves it is working :-)
The permissions were missing, and e2e tests were failing.

Are we expanding the olm controller's rbac scope here? I think we should be expanding the rbac scope for the service account in .spec.serviceAccount for the CE. Right?

According to the docs, we do need those permissions. When we started using the SA with Buxcutter, we began hitting issues in the experimental-e2e tests: the install gets stuck because the SA is missing required permissions. See run: https://github.com/operator-framework/operator-controller/actions/runs/20789162910/job/59706814076

I'm having trouble identifying the root cause from the error messages. It just says the condition hasn't been met. My expectation would be that the only thing missing from the install service account as its modeled in main is the ability to update clusterextensionrevisions/finalizers. In principle all other permissions should be the same, no?

“OLMv1 operator preflight checks should report error when {escalate, bind} is not specified”
Essentially, without bind/escalate in the ClusterRole, the controller could not proceed, causing the operator to “fail to start” and tests to timeout waiting for it to become ready.
We need that to create ClusterRoles and ClusterRoleBindings defined in that Operator’s bundle so we need that.
Now, we are using the SA permissions and without the required permissions in the SA the install will not work out. The missing permission was not obtained before because we are not using the SA.

We need that to create ClusterRoles and ClusterRoleBindings defined in that Operator’s bundle so we need that.

This should be true whether we use Boxcutter or not, right? Why does this test pass with the Helm applier but not with Boxcutter + scoped client? Where is the need for bind and escalate coming from?

This is a pretty drastic increase in permission scope. I'm just trying to understand why it wasn't needed before, but now it is.

After many tests and long deep analyse, and help from AI ;-) to speed up the reason for we need is:
(Adding content generated with CLAUDE to help out)

THE REAL DIFFERENCE: Server-Side Apply!

Helm Applier (Main Branch)

Uses Helm library (helm.sh/helm/v3)

Helm library uses traditional CREATE/UPDATE operations

Less strict RBAC enforcement

Permission matching works → PASSES ✅

Boxcutter Applier (PR Branch)

Uses pkg.package-operator.run/boxcutter machinery

Machinery uses Server-Side Apply (SSA) with field ownership

STRICTER RBAC enforcement with SSA!

SSA requires explicit bind verb → FAILS ❌

Code Evidence

Boxcutter Machinery (objects.go)

ctx, desiredObject, client.Apply, // ← Server-Side Apply! options, client.ForceOwnership, // ← Field ownership enforcement

Why SSA Is Stricter

From Kubernetes documentation:

Server-Side Apply enforces stricter field ownership and permissions.
When using SSA to create RBAC resources like ClusterRoleBindings,
the user MUST have the explicit bind verb, even if they already
have all the permissions being granted.

Traditional Create/Update:

Checks: "Does user have all the permissions?" → YES → Allow

Server-Side Apply:

Checks: "Does user have bind verb for field ownership?" → NO → Deny

Permission matching fallback doesn't work with SSA

Complete Comparison Table

Aspect Helm (Main) Boxcutter (PR)

Client Type SA-scoped ✅ SA-scoped ✅

Apply Method Helm lib (Create/Update) SSA (client.Apply)

RBAC Enforcement Traditional (lenient) SSA (strict)

bind verb required? ❌ No (perms match) ✅ YES (SSA strict)

Test result ✅ PASS ❌ FAIL

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/revision_engine_factory.go

internal/operator-controller/labels/labels.go

internal/operator-controller/applier/boxcutter.go

test/e2e/steps/testdata/rbac-template.yaml

camilamacedo86 · 2026-01-08T07:45:46Z

hack/test/pre-upgrade-setup.sh

+    - "get"
+    - "list"
+    - "watch"
 EOF


To fix: https://github.com/operator-framework/operator-controller/actions/runs/20792120869/job/59718591823

internal/operator-controller/controllers/revision_engine_factory.go

internal/operator-controller/controllers/clusterextensionrevision_controller.go

internal/operator-controller/controllers/revision_engine_factory.go

pedjak · 2026-01-08T10:27:16Z

internal/operator-controller/controllers/revision_engine_factory.go

+	return f.createScopedClient(saNamespace, saName)
+}
+
+func (f *DefaultRevisionEngineFactory) createScopedClient(namespace, serviceAccountName string) (client.Client, error) {


we should cache the clients, i.e. avoid returning a new instance on each createScopeClient invocation.

Lets disccuss in: #2429 (comment)

pedjak · 2026-01-08T10:29:00Z

internal/operator-controller/controllers/revision_engine_factory.go

+
+// CreateRevisionEngine constructs a boxcutter RevisionEngine for the given ClusterExtensionRevision.
+// It reads the ServiceAccount from annotations and creates a scoped client.
+func (f *DefaultRevisionEngineFactory) CreateRevisionEngine(_ context.Context, rev *ocv1.ClusterExtensionRevision) (RevisionEngine, error) {


We should cache RevisionEngines, i.e. return the same one for a given ClusterExtensionRevision.

I'm not against going down this path, but since RevisionEngine doesn't really hold state I'm curious to know what's behind the suggestion. Making the resource profile less choppy?

yeah, to lower the number of created resources. With this, we are going to create new revision engine instance at each reconcile.

IMO, caching should be done in a separate PR/JIRA/task so it can get proper focus. We can track it as tech debt and (for now) I wouldn’t prioritize it until we meet the main goals first—things may still change a lot.

It’s also not as simple as it first looks:

CER should be independent from CE
ClusterExtensionRevision (CER) should work without depending on ClusterExtension (CE).
If we cache by ServiceAccount, the same key can be shared by multiple CEs and their revisions, which makes cleanup harder.

Cleanup is tricky
When do we evict cache entries?

on CE deletion?

on CER deletion?

on ServiceAccount deletion?

Since multiple CEs can share the same ServiceAccount, evicting on CE deletion could break other CEs.
We’d likely need reference counting or a smarter eviction strategy.

Risk of unbounded growth
Without eviction, the cache could grow indefinitely in clusters with many unique ServiceAccounts, which could cause memory issues.

Scope creep
Adding caching plus a safe eviction/lifecycle policy would significantly expand the scope of this PR.

For now: we create a new RevisionEngine per reconcile. This is safe and correct (just less optimal).

WDYT? Could we agree to revisit caching as an optimisation after we land the other changes (e.g., once SA is optional, etc.)?

My thought is: let’s keep this PR focused on the core behaviour first, and track caching as tech debt / a follow-up task once the design stabilises.

I'm ok with couching it under a premature optimization. It's still not super clear how much and what we gain by caching or what the memory or latency footprint of the RE is.

I have similar feeling on the client side. It could potentially make sense to keep the token in memory only for as long as its needed. On the other hand, we'd be hitting the kube-api server for a new token every time (though maybe this is obviated by switching to impersonate, eventually). In any case, I think it's ok to address this as a follow up.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/revision_engine_factory.go

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

camilamacedo86 · 2026-01-12T11:40:16Z

Hi @pedjak and @perdasilva

Could we move with this one?
That is anyway just a step towards the end goal.
I think the goal that we want to achieve here is done.

Now we are restricting the permissions for OLMv1 based on the SA provided for Boxcutter as we do for Helm applier.
WDYT?

…operations Implement serviceAccount-scoped token-based authentication for ClusterExtensionRevision controller using annotation-based configuration. - Add RevisionEngineFactory with CreateRevisionEngine(ctx, rev) interface - Read ServiceAccount from annotations (no ClusterExtension dependency) - Token-based auth using TokenInjectingRoundTripper - ServiceAccount name and namespace in annotations for observability - TrackingCache uses global client for caching/cleanup - Comprehensive error path tests ClusterExtensionRevision can exist independently. Easy mode impersonation deferred until API is finalized. Assisted-by: Cursor

Adds documentation comments to all label/annotation constants explaining: - What each constant represents - Where they are applied (labels vs annotations) - ServiceAccount constants document their relationship to ClusterExtension spec Addresses code review feedback for improved maintainability.

The upgrade test ServiceAccount needs permissions to manage ClusterExtensionRevisions when BoxcutterRuntime is enabled. Without these permissions, the upgraded controller cannot create or update ClusterExtensionRevision resources, causing the ClusterExtension to fail reconciliation after upgrade.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Add `bind` and `escalate` RBAC verbs to e2e test ServiceAccount to support Boxcutter applier's use of Kubernetes Server-Side Apply (SSA). Experimental e2e tests fail when Boxcutter uses ServiceAccount-scoped clients to apply bundle RBAC resources (ClusterRoles and ClusterRoleBindings): ``` clusterrolebindings.rbac.authorization.k8s.io is forbidden: User "system:serviceaccount:olmv1-e2e:olm-sa" cannot bind ClusterRole: RBAC: attempting to grant RBAC permissions not currently held ``` - Uses helm.sh/helm/v3 library - Applies resources with traditional CREATE/UPDATE operations - Kubernetes RBAC allows ClusterRoleBinding creation when the ServiceAccount already has all the permissions being granted (permission matching) - **Works WITHOUT `bind`/`escalate` verbs** ✅ - Uses pkg.package-operator.run/boxcutter machinery - Applies resources with **Server-Side Apply (SSA)** (`client.Apply`) - SSA enforces field-level ownership and **stricter RBAC enforcement** - Kubernetes API server **requires explicit `bind` verb** for ClusterRoleBindings - Permission matching fallback does NOT work reliably with SSA - **REQUIRES `bind`/`escalate` verbs** ❌ Validated by running actual tests: **Test 1: Main branch standard-e2e (Helm, NO bind/escalate)** ```bash make test-e2e ``` Result: ✅ PASS (21 scenarios passed) **Test 2: PR branch experimental-e2e (Boxcutter, NO bind/escalate)** ```bash make test-experimental-e2e ``` Result: ❌ FAIL (cannot bind ClusterRole error) **Test 3: PR branch experimental-e2e (Boxcutter, WITH bind/escalate)** Result: ✅ PASS (all tests pass) Add `bind` and `escalate` verbs to the e2e test RBAC template: ```yaml - apiGroups: ["rbac.authorization.k8s.io"] resources: [clusterroles, roles, clusterrolebindings, rolebindings] verbs: [ update, create, list, watch, get, delete, patch, bind, escalate ] ``` These verbs allow the ServiceAccount to: - `bind`: Create ClusterRoleBindings that reference roles with permissions the ServiceAccount doesn't have - `escalate`: Create ClusterRoles with permissions the ServiceAccount doesn't have This is the documented requirement in `docs/concepts/permission-model.md` for extension installers and aligns with Kubernetes RBAC best practices. 1. **Required for SSA**: Server-Side Apply has stricter RBAC enforcement 2. **Documented requirement**: OLMv1 docs specify bind/escalate as proper approach 3. **Industry best practice**: Operator installers should have these verbs 4. **Supports all operators**: Not just test-operator with matching permissions 5. **Maintains SSA benefits**: Field ownership, conflict resolution, GitOps support - Kubernetes RBAC: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#privilege-escalation-prevention-and-bootstrapping - OLMv1 Permission Model: docs/concepts/permission-model.md - Boxcutter machinery: pkg.package-operator.run/boxcutter/machinery (uses client.Apply) - Testing evidence: FINAL_TESTED_ANSWER.md, SERVER_SIDE_APPLY_ANSWER.md Tested-by: Actual e2e test runs comparing Helm vs Boxcutter behavior Signed-off-by: Camila <camil@example.com>

Copilot AI review requested due to automatic review settings January 6, 2026 17:22

camilamacedo86 requested a review from a team as a code owner January 6, 2026 17:22

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 6, 2026

openshift-ci bot requested review from grokspawn and oceanc80 January 6, 2026 17:22

Copilot started reviewing on behalf of camilamacedo86 January 6, 2026 17:22 View session

This comment was marked as outdated.

Sign in to view

camilamacedo86 force-pushed the admin-install branch from f7ebc0c to 2c82746 Compare January 6, 2026 18:00

Copilot AI review requested due to automatic review settings January 6, 2026 18:41

camilamacedo86 force-pushed the admin-install branch from 2c82746 to 7bcd536 Compare January 6, 2026 18:41

Copilot started reviewing on behalf of camilamacedo86 January 6, 2026 18:41 View session

operator-framework deleted a comment from Copilot AI Jan 6, 2026

camilamacedo86 force-pushed the admin-install branch from 7bcd536 to d7760da Compare January 6, 2026 18:45

camilamacedo86 changed the title ~~WIP: (feat): [Boxcutter] Use serviceAccount for ClusterExtensionRevision operations~~ ✨ Boxcutter ClusterExtensionRevision operations now uses .spec.serviceAccount Jan 6, 2026

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 6, 2026

camilamacedo86 changed the title ~~✨ Boxcutter ClusterExtensionRevision operations now uses .spec.serviceAccount~~ WIP ✨ Boxcutter ClusterExtensionRevision operations now uses .spec.serviceAccount Jan 6, 2026

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 6, 2026

Copilot AI reviewed Jan 6, 2026

View reviewed changes

internal/operator-controller/controllers/revision_engine_factory_test.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the admin-install branch from d7760da to 586787b Compare January 6, 2026 19:01

operator-framework deleted a comment from Copilot AI Jan 6, 2026

Copilot AI review requested due to automatic review settings January 7, 2026 11:32

camilamacedo86 force-pushed the admin-install branch from 586787b to b6ad85e Compare January 7, 2026 11:32

Copilot started reviewing on behalf of camilamacedo86 January 7, 2026 11:32 View session

camilamacedo86 changed the title ~~WIP ✨ Boxcutter ClusterExtensionRevision operations now uses .spec.serviceAccount~~ ✨ Boxcutter ClusterExtensionRevision operations now uses .spec.serviceAccount Jan 7, 2026

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2026

camilamacedo86 commented Jan 7, 2026

View reviewed changes

Copilot AI reviewed Jan 7, 2026

View reviewed changes

camilamacedo86 changed the title ~~✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations~~ WIP ✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations Jan 7, 2026

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2026

camilamacedo86 force-pushed the admin-install branch 2 times, most recently from 3d14643 to 76b836b Compare January 7, 2026 19:58

camilamacedo86 changed the title ~~WIP ✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations~~ ✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations Jan 7, 2026

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2026

camilamacedo86 commented Jan 8, 2026

View reviewed changes

pedjak reviewed Jan 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 8, 2026 17:34

Copilot started reviewing on behalf of camilamacedo86 January 8, 2026 17:34 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

camilamacedo86 force-pushed the admin-install branch from 6321b79 to 000cb09 Compare January 8, 2026 17:46

Copilot AI review requested due to automatic review settings January 8, 2026 17:59

camilamacedo86 force-pushed the admin-install branch from 000cb09 to 55e51dc Compare January 8, 2026 17:59

Copilot started reviewing on behalf of camilamacedo86 January 8, 2026 18:00 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

camilamacedo86 requested a review from pedjak January 8, 2026 18:08

camilamacedo86 force-pushed the admin-install branch from 55e51dc to 6b03fe3 Compare January 12, 2026 12:05

camilamacedo86 added 3 commits January 12, 2026 15:24

review changes

a9338e6

Copilot AI review requested due to automatic review settings January 12, 2026 15:24

camilamacedo86 force-pushed the admin-install branch from 6b03fe3 to a9338e6 Compare January 12, 2026 15:24

Copilot started reviewing on behalf of camilamacedo86 January 12, 2026 15:30 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

Aspect	Helm (Main)	Boxcutter (PR)
Client Type	SA-scoped ✅	SA-scoped ✅
Apply Method	Helm lib (Create/Update)	SSA (client.Apply)
RBAC Enforcement	Traditional (lenient)	SSA (strict)
bind verb required?	❌ No (perms match)	✅ YES (SSA strict)
Test result	✅ PASS	❌ FAIL

✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations #2429

Are you sure you want to change the base?

✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations #2429

Uh oh!

Conversation

camilamacedo86 commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's New

What This Means for You

Example

Uh oh!

netlify bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Helm Applier (Main Branch)

Boxcutter Applier (PR Branch)

Code Evidence

Boxcutter Machinery (objects.go)

Why SSA Is Stricter

Complete Comparison Table

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 commented Jan 6, 2026 •

edited

Loading

netlify bot commented Jan 6, 2026 •

edited

Loading

perdasilva Jan 8, 2026 •

edited

Loading

camilamacedo86 Jan 8, 2026 •

edited

Loading

camilamacedo86 Jan 12, 2026 •

edited

Loading

perdasilva Jan 12, 2026 •

edited

Loading

camilamacedo86 Jan 8, 2026 •

edited

Loading

perdasilva Jan 12, 2026 •

edited

Loading