test ci

[gangwgr]

Summary by CodeRabbit

• Tests
  • Added end-to-end test suite validating OIDC authentication configuration, operator health status, and component deployment behavior.

[openshift-merge-bot]

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: automatic mode

[coderabbitai]

Walkthrough

This pull request adds a new end-to-end test suite validating the ExternalOIDCExternalClaimsSourcing feature gate behavior. The test deploys a Keycloak instance, configures OIDC authentication with the feature gate enabled, and verifies that structured authentication configuration is absent and the integrated OAuth stack is removed.

Changes

OIDC Feature Gate Test Suite

Layer / File(s)	Summary
Test suite documentation and infrastructure setup test/extended/authentication/external_oidc_claims_sourcing.go	Package declaration and documentation describe CNTRLPLANE-2991 expectations. Ginkgo test harness is initialized with CLI context and shared state. Keycloak is deployed, an OIDC client secret is created in openshift-config, and the original authentication configuration is captured for restoration.
OIDC feature gate behavior assertions test/extended/authentication/external_oidc_claims_sourcing.go	Nested ordered test scope configures OIDC with feature gate enabled and verifies that authConfig is absent from KubeAPI Servers operator observed config, auth-config ConfigMap does not exist, oauth-metadata ConfigMap is eventually deleted, operator is healthy, and oauth-openshift Deployment is removed.
Test teardown and resource cleanup test/extended/authentication/external_oidc_claims_sourcing.go	AfterAll cleanup restores authentication to the original state with conditional rollout and executes Keycloak resource teardown.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	Lines 111-112 have syntax error with extra err parameter; test 1 violates single responsibility; missing assertion message on line 122 Eventually.Should().	Fix syntax at 111-112; split test 1 into separate assertions; add message to line 122 Eventually.Should call.
Microshift Test Compatibility	⚠️ Warning	Test uses config.openshift.io and operator.openshift.io APIs, and openshift-kube-apiserver namespace unavailable on MicroShift, with no apigroup tags or runtime checks.	Add [apigroup:config.openshift.io] tag to test name or guard with exutil.IsMicroShiftCluster() check with g.Skip(). Test requires OpenShift APIs absent on MicroShift.
Title check	❓ Inconclusive	The title 'test ci' is vague and generic, failing to convey meaningful information about the changeset, which adds a comprehensive end-to-end test suite for external OIDC claims sourcing.	Replace with a more descriptive title such as 'Add E2E test for external OIDC claims sourcing with feature gate' to clearly communicate the primary change.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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	All Ginkgo test names are stable and deterministic, containing no dynamic identifiers, timestamps, or generated values. Dynamic infrastructure is only in setup code, not test titles.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Test validates OIDC auth via cluster-wide API and operator checks. No multi-node assumptions: no pod anti-affinity, topology constraints, node communication, or multiple replica expectations.
Topology-Aware Scheduling Compatibility	✅ Passed	This PR adds only a Ginkgo test file with no deployment manifests, operator code, or controllers. The file contains no scheduling constraints, affinity rules, or topology-related configurations.
Ote Binary Stdout Contract	✅ Passed	File contains only Ginkgo test code with proper block structure. Process-level code has no stdout writes; fmt.Sprintf calls are inside g.BeforeAll (test-level), not at process level.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Test uses image mirroring for Keycloak, no hardcoded IPv4 addresses, no IPv4-specific IP parsing, safe URL building with net.JoinPath(), and only internal cluster connectivity.
No-Weak-Crypto	✅ Passed	The test file contains no MD5, SHA1, DES, RC4, 3DES, Blowfish, or ECB usage; no custom crypto implementations; and no non-constant-time secret comparisons.
Container-Privileges	✅ Passed	Test file adds Keycloak deployment without container-level privileged settings: no privileged=true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs	✅ Passed	No direct logging of sensitive data (passwords, tokens, API keys, PII, etc.) found in the new test file external_oidc_claims_sourcing.go. Credentials are used but not logged.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gangwgr
Once this PR has been reviewed and has the lgtm label, please assign liouk 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:

• test/extended/authentication/OWNERS

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

[coderabbitai]

🧹 Nitpick comments (2)

test/extended/authentication/external_oidc_claims_sourcing.go (2)

125-139: ⚡ Quick win

Health check passes vacuously if conditions are missing.

The loop only asserts Available/Degraded when those condition types are present in Status.Conditions. If either condition is absent (e.g., the operator hasn't published it yet), the test silently passes without verifying health. Consider asserting that both conditions were actually observed.

♻️ Proposed change

+		var sawAvailable, sawDegraded bool
 		for _, cond := range kas.Status.Conditions {
 			switch cond.Type {
 			case "Available":
+				sawAvailable = true
 				o.Expect(cond.Status).To(o.Equal(operatorv1.ConditionTrue),
 					"kube-apiserver operator should be Available")
 			case "Degraded":
+				sawDegraded = true
 				o.Expect(cond.Status).To(o.Equal(operatorv1.ConditionFalse),
 					"kube-apiserver operator should not be Degraded")
 			}
 		}
+		o.Expect(sawAvailable).To(o.BeTrue(), "kube-apiserver operator should report an Available condition")
+		o.Expect(sawDegraded).To(o.BeTrue(), "kube-apiserver operator should report a Degraded condition")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/authentication/external_oidc_claims_sourcing.go` around lines
125 - 139, The health-check loop over kas.Status.Conditions can pass vacuously
if the "Available" or "Degraded" conditions are missing; update the test in the
It block that fetches kas (variable kas from
oc.AdminOperatorClient().OperatorV1().KubeAPIServers().Get) to track whether
"Available" and "Degraded" were seen (e.g., sawAvailable, sawDegraded) while
iterating kas.Status.Conditions, keep the existing expectations for Available ==
operatorv1.ConditionTrue and Degraded == operatorv1.ConditionFalse, and after
the loop add explicit assertions that both sawAvailable and sawDegraded are true
so the test fails if either condition is absent.

---

27-32: 💤 Low value

Fix misplaced defer g.GinkgoRecover() and tighten operator health assertions

• defer g.GinkgoRecover() is placed in the top-level Describe container body, so it doesn’t protect running specs; move/remove it (only needed in goroutines).

  ♻️ Proposed change

  var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow][Disruptive]", g.Ordered, func() {

• defer g.GinkgoRecover()
  oc := exutil.NewCLIWithoutNamespace("oidc-claims-sourcing-e2e")

</details>
- “should keep the kube-apiserver operator healthy” only asserts when it sees `Available`/`Degraded` condition types in `kas.Status.Conditions`; if those conditions are missing, the spec can pass without checking health—add explicit expectations that both condition types are present (and asserted) before finishing.

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @test/extended/authentication/external_oidc_claims_sourcing.go around lines
27 - 32, Move the misplaced defer g.GinkgoRecover() out of the top-level
g.Describe body (remove it there and only use defer g.GinkgoRecover() inside any
goroutines or helper functions that spawn goroutines) so it actually protects
goroutines rather than the Describe registration; then update the
health-checking spec that inspects kas.Status.Conditions (the "should keep the
kube-apiserver operator healthy" check) to explicitly assert that both condition
types "Available" and "Degraded" are present in kas.Status.Conditions before
asserting their Status values—add expectations that the condition entries exist
(and fail the test if missing) and only then check their .Status values to
tighten the operator health assertions.

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @test/extended/authentication/external_oidc_claims_sourcing.go:

• Around line 125-139: The health-check loop over kas.Status.Conditions can pass
  vacuously if the "Available" or "Degraded" conditions are missing; update the
  test in the It block that fetches kas (variable kas from
  oc.AdminOperatorClient().OperatorV1().KubeAPIServers().Get) to track whether
  "Available" and "Degraded" were seen (e.g., sawAvailable, sawDegraded) while
  iterating kas.Status.Conditions, keep the existing expectations for Available ==
  operatorv1.ConditionTrue and Degraded == operatorv1.ConditionFalse, and after
  the loop add explicit assertions that both sawAvailable and sawDegraded are true
  so the test fails if either condition is absent.
• Around line 27-32: Move the misplaced defer g.GinkgoRecover() out of the
  top-level g.Describe body (remove it there and only use defer g.GinkgoRecover()
  inside any goroutines or helper functions that spawn goroutines) so it actually
  protects goroutines rather than the Describe registration; then update the
  health-checking spec that inspects kas.Status.Conditions (the "should keep the
  kube-apiserver operator healthy" check) to explicitly assert that both condition
  types "Available" and "Degraded" are present in kas.Status.Conditions before
  asserting their Status values—add expectations that the condition entries exist
  (and fail the test if missing) and only then check their .Status values to
  tighten the operator health assertions.

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

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

**Review profile**: CHILL

**Plan**: Enterprise

**Run ID**: `6da88fe8-744f-43d6-91c1-d04a4a420837`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 76ed5a820c471faf71cdf93ec06faebff542bf20 and 246903274dd6b26fcc6c3898ab17543b14145d0e.

</details>

<details>
<summary>📒 Files selected for processing (1)</summary>

* `test/extended/authentication/external_oidc_claims_sourcing.go`

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[openshift-ci]

@gangwgr: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify	2469032	link	true	/test verify

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.