Skip to content

WIP: NO-JIRA: Update authentication ClusterOperator manifest versions #799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
+4 −2
Closed

WIP: NO-JIRA: Update authentication ClusterOperator manifest versions #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b562af
manifests: remove oauth-openshift from expected versions
liouk Oct 14, 2025
f64ab6d
drop! pull fix from #798
liouk Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions manifests/08_clusteroperator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,3 @@ status:
versions:
- name: operator
version: "0.0.1-snapshot"
- name: oauth-openshift
version: "0.0.1-snapshot_openshift"
4 changes: 4 additions & 0 deletions pkg/operator/starter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,10 @@ func prepareOauthAPIServerOperator(

const apiServerConditionsPrefix = "APIServer"

statusControllerOptions = append(statusControllerOptions, func(ss *status.StatusSyncer) *status.StatusSyncer {
return ss.WithVersionRemoval()
})
Comment on lines +479 to +481
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify that version removal targets only the oauth-openshift version.

The addition of WithVersionRemoval() enables version removal functionality on the StatusSyncer, but it's unclear from this code which versions will be removed and under what conditions. Given the PR objective to remove the oauth-openshift version in OIDC scenarios, please verify that:

  1. The WithVersionRemoval() method implementation correctly identifies and removes only the oauth-openshift version
  2. The removal logic properly handles both OIDC and non-OIDC scenarios
  3. Other versions (e.g., for different components) are not inadvertently affected

Since the PR is marked WIP while investigating repercussions, it would be helpful to understand what concerns or edge cases are being investigated.

Run the following script to examine the WithVersionRemoval() implementation and verify its behavior:

🏁 Script executed:

#!/bin/bash
# Description: Examine the WithVersionRemoval implementation and its usage

# Find the WithVersionRemoval method definition
echo "=== WithVersionRemoval method definition ==="
ast-grep --pattern $'func ($_ *StatusSyncer) WithVersionRemoval() $_ {
  $$$
}'

echo ""
echo "=== Usage of WithVersionRemoval in codebase ==="
rg -n 'WithVersionRemoval' -A 3 -B 3

echo ""
echo "=== Related manifest changes ==="
# Check if manifests/08_clusteroperator.yaml exists and contains relevant changes
fd -t f '08_clusteroperator.yaml' --exec cat {}

Length of output: 3628

Guard version removal behind OIDC mode

Unconditionally enabling WithVersionRemoval() will strip all missing versions, not just oauth-openshift. Wrap this option in the OIDC-specific path (or equivalent flag) to avoid unintended removal elsewhere, add an inline comment clarifying the scope, and ensure tests cover both OIDC and non-OIDC scenarios.

🤖 Prompt for AI Agents 
In pkg/operator/starter.go around lines 479–481, the call that unconditionally
appends ss.WithVersionRemoval() must be guarded so version removal only happens
in OIDC mode; change the code to append that option only when the
OIDC/oauth-openshift mode flag or configuration is true (e.g., if oIDCEnabled {
statusControllerOptions = append(...)}), add an inline comment explaining this
is OIDC-specific behavior to avoid stripping missing versions for other auth
modes, and update/add unit/integration tests to assert WithVersionRemoval is
applied when OIDC is enabled and not applied when OIDC is disabled.


apiServerControllers, err := apiservercontrollerset.NewAPIServerControllerSet(
"oauth-apiserver",
authOperatorInput.authenticationOperatorClient,
Expand Down