Skip to content

Commit 62f4122

Browse files
authored
Fix VirtualMCPServer OIDC configuration by implementing OIDCConfigurable interface (#2821)
* Add mockgen directive for OIDC Resolver interface 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. * Add OIDCConfigurable interface implementation to VirtualMCPServer 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. * Refactor Converter to require OIDC resolver for all OIDC types 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.
1 parent f57d84a commit 62f4122

File tree

7 files changed

+431
-55
lines changed

7 files changed

+431
-55
lines changed

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,23 @@ type VirtualMCPServerList struct {
566566
Items []VirtualMCPServer `json:"items"`
567567
}
568568

569+
// GetOIDCConfig returns the OIDC configuration reference for incoming auth.
570+
// This implements the OIDCConfigurable interface to allow the OIDC resolver
571+
// to resolve Kubernetes and ConfigMap OIDC configurations.
572+
func (v *VirtualMCPServer) GetOIDCConfig() *OIDCConfigRef {
573+
if v.Spec.IncomingAuth == nil {
574+
return nil
575+
}
576+
return v.Spec.IncomingAuth.OIDCConfig
577+
}
578+
579+
// GetProxyPort returns the proxy port for the VirtualMCPServer.
580+
// This implements the OIDCConfigurable interface.
581+
// vMCP uses port 4483 by default.
582+
func (*VirtualMCPServer) GetProxyPort() int32 {
583+
return 4483
584+
}
585+
569586
func init() {
570587
SchemeBuilder.Register(&VirtualMCPServer{}, &VirtualMCPServerList{})
571588
}

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sigs.k8s.io/controller-runtime/pkg/log"
1414

1515
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
16+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc"
1617
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
1718
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/vmcpconfig"
1819
"github.com/stacklok/toolhive/pkg/vmcp/workloads"
@@ -25,8 +26,14 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
2526
) error {
2627
ctxLogger := log.FromContext(ctx)
2728

28-
// Convert CRD to vmcp config using converter
29-
converter := vmcpconfig.NewConverter()
29+
// Create OIDC resolver to handle all OIDC types (kubernetes, configMap, inline)
30+
oidcResolver := oidc.NewResolver(r.Client)
31+
32+
// Convert CRD to vmcp config using converter with OIDC resolver
33+
converter, err := vmcpconfig.NewConverter(oidcResolver)
34+
if err != nil {
35+
return fmt.Errorf("failed to create vmcp converter: %w", err)
36+
}
3037
config, err := converter.Convert(ctx, vmcp)
3138
if err != nil {
3239
return fmt.Errorf("failed to create vmcp Config from VirtualMCPServer: %w", err)

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/stretchr/testify/assert"
2222
"github.com/stretchr/testify/require"
23+
"go.uber.org/mock/gomock"
2324
"gopkg.in/yaml.v3"
2425
corev1 "k8s.io/api/core/v1"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -28,10 +29,29 @@ import (
2829
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2930

3031
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
32+
oidcmocks "github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc/mocks"
3133
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/vmcpconfig"
3234
"github.com/stacklok/toolhive/pkg/vmcp"
3335
)
3436

37+
// newNoOpMockResolver creates a mock resolver that returns (nil, nil) for all calls.
38+
// Use this in tests that don't care about OIDC configuration.
39+
func newNoOpMockResolver(t *testing.T) *oidcmocks.MockResolver {
40+
t.Helper()
41+
ctrl := gomock.NewController(t)
42+
mockResolver := oidcmocks.NewMockResolver(ctrl)
43+
mockResolver.EXPECT().Resolve(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
44+
return mockResolver
45+
}
46+
47+
// newTestConverter creates a Converter with the given resolver, failing the test if creation fails.
48+
func newTestConverter(t *testing.T, resolver *oidcmocks.MockResolver) *vmcpconfig.Converter {
49+
t.Helper()
50+
converter, err := vmcpconfig.NewConverter(resolver)
51+
require.NoError(t, err)
52+
return converter
53+
}
54+
3555
// TestCreateVmcpConfigFromVirtualMCPServer tests vmcp config generation
3656
func TestCreateVmcpConfigFromVirtualMCPServer(t *testing.T) {
3757
t.Parallel()
@@ -65,7 +85,7 @@ func TestCreateVmcpConfigFromVirtualMCPServer(t *testing.T) {
6585
t.Run(tt.name, func(t *testing.T) {
6686
t.Parallel()
6787

68-
converter := vmcpconfig.NewConverter()
88+
converter := newTestConverter(t, newNoOpMockResolver(t))
6989
config, err := converter.Convert(context.Background(), tt.vmcp)
7090

7191
require.NoError(t, err)
@@ -138,7 +158,7 @@ func TestConvertOutgoingAuth(t *testing.T) {
138158
},
139159
}
140160

141-
converter := vmcpconfig.NewConverter()
161+
converter := newTestConverter(t, newNoOpMockResolver(t))
142162
config, err := converter.Convert(context.Background(), vmcpServer)
143163
require.NoError(t, err)
144164

@@ -198,7 +218,7 @@ func TestConvertBackendAuthConfig(t *testing.T) {
198218
},
199219
}
200220

201-
converter := vmcpconfig.NewConverter()
221+
converter := newTestConverter(t, newNoOpMockResolver(t))
202222
config, err := converter.Convert(context.Background(), vmcpServer)
203223
require.NoError(t, err)
204224

@@ -292,7 +312,7 @@ func TestConvertAggregation(t *testing.T) {
292312
},
293313
}
294314

295-
converter := vmcpconfig.NewConverter()
315+
converter := newTestConverter(t, newNoOpMockResolver(t))
296316
config, err := converter.Convert(context.Background(), vmcpServer)
297317
require.NoError(t, err)
298318

@@ -385,7 +405,7 @@ func TestConvertCompositeTools(t *testing.T) {
385405
},
386406
}
387407

388-
converter := vmcpconfig.NewConverter()
408+
converter := newTestConverter(t, newNoOpMockResolver(t))
389409
config, err := converter.Convert(context.Background(), vmcpServer)
390410
require.NoError(t, err)
391411

@@ -564,7 +584,7 @@ func TestYAMLMarshalingDeterminism(t *testing.T) {
564584
},
565585
}
566586

567-
converter := vmcpconfig.NewConverter()
587+
converter := newTestConverter(t, newNoOpMockResolver(t))
568588

569589
// Marshal the config 10 times to ensure deterministic output
570590
const iterations = 10

cmd/thv-operator/pkg/oidc/mocks/mock_resolver.go

Lines changed: 138 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/pkg/oidc/resolver.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ type OIDCConfigurable interface {
4747
GetProxyPort() int32
4848
}
4949

50+
//go:generate mockgen -destination=mocks/mock_resolver.go -package=mocks -source=resolver.go Resolver
51+
5052
// Resolver is the interface for resolving OIDC configuration from various sources
5153
type Resolver interface {
5254
// Resolve takes any resource implementing OIDCConfigurable and resolves its OIDC config

0 commit comments

Comments
 (0)