Skip to content

🌱 Use dedicated least-privilege service accounts for revision probe e2e tests#2547

Merged
openshift-merge-bot[bot] merged 1 commit intooperator-framework:mainfrom
perdasilva:fixup-cer-probe-e2e-perms
Mar 9, 2026
Merged

🌱 Use dedicated least-privilege service accounts for revision probe e2e tests#2547
openshift-merge-bot[bot] merged 1 commit intooperator-framework:mainfrom
perdasilva:fixup-cer-probe-e2e-perms

Conversation

@perdasilva
Copy link
Contributor

@perdasilva perdasilva commented Mar 9, 2026

Description

The revision probe e2e tests are failing the test-experimental-e2e job. This PR addresses that issue.

The revision probe e2e tests previously shared the generic olm-sa service account, which has RBAC permissions related to the scope required for the test-operator. This change updates the service account + RBAC creation step to derive the appropriate rbac template from the service account and the olm kernel used (helm or boxcutter). It also adds a dedicated rbac template for the revision test with only the rbac required for the tests (i.e. the resources in the revision object).

Changes

  • Convention-based RBAC template lookup: Replaced hardcoded RBAC template constants with a fmt.Sprintf pattern: <service-account>-<helm|boxcutter>-rbac-template.yaml, keyed off the service account name and the BoxcutterRuntime feature gate
  • RBAC template renames: Renamed existing templates with olm-sa- prefix to match the new convention (rbac-template.yamlolm-sa-helm-rbac-template.yaml, boxcutter-rbac-template.yamlolm-sa-boxcutter-rbac-template.yaml)
  • Dedicated pvc-probe-sa service account: PVC revision scenarios now use a dedicated SA with a least-privilege RBAC template granting only CER finalizer update, PersistentVolume CRUD, PVC CRUD, and ConfigMap CRUD

Reviewer Checklist

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

Copilot AI review requested due to automatic review settings March 9, 2026 16:20
@perdasilva perdasilva changed the title 🌱 : Fix revision probe e2e tests 🌱 Fix revision probe e2e tests Mar 9, 2026
@netlify
Copy link

netlify bot commented Mar 9, 2026

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 5811289
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/69af0533e1e9d00008b0735e
😎 Deploy Preview https://deploy-preview-2547--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the revision e2e tests to run with dedicated ServiceAccounts and purpose-built RBAC templates rather than relying on the shared “needed permissions” setup.

Changes:

  • Added per-scenario RBAC templates for revision probe-related ServiceAccounts.
  • Introduced a new Godog step that applies an RBAC template derived from the ServiceAccount name.
  • Updated revision.feature to use the new ServiceAccounts and step.

Reviewed changes

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

File Description
test/e2e/steps/testdata/pvc-probe-sa-rbac-template.yaml Adds a dedicated RBAC template for the PVC probe failure scenario.
test/e2e/steps/testdata/pvc-bound-sa-rbac-template.yaml Adds a dedicated RBAC template for the PVC “Bound” progression scenario (includes PV permissions).
test/e2e/steps/steps.go Adds a new step to apply <serviceAccount>-rbac-template.yaml RBAC templates.
test/e2e/features/revision.feature Switches scenarios to use the new dedicated ServiceAccounts and RBAC-template step.

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

@perdasilva
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2026
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from 10047fe to 3a1a7d8 Compare March 9, 2026 16:33
Copilot AI review requested due to automatic review settings March 9, 2026 16:35
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from 3a1a7d8 to 4415ed9 Compare March 9, 2026 16:35
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from a71cce6 to 4415ed9 Compare March 9, 2026 16:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 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.

@perdasilva perdasilva changed the title 🌱 Fix revision probe e2e tests 🌱 Use dedicated least-privilege service accounts for revision probe e2e tests Mar 9, 2026
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from 4415ed9 to 1095a15 Compare March 9, 2026 16:56
@perdasilva
Copy link
Contributor Author

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2026
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from 1095a15 to 0091b38 Compare March 9, 2026 17:31
Copilot AI review requested due to automatic review settings March 9, 2026 17:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

… tests

The revision probe e2e tests previously shared the generic `olm-sa`
service account, which had broad permissions that masked potential
RBAC issues. This change:

- Refactors RBAC template selection to use a convention-based naming
  pattern: `<service-account>-<helm|boxcutter>-rbac-template.yaml`,
  replacing the hardcoded constants with `fmt.Sprintf`
- Renames existing RBAC templates with `olm-sa-` prefix to follow the
  new convention (`rbac-template.yaml` → `olm-sa-helm-rbac-template.yaml`,
  `boxcutter-rbac-template.yaml` → `olm-sa-boxcutter-rbac-template.yaml`)
- Introduces a dedicated `pvc-probe-sa` service account for PVC probe
  scenarios with a least-privilege RBAC template granting only CER
  finalizer update, PersistentVolume CRUD, PVC CRUD, and ConfigMap CRUD

Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
@perdasilva perdasilva force-pushed the fixup-cer-probe-e2e-perms branch from 0091b38 to 5811289 Compare March 9, 2026 17:36
@tmshort
Copy link
Contributor

tmshort commented Mar 9, 2026

/approve

@openshift-ci
Copy link

openshift-ci bot commented Mar 9, 2026

[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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2026
@tmshort
Copy link
Contributor

tmshort commented Mar 9, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 9, 2026
@tmshort
Copy link
Contributor

tmshort commented Mar 9, 2026

/override ".github/workflows/pr-title.yaml / Verify PR title (pull_request_target)"

@openshift-ci
Copy link

openshift-ci bot commented Mar 9, 2026

@tmshort: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • .github/workflows/pr-title.yaml / Verify PR title (pull_request_target)

Only the following failed contexts/checkruns were expected:

  • Verify PR title
  • crd-diff
  • e2e
  • experimental-e2e
  • extension-developer-e2e
  • go-apidiff
  • go-verdiff
  • goreleaser
  • lint
  • netlify/olmv1/deploy-preview
  • st2ex-e2e
  • tide
  • unit-test-basic
  • upgrade-st2st-e2e
  • verify

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

Details

In response to this:

/override ".github/workflows/pr-title.yaml / Verify PR title (pull_request_target)"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@tmshort
Copy link
Contributor

tmshort commented Mar 9, 2026

/override "Verify PR title"

@openshift-ci
Copy link

openshift-ci bot commented Mar 9, 2026

@tmshort: Overrode contexts on behalf of tmshort: Verify PR title

Details

In response to this:

/override "Verify PR title"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@tmshort
Copy link
Contributor

tmshort commented Mar 9, 2026

/override "Verify PR title"

@openshift-ci
Copy link

openshift-ci bot commented Mar 9, 2026

@tmshort: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • Verify PR title

Only the following failed contexts/checkruns were expected:

  • crd-diff
  • e2e
  • experimental-e2e
  • extension-developer-e2e
  • go-apidiff
  • go-verdiff
  • goreleaser
  • lint
  • netlify/olmv1/deploy-preview
  • st2ex-e2e
  • tide
  • unit-test-basic
  • upgrade-st2st-e2e
  • verify

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

Details

In response to this:

/override "Verify PR title"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 33a0224 into operator-framework:main Mar 9, 2026
23 of 24 checks passed
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.58%. Comparing base (bca7a49) to head (5811289).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2547      +/-   ##
==========================================
+ Coverage   64.28%   68.58%   +4.29%     
==========================================
  Files         131      131              
  Lines        9330     9330              
==========================================
+ Hits         5998     6399     +401     
+ Misses       2855     2439     -416     
- Partials      477      492      +15     
Flag Coverage Δ
e2e 42.22% <ø> (ø)
experimental-e2e 51.57% <ø> (?)
unit 53.81% <ø> (ø)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants