Skip to content

Commit 12ab045

Browse files
author
Rohit Patil
committed
Fix TestExternalOIDCWithKeycloak timeout and add validation checks
- Increase OAuth validation timeout from 5min to 10min - Add authentication type verification before validation - Add post-update verification to detect config resets - Add progress logging during validation - Enhance debugging for authentication type changes Must-gather analysis showed OAuth cleanup was in progress but didn't complete within the original 5-minute timeout. The operator was actively deleting OAuth resources when the test timed out. Additional validation checks detect if spec.type is unexpectedly reset during reconciliation, providing actionable error messages instead of generic timeout failures.
1 parent 6246aa3 commit 12ab045

1 file changed

Lines changed: 50 additions & 2 deletions

File tree

test/e2e-oidc/external_oidc.go

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ func testExternalOIDCWithKeycloak(ctx context.Context, t testing.TB) {
154154
}
155155

156156
// Test 4: Validate OAuth state after OIDC configuration
157+
// First, verify the authentication resource still has the OIDC configuration
158+
currentAuth := testClient.getAuth(t, testCtx)
159+
require.Equal(t, testSpec.Type, currentAuth.Spec.Type,
160+
"authentication type changed after KAS rollout - expected %q, got %q (possible controller reset)",
161+
testSpec.Type, currentAuth.Spec.Type)
162+
t.Logf("Confirmed authentication type is still %q before OAuth state validation", currentAuth.Spec.Type)
163+
157164
testClient.validateOAuthState(t, testCtx, false, newExternalOIDCArchitectureEnabled)
158165
t.Logf("OAuth state validation passed")
159166

@@ -413,13 +420,31 @@ func (tc *testClient) updateAuthResource(t testing.TB, ctx context.Context, base
413420
spec := baseSpec.DeepCopy()
414421
updateAuthSpec(spec)
415422

423+
// Log the authentication type change for debugging
424+
t.Logf("Updating authentication resource: type %q -> %q", auth.Spec.Type, spec.Type)
425+
if len(spec.OIDCProviders) > 0 {
426+
t.Logf(" OIDC providers: %d", len(spec.OIDCProviders))
427+
}
428+
if spec.WebhookTokenAuthenticator != nil {
429+
t.Logf(" Webhook authenticator: %s", spec.WebhookTokenAuthenticator.KubeConfig.Name)
430+
}
431+
416432
auth.Spec = *spec
417433
auth, err := tc.configClient.ConfigV1().Authentications().Update(ctx, auth, metav1.UpdateOptions{})
418434
if err != nil {
419435
return nil, err
420436
}
421437

422-
require.True(t, equality.Semantic.DeepEqual(auth.Spec, *spec))
438+
require.True(t, equality.Semantic.DeepEqual(auth.Spec, *spec),
439+
"authentication spec mismatch after update - expected type=%q, got type=%q",
440+
spec.Type, auth.Spec.Type)
441+
442+
// Verify the update persisted by reading it back
443+
updatedAuth := tc.getAuth(t, ctx)
444+
require.Equal(t, spec.Type, updatedAuth.Spec.Type,
445+
"authentication type changed after update - expected %q, got %q (possible webhook or controller reset)",
446+
spec.Type, updatedAuth.Spec.Type)
447+
t.Logf("Confirmed authentication resource updated successfully with type=%q", updatedAuth.Spec.Type)
423448

424449
return auth, nil
425450
}
@@ -546,14 +571,37 @@ func (tc *testClient) validateOAuthState(t testing.TB, ctx context.Context, requ
546571
dynamicClient, err := dynamic.NewForConfig(tc.kubeConfig)
547572
require.NoError(t, err, "unexpected error while creating dynamic client")
548573

574+
// Verify the authentication resource has the expected type before validating OAuth state
575+
auth := tc.getAuth(t, ctx)
576+
if requireMissing {
577+
// When OAuth resources should be missing, we expect type=OIDC (or webhook for new arch)
578+
expectedType := configv1.AuthenticationTypeOIDC
579+
if newExternalOIDCArchitectureEnabled && len(auth.Spec.WebhookTokenAuthenticators) > 0 {
580+
expectedType = ""
581+
}
582+
actualType := auth.Spec.Type
583+
if actualType != expectedType {
584+
t.Logf("WARNING: Authentication type mismatch before validation - expected %q, got %q", expectedType, actualType)
585+
t.Logf("Full authentication spec: %+v", auth.Spec)
586+
}
587+
}
588+
549589
var validationErrs []error
550-
waitErr := wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, false, func(_ context.Context) (bool, error) {
590+
// Increased timeout from 5 minutes to 10 minutes to allow for slower OAuth resource cleanup
591+
// See: https://issues.redhat.com/browse/OCPBUGS-XXXXX
592+
waitErr := wait.PollUntilContextTimeout(ctx, 30*time.Second, 10*time.Minute, false, func(_ context.Context) (bool, error) {
551593
validationErrs = make([]error, 0)
552594
validationErrs = append(validationErrs, validateOAuthResources(ctx, dynamicClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
553595
validationErrs = append(validationErrs, validateOAuthRoutes(ctx, tc.routeClient, tc.configClient, requireMissing)...)
554596
validationErrs = append(validationErrs, validateOAuthControllerConditions(tc.operatorClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
555597
validationErrs = append(validationErrs, validateOperandVersions(ctx, tc.configClient, requireMissing, newExternalOIDCArchitectureEnabled)...)
556598
validationErrs = append(validationErrs, validateOAuthRelatedObjects(ctx, tc.configClient, requireMissing)...)
599+
600+
// Log progress every iteration to help debug timeout issues
601+
if len(validationErrs) > 0 {
602+
t.Logf("OAuth state validation in progress, %d validation errors remaining", len(validationErrs))
603+
}
604+
557605
return len(validationErrs) == 0, nil
558606
})
559607

0 commit comments

Comments
 (0)