Skip to content

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Dec 1, 2025

Summary

VirtualMCPServer with type: kubernetes or type: configMap OIDC configuration was not working because the converter used hand-rolled conversions instead of implementing the pre-existing OIDCConfigurable interface that the OIDC resolver requires.

This PR:

  • Implements OIDCConfigurable interface on VirtualMCPServer (adds GetOIDCConfig() and GetProxyPort() methods)
  • Refactors Converter to require an OIDC resolver and use it for all OIDC types (kubernetes, configMap, inline)
  • Ensures fail-closed behavior: if OIDC resolution fails, deployment fails rather than silently deploying without authentication
  • Adds comprehensive tests using gomock for OIDC resolution

Changes

  1. Add mockgen directive for OIDC Resolver - Enables proper unit testing with mock resolvers
  2. Implement OIDCConfigurable on VirtualMCPServer - Adds the interface methods needed by the OIDC resolver
  3. Refactor Converter to use OIDC resolver - Removes hand-rolled inline-only handling and uses the unified resolver for all OIDC types

Test plan

  • Unit tests pass for all OIDC config types (kubernetes, configMap, inline)
  • Fail-closed behavior verified: resolver errors cause conversion to fail
  • Existing tests updated to use mock resolver
  • Manual testing with kubernetes OIDC type in a real cluster

Fixes #2820

🤖 Generated with Claude Code

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Dec 1, 2025
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 77.35849% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.65%. Comparing base (f57d84a) to head (eeb373e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...hv-operator/api/v1alpha1/virtualmcpserver_types.go 0.00% 6 Missing ⚠️
cmd/thv-operator/pkg/vmcpconfig/converter.go 90.69% 2 Missing and 2 partials ⚠️
...perator/controllers/virtualmcpserver_vmcpconfig.go 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@ ## main #2821 +/- ## ========================================== + Coverage 56.59% 56.65% +0.05%  ========================================== Files 322 322 Lines 31247 31279 +32 ========================================== + Hits 17685 17721 +36  + Misses 12042 12040 -2  + Partials 1520 1518 -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.
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 3, 2025
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 3, 2025
Add go:generate directive to generate gomock mock for the oidc.Resolver interface. This enables proper unit testing of components that depend on OIDC resolution without requiring a real Kubernetes client.
Implement GetOIDCConfig() and GetProxyPort() methods on VirtualMCPServer to satisfy the oidc.OIDCConfigurable interface. This allows the OIDC resolver to resolve Kubernetes and ConfigMap OIDC configurations for VirtualMCPServer resources. GetProxyPort returns 4483, the default vMCP server port.
Previously, the Converter only handled inline OIDC configuration directly and had a TODO for kubernetes and configMap types. This change: - Makes oidcResolver a required parameter for NewConverter (returns error if nil) - Uses the resolver for all OIDC types (kubernetes, configMap, inline) - Adds mapResolvedOIDCToVmcpConfig() to map resolver output to vmcp config - Handles ClientSecretEnv for all OIDC types that may have client secrets - Updates controller to pass OIDC resolver to converter - Adds comprehensive tests using gomock for OIDC resolution - Adds compile-time interface assertion for VirtualMCPServer This ensures fail-closed behavior: if OIDC is configured but resolution fails, the converter returns an error rather than silently deploying without authentication.
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 3, 2025
@jhrozek jhrozek merged commit 62f4122 into main Dec 3, 2025
32 checks passed
@jhrozek jhrozek deleted the oidc_refactor branch December 3, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

3 participants