✨ Add ServiceAccount validation to ClusterExtension reconciliation #2488

perdasilva · 2026-02-05T14:19:52Z

Summary

This PR introduces an extensible validation framework for ClusterExtension reconciliation that validates configuration before attempting bundle installation. The first validator ensures the specified ServiceAccount exists.

Problem

Previously, ServiceAccount validation happened during revision state retrieval, which meant:

Configuration errors were discovered late in the reconciliation process
Error handling was mixed with business logic
No way to validate multiple aspects and report all errors at once

Solution

New Validation Framework

ValidateClusterExtension(validators ...ClusterExtensionValidator)

Orchestrates multiple validators in a single reconcile step
Executes ALL validators even if some fail (collects all errors)
Sets appropriate status conditions on failure:
- Installed: Unknown
- Progressing: True (Reason: Retrying)
Returns aggregated errors with clear user-facing messages

ServiceAccountValidator(tokenGetter)

Validates ServiceAccount existence using the TokenGetter
Reuses the same TokenGetter instance used for bundle operations
Leverages existing token cache (no unnecessary API calls)
Returns friendly error: service account "name" not found in namespace "ns"

Integration Points

Added as the first reconcile step (after finalizer handling) in both:

Boxcutter runtime configuration (cmd/operator-controller/main.go:630-632)
Helm runtime configuration (cmd/operator-controller/main.go:748-750)

The same tokenGetter instance is created once and shared across validation and bundle operations.

Cleanup

Removed ServiceAccount-specific error handling from RetrieveRevisionStates since validation now happens earlier in the pipeline.

Testing

Unit Tests (`clusterextension_controller_test.go`)

TestValidateClusterExtension - 7 scenarios:

✅ All validators pass
✅ Single validator fails
✅ Multiple validators collect all failures
✅ Multiple validators all pass
✅ Mixed pass/fail validators
✅ ServiceAccount not found
✅ ServiceAccount found

TestClusterExtensionServiceAccountNotFound

✅ Updated to use new validation framework
✅ Verifies status conditions and error messages

E2E Tests (`test/e2e/features/install.feature`)

✅ New scenario: "Report validation error when ServiceAccount does not exist"
✅ Validates end-to-end user experience

Benefits

Extensibility

Clean pattern for adding new validators (RBAC, namespace, resource quotas, etc.)
Validators are composable and testable in isolation

User Experience

Clear, actionable error messages
All validation errors reported at once (not one-at-a-time)
Early failure detection before expensive operations

Performance

Reuses TokenGetter cache (no redundant API calls)
Fails fast on validation errors
Same token used for validation and bundle operations

Maintainability

Separation of concerns (validation vs. business logic)
Easier to test individual validators
Framework supports both unit and integration testing

Files Changed

cmd/operator-controller/main.go (+9/-9)
- Moved TokenGetter creation earlier
- Added validation step to both reconciler configurations
internal/operator-controller/controllers/clusterextension_reconcile_steps.go (+57/-6)
- New validation framework
- ServiceAccount validator implementation
- Removed SA error handling from RetrieveRevisionStates
internal/operator-controller/controllers/clusterextension_controller_test.go (+193/-15)
- Comprehensive test coverage
- Helper functions for test setup
test/e2e/features/install.feature (+24)
- End-to-end validation scenario

🤖 Generated with Claude Code

netlify · 2026-02-05T14:19:57Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`6d40acc`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6989efc406fde500086fd90e
😎 Deploy Preview	https://deploy-preview-2488--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.

openshift-ci · 2026-02-05T14:19:58Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign oceanc80 for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

Copilot

Pull request overview

This PR adds ServiceAccount validation to the ClusterExtension reconciliation pipeline. The validation checks whether the ServiceAccount specified in .spec.serviceAccount exists on the cluster before proceeding with bundle installation, moving this check from RetrieveRevisionStates to an earlier validation stage.

Changes:

Introduces a new validation framework with ValidateClusterExtension orchestrator and ServiceAccountValidator implementation
Moves ServiceAccount existence checks earlier in the reconciliation pipeline (after finalizer handling)
Updates both Boxcutter and Helm reconciler configurations to include the validation step

Reviewed changes

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

Show a summary per file

File	Description
`internal/operator-controller/controllers/clusterextension_reconcile_steps.go`	Adds validation framework (ValidateClusterExtension, ClusterExtensionValidator, ServiceAccountValidator) and removes ServiceAccount error handling from RetrieveRevisionStates
`internal/operator-controller/controllers/clusterextension_reconcile_steps_test.go`	Adds comprehensive unit tests for validator functions (10 test cases total)
`internal/operator-controller/controllers/clusterextension_controller_test.go`	Updates existing ServiceAccount test to use new validation approach with mock TokenGetter
`cmd/operator-controller/main.go`	Integrates validation step into both Boxcutter and Helm reconciler configurations
`test/e2e/features/install.feature`	Adds E2E Gherkin scenario for ServiceAccount validation error case

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

internal/operator-controller/controllers/clusterextension_controller_test.go

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

internal/operator-controller/controllers/clusterextension_reconcile_steps_test.go

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

internal/operator-controller/controllers/clusterextension_reconcile_steps_test.go

codecov · 2026-02-05T14:38:38Z

Codecov Report

❌ Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.84%. Comparing base (910fa59) to head (6d40acc).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/operator-controller/main.go	37.50%	5 Missing ⚠️
...er/controllers/clusterextension_reconcile_steps.go	96.15%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2488      +/-   ##
==========================================
+ Coverage   69.73%   69.84%   +0.11%     
==========================================
  Files         102      102              
  Lines        8471     8520      +49     
==========================================
+ Hits         5907     5951      +44     
- Misses       2091     2096       +5     
  Partials      473      473

Flag	Coverage Δ
e2e	`46.27% <82.35%> (+<0.01%)`	⬆️
experimental-e2e	`12.91% <0.00%> (-0.09%)`	⬇️
unit	`58.05% <73.52%> (+0.19%)`	⬆️

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

Pull request overview

Copilot reviewed 5 out of 5 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/clusterextension_reconcile_steps.go

internal/operator-controller/controllers/clusterextension_reconcile_steps_test.go

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

perdasilva · 2026-02-09T09:25:32Z

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

 		revisionStates, err := r.GetRevisionStates(ctx, ext)
 		if err != nil {
 			setInstallStatus(ext, nil)
-			var saerr *authentication.ServiceAccountNotFoundError


this got split between the service account validator and the cluster extension validator

Copilot

Pull request overview

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

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

internal/operator-controller/controllers/clusterextension_controller_test.go

Copilot

Pull request overview

Copilot reviewed 5 out of 5 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/controllers/clusterextension_controller_test.go

…erExtension Introduces an extensible validation framework that runs early in the ClusterExtension reconciliation pipeline to catch configuration errors before expensive operations begin. The first validator checks whether the specified ServiceAccount exists, providing clear feedback when it's missing. The validation step: - Executes all validators and collects all errors (no fail-fast) - Reuses the same TokenGetter cache for both validation and bundle operations - Sets appropriate status conditions (Installed=Unknown, Progressing=Retrying) - Provides user-friendly error messages This architectural improvement separates validation concerns from revision state retrieval, making it easier to add new validators in the future. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Copilot AI review requested due to automatic review settings February 5, 2026 14:19

openshift-ci bot requested review from anik120 and grokspawn February 5, 2026 14:19

Copilot started reviewing on behalf of perdasilva February 5, 2026 14:20 View session

perdasilva changed the title ~~feat: Add ServiceAccount validation to ClusterExtension reconciliation~~ ✨ Add ServiceAccount validation to ClusterExtension reconciliation Feb 5, 2026

perdasilva marked this pull request as draft February 5, 2026 14:20

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 Feb 5, 2026

Copilot AI reviewed Feb 5, 2026

View reviewed changes

perdasilva force-pushed the validation-stage branch from 26b9052 to 58f8e47 Compare February 5, 2026 14:46

Copilot AI review requested due to automatic review settings February 5, 2026 16:54

perdasilva force-pushed the validation-stage branch from 58f8e47 to 64a00b6 Compare February 5, 2026 16:54

Copilot started reviewing on behalf of perdasilva February 5, 2026 16:56 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

perdasilva force-pushed the validation-stage branch from 64a00b6 to 8e814fb Compare February 6, 2026 13:40

perdasilva commented Feb 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 9, 2026 09:31

perdasilva force-pushed the validation-stage branch from 8e814fb to cf4cdb1 Compare February 9, 2026 09:31

Copilot started reviewing on behalf of perdasilva February 9, 2026 09:31 View session

perdasilva marked this pull request as ready for review February 9, 2026 09:33

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 Feb 9, 2026

openshift-ci bot requested a review from pedjak February 9, 2026 09:33

Copilot AI reviewed Feb 9, 2026

View reviewed changes

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

internal/operator-controller/controllers/clusterextension_controller_test.go Show resolved Hide resolved

perdasilva force-pushed the validation-stage branch from cf4cdb1 to a946319 Compare February 9, 2026 13:28

Copilot AI review requested due to automatic review settings February 9, 2026 14:09

perdasilva force-pushed the validation-stage branch from a946319 to 81c68c3 Compare February 9, 2026 14:09

Copilot started reviewing on behalf of perdasilva February 9, 2026 14:10 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

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

perdasilva force-pushed the validation-stage branch from 81c68c3 to 6d40acc Compare February 9, 2026 14:31

✨ Add ServiceAccount validation to ClusterExtension reconciliation #2488

Are you sure you want to change the base?

✨ Add ServiceAccount validation to ClusterExtension reconciliation #2488

Conversation

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

Uh oh!

Summary

Problem

Solution

New Validation Framework

Integration Points

Cleanup

Testing

Unit Tests (clusterextension_controller_test.go)

E2E Tests (test/e2e/features/install.feature)

Benefits

Extensibility

User Experience

Performance

Maintainability

Files Changed

Uh oh!

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

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

openshift-ci bot commented Feb 5, 2026

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!

Uh oh!

Uh oh!

codecov bot commented Feb 5, 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

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

perdasilva Feb 9, 2026

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

perdasilva commented Feb 5, 2026 •

edited

Loading

Unit Tests (`clusterextension_controller_test.go`)

E2E Tests (`test/e2e/features/install.feature`)

netlify bot commented Feb 5, 2026 •

edited

Loading

codecov bot commented Feb 5, 2026 •

edited

Loading