Skip to content

Commit 8050a55

Browse files
jhrozekjhrozek
committed
Wire embedded auth server into VirtualMCPServer converter and deployment
The VirtualMCPServer operator lacked the ability to convert an inline EmbeddedAuthServerConfig into an authserver.RunConfig and mount the necessary secrets onto the vMCP pod. Export BuildAuthServerRunConfig from controllerutil (previously the unexported buildEmbeddedAuthServerRunnerConfig used only by MCPServer) and call it from the VirtualMCPServer converter. This avoids duplicating ~450 lines of conversion logic (signing keys, HMAC secrets, upstream providers, Redis storage with Sentinel) and ensures both controllers use identical mount paths, env var naming, and Redis key prefix format. Operator side: serialize the RunConfig as a separate ConfigMap key (authserver-config.yaml) alongside config.yaml. Mount auth server volumes and env vars onto the deployment via GenerateAuthServerVolumes and GenerateAuthServerEnvVars. Cross-validate auth server config against backend strategies via ValidateAuthServerIntegration. Binary side: load sibling authserver-config.yaml if present, construct EmbeddedAuthServer, and pass it to the server config. Fail closed on read errors other than file-not-found. Fixes #4284
1 parent 21cbbd6 commit 8050a55

10 files changed

Lines changed: 652 additions & 74 deletions

‎cmd/thv-operator/controllers/virtualmcpserver_controller.go‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"crypto/rand"
1111
"encoding/base64"
12+
stderrors "errors"
1213
"fmt"
1314
"maps"
1415
"reflect"
@@ -71,6 +72,17 @@ type AuthConfigError struct {
7172
Error error
7273
}
7374

75+
// SpecValidationError represents a spec validation failure that the user must fix.
76+
// Unlike transient errors, these should NOT trigger requeue — the controller sets
77+
// a status condition and waits for the user to update the spec.
78+
type SpecValidationError struct {
79+
Message string
80+
}
81+
82+
func (e *SpecValidationError) Error() string {
83+
return e.Message
84+
}
85+
7486
// VirtualMCPServerReconciler reconciles a VirtualMCPServer object
7587
//
7688
// Resource Cleanup Strategy:
@@ -383,6 +395,26 @@ func (*VirtualMCPServerReconciler) validateAuthServerConfig(
383395
return nil
384396
}
385397

398+
// handleSpecValidationError checks whether err is a SpecValidationError (user must fix the spec).
399+
// If so, it applies the already-set status conditions and returns nil (no requeue).
400+
// Otherwise it returns the original error unchanged for normal requeue handling.
401+
func (r *VirtualMCPServerReconciler) handleSpecValidationError(
402+
ctx context.Context,
403+
vmcp *mcpv1alpha1.VirtualMCPServer,
404+
statusManager virtualmcpserverstatus.StatusManager,
405+
err error,
406+
) error {
407+
var specErr *SpecValidationError
408+
if !stderrors.As(err, &specErr) {
409+
return err
410+
}
411+
ctxLogger := log.FromContext(ctx)
412+
if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
413+
ctxLogger.Error(applyErr, "Failed to apply status updates after spec validation error")
414+
}
415+
return nil
416+
}
417+
386418
// validateGroupRef validates that the referenced MCPGroup exists and is ready
387419
func (r *VirtualMCPServerReconciler) validateGroupRef(
388420
ctx context.Context,
@@ -660,8 +692,11 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
660692
return ctrl.Result{}, err
661693
}
662694

663-
// Ensure vmcp Config ConfigMap
664-
if err := r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames, statusManager); err != nil {
695+
// Ensure vmcp Config ConfigMap.
696+
// handleSpecValidationError converts SpecValidationError to nil (no requeue)
697+
// after applying status conditions, while passing through transient errors.
698+
if err := r.handleSpecValidationError(ctx, vmcp, statusManager,
699+
r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames, statusManager)); err != nil {
665700
ctxLogger.Error(err, "Failed to ensure vmcp Config ConfigMap")
666701
return ctrl.Result{}, err
667702
}

‎cmd/thv-operator/controllers/virtualmcpserver_deployment.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer(
123123
args := r.buildContainerArgsForVmcp(vmcp)
124124
volumeMounts, volumes := r.buildVolumesForVmcp(vmcp)
125125
env := r.buildEnvVarsForVmcp(ctx, vmcp, typedWorkloads)
126+
127+
// Add embedded auth server volumes and env vars if configured (inline config)
128+
if vmcp.Spec.AuthServerConfig != nil {
129+
authServerVolumes, authServerMounts := ctrlutil.GenerateAuthServerVolumes(vmcp.Spec.AuthServerConfig)
130+
authServerEnvVars := ctrlutil.GenerateAuthServerEnvVars(vmcp.Spec.AuthServerConfig)
131+
volumes = append(volumes, authServerVolumes...)
132+
volumeMounts = append(volumeMounts, authServerMounts...)
133+
env = append(env, authServerEnvVars...)
134+
}
126135
deploymentLabels, deploymentAnnotations := r.buildDeploymentMetadataForVmcp(ls, vmcp)
127136
deploymentTemplateLabels, deploymentTemplateAnnotations := r.buildPodTemplateMetadata(ls, vmcp, vmcpConfigChecksum)
128137
podSecurityContext, containerSecurityContext := r.buildSecurityContextsForVmcp(ctx, vmcp)

‎cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
4141
if err != nil {
4242
return fmt.Errorf("failed to create vmcp converter: %w", err)
4343
}
44-
config, err := converter.Convert(ctx, vmcp)
44+
config, authServerRC, err := converter.Convert(ctx, vmcp)
4545
if err != nil {
4646
return fmt.Errorf("failed to create vmcp Config from VirtualMCPServer: %w", err)
4747
}
@@ -62,7 +62,23 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
6262
return fmt.Errorf("invalid vmcp Config: %w", err)
6363
}
6464

65-
// Marshal to YAML for storage in ConfigMap
65+
// Cross-validate auth server RunConfig against backend strategies.
66+
// TODO: Move this into the operator's vmcpconfig.Validator wrapper so callers
67+
// don't need to know about the two-step validation sequence.
68+
if err := vmcpconfig.ValidateAuthServerIntegration(config, authServerRC); err != nil {
69+
message := fmt.Sprintf("invalid auth server integration: %v", err)
70+
statusManager.SetPhase(mcpv1alpha1.VirtualMCPServerPhaseFailed)
71+
statusManager.SetMessage(message)
72+
statusManager.SetAuthServerConfigValidatedCondition(
73+
mcpv1alpha1.ConditionReasonAuthServerConfigInvalid,
74+
message,
75+
metav1.ConditionFalse,
76+
)
77+
statusManager.SetObservedGeneration(vmcp.Generation)
78+
return &SpecValidationError{Message: message}
79+
}
80+
81+
// Marshal the serializable Config to YAML for storage in ConfigMap.
6682
// Note: gopkg.in/yaml.v3 produces deterministic output by sorting map keys alphabetically.
6783
// This ensures stable checksums for triggering pod rollouts only when content actually changes.
6884
vmcpConfigYAML, err := yaml.Marshal(config)
@@ -71,15 +87,28 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
7187
}
7288

7389
configMapName := vmcpConfigMapName(vmcp.Name)
90+
configMapData := map[string]string{
91+
"config.yaml": string(vmcpConfigYAML),
92+
}
93+
94+
// If an embedded auth server is configured, serialize its RunConfig as a separate key.
95+
// RunConfig contains only references (file paths, env var names) — never actual secrets —
96+
// so it is safe for ConfigMap storage. The vMCP binary loads this alongside config.yaml.
97+
if authServerRC != nil {
98+
authServerYAML, marshalErr := yaml.Marshal(authServerRC)
99+
if marshalErr != nil {
100+
return fmt.Errorf("failed to marshal auth server config: %w", marshalErr)
101+
}
102+
configMapData["authserver-config.yaml"] = string(authServerYAML)
103+
}
104+
74105
configMap := &corev1.ConfigMap{
75106
ObjectMeta: metav1.ObjectMeta{
76107
Name: configMapName,
77108
Namespace: vmcp.Namespace,
78109
Labels: labelsForVmcpConfig(vmcp.Name),
79110
},
80-
Data: map[string]string{
81-
"config.yaml": string(vmcpConfigYAML),
82-
},
111+
Data: configMapData,
83112
}
84113

85114
// Compute and add content checksum annotation using robust SHA256-based checksum

‎cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go‎

Lines changed: 120 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package controllers
55

66
import (
77
"context"
8+
stderrors "errors"
89
"fmt"
910
"testing"
1011
"time"
@@ -83,7 +84,7 @@ func TestCreateVmcpConfigFromVirtualMCPServer(t *testing.T) {
8384
t.Parallel()
8485

8586
converter := newTestConverter(t, newNoOpMockResolver(t))
86-
config, err := converter.Convert(context.Background(), tt.vmcp)
87+
config, _, err := converter.Convert(context.Background(), tt.vmcp)
8788

8889
require.NoError(t, err)
8990
assert.NotNil(t, config)
@@ -154,7 +155,7 @@ func TestConvertOutgoingAuth(t *testing.T) {
154155
}
155156

156157
converter := newTestConverter(t, newNoOpMockResolver(t))
157-
config, err := converter.Convert(context.Background(), vmcpServer)
158+
config, _, err := converter.Convert(context.Background(), vmcpServer)
158159
require.NoError(t, err)
159160

160161
require.NotNil(t, config.OutgoingAuth)
@@ -245,7 +246,7 @@ func TestConvertBackendAuthConfig(t *testing.T) {
245246
converter = newTestConverter(t, newNoOpMockResolver(t))
246247
}
247248

248-
config, err := converter.Convert(context.Background(), vmcpServer)
249+
config, _, err := converter.Convert(context.Background(), vmcpServer)
249250
require.NoError(t, err)
250251

251252
require.NotNil(t, config.OutgoingAuth)
@@ -339,7 +340,7 @@ func TestConvertAggregation(t *testing.T) {
339340
}
340341

341342
converter := newTestConverter(t, newNoOpMockResolver(t))
342-
config, err := converter.Convert(context.Background(), vmcpServer)
343+
config, _, err := converter.Convert(context.Background(), vmcpServer)
343344
require.NoError(t, err)
344345

345346
require.NotNil(t, config.Aggregation)
@@ -432,7 +433,7 @@ func TestConvertCompositeTools(t *testing.T) {
432433
}
433434

434435
converter := newTestConverter(t, newNoOpMockResolver(t))
435-
config, err := converter.Convert(context.Background(), vmcpServer)
436+
config, _, err := converter.Convert(context.Background(), vmcpServer)
436437
require.NoError(t, err)
437438

438439
tools := config.CompositeTools
@@ -1072,10 +1073,11 @@ func TestYAMLMarshalingDeterminism(t *testing.T) {
10721073
results := make([]string, iterations)
10731074

10741075
for i := 0; i < iterations; i++ {
1075-
config, err := converter.Convert(context.Background(), testVmcp)
1076+
cfg, _, err := converter.Convert(context.Background(), testVmcp)
10761077
require.NoError(t, err)
10771078

1078-
yamlBytes, err := yaml.Marshal(config)
1079+
// Marshal the Config to YAML.
1080+
yamlBytes, err := yaml.Marshal(cfg)
10791081
require.NoError(t, err)
10801082

10811083
results[i] = string(yamlBytes)
@@ -1951,3 +1953,114 @@ func TestOptimizerEmbeddingServiceURL(t *testing.T) {
19511953
})
19521954
}
19531955
}
1956+
1957+
// TestEnsureVmcpConfigConfigMap_AuthServerIntegrationValidationError verifies that
1958+
// ensureVmcpConfigConfigMap returns a SpecValidationError and sets the correct status
1959+
// conditions when ValidateAuthServerIntegration fails.
1960+
//
1961+
// The test triggers the issuer-mismatch path: AuthServerConfig.Issuer differs from
1962+
// IncomingAuth.OIDC.Issuer, causing validateAuthServerIncomingAuthConsistency to fail.
1963+
func TestEnsureVmcpConfigConfigMap_AuthServerIntegrationValidationError(t *testing.T) {
1964+
t.Parallel()
1965+
1966+
const (
1967+
incomingIssuer = "https://incoming-auth.example.com"
1968+
authServerIssuer = "https://different-auth-server.example.com"
1969+
audience = "https://api.example.com"
1970+
clientID = "test-client-id"
1971+
upstreamIssuerURL = "https://upstream-idp.example.com"
1972+
)
1973+
1974+
testVmcp := &mcpv1alpha1.VirtualMCPServer{
1975+
ObjectMeta: metav1.ObjectMeta{
1976+
Name: "test-vmcp",
1977+
Namespace: "default",
1978+
Generation: 3,
1979+
},
1980+
Spec: mcpv1alpha1.VirtualMCPServerSpec{
1981+
Config: vmcpconfig.Config{Group: "test-group"},
1982+
IncomingAuth: &mcpv1alpha1.IncomingAuthConfig{
1983+
Type: "oidc",
1984+
OIDCConfig: &mcpv1alpha1.OIDCConfigRef{
1985+
Type: mcpv1alpha1.OIDCConfigTypeInline,
1986+
Inline: &mcpv1alpha1.InlineOIDCConfig{
1987+
Issuer: incomingIssuer,
1988+
Audience: audience,
1989+
ClientID: clientID,
1990+
},
1991+
},
1992+
},
1993+
AuthServerConfig: &mcpv1alpha1.EmbeddedAuthServerConfig{
1994+
Issuer: authServerIssuer,
1995+
SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{
1996+
{Name: "signing-key-secret", Key: "key.pem"},
1997+
},
1998+
UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{
1999+
{
2000+
Name: "corporate-idp",
2001+
Type: mcpv1alpha1.UpstreamProviderTypeOIDC,
2002+
OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{
2003+
IssuerURL: upstreamIssuerURL,
2004+
ClientID: "upstream-client-id",
2005+
},
2006+
},
2007+
},
2008+
},
2009+
},
2010+
}
2011+
2012+
mcpGroup := &mcpv1alpha1.MCPGroup{
2013+
ObjectMeta: metav1.ObjectMeta{
2014+
Name: "test-group",
2015+
Namespace: "default",
2016+
},
2017+
Spec: mcpv1alpha1.MCPGroupSpec{},
2018+
}
2019+
2020+
scheme := runtime.NewScheme()
2021+
require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
2022+
require.NoError(t, corev1.AddToScheme(scheme))
2023+
2024+
fakeClient := fake.NewClientBuilder().
2025+
WithScheme(scheme).
2026+
WithObjects(testVmcp, mcpGroup).
2027+
Build()
2028+
2029+
r := &VirtualMCPServerReconciler{
2030+
Client: fakeClient,
2031+
Scheme: scheme,
2032+
}
2033+
2034+
ctx := context.Background()
2035+
workloadDiscoverer := workloads.NewK8SDiscovererWithClient(fakeClient, testVmcp.Namespace)
2036+
workloadNames, err := workloadDiscoverer.ListWorkloadsInGroup(ctx, testVmcp.Spec.Config.Group)
2037+
require.NoError(t, err)
2038+
2039+
// Use a mock StatusManager so we can verify the exact conditions set on failure.
2040+
mockCtrl := gomock.NewController(t)
2041+
mockStatus := statusmocks.NewMockStatusManager(mockCtrl)
2042+
2043+
// processOutgoingAuth (discovered mode, no OutgoingAuth on CRD) cleans up stale conditions.
2044+
mockStatus.EXPECT().RemoveConditionsWithPrefix("DefaultAuthConfig", []string{}).Times(1)
2045+
mockStatus.EXPECT().RemoveConditionsWithPrefix("DiscoveredAuthConfig-", []string{}).Times(1)
2046+
mockStatus.EXPECT().RemoveConditionsWithPrefix("BackendAuthConfig-", []string{}).Times(1)
2047+
2048+
// ValidateAuthServerIntegration failure: issuer mismatch sets Failed phase and condition.
2049+
mockStatus.EXPECT().SetPhase(mcpv1alpha1.VirtualMCPServerPhaseFailed).Times(1)
2050+
mockStatus.EXPECT().SetMessage(gomock.Any()).Times(1).Do(func(message string) {
2051+
assert.Contains(t, message, "invalid auth server integration")
2052+
})
2053+
mockStatus.EXPECT().SetAuthServerConfigValidatedCondition(
2054+
mcpv1alpha1.ConditionReasonAuthServerConfigInvalid,
2055+
gomock.Any(),
2056+
metav1.ConditionFalse,
2057+
).Times(1)
2058+
mockStatus.EXPECT().SetObservedGeneration(testVmcp.Generation).Times(1)
2059+
2060+
err = r.ensureVmcpConfigConfigMap(ctx, testVmcp, workloadNames, mockStatus)
2061+
2062+
// Verify the error is a SpecValidationError with the expected message.
2063+
var specErr *SpecValidationError
2064+
require.True(t, stderrors.As(err, &specErr), "expected a *SpecValidationError, got %T: %v", err, err)
2065+
assert.Contains(t, specErr.Message, "invalid auth server integration")
2066+
}

‎cmd/thv-operator/pkg/controllerutil/authserver.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,10 @@ func AddEmbeddedAuthServerConfigOptions(
396396
}
397397

398398
// Build the embedded auth server config for runner
399-
embeddedConfig, err := buildEmbeddedAuthServerRunnerConfig(namespace, mcpServerName, authServerConfig, oidcConfig)
399+
embeddedConfig, err := BuildAuthServerRunConfig(
400+
namespace, mcpServerName, authServerConfig,
401+
[]string{oidcConfig.ResourceURL}, oidcConfig.Scopes,
402+
)
400403
if err != nil {
401404
return fmt.Errorf("failed to build embedded auth server config: %w", err)
402405
}
@@ -407,23 +410,24 @@ func AddEmbeddedAuthServerConfigOptions(
407410
return nil
408411
}
409412

410-
// buildEmbeddedAuthServerRunnerConfig converts CRD EmbeddedAuthServerConfig to authserver.RunConfig.
413+
// BuildAuthServerRunConfig converts CRD EmbeddedAuthServerConfig to authserver.RunConfig.
411414
// The RunConfig is serializable and contains file paths for secrets (not the secrets themselves).
412415
//
413-
// The oidcConfig parameter provides:
414-
// - AllowedAudiences: from oidcConfig.ResourceURL (required, validated in AddEmbeddedAuthServerConfigOptions)
415-
// - ScopesSupported: from oidcConfig.Scopes (optional, nil uses auth server defaults)
416-
func buildEmbeddedAuthServerRunnerConfig(
416+
// AllowedAudiences and ScopesSupported are caller-provided because different controllers
417+
// derive them from different sources (MCPServer uses oidcConfig.ResourceURL/Scopes;
418+
// VirtualMCPServer derives from the resolved vmcp Config).
419+
func BuildAuthServerRunConfig(
417420
namespace string,
418-
mcpServerName string,
421+
name string,
419422
authConfig *mcpv1alpha1.EmbeddedAuthServerConfig,
420-
oidcConfig *oidc.OIDCConfig,
423+
allowedAudiences []string,
424+
scopesSupported []string,
421425
) (*authserver.RunConfig, error) {
422426
config := &authserver.RunConfig{
423427
SchemaVersion: authserver.CurrentSchemaVersion,
424428
Issuer: authConfig.Issuer,
425-
AllowedAudiences: []string{oidcConfig.ResourceURL},
426-
ScopesSupported: oidcConfig.Scopes,
429+
AllowedAudiences: allowedAudiences,
430+
ScopesSupported: scopesSupported,
427431
}
428432

429433
// Build signing key configuration
@@ -465,7 +469,7 @@ func buildEmbeddedAuthServerRunnerConfig(
465469
}
466470

467471
// Build storage configuration
468-
storageCfg, err := buildStorageRunConfig(namespace, mcpServerName, authConfig)
472+
storageCfg, err := buildStorageRunConfig(namespace, name, authConfig)
469473
if err != nil {
470474
return nil, fmt.Errorf("failed to build storage config: %w", err)
471475
}

0 commit comments

Comments
 (0)