CNTRLPLANE-2589: Migrate test/e2e-oidc to OTE + bug fixes(imagestream,scc,Pathological Event Monitor)#945
Open
ropatil010 wants to merge 3 commits into
Open
CNTRLPLANE-2589: Migrate test/e2e-oidc to OTE + bug fixes(imagestream,scc,Pathological Event Monitor)#945ropatil010 wants to merge 3 commits into
ropatil010 wants to merge 3 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@ropatil010: This pull request references CNTRLPLANE-2589 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR wires a new external OIDC e2e suite into the test binary, splits the test entrypoint into a shared helper, updates shared wait and Keycloak helpers, and adds validation for OIDC configuration, OAuth state, and real Keycloak-backed authentication. ChangesExternal OIDC e2e suite
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Main as cmd/cluster-authentication-operator-tests-ext/main.go
participant Wrapper as test/e2e-oidc/external_oidc_test.go
participant Helper as testExternalOIDCWithKeycloak
participant Keycloak as Keycloak
participant Auth as authentication/cluster
participant KAS as kube-apiserver
Main->>Main: import test/e2e-oidc
Main->>Main: exclude [Disruptive] from serial suite
Wrapper->>Helper: delegate tt.Context() and tt
Helper->>Auth: update Authentication spec
Helper->>Keycloak: configure IdP and authenticate user
Helper->>KAS: validate rollout and submit SelfSubjectReview
Helper->>Auth: rollback original spec on cleanup
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 10❌ Failed checks (10 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ropatil010
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e-oidc/external_oidc.go (1)
188-202: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueTLS verification disabled +
Getwithout context.
InsecureSkipVerify: true(Lines 191-193) disables certificate validation for the issuer/JWKS fetch, andclient.Get(Lines 198, 218) ignores the ambientcontext.Context, so these calls won't honor test cancellation/timeouts. The IdP's CA is already available (thedefault-ingress-cert-derived bundle used at Line 510), so you can populateRootCAsinstead of skipping verification, and threadctxviahttp.NewRequestWithContext.🤖 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/e2e-oidc/external_oidc.go` around lines 188 - 202, The issuer/JWKS fetch in fetchIssuerJWKS is disabling TLS verification and using client.Get without any context, so update it to trust the IdP CA by configuring tls.Config.RootCAs from the existing default-ingress-cert-derived bundle instead of InsecureSkipVerify, and switch the OpenID/JWKS HTTP calls to http.NewRequestWithContext so they honor cancellation and timeouts. Apply the same pattern to both requests in fetchIssuerJWKS (and any related JWKS fetch helper) so the issuerURL lookups use the provided context and proper certificate validation.Source: Linters/SAST tools
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e-oidc/external_oidc.go`:
- Line 98: The `t.(*testing.T)` cast in `testExternalOIDCWithKeycloak` will
panic when the test runs through Ginkgo because `g.GinkgoTB()` is not a
`*testing.T`. Update `test.WaitForNewKASRollout` to accept `testing.TB` instead
of `*testing.T`, then pass `t` through directly at each call site in
`external_oidc.go` so both standard Go tests and Ginkgo paths work safely.
---
Nitpick comments:
In `@test/e2e-oidc/external_oidc.go`:
- Around line 188-202: The issuer/JWKS fetch in fetchIssuerJWKS is disabling TLS
verification and using client.Get without any context, so update it to trust the
IdP CA by configuring tls.Config.RootCAs from the existing
default-ingress-cert-derived bundle instead of InsecureSkipVerify, and switch
the OpenID/JWKS HTTP calls to http.NewRequestWithContext so they honor
cancellation and timeouts. Apply the same pattern to both requests in
fetchIssuerJWKS (and any related JWKS fetch helper) so the issuerURL lookups use
the provided context and proper certificate validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3abe9ac9-8754-40f0-9aa0-b88e55a0553d
📒 Files selected for processing (3)
cmd/cluster-authentication-operator-tests-ext/main.gotest/e2e-oidc/external_oidc.gotest/e2e-oidc/external_oidc_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e-oidc/external_oidc.go (1)
188-234: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPropagate the request context into
fetchIssuerJWKS.
fetchIssuerJWKStakes nocontext.Context, so bothclient.Getcalls run with an implicitcontext.Background()and cannot be cancelled with the test context. The only caller (testOIDCAuthentication, Line 845) already hasctxavailable. Thread it through and usehttp.NewRequestWithContext+client.Do(this also clears thenoctxlinter errors on Lines 198 and 218).As per coding guidelines: "When a function accepts or has access to a
context.Context, pass it through to downstream calls that accept one."🤖 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/e2e-oidc/external_oidc.go` around lines 188 - 234, `fetchIssuerJWKS` currently ignores the test context, so its HTTP requests cannot be cancelled and trigger the `noctx` issue. Update `fetchIssuerJWKS` to accept a `context.Context` from `testOIDCAuthentication`, then replace both `client.Get` calls with context-aware requests using `http.NewRequestWithContext` and `client.Do` so cancellation and timeouts propagate correctly.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e-oidc/external_oidc.go`:
- Around line 189-193: The fetchIssuerJWKS client is still bypassing TLS
verification with InsecureSkipVerify, so update that http.Client setup to trust
the staged ingress CA instead. Reuse the CA data synced into openshift-config
for this IdP, build a RootCAs cert pool, and wire it through tls.Config in
fetchIssuerJWKS so the discovery/JWKS requests validate certificates normally.
Keep the fix localized to the fetchIssuerJWKS path and the related ingress CA
handling in external_oidc.go.
---
Nitpick comments:
In `@test/e2e-oidc/external_oidc.go`:
- Around line 188-234: `fetchIssuerJWKS` currently ignores the test context, so
its HTTP requests cannot be cancelled and trigger the `noctx` issue. Update
`fetchIssuerJWKS` to accept a `context.Context` from `testOIDCAuthentication`,
then replace both `client.Get` calls with context-aware requests using
`http.NewRequestWithContext` and `client.Do` so cancellation and timeouts
propagate correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d6c703ba-a314-4200-81b9-0083aa72deed
📒 Files selected for processing (4)
cmd/cluster-authentication-operator-tests-ext/main.gotest/e2e-oidc/external_oidc.gotest/e2e-oidc/external_oidc_test.gotest/library/waits.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/cluster-authentication-operator-tests-ext/main.go
- test/e2e-oidc/external_oidc_test.go
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e-oidc/external_oidc.go`:
- Around line 1207-1215: The rollback cleanup in the authentication Get/Update
flow dereferences auth.Name even when Get returns an error and auth may be nil.
Update the error handling in the block that calls
tc.configClient.ConfigV1().Authentications().Get so it does not reference
auth.Name on failure; use a stable identifier like the cluster authentication
name directly in the fmt.Errorf messages, and keep the existing update path
unchanged.
- Around line 353-357: The retry callback in PollUntilContextTimeout for the
auth ConfigMap lookup is intentionally swallowing cmErr2 by returning nil, which
triggers nilerr; update the closure in the related wait logic to make the
retry-on-error behavior explicit by either returning cmErr2 when the error
should stop polling or adding a targeted suppression with a clear reason if
retries on transient Get failures are intended.
- Around line 399-409: The oidcAuthResponse struct is unused and should be
removed to satisfy linting. Delete the unused type definition from
external_oidc.go, and make sure no other code references oidcAuthResponse before
removing it.
- Line 111: Remove the debug log in external_oidc.go that prints
kcClient.AdminURL() from the e2e setup. The issue is the t.Logf call in the
external OIDC test path exposes Keycloak admin URL/hostname details; replace it
by either deleting the log or changing it to a non-sensitive message that does
not include the URL, keeping the rest of the test flow unchanged.
- Around line 432-463: The fetchIssuerJWKS helper currently performs blocking
HTTP discovery and JWKS retrieval without honoring test cancellation. Update
fetchIssuerJWKS to accept a ctx parameter from the caller and use it for both
the OpenID configuration request and the JWKS request via NewRequestWithContext
plus client.Do, so the test can stop these fetches when the context is canceled.
Make sure the existing fetchIssuerJWKS call sites pass the test context through
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2a70907d-032d-458f-81b5-46984472c74c
📒 Files selected for processing (4)
cmd/cluster-authentication-operator-tests-ext/main.gotest/e2e-oidc/external_oidc.gotest/e2e-oidc/external_oidc_test.gotest/library/waits.go
🚧 Files skipped from review as they are similar to previous changes (3)
- test/e2e-oidc/external_oidc_test.go
- test/library/waits.go
- cmd/cluster-authentication-operator-tests-ext/main.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/library/keycloakidp.go (1)
265-279: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
AuthenticatePassworddiscards caller's context instead of propagating it.The function creates its own timeout context from
context.Background()rather than accepting acontext.Contextparameter. The known caller (testOIDCAuthenticationintest/e2e-oidc/external_oidc.go) already holds a livectxwhen invokingkcClient.AuthenticatePassword(...)insidewait.PollUntilContextTimeout(ctx, ...), but that context is not passed through, so cancellation/deadline of the outer context won't propagate into this request.As per coding guidelines, "Propagate
context.Context- ... Never discard a context or substitutecontext.Background()/context.TODO()when a context is already available."♻️ Proposed fix
-func (kc *KeycloakClient) AuthenticatePassword(clientID, clientSecret, name, password string) error { +func (kc *KeycloakClient) AuthenticatePassword(ctx context.Context, clientID, clientSecret, name, password string) error { data := url.Values{ "username": []string{name}, "password": []string{password}, "grant_type": []string{"password"}, "client_id": []string{clientID}, "scope": []string{"openid"}, } if len(clientSecret) > 0 { data.Add("client_secret", clientSecret) } - reqCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + reqCtx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() authReq, err := http.NewRequestWithContext(reqCtx, http.MethodPost, kc.TokenURL(), bytes.NewBufferString(data.Encode()))This requires updating call sites accordingly.
🤖 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/library/keycloakidp.go` around lines 265 - 279, `AuthenticatePassword` is dropping the caller’s `context.Context` by building a new timeout from `context.Background()`, so cancellation and deadlines from `testOIDCAuthentication` and `wait.PollUntilContextTimeout` cannot flow through. Change `KeycloakClient.AuthenticatePassword` to accept a context parameter and use it for `http.NewRequestWithContext`, then update its call sites (including the one in `testOIDCAuthentication`) to pass the existing context instead of creating a new one.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e-oidc/external_oidc.go`:
- Around line 170-171: The failure message in the auth configmap deletion check
uses the wrong variable, so the timeout error from waitErr is hidden. Update the
assertion in the external_oidc test to format waitErr in the message for the
configmap deletion wait, keeping the cmErr assertion unchanged and ensuring the
message reflects the variable checked by the waitForAuthConfigMap deletion
logic.
---
Outside diff comments:
In `@test/library/keycloakidp.go`:
- Around line 265-279: `AuthenticatePassword` is dropping the caller’s
`context.Context` by building a new timeout from `context.Background()`, so
cancellation and deadlines from `testOIDCAuthentication` and
`wait.PollUntilContextTimeout` cannot flow through. Change
`KeycloakClient.AuthenticatePassword` to accept a context parameter and use it
for `http.NewRequestWithContext`, then update its call sites (including the one
in `testOIDCAuthentication`) to pass the existing context instead of creating a
new one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 72eee9d3-5192-4de5-af16-92f7c11ed976
📒 Files selected for processing (5)
cmd/cluster-authentication-operator-tests-ext/main.gotest/e2e-oidc/external_oidc.gotest/e2e-oidc/external_oidc_test.gotest/library/keycloakidp.gotest/library/waits.go
🚧 Files skipped from review as they are similar to previous changes (3)
- test/library/waits.go
- test/e2e-oidc/external_oidc_test.go
- cmd/cluster-authentication-operator-tests-ext/main.go
ropatil010
pushed a commit
to ropatil010/cluster-authentication-operator
that referenced
this pull request
Jul 6, 2026
This commit fixes three monitor test failures in PR openshift#945: 1. [Monitor:known-image-checker] - External image usage 2. [Monitor:audit-log-analyzer] PodSecurityViolation 3. [Monitor:legacy-test-framework-invariants-alerts] PodSecurityViolation Changes: 1. ImageStream Import Pattern (test/library/imagestream.go) - New ImportImageToImageStream() function imports external images (quay.io/keycloak:25.0, docker.io/gitlab-ce:13.8.4-ce.0) into OpenShift's internal registry - Ensures pods pull from image-registry.openshift-image-registry.svc:5000 - Fixes known-image-checker failures (100% compliance) 2. Configurable Security Context (test/library/idpdeployment.go) - New createContainerSecurityContext() factory for privileged vs restricted - New deployPodWithImageStream() wrapper integrates ImageStream import - Keycloak: Uses restricted PSA (non-root, no caps, no escalation) - GitLab: Uses privileged PSA (required for system services) - Conditional PSA enforcement and SCC RoleBinding creation - Fixes 50% of PodSecurityViolation errors (Keycloak now compliant) 3. IDP Integration - test/library/keycloakidp.go: Use deployPodWithImageStream + restricted mode - test/library/gitlabidp.go: Use deployPodWithImageStream + privileged mode Result: ✅ known-image-checker: 100% compliance (all images from internal registry) ✅ PodSecurityViolation: 50% reduction (Keycloak compliant, GitLab still requires privileged for PostgreSQL/Redis/nginx management) Vendored: github.com/openshift/client-go/image package Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ropatil010
force-pushed
the
test-e2e-oidc
branch
3 times, most recently
from
0d627ad to
16070ef
Compare
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
ropatil010
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
ropatil010
changed the title
Contributor
Author
|
/assign @liouk @everettraven The failure bug cases are not caused bec of OTE migrations. The failure cases were existing before migration itself. |
ropatil010
changed the title
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
@ropatil010, |
Contributor
|
@ropatil010, |
Contributor
Author
|
Ignore the CI profile e2e-aws-operator-encryption-serial-ote-1of2, e2e-aws-operator-encryption-serial-ote-2of2 Tested 945 with 944 PR. CI logs from this PR: |
Contributor
Author
|
/test e2e-agnostic |
liouk
suggested changes
Jul 8, 2026
liouk
left a comment
liouk
left a comment
Member
There was a problem hiding this comment.
Overall I believe that the current suite structure w.r.t OIDC tests can be improved. My suggestion would be:
- remove
[OIDC]fromTestKeycloakAsOIDCPasswordGrantCheckAndGroupSyncandTestGitLabAsOIDCPasswordGrantCheck; these two tests are testing OIDC providers over IntegratedOAuth functionality, so the tag is misleading. - Drop
[OIDC]from the serial suite qualifier entirely; also remove the Disruptive cluster stability from it. - Keep
[OIDC]only for external OIDC e2e tests, i.e.TestExternalOIDCWithKeycloak; also, give these tests their own long-running suite, similar to what has been currently defined in the Makefile.
Contributor
Author
Thanks @liouk for the inputs. I guess we need to add new CI profile for this OIDC suite. Do let me know your inputs on this |
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
ropatil010
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
ropatil010
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
All CI profiles are PASS. Currently there is no dedicated OTE CI profile for execution on e2e-oidc cases. |
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
Migrates OIDC and encryption test suites to OpenShift Tests Extension framework, consolidating serial test execution and fixing test suite qualifier issues that caused encryption test interference. Key changes: - Refactored OIDC test suite structure with improved helper functions - Fixed Keycloak token expiration issues - Resolved PSA violations in IDP deployments - Added ImageStream client support for Keycloak/GitLab image management - Fixed test suite qualifiers to prevent encryption test interference Test Suite Fixes: Previously, the operator/serial suite qualifier was too broad, catching ALL [Serial] tests including encryption tests. This caused TestPerfEncryptionTypeAESCBC (creates 15k tokens) and TestEncryptionTypeUnset (expects clean state) to run sequentially in the same CI job, leading to test failures when encrypted tokens persisted between tests. Changes to test suite qualifiers: - operator/parallel: Removed [Serial] restriction to include all operator, template, and token tests for parallel execution - operator/serial: Added exclusions for [Encryption] and [OIDC] tests, now only runs IntegratedOAuth IDP tests (Keycloak/GitLab) This ensures each encryption test suite runs in isolated CI jobs: - e2e-aws-operator-encryption-perf-serial-ote: Performance tests - e2e-aws-operator-encryption-serial-ote: Basic encryption tests - e2e-aws-operator-encryption-rotation-serial-ote: Rotation tests - e2e-aws-operator-oidc-serial-ote: External OIDC tests Root Cause Analysis: TestEncryptionTypeUnset failed because it ran immediately after TestPerfEncryptionTypeAESCBC in the same suite. The perf test created 15,007 OAuth tokens and started AES-CBC encryption migration, which takes 15-20 minutes to complete. TestEncryptionTypeUnset started before migration finished and found residual AES-CBC encrypted tokens instead of the expected unencrypted state, causing the test to fail with: "unexpected encrypted state, expected data to be 'unencrypted' but was 'encrypted' with mode 'aescbc'" The fix ensures clean cluster state between encryption test suites by running them in separate isolated CI jobs. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
Author
|
Earlier discussion to have single CI profiles and after execution concluded to use separate CI profiles. Waiting for dev review. Not sure if we still need to worry for the failure cases that are not related to this PR. How ever re-running the below failure CI profile. |
liouk
suggested changes
Jul 10, 2026
Member
|
… failures This commit consolidates test suite configuration and fixes critical test failures in the encryption test suite. Changes: 1. Suite Consolidation (main.go): - Consolidated serial tests into operator-encryption/serial suite - Removed redundant suites (operator/serial, encryption-perf, encryption-rotation) - Added [Parallel] tag to parallel tests for consistent filtering - Applied uniform exclusion filters across all suites - Simplified suite filters using KMS-style substring matching 2. Test Tag Updates: - Updated [OIDC] to [ExternalOIDC] for external OIDC tests - Added [Parallel] tags to: certs, custom_route, templates, tokentimeout - Added [Timeout:30m] to TestPerfEncryptionTypeAESCBC - Added [Timeout:30m] to TestEncryptionTypeUnset 3. Critical Fix: TestEncryptionTypeUnset (encryption.go): - Root cause: Test asserted encryption state before StorageVersionMigration completed - Added waitForOAuthMigrationComplete() to poll migration status (20min timeout) - Added waitForAuthOperatorReady() to wait for ClusterOperator Progressing=False - Migration takes 8-15 minutes; previous code only waited for key changes (2-3 min) - Test now waits for actual data migration before asserting state Test Results: - All unit tests pass - All packages compile successfully - Test binaries build successfully Fixes: - TestEncryptionTypeUnset assertion failure (data encrypted when expected unencrypted) - TestPerfEncryptionTypeAESCBC user interruption (insufficient timeout) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update suite name from operator-encryption/serial to operator/serial to better reflect that it contains both encryption and OAuth/IDP tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
|
@ropatil010: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Team,
This PR is about:
Part 1: OTE Migration
1.1 Test Architecture Transformation: Dual-Mode Pattern Implementation:
1.2 Test Tags & Metadata
1.3 Comprehensive Test Coverage
The test suite now includes 6 complete sub-tests (previously only precondition checks):
- Verifies ConfigMap doesn't exist when authentication type is not OIDC
- Tests CAO auto-deletion when manually created
- Tests invalid CEL expressions for uid claim mapping
- Tests invalid CEL expressions for extras claim mapping
- Verifies admission controller rejects malformed expressions
- Tests degradation with invalid CA bundle
- Tests degradation with invalid issuer URL
- Confirms operator enters Degraded state appropriately
- Tests email claim with custom prefix (oidc-test:)
- Tests email claim with no prefix
- Tests sub claim with default issuer-based prefix
- Tests email claim with NoOpinion policy (auto-prefix)
- Verifies successful KAS rollout (when old architecture enabled)
- Full OAuth/OIDC state validation including JWT/JWKS verification
- Verifies ConfigMap exists when type is OIDC
- Tests CAO auto-overwrite when manually modified
- Tests configuration with non-existent claim
- Verifies rollout succeeds but authentication fails gracefully
Part 2: Critical Bug Fixes
2.1 Keycloak Token Expiration Bug
2.2 Pod Security Admission (PSA) Violations
Part 3: Library Enhancements
3.1 ImageStream Support (NEW - Addresses Known-Image-Checker Violations)
3.2 IDP Deployment Enhancements
3.3 Keycloak/GitLab Integration Updates
3.4 Wait Utilities Refactoring