Skip to content

Conversation

@intojhanurag
Copy link
Member

@intojhanurag intojhanurag commented Jan 18, 2026

Fixes #8314

Proposed Changes

  • Fix unit tests timing out when run inside Kubernetes-based CI environments
  • Refactor OIDC provider initialization from eager to lazy loading
  • Improve test isolation by removing network calls from constructor

Root Cause: The auth.NewVerifier() constructor eagerly initialized the OIDC provider by making HTTP calls to https://kubernetes.default.svc/.well-known/openid-configuration. In Kubernetes environments, DNS resolution succeeds but HTTP requests hang, blocking test execution. Locally, DNS resolution fails quickly and tests pass.

This violated unit test isolation by making real network calls during object construction, creating environment-dependent test behavior.

Solution

Implemented lazy initialization of the OIDC provider:

  1. Removed eager initialization from NewVerifier() constructor
  2. Added ensureProvider() method using double-checked locking pattern for thread-safe lazy initialization
  3. Provider now initializes only when OIDC authentication is enabled and verification is actually needed
  4. All verification methods (VerifyRequest, VerifyRequestFromSubject, VerifyRequestFromSubjectsWithFilters) now call ensureProvider() before performing authentication

Modified Files

  • pkg/auth/verifier.go: Implemented lazy OIDC provider initialization

Pre-review Checklist

  • At least 80% unit test coverage - Existing unit test coverage maintained
  • E2E tests for any new behavior - No new behavior; refactoring maintains existing functionality
  • Docs PR for any user-visible impact - No user-visible impact; internal implementation detail
  • Spec PR for any new API feature - No API changes
  • Conformance test for any change to the spec - No spec changes

Release Note

Fixed unit tests timing out when run in Kubernetes-based CI environments due to eager OIDC provider initialization making network calls during object construction.

@knative-prow knative-prow bot requested review from Leo6Leo and aliok January 18, 2026 05:58
@knative-prow knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 18, 2026
@knative-prow
Copy link

knative-prow bot commented Jan 18, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@intojhanurag intojhanurag marked this pull request as draft January 18, 2026 05:58
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 18, 2026
@intojhanurag intojhanurag marked this pull request as ready for review January 18, 2026 05:59
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 18, 2026
@intojhanurag
Copy link
Member Author

It is not ready for review . I am testing something. I will convert in draft in some time .

@codecov
Copy link

codecov bot commented Jan 18, 2026

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.09%. Comparing base (59b517c) to head (4091bef).

Files with missing lines Patch % Lines
pkg/auth/verifier.go 0.00% 23 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8844      +/-   ##
==========================================
- Coverage   51.12%   51.09%   -0.04%     
==========================================
  Files         409      409              
  Lines       21369    21383      +14     
==========================================
  Hits        10925    10925              
- Misses       9592     9606      +14     
  Partials      852      852              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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


// provider is valid, update it
v.m.Lock()
defer v.m.Unlock()
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the lock from initOIDCProvider() because the caller now holds the lock.

@intojhanurag intojhanurag marked this pull request as draft January 18, 2026 06:39
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit tests fails when running them inside a Kubernetes cluster

1 participant