Skip to content

CNTRLPLANE-3772: promote ExternalOIDCWithUpstreamParity to Default - #8952

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
ShazaAldawamneh:3772
Aug 13, 2026
Merged

CNTRLPLANE-3772: promote ExternalOIDCWithUpstreamParity to Default#8952
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
ShazaAldawamneh:3772

Conversation

@ShazaAldawamneh

@ShazaAldawamneh ShazaAldawamneh commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

feat: Promote ExternalOIDCWithUpstreamParity to Default feature set Promote the ExternalOIDCWithUpstreamParity feature gate from TechPreviewNoUpgrade to the Default feature set, enabling upstream parity authentication configuration by default.

This change aligns with the openshift/api promotion in PR #2915 and allows the feature to be available in the default feature set once the API changes are merged.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

    • Expanded availability of external OIDC integration by enabling it by default, in addition to the technology preview feature set.
    • External OIDC is now enabled in the default feature configurations for supported deployment modes.
  • Bug Fixes

    • Improved consistency in authentication configuration handling for valid OIDC identity providers.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 7, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 7, 2026

Copy link
Copy Markdown

@ShazaAldawamneh: This pull request references CNTRLPLANE-3772 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

feat: Promote ExternalOIDCWithUpstreamParity to Default feature set Promote the ExternalOIDCWithUpstreamParity feature gate from TechPreviewNoUpgrade to the Default feature set, enabling upstream parity authentication configuration by default.

This change aligns with the openshift/api promotion in PR #2915 and allows the feature to be available in the default feature set once the API changes are merged.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci 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 Jul 7, 2026
@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added do-not-merge/needs-area area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release and removed do-not-merge/needs-area labels Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 3580d35b-3b48-418d-8fbe-840ea1c0a007

📥 Commits

Reviewing files that changed from the base of the PR and between ef696b8 and b1446d9.

⛔ Files ignored due to path filters (4)
  • cmd/install/assets/crds/hypershift-operator/payload-manifests/featuregates/featureGate-Hypershift-Default.yaml is excluded by !cmd/install/assets/**/*.yaml
  • cmd/install/assets/crds/hypershift-operator/payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml is excluded by !cmd/install/assets/**/*.yaml
  • cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yaml is excluded by !**/zz_generated.crd-manifests/**, !cmd/install/assets/**/*.yaml
  • cmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-Default.crd.yaml is excluded by !**/zz_generated.crd-manifests/**, !cmd/install/assets/**/*.yaml
📒 Files selected for processing (1)
  • api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml

📝 Walkthrough

Walkthrough

The ExternalOIDCWithUpstreamParity feature gate is enabled for configv1.Default and both default feature-gate manifests. The valid OIDC authentication test now expects an empty UserValidationRules collection.

Suggested reviewers: clebs

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes promoting ExternalOIDCWithUpstreamParity to the Default feature set.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The diff adds no Ginkgo title calls or test-name changes. The only test edit adds an empty UserValidationRules field; existing t.Run labels are literal and stable.
Test Structure And Quality ✅ Passed The only test change adds an expected empty field in a table-driven Go test; the affected package has no Ginkgo It blocks, cluster waits, or resource setup.
Topology-Aware Scheduling Compatibility ✅ Passed The commit promotes an auth feature gate and updates CRD schemas/tests; its diff adds no affinity, topology spread, node selector, toleration, replica, or PDB scheduling constraint.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The diff adds no Ginkgo e2e test. It only updates a standard Go unit-test expectation and feature-gate data, with no new IPv4 or external-connectivity assumption.
No-Weak-Crypto ✅ Passed The commit diff adds only feature-gate, test expectation, and CRD validation data; the complete added-line scan found no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, or crypto code.
Container-Privileges ✅ Passed The diff changes FeatureGate data, Go logic, a test, and CRD schemas; no added line sets privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs ✅ Passed The diff adds only feature-gate settings, a test expectation, and generated schema entries; it introduces no logging calls or logged sensitive values.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@ShazaAldawamneh
ShazaAldawamneh marked this pull request as ready for review July 7, 2026 14:37
@openshift-ci 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 Jul 7, 2026
@openshift-ci
openshift-ci Bot requested review from clebs and jparrill July 7, 2026 14:38

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but needs to be held until the feature has been promoted in openshift/api first.

/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 Jul 7, 2026
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.58%. Comparing base (dda6055) to head (b1446d9).
⚠️ Report is 458 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8952      +/-   ##
==========================================
+ Coverage   43.45%   45.58%   +2.12%     
==========================================
  Files         771      788      +17     
  Lines       95718    99455    +3737     
==========================================
+ Hits        41597    45333    +3736     
+ Misses      51234    50962     -272     
- Partials     2887     3160     +273     

see 175 files with indirect coverage changes

Flag Coverage Δ
cmd-support 38.79% <ø> (+1.66%) ⬆️
cpo-hostedcontrolplane 47.73% <ø> (+2.51%) ⬆️
cpo-other 45.93% <ø> (+0.83%) ⬆️
hypershift-operator 56.97% <ø> (+3.31%) ⬆️
other 35.32% <ø> (+3.23%) ⬆️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hypershift-jira-solve-ci

hypershift-jira-solve-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

I now have all the evidence I need. The root cause is completely clear. Here's my final report:

Test Failure Analysis Complete

Job Information

  • Prow Job: test / Unit Tests (cpo-hostedcontrolplane)
  • Build ID: 28868204594 / job 85721309553
  • PR: CNTRLPLANE-3772: promote ExternalOIDCWithUpstreamParity to Default #8952CNTRLPLANE-3772: promote ExternalOIDCWithUpstreamParity to Default
  • Test: TestGenerateAuthConfig/When_valid_OIDC_provider_is_provided,_it_should_generate_valid_authentication_configuration
  • Package: github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/kas

Test Failure Analysis

Error

auth_test.go:269: actual authentication configuration does not match expected (-want +got):
      &kas.AuthenticationConfiguration{
          ...
          ClaimMappings:        {Username: {Claim: "email", Prefix: &""}, Groups: {Prefix: &""}, UID: {Claim: "sub"}, Extra: {}},
      -   UserValidationRules:  nil,
      +   UserValidationRules:  []kas.UserValidationRule{},
          ...
      }

Summary

The PR promotes the ExternalOIDCWithUpstreamParity feature gate to the Default feature set. This causes a previously-skipped code path in generateJWTForProvider (auth.go:102-108) to now execute during unit tests, which populates UserValidationRules with an empty slice []UserValidationRule{} instead of leaving it as the zero-value nil. The test expectation in auth_test.go does not include UserValidationRules in its expected struct literal, so it expects nil. The cmp.Diff comparison catches this nil-vs-empty-slice difference and the test fails.

Root Cause

The failure is a test expectation that was not updated to account for the behavioral change introduced by promoting the feature gate.

Here is the complete chain of causation:

  1. Feature gate initialization: control-plane-operator/featuregates/featuregates.go line 30 calls ConfigureFeatureSet(string(configv1.Default)) in init(), meaning all unit tests run with the Default feature set active unless explicitly overridden.

  2. The PR change (single line at featuregates.go:22): Adds configv1.Default to ExternalOIDCWithUpstreamParity's enabled feature sets:

    // Before:
    featuregates.WithEnableForFeatureSets(configv1.TechPreviewNoUpgrade)
    // After:
    featuregates.WithEnableForFeatureSets(configv1.TechPreviewNoUpgrade, configv1.Default)
  3. Newly-activated code path (auth.go:102-108): When ExternalOIDCWithUpstreamParity is enabled, generateJWTForProvider now executes this block:

    if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) {
        userValidationRules, err := generateUserValidationRules(provider.UserValidationRules...)
        out.UserValidationRules = userValidationRules
    }
  4. Empty-slice initialization (auth.go:416): generateUserValidationRules initializes its return value as out := []UserValidationRule{} (an empty, non-nil slice). When called with no input rules (the test's OIDC provider has no UserValidationRules), the loop body never executes, and the function returns this empty slice.

  5. Test expectation mismatch (auth_test.go:118-141): The test case "When valid OIDC provider is provided..." builds its expected JWTAuthenticator struct but omits the UserValidationRules field entirely. In Go, an omitted slice field in a struct literal defaults to nil. Since cmp.Diff distinguishes nil from []UserValidationRule{}, the test fails.

This is purely a test update issue — the production behavior change is correct and intentional.

Recommendations

Option 1 (Preferred): Update the test expectation — Add UserValidationRules: []UserValidationRule{} to the expected JWTAuthenticator in the failing test case at auth_test.go line ~141, consistent with how the code now behaves with the feature gate enabled by default:

ClaimValidationRules: []ClaimValidationRule{},
UserValidationRules:  []UserValidationRule{},  // ← add this line

Option 2 (Alternative): Fix the production code to return nil for empty input — Change generateUserValidationRules at auth.go line 416 from out := []UserValidationRule{} to var out []UserValidationRule. This would make the function return nil when called with no rules. However, this could affect JSON serialization behavior (null vs [] with omitempty tag), so Option 1 is safer.

Evidence
Evidence Detail
Failing test TestGenerateAuthConfig/When_valid_OIDC_provider_is_provided... in auth_test.go:269
Diff output UserValidationRules: nil (expected) vs []kas.UserValidationRule{} (actual)
PR change featuregates.go:22 — adds configv1.Default to ExternalOIDCWithUpstreamParity enabled feature sets
Feature gate init featuregates.go:30init() configures Default feature set, so all tests run with it
Activated code path auth.go:102-108 — gate-protected block now executes, calling generateUserValidationRules
Empty slice source auth.go:416out := []UserValidationRule{} returns non-nil empty slice when no rules are provided
Missing test field auth_test.go:118-141 — expected struct omits UserValidationRules, defaulting to nil
Feature gate warning Logs show Setting GA feature gate ExternalOIDCWithUpstreamParity=true. It will be removed in a future release. — confirms gate is now active
Single failing package Only github.com/openshift/hypershift/.../v2/kas failed (2.951s); all other packages passed

@jparrill jparrill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dropped some comments. Thanks!

CI is red — Unit Tests (cpo-hostedcontrolplane) fails in TestGenerateAuthConfig/When_valid_OIDC_provider_is_provided at auth_test.go:269. The test expected UserValidationRules: nil but after the promotion the parity code path runs by default and produces UserValidationRules: []kas.UserValidationRule{}. The test expectation needs updating for the new default behavior.

/hold

Please hold until openshift/api#2915 merges and we do the vendor bump.

Comment thread control-plane-operator/featuregates/featuregates.go

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we also need to update

{
"name": "ExternalOIDCWithUpstreamParity"
},
so that it is in the set of enabled gates by default.

Without that, the new API fields will not be available by default.

@openshift-ci openshift-ci Bot added the area/api Indicates the PR includes changes for the API label Aug 12, 2026
@openshift-ci openshift-ci Bot added the area/cli Indicates the PR includes changes for CLI label Aug 12, 2026
Promote the ExternalOIDCWithUpstreamParity feature gate from
TechPreviewNoUpgrade to the Default feature set, enabling upstream
parity authentication configuration by default.

This change aligns with the openshift/api promotion in PR openshift#2915 and
allows the feature to be available in the default feature set once
the API changes are merged.

Signed-off-by: Shaza Aldawamneh <shaza.aldawamneh@hotmail.com>

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-azure-self-managed
/test e2e-v2-gke

@everettraven

Copy link
Copy Markdown
Contributor

/retest-required

1 similar comment
@ShazaAldawamneh

Copy link
Copy Markdown
Contributor Author

/retest-required

@everettraven

Copy link
Copy Markdown
Contributor

/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-external-oidc

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@everettraven: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-external-oidc

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0c26c830-971d-11f1-83f9-3097cf304ca8-0

@ShazaAldawamneh

Copy link
Copy Markdown
Contributor Author

/verified by @ShazaAldawamneh

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Aug 13, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@ShazaAldawamneh: This PR has been marked as verified by @ShazaAldawamneh.

Details

In response to this:

/verified by @ShazaAldawamneh

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 openshift-eng/jira-lifecycle-plugin repository.

@bryan-cox

Copy link
Copy Markdown
Member

/override ci/prow/e2e-v2-gke
/hold cancel

@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 Aug 13, 2026
@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-v2-gke

Details

In response to this:

/override ci/prow/e2e-v2-gke
/hold cancel

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.

@bryan-cox bryan-cox left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve

@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, everettraven, ShazaAldawamneh

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 Aug 13, 2026
@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@ShazaAldawamneh: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 2ac5e2f into openshift:main Aug 13, 2026
52 checks passed
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. area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants