✨ Add preauthorizer checks to Boxcutter applier #2443
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
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.
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
6f94c27 to
9d08956
Compare
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
9d08956 to
7cdc319
Compare
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
2 times, most recently
from
876225e to
7f4a867
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2443 +/- ##
==========================================
+ Coverage 69.48% 73.62% +4.13%
==========================================
Files 101 101
Lines 7701 7742 +41
==========================================
+ Hits 5351 5700 +349
+ Misses 1914 1594 -320
- Partials 436 448 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
7f4a867 to
d542d16
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
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.
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
d542d16 to
53a9309
Compare
openshift-merge-robot
added
the
needs-rebase
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
53a9309 to
5cd737f
Compare
There was a problem hiding this comment.
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.
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
5cd737f to
2b041e5
Compare
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
2b041e5 to
13cdb4b
Compare
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
13cdb4b to
9e07dab
Compare
openshift-merge-robot
removed
the
needs-rebase
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 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.
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
perdasilva
force-pushed
the
boxcutter-preflight-auth-checks
branch
from
9e07dab to
387691d
Compare
| Verbs: []string{"update"}, | ||
| APIGroups: []string{""}, | ||
| Resources: []string{"clusterextensions/finalizers"}, | ||
| ResourceNames: []string{"test-cluster-extension"}, |
There was a problem hiding this comment.
So now if we do not have the permissions for those, will all work?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 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.
3 participants
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:
createOrUpdatemethod to call perform the PreAuthorization checksReviewer Checklist