Wire embedded auth server into VirtualMCPServer converter and deployment (#4383)

* 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

* Fix E2E test to use OIDC incoming auth with auth server config

ValidateAuthServerIntegration (added in 0810bda) requires OIDC
incoming auth when AuthServerConfig is present. The pre-existing E2E
test used anonymous auth, causing the condition to be overwritten to
False and the test to time out.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Extract convertSessionStorage to reduce Convert cyclomatic complexity

The Convert function exceeded the gocyclo threshold (17 > 15) after
the rebase added both SessionStorage and AuthServerConfig conversion.
Extract the SessionStorage block into a standalone helper.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: jhrozek

cmd/thv-operator/controllers/virtualmcpserver_controller.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/base64"
+	stderrors "errors"
 	"fmt"
 	"maps"
 	"reflect"
@@ -71,6 +72,17 @@ type AuthConfigError struct {
 	Error error
 }
 
+// SpecValidationError represents a spec validation failure that the user must fix.
+// Unlike transient errors, these should NOT trigger requeue — the controller sets
+// a status condition and waits for the user to update the spec.
+type SpecValidationError struct {
+	Message string
+}
+
+func (e *SpecValidationError) Error() string {
+	return e.Message
+}
+
 // VirtualMCPServerReconciler reconciles a VirtualMCPServer object
 //
 // Resource Cleanup Strategy:
@@ -419,6 +431,26 @@ func (*VirtualMCPServerReconciler) validateAuthServerConfig(
 	return nil
 }
 
+// handleSpecValidationError checks whether err is a SpecValidationError (user must fix the spec).
+// If so, it applies the already-set status conditions and returns nil (no requeue).
+// Otherwise it returns the original error unchanged for normal requeue handling.
+func (r *VirtualMCPServerReconciler) handleSpecValidationError(
+	ctx context.Context,
+	vmcp *mcpv1alpha1.VirtualMCPServer,
+	statusManager virtualmcpserverstatus.StatusManager,
+	err error,
+) error {
+	var specErr *SpecValidationError
+	if !stderrors.As(err, &specErr) {
+		return err
+	}
+	ctxLogger := log.FromContext(ctx)
+	if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
+		ctxLogger.Error(applyErr, "Failed to apply status updates after spec validation error")
+	}
+	return nil
+}
+
 // validateGroupRef validates that the referenced MCPGroup exists and is ready
 func (r *VirtualMCPServerReconciler) validateGroupRef(
 	ctx context.Context,
@@ -696,8 +728,11 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
 		return ctrl.Result{}, err
 	}
 
-	// Ensure vmcp Config ConfigMap
-	if err := r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames, statusManager); err != nil {
+	// Ensure vmcp Config ConfigMap.
+	// handleSpecValidationError converts SpecValidationError to nil (no requeue)
+	// after applying status conditions, while passing through transient errors.
+	if err := r.handleSpecValidationError(ctx, vmcp, statusManager,
+		r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames, statusManager)); err != nil {
 		ctxLogger.Error(err, "Failed to ensure vmcp Config ConfigMap")
 		return ctrl.Result{}, err
 	}

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

@@ -126,6 +126,15 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer(
 	args := r.buildContainerArgsForVmcp(vmcp)
 	volumeMounts, volumes := r.buildVolumesForVmcp(vmcp)
 	env := r.buildEnvVarsForVmcp(ctx, vmcp, typedWorkloads)
+
+	// Add embedded auth server volumes and env vars if configured (inline config)
+	if vmcp.Spec.AuthServerConfig != nil {
+		authServerVolumes, authServerMounts := ctrlutil.GenerateAuthServerVolumes(vmcp.Spec.AuthServerConfig)
+		authServerEnvVars := ctrlutil.GenerateAuthServerEnvVars(vmcp.Spec.AuthServerConfig)
+		volumes = append(volumes, authServerVolumes...)
+		volumeMounts = append(volumeMounts, authServerMounts...)
+		env = append(env, authServerEnvVars...)
+	}
 	deploymentLabels, deploymentAnnotations := r.buildDeploymentMetadataForVmcp(ls, vmcp)
 	deploymentTemplateLabels, deploymentTemplateAnnotations := r.buildPodTemplateMetadata(ls, vmcp, vmcpConfigChecksum)
 	podSecurityContext, containerSecurityContext := r.buildSecurityContextsForVmcp(ctx, vmcp)

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go

@@ -41,7 +41,7 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
 	if err != nil {
 		return fmt.Errorf("failed to create vmcp converter: %w", err)
 	}
-	config, err := converter.Convert(ctx, vmcp)
+	config, authServerRC, err := converter.Convert(ctx, vmcp)
 	if err != nil {
 		return fmt.Errorf("failed to create vmcp Config from VirtualMCPServer: %w", err)
 	}
@@ -62,7 +62,23 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
 		return fmt.Errorf("invalid vmcp Config: %w", err)
 	}
 
-	// Marshal to YAML for storage in ConfigMap
+	// Cross-validate auth server RunConfig against backend strategies.
+	// TODO: Move this into the operator's vmcpconfig.Validator wrapper so callers
+	// don't need to know about the two-step validation sequence.
+	if err := vmcpconfig.ValidateAuthServerIntegration(config, authServerRC); err != nil {
+		message := fmt.Sprintf("invalid auth server integration: %v", err)
+		statusManager.SetPhase(mcpv1alpha1.VirtualMCPServerPhaseFailed)
+		statusManager.SetMessage(message)
+		statusManager.SetAuthServerConfigValidatedCondition(
+			mcpv1alpha1.ConditionReasonAuthServerConfigInvalid,
+			message,
+			metav1.ConditionFalse,
+		)
+		statusManager.SetObservedGeneration(vmcp.Generation)
+		return &SpecValidationError{Message: message}
+	}
+
+	// Marshal the serializable Config to YAML for storage in ConfigMap.
 	// Note: gopkg.in/yaml.v3 produces deterministic output by sorting map keys alphabetically.
 	// This ensures stable checksums for triggering pod rollouts only when content actually changes.
 	vmcpConfigYAML, err := yaml.Marshal(config)
@@ -71,15 +87,28 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
 	}
 
 	configMapName := vmcpConfigMapName(vmcp.Name)
+	configMapData := map[string]string{
+		"config.yaml": string(vmcpConfigYAML),
+	}
+
+	// If an embedded auth server is configured, serialize its RunConfig as a separate key.
+	// RunConfig contains only references (file paths, env var names) — never actual secrets —
+	// so it is safe for ConfigMap storage. The vMCP binary loads this alongside config.yaml.
+	if authServerRC != nil {
+		authServerYAML, marshalErr := yaml.Marshal(authServerRC)
+		if marshalErr != nil {
+			return fmt.Errorf("failed to marshal auth server config: %w", marshalErr)
+		}
+		configMapData["authserver-config.yaml"] = string(authServerYAML)
+	}
+
 	configMap := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      configMapName,
 			Namespace: vmcp.Namespace,
 			Labels:    labelsForVmcpConfig(vmcp.Name),
 		},
-		Data: map[string]string{
-			"config.yaml": string(vmcpConfigYAML),
-		},
+		Data: configMapData,
 	}
 
 	// Compute and add content checksum annotation using robust SHA256-based checksum

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go

@@ -5,6 +5,7 @@ package controllers
 
 import (
 	"context"
+	stderrors "errors"
 	"fmt"
 	"testing"
 	"time"
@@ -83,7 +84,7 @@ func TestCreateVmcpConfigFromVirtualMCPServer(t *testing.T) {
 			t.Parallel()
 
 			converter := newTestConverter(t, newNoOpMockResolver(t))
-			config, err := converter.Convert(context.Background(), tt.vmcp)
+			config, _, err := converter.Convert(context.Background(), tt.vmcp)
 
 			require.NoError(t, err)
 			assert.NotNil(t, config)
@@ -154,7 +155,7 @@ func TestConvertOutgoingAuth(t *testing.T) {
 			}
 
 			converter := newTestConverter(t, newNoOpMockResolver(t))
-			config, err := converter.Convert(context.Background(), vmcpServer)
+			config, _, err := converter.Convert(context.Background(), vmcpServer)
 			require.NoError(t, err)
 
 			require.NotNil(t, config.OutgoingAuth)
@@ -245,7 +246,7 @@ func TestConvertBackendAuthConfig(t *testing.T) {
 				converter = newTestConverter(t, newNoOpMockResolver(t))
 			}
 
-			config, err := converter.Convert(context.Background(), vmcpServer)
+			config, _, err := converter.Convert(context.Background(), vmcpServer)
 			require.NoError(t, err)
 
 			require.NotNil(t, config.OutgoingAuth)
@@ -339,7 +340,7 @@ func TestConvertAggregation(t *testing.T) {
 			}
 
 			converter := newTestConverter(t, newNoOpMockResolver(t))
-			config, err := converter.Convert(context.Background(), vmcpServer)
+			config, _, err := converter.Convert(context.Background(), vmcpServer)
 			require.NoError(t, err)
 
 			require.NotNil(t, config.Aggregation)
@@ -432,7 +433,7 @@ func TestConvertCompositeTools(t *testing.T) {
 			}
 
 			converter := newTestConverter(t, newNoOpMockResolver(t))
-			config, err := converter.Convert(context.Background(), vmcpServer)
+			config, _, err := converter.Convert(context.Background(), vmcpServer)
 			require.NoError(t, err)
 
 			tools := config.CompositeTools
@@ -1072,10 +1073,11 @@ func TestYAMLMarshalingDeterminism(t *testing.T) {
 	results := make([]string, iterations)
 
 	for i := 0; i < iterations; i++ {
-		config, err := converter.Convert(context.Background(), testVmcp)
+		cfg, _, err := converter.Convert(context.Background(), testVmcp)
 		require.NoError(t, err)
 
-		yamlBytes, err := yaml.Marshal(config)
+		// Marshal the Config to YAML.
+		yamlBytes, err := yaml.Marshal(cfg)
 		require.NoError(t, err)
 
 		results[i] = string(yamlBytes)
@@ -2074,3 +2076,114 @@ func TestConfigMapContent_SessionStorage(t *testing.T) {
 		})
 	}
 }
+
+// TestEnsureVmcpConfigConfigMap_AuthServerIntegrationValidationError verifies that
+// ensureVmcpConfigConfigMap returns a SpecValidationError and sets the correct status
+// conditions when ValidateAuthServerIntegration fails.
+//
+// The test triggers the issuer-mismatch path: AuthServerConfig.Issuer differs from
+// IncomingAuth.OIDC.Issuer, causing validateAuthServerIncomingAuthConsistency to fail.
+func TestEnsureVmcpConfigConfigMap_AuthServerIntegrationValidationError(t *testing.T) {
+	t.Parallel()
+
+	const (
+		incomingIssuer    = "https://incoming-auth.example.com"
+		authServerIssuer  = "https://different-auth-server.example.com"
+		audience          = "https://api.example.com"
+		clientID          = "test-client-id"
+		upstreamIssuerURL = "https://upstream-idp.example.com"
+	)
+
+	testVmcp := &mcpv1alpha1.VirtualMCPServer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "test-vmcp",
+			Namespace:  "default",
+			Generation: 3,
+		},
+		Spec: mcpv1alpha1.VirtualMCPServerSpec{
+			Config: vmcpconfig.Config{Group: "test-group"},
+			IncomingAuth: &mcpv1alpha1.IncomingAuthConfig{
+				Type: "oidc",
+				OIDCConfig: &mcpv1alpha1.OIDCConfigRef{
+					Type: mcpv1alpha1.OIDCConfigTypeInline,
+					Inline: &mcpv1alpha1.InlineOIDCConfig{
+						Issuer:   incomingIssuer,
+						Audience: audience,
+						ClientID: clientID,
+					},
+				},
+			},
+			AuthServerConfig: &mcpv1alpha1.EmbeddedAuthServerConfig{
+				Issuer: authServerIssuer,
+				SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{
+					{Name: "signing-key-secret", Key: "key.pem"},
+				},
+				UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{
+					{
+						Name: "corporate-idp",
+						Type: mcpv1alpha1.UpstreamProviderTypeOIDC,
+						OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{
+							IssuerURL: upstreamIssuerURL,
+							ClientID:  "upstream-client-id",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	mcpGroup := &mcpv1alpha1.MCPGroup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-group",
+			Namespace: "default",
+		},
+		Spec: mcpv1alpha1.MCPGroupSpec{},
+	}
+
+	scheme := runtime.NewScheme()
+	require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
+	require.NoError(t, corev1.AddToScheme(scheme))
+
+	fakeClient := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(testVmcp, mcpGroup).
+		Build()
+
+	r := &VirtualMCPServerReconciler{
+		Client: fakeClient,
+		Scheme: scheme,
+	}
+
+	ctx := context.Background()
+	workloadDiscoverer := workloads.NewK8SDiscovererWithClient(fakeClient, testVmcp.Namespace)
+	workloadNames, err := workloadDiscoverer.ListWorkloadsInGroup(ctx, testVmcp.Spec.Config.Group)
+	require.NoError(t, err)
+
+	// Use a mock StatusManager so we can verify the exact conditions set on failure.
+	mockCtrl := gomock.NewController(t)
+	mockStatus := statusmocks.NewMockStatusManager(mockCtrl)
+
+	// processOutgoingAuth (discovered mode, no OutgoingAuth on CRD) cleans up stale conditions.
+	mockStatus.EXPECT().RemoveConditionsWithPrefix("DefaultAuthConfig", []string{}).Times(1)
+	mockStatus.EXPECT().RemoveConditionsWithPrefix("DiscoveredAuthConfig-", []string{}).Times(1)
+	mockStatus.EXPECT().RemoveConditionsWithPrefix("BackendAuthConfig-", []string{}).Times(1)
+
+	// ValidateAuthServerIntegration failure: issuer mismatch sets Failed phase and condition.
+	mockStatus.EXPECT().SetPhase(mcpv1alpha1.VirtualMCPServerPhaseFailed).Times(1)
+	mockStatus.EXPECT().SetMessage(gomock.Any()).Times(1).Do(func(message string) {
+		assert.Contains(t, message, "invalid auth server integration")
+	})
+	mockStatus.EXPECT().SetAuthServerConfigValidatedCondition(
+		mcpv1alpha1.ConditionReasonAuthServerConfigInvalid,
+		gomock.Any(),
+		metav1.ConditionFalse,
+	).Times(1)
+	mockStatus.EXPECT().SetObservedGeneration(testVmcp.Generation).Times(1)
+
+	err = r.ensureVmcpConfigConfigMap(ctx, testVmcp, workloadNames, mockStatus)
+
+	// Verify the error is a SpecValidationError with the expected message.
+	var specErr *SpecValidationError
+	require.True(t, stderrors.As(err, &specErr), "expected a *SpecValidationError, got %T: %v", err, err)
+	assert.Contains(t, specErr.Message, "invalid auth server integration")
+}

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

@@ -396,7 +396,10 @@ func AddEmbeddedAuthServerConfigOptions(
 	}
 
 	// Build the embedded auth server config for runner
-	embeddedConfig, err := buildEmbeddedAuthServerRunnerConfig(namespace, mcpServerName, authServerConfig, oidcConfig)
+	embeddedConfig, err := BuildAuthServerRunConfig(
+		namespace, mcpServerName, authServerConfig,
+		[]string{oidcConfig.ResourceURL}, oidcConfig.Scopes,
+	)
 	if err != nil {
 		return fmt.Errorf("failed to build embedded auth server config: %w", err)
 	}
@@ -407,24 +410,25 @@ func AddEmbeddedAuthServerConfigOptions(
 	return nil
 }
 
-// buildEmbeddedAuthServerRunnerConfig converts CRD EmbeddedAuthServerConfig to authserver.RunConfig.
+// BuildAuthServerRunConfig converts CRD EmbeddedAuthServerConfig to authserver.RunConfig.
 // The RunConfig is serializable and contains file paths for secrets (not the secrets themselves).
 //
-// The oidcConfig parameter provides:
-//   - AllowedAudiences: from oidcConfig.ResourceURL (required, validated in AddEmbeddedAuthServerConfigOptions)
-//   - ScopesSupported: from oidcConfig.Scopes (optional, nil uses auth server defaults)
-func buildEmbeddedAuthServerRunnerConfig(
+// AllowedAudiences and ScopesSupported are caller-provided because different controllers
+// derive them from different sources (MCPServer uses oidcConfig.ResourceURL/Scopes;
+// VirtualMCPServer derives from the resolved vmcp Config).
+func BuildAuthServerRunConfig(
 	namespace string,
-	mcpServerName string,
+	name string,
 	authConfig *mcpv1alpha1.EmbeddedAuthServerConfig,
-	oidcConfig *oidc.OIDCConfig,
+	allowedAudiences []string,
+	scopesSupported []string,
 ) (*authserver.RunConfig, error) {
 	config := &authserver.RunConfig{
 		SchemaVersion:                authserver.CurrentSchemaVersion,
 		Issuer:                       authConfig.Issuer,
 		AuthorizationEndpointBaseURL: authConfig.AuthorizationEndpointBaseURL,
-		AllowedAudiences:             []string{oidcConfig.ResourceURL},
-		ScopesSupported:              oidcConfig.Scopes,
+		AllowedAudiences:             allowedAudiences,
+		ScopesSupported:              scopesSupported,
 	}
 
 	// Build signing key configuration
@@ -466,7 +470,7 @@ func buildEmbeddedAuthServerRunnerConfig(
 	}
 
 	// Build storage configuration
-	storageCfg, err := buildStorageRunConfig(namespace, mcpServerName, authConfig)
+	storageCfg, err := buildStorageRunConfig(namespace, name, authConfig)
 	if err != nil {
 		return nil, fmt.Errorf("failed to build storage config: %w", err)
 	}