✨ Add preauthorizer checks to Boxcutter applier #2443

perdasilva · 2026-01-13T14:42:13Z

Description

Adds the PreAuthorizer checks to the Boxcutter applier for feature-gate parity between Helm and Boxcutter appliers.

The Boxcutter applier's PreAuthorization check requires clusterextensions/finalizers and clusterextensionrevisions/finalizers update permissions (on top of the permissions to manage the bundle's resources).

Changes:

Refactors PreAuthorizer away from handling ClusterExtension specific permissions
Refactors Helm applier to add ClusterExtension permissions for the PreAuthorization check
Updates the Helm applier PreAuthorization unit tests and adds a component integration test to ensure it is being called with the right parameters
Adds PreAuthorization checks to the Boxcutter Applier and adds the clusterextensionrevisions/finalizers update permission for the check
Refactors Boxcutters createOrUpdate method to call perform the PreAuthorization checks
Adds PreAuthorizator intergration tests to Boxcutter unit test suite

PreAuthorizer Refactoring Notes

Previously, the PreAuthorizer.PreAuthorize method took a ClusterExtension as a parameter and used it to derive the user to check the permissions against and to generate the clusterextensions/finalizers update permission implicitly required by the applier to manage update ownerReferences blockerOwnerDeletion.

This PR makes refactors the PreAuthorize methods to substitute the ClusterExtension parameter by two parameters:

manifestManager -> the user to check the permissions agains
additionsRequiredPermissions -> permissions that are required on top of the permissions strictly required to manage the manifests input through the manifestReader parameter.

This makes the PreAuthorizer more generic by removing ClusterExtension concerns, and allows the applier to define which permissions are needed for its operation beyond those dictated by the bundle manifests. Making the PreAuthorizer more generic, and moving applier specific concerns to the applier. The PreAuthorizer and Applier unit tests are update for this change (removing the clusterextensionrevision perms from the PreAuthorizer tests and adding that check to the applier).

E2E Notes

Added the ClusterExtension reports <condition> as <status> with Reason <reason> and Message including <message fragment> to avoid checking the entire error message but only the salient points as the set could change in the future
Refactored the templating functions to split concerns
Split the namespace and service account from the RBAC step into its own step (RBAC steps call the service account step - so their behavior hasn't changes completely) - this enables testing the ClusterExtension before the service account gets its permissions

Note

The Boxcutter applier still uses the manager client to create and update the revisions. The PreAuthorizer just ensures the service account has all the necessary permissions to do its job.

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2026-01-13T14:42:20Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`d0a8787`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/696f34854c5def00085929e0
😎 Deploy Preview	https://deploy-preview-2443--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

This PR adds PreAuthorizer checks to the Boxcutter applier to achieve feature-gate parity with the Helm applier. The implementation validates that service accounts have the necessary RBAC permissions before applying cluster extensions, including the ability to update clusterextensionrevisions/finalizers which is specific to the Boxcutter workflow.

Changes:

Added an Option pattern to configure PreAuthorizer with ClusterExtensionRevision finalizer permission checks
Integrated PreAuthorizer into the Boxcutter applier with manifest generation and permission validation
Updated main.go to initialize PreAuthorizer with the new option when the PreflightPermissions feature gate is enabled

Reviewed changes

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

Show a summary per file

File	Description
internal/operator-controller/authorization/rbac.go	Added Option pattern and WithClusterExtensionRevisionPerms to optionally check for update permissions on clusterextensionrevisions/finalizers
internal/operator-controller/authorization/rbac_test.go	Added test case for PreAuthorizer with ClusterExtensionRevision permissions
internal/operator-controller/applier/boxcutter.go	Added PreAuthorizer field and runPreAuthorizationChecks method to validate permissions before applying revisions
internal/operator-controller/applier/boxcutter_test.go	Added integration test for PreAuthorizer with fake implementation
cmd/operator-controller/main.go	Initialize PreAuthorizer with WithClusterExtensionRevisionPerms option when PreflightPermissions feature gate is enabled

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

internal/operator-controller/authorization/rbac.go

internal/operator-controller/authorization/rbac_test.go

internal/operator-controller/authorization/rbac.go

internal/operator-controller/applier/boxcutter.go

codecov · 2026-01-13T15:40:54Z

Codecov Report

❌ Patch coverage is 90.12346% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.58%. Comparing base (a9e5614) to head (d0a8787).

Files with missing lines	Patch %	Lines
cmd/operator-controller/main.go	0.00%	4 Missing ⚠️
internal/operator-controller/applier/boxcutter.go	90.47%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2443      +/-   ##
==========================================
- Coverage   73.01%   69.58%   -3.44%     
==========================================
  Files         101      101              
  Lines        7730     7768      +38     
==========================================
- Hits         5644     5405     -239     
- Misses       1635     1928     +293     
+ Partials      451      435      -16

Flag	Coverage Δ
e2e	`45.87% <0.00%> (-0.38%)`	⬇️
experimental-e2e	`13.72% <0.00%> (-36.04%)`	⬇️
unit	`57.20% <90.12%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

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

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

internal/operator-controller/applier/boxcutter.go

internal/operator-controller/applier/boxcutter_test.go

internal/operator-controller/applier/boxcutter.go

internal/operator-controller/authorization/rbac_test.go

Copilot

Pull request overview

Copilot reviewed 7 out of 7 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/applier/helm_test.go

internal/operator-controller/authorization/rbac.go

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

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

internal/operator-controller/authorization/rbac.go

internal/operator-controller/applier/helm.go

test/e2e/steps/steps.go

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

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

internal/operator-controller/applier/boxcutter_test.go

camilamacedo86

All feedbacks and requests seems addressed
Great work folks. !!!

/lgtm

pedjak · 2026-01-19T14:48:37Z

internal/operator-controller/applier/boxcutter.go

+		objBytes, err := yaml.Marshal(obj)
+		if err != nil {
+			return nil, fmt.Errorf("error generating revision manifest: %w", err)
+		}
+		manifestBuilder.WriteString("---\n")
+		manifestBuilder.WriteString(string(objBytes))
+		manifestBuilder.WriteString("\n")


We could use YAML printer for that:

import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/printers" ) func WriteToYAML(objects []runtime.Object, w io.Writer) error { printer := printers.YAMLPrinter{} for _, obj := range objects { if err := printer.PrintObj(obj, w); err != nil { return err } } return nil }

I've updated to use the YAMLPrinter and a byte buffer ^^

pedjak · 2026-01-19T14:54:35Z

internal/operator-controller/authorization/rbac.go

+
 type PreAuthorizer interface {
-	PreAuthorize(ctx context.Context, ext *ocv1.ClusterExtension, manifestReader io.Reader) ([]ScopedPolicyRules, error)
+	PreAuthorize(ctx context.Context, user user.Info, manifestReader io.Reader, additionalRequiredPerms ...UserAuthorizerAttributesFactory) ([]ScopedPolicyRules, error)


please document

I've moved the godoc string from the implementation to the interface

pedjak · 2026-01-19T15:09:36Z

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

-apiVersion: v1
-kind: Namespace
-metadata:
-  name: ${TEST_NAMESPACE}
---
-apiVersion: v1
-kind: ServiceAccount
-metadata:
-  name: ${SERVICEACCOUNT_NAME}
-  namespace: ${TEST_NAMESPACE}
---


I would keep within the same template all needed resources, so to keep the step logic simpler.

I needed to split the steps for the e2e test

I need to have the service account and namespace stamped out for the ClusterExtension and then to apply the perms. I'd prefer not having to maintain two files with the same template.

pedjak · 2026-01-19T15:15:52Z

test/e2e/features/recover.feature

+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    And ClusterExtension reports Progressing as True with Reason Retrying and Message includes:


can we match against the full message?

It's a bit hard right now because there's a difference b/w what is required forthe Helm applier and what is required by the Boxcutter applier. I also didn't want the test to break every time something changes in the bundle and having to manage long winded and message messages. The other tests should fail.

pedjak · 2026-01-19T15:21:41Z

test/e2e/steps/steps.go

 			return false
 		}
-		if msg != nil && condition["message"] != *msg {
+		if msgCmp != nil && !(*msgCmp)(condition["message"].(string)) {


can we avoid type casts?

moved the deserialization to meta.Condition instead of map[string]interface{}

pedjak · 2026-01-19T15:34:47Z

test/e2e/features/recover.feature

    And ClusterExtension reports Installed as True
+
+  @PreflightPermissions
+  Scenario: Add missing permissions


let's be more descriptive with the scenario.

Updated to something more descriptive. Thanks for calling this out. One of those things you leave last and then forget about =SS

openshift-ci · 2026-01-19T17:30:01Z

New changes are detected. LGTM label has been removed.

tmshort · 2026-01-19T21:34:18Z

/approve

Approved for inclusion, but still needs LGTM (@pedjak)

openshift-ci · 2026-01-19T21:34:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 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.

Copilot AI review requested due to automatic review settings January 13, 2026 14:42

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 13, 2026

Copilot started reviewing on behalf of perdasilva January 13, 2026 14:42 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 6f94c27 to 9d08956 Compare January 13, 2026 14:56

Copilot AI review requested due to automatic review settings January 13, 2026 15:00

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 9d08956 to 7cdc319 Compare January 13, 2026 15:00

Copilot started reviewing on behalf of perdasilva January 13, 2026 15:00 View session

perdasilva force-pushed the boxcutter-preflight-auth-checks branch 2 times, most recently from 876225e to 7f4a867 Compare January 13, 2026 15:25

Copilot AI reviewed Jan 13, 2026

View reviewed changes

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 7f4a867 to d542d16 Compare January 13, 2026 17:09

perdasilva marked this pull request as ready for review January 13, 2026 17:09

Copilot AI review requested due to automatic review settings January 13, 2026 17:09

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 13, 2026

openshift-ci bot requested review from joelanford and tmshort January 13, 2026 17:09

Copilot started reviewing on behalf of perdasilva January 13, 2026 17:11 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from d542d16 to 53a9309 Compare January 14, 2026 16:06

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2026

Copilot AI review requested due to automatic review settings January 14, 2026 16:08

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 53a9309 to 5cd737f Compare January 14, 2026 16:08

Copilot started reviewing on behalf of perdasilva January 14, 2026 16:09 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

internal/operator-controller/applier/helm_test.go Outdated Show resolved Hide resolved

internal/operator-controller/applier/helm_test.go Outdated Show resolved Hide resolved

internal/operator-controller/authorization/rbac.go Outdated Show resolved Hide resolved

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 5cd737f to 2b041e5 Compare January 14, 2026 16:26

Copilot AI review requested due to automatic review settings January 14, 2026 16:28

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 2b041e5 to 13cdb4b Compare January 14, 2026 16:28

Copilot AI review requested due to automatic review settings January 16, 2026 15:19

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from c96c4bc to 40a2645 Compare January 16, 2026 15:19

Copilot started reviewing on behalf of perdasilva January 16, 2026 15:19 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 40a2645 to 325f80d Compare January 16, 2026 15:35

Copilot AI review requested due to automatic review settings January 16, 2026 15:45

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 325f80d to 12adada Compare January 16, 2026 15:45

Copilot started reviewing on behalf of perdasilva January 16, 2026 15:46 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

internal/operator-controller/applier/boxcutter_test.go Show resolved Hide resolved

camilamacedo86 reviewed Jan 19, 2026

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2026

pedjak reviewed Jan 19, 2026

View reviewed changes

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from 12adada to dd63342 Compare January 19, 2026 17:29

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2026

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2026

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 19, 2026

Add preflight checks to Boxcutter applier

d0a8787

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Copilot AI review requested due to automatic review settings January 20, 2026 07:53

perdasilva force-pushed the boxcutter-preflight-auth-checks branch from dd63342 to d0a8787 Compare January 20, 2026 07:53

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2026

Copilot started reviewing on behalf of perdasilva January 20, 2026 07:54 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

✨ Add preauthorizer checks to Boxcutter applier #2443

Are you sure you want to change the base?

✨ Add preauthorizer checks to Boxcutter applier #2443

Uh oh!

Conversation

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

Uh oh!

Description

PreAuthorizer Refactoring Notes

E2E Notes

Note

Reviewer Checklist

Uh oh!

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

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

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!

Uh oh!

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!

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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

camilamacedo86 left a comment

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!

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

perdasilva commented Jan 13, 2026 •

edited

Loading

netlify bot commented Jan 13, 2026 •

edited

Loading

codecov bot commented Jan 13, 2026 •

edited

Loading