Skip to content

Commit 3fedad2

Browse files
Rohit Patilclaude
authored andcommitted
Refactor OIDC test suite structure and fix review feedback
This commit improves the OIDC test suite organization by making tag semantics more accurate and addressing code review feedback. Changes: - Remove [OIDC] tag from IntegratedOAuth IDP tests (Keycloak/GitLab) These tests use OIDC providers as identity sources for OpenShift's IntegratedOAuth, not external OIDC authentication. - Simplify External OIDC test tags: [OIDC] only (remove [Serial][Disruptive]) This test runs in its own dedicated suite via OTE framework. - Add new OTE suite: operator-oidc/serial Dedicated suite for external OIDC tests that bypass IntegratedOAuth. - Update serial suite qualifier to properly match [Serial] tests and non-disruptive operator/template/token tests. - Fix context usage in external_oidc.go poll function Use poll-derived context instead of outer context to ensure API calls are cancelled when poll times out. - Add nsCleanup() to idpdeployment.go cleanup function Fixes resource leak by properly cleaning up test namespaces. - Remove placeholder bug reference (OCPBUGS-XXXXX) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 73c453f commit 3fedad2

5 files changed

Lines changed: 24 additions & 13 deletions

File tree

cmd/cluster-authentication-operator-tests-ext/main.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,16 @@ func prepareOperatorTestsRegistry() (*oteextension.Registry, error) {
7777

7878
// The following suite runs tests that must execute serially (one at a time)
7979
// because they modify cluster-wide resources like OAuth configuration.
80-
// Tests tagged with [Serial] and any of [Operator], [OIDC], [Templates], [Tokens] are included in this suite.
80+
// Tests tagged with [Serial] are included, as well as tests with [Operator], [Templates], or [Tokens].
8181
// Excludes [Disruptive] tests which run in the disruptive suite instead.
82-
// ClusterStability set to Disruptive: OIDC tests temporarily put authentication operator
83-
// in Degraded state during cleanup when IDP configuration is rolled back.
82+
// ClusterStability set to Disruptive: IntegratedOAuth IDP tests (Keycloak/GitLab) temporarily
83+
// put authentication operator in Degraded state during cleanup when IDP configuration is rolled back.
8484
extension.AddSuite(oteextension.Suite{
8585
Name: "openshift/cluster-authentication-operator/operator/serial",
8686
Parallelism: 1,
8787
ClusterStability: oteextension.ClusterStabilityDisruptive,
8888
Qualifiers: []string{
89-
`name.contains("[Serial]") && !name.contains("[Disruptive]") && (name.contains("[Operator]") || name.contains("[OIDC]") || name.contains("[Templates]") || name.contains("[Tokens]"))`,
89+
`!name.contains("[Disruptive]") && (name.contains("[Serial]") || name.contains("[Operator]") || name.contains("[Templates]") || name.contains("[Tokens]"))`,
9090
},
9191
})
9292

@@ -110,6 +110,17 @@ func prepareOperatorTestsRegistry() (*oteextension.Registry, error) {
110110
},
111111
})
112112

113+
// The following suite runs external OIDC tests that authenticate directly via external OIDC
114+
// (bypassing OpenShift's IntegratedOAuth). These are long-running tests in their own suite.
115+
extension.AddSuite(oteextension.Suite{
116+
Name: "openshift/cluster-authentication-operator/operator-oidc/serial",
117+
Parallelism: 1,
118+
ClusterStability: oteextension.ClusterStabilityDisruptive,
119+
Qualifiers: []string{
120+
`name.contains("[OIDC]")`,
121+
},
122+
})
123+
113124
// ClusterStability set to Disruptive: encryption perf tests trigger API server rollouts.
114125
extension.AddSuite(oteextension.Suite{
115126
Name: "openshift/cluster-authentication-operator/operator-encryption-perf/serial",

test/e2e-oidc/external_oidc.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import (
5151
)
5252

5353
var _ = g.Describe("[sig-auth] authentication operator", func() {
54-
g.It("[OIDC][Serial][Disruptive] TestExternalOIDCWithKeycloak [Timeout:3h]", func(ctx context.Context) {
54+
g.It("[OIDC] TestExternalOIDCWithKeycloak [Timeout:3h]", func(ctx context.Context) {
5555
testExternalOIDCWithKeycloak(ctx, g.GinkgoTB())
5656
})
5757
})
@@ -849,14 +849,13 @@ func (tc *testClient) validateOAuthState(t testing.TB, ctx context.Context, requ
849849

850850
var validationErrs []error
851851
// Increased timeout from 5 minutes to 10 minutes to allow for slower OAuth resource cleanup
852-
// See: https://issues.redhat.com/browse/OCPBUGS-XXXXX
853-
waitErr := wait.PollUntilContextTimeout(ctx, 30*time.Second, 10*time.Minute, false, func(_ context.Context) (bool, error) {
852+
waitErr := wait.PollUntilContextTimeout(ctx, 30*time.Second, 10*time.Minute, false, func(pollCtx context.Context) (bool, error) {
854853
validationErrs = make([]error, 0)
855-
validationErrs = append(validationErrs, validateOAuthResources(ctx, dynamicClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
856-
validationErrs = append(validationErrs, validateOAuthRoutes(ctx, tc.routeClient, tc.configClient, requireMissing)...)
854+
validationErrs = append(validationErrs, validateOAuthResources(pollCtx, dynamicClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
855+
validationErrs = append(validationErrs, validateOAuthRoutes(pollCtx, tc.routeClient, tc.configClient, requireMissing)...)
857856
validationErrs = append(validationErrs, validateOAuthControllerConditions(tc.operatorClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
858-
validationErrs = append(validationErrs, validateOperandVersions(ctx, tc.configClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
859-
validationErrs = append(validationErrs, validateOAuthRelatedObjects(ctx, tc.configClient, requireMissing)...)
857+
validationErrs = append(validationErrs, validateOperandVersions(pollCtx, tc.configClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
858+
validationErrs = append(validationErrs, validateOAuthRelatedObjects(pollCtx, tc.configClient, requireMissing)...)
860859

861860
// Log progress every iteration to help debug timeout issues
862861
if len(validationErrs) > 0 {

test/e2e/gitlab.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
)
1111

1212
var _ = g.Describe("[sig-auth] authentication operator", func() {
13-
g.It("[OIDC][Serial] TestGitLabAsOIDCPasswordGrantCheck", func() {
13+
g.It("[Serial] TestGitLabAsOIDCPasswordGrantCheck", func() {
1414
testGitLabAsOIDCPasswordGrantCheck(g.GinkgoTB())
1515
})
1616
})

test/e2e/keycloak.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
)
2727

2828
var _ = g.Describe("[sig-auth] authentication operator", func() {
29-
g.It("[OIDC][Serial] TestKeycloakAsOIDCPasswordGrantCheckAndGroupSync", func() {
29+
g.It("[Serial] TestKeycloakAsOIDCPasswordGrantCheckAndGroupSync", func() {
3030
testKeycloakAsOIDCPasswordGrantCheckAndGroupSync(g.GinkgoTB())
3131
})
3232
})

test/library/idpdeployment.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ func deployPodWithImageStream(
136136
cleanup = func() {
137137
isCleanup()
138138
podCleanup()
139+
nsCleanup()
139140
}
140141

141142
return namespace, host, cleanup

0 commit comments

Comments
 (0)