Skip to content

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Jan 21, 2026

The WithOIDCConfig() function receives thvCABundle and jwksAuthTokenFile parameters but was not mapping them to OIDCConfig.CACertPath and OIDCConfig.AuthTokenFile. This meant custom CA certificates for OIDC validation were never used by the token validator.

Add the two missing field mappings so the proxyrunner can use custom CA certificates when validating OIDC tokens against issuers with non-public certificates (e.g., corporate Keycloak instances).

Related: #3141

Large PR Justification

There's a bunch of autogenerated changes. I don't know how to split the PR so that it would still cover the feature but be small. I could have added the types and then implementation but that seems suboptimal.

The WithOIDCConfig() function receives thvCABundle and jwksAuthTokenFile
parameters but was not mapping them to OIDCConfig.CACertPath and
OIDCConfig.AuthTokenFile. This meant custom CA certificates for OIDC
validation were never used by the token validator.

Add the two missing field mappings so the proxyrunner can use custom CA
certificates when validating OIDC tokens against issuers with non-public
certificates (e.g., corporate Keycloak instances).

Related: #3141
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Jan 21, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 57.02479% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.82%. Comparing base (8571282) to head (520f799).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...d/thv-operator/controllers/mcpserver_controller.go 13.79% 49 Missing and 1 partial ⚠️
cmd/thv-operator/pkg/oidc/resolver.go 88.88% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3391      +/-   ##
==========================================
- Coverage   64.85%   64.82%   -0.03%     
==========================================
  Files         375      377       +2     
  Lines       36626    36746     +120     
==========================================
+ Hits        23753    23821      +68     
- Misses      10999    11049      +50     
- Partials     1874     1876       +2     

☔ 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.

@jhrozek jhrozek dismissed github-actions[bot]’s stale review January 22, 2026 13:05

I provided the large PR justification in the opening comment.

@jhrozek jhrozek merged commit 1853ae9 into main Jan 22, 2026
38 checks passed
@jhrozek jhrozek deleted the k8s-ca-certs branch January 22, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants