Wire MCPOIDCConfig into VirtualMCPServer controller

VirtualMCPServer can now reference a shared MCPOIDCConfig resource via
incomingAuth.oidcConfigRef, matching the pattern already established for
MCPServer. The legacy inline oidcConfig field is deprecated and will be
removed in v1beta1.

Key changes:
- Add oidcConfigRef field to IncomingAuthConfig with CEL mutual-exclusivity
  validation against the deprecated oidcConfig field
- Converter resolves OIDC config from MCPOIDCConfig references using
  ResolveFromConfigRef, including client secret and CA bundle handling
- VirtualMCPServer controller validates the referenced MCPOIDCConfig,
  tracks config hash changes, and watches for MCPOIDCConfig updates
- MCPOIDCConfig controller now tracks VirtualMCPServer references in
  ReferencingWorkloads and blocks deletion while referenced
- Deployment builder propagates errors from MCPOIDCConfig fetch failures
  rather than silently producing misconfigured deployments

Closes #4253
Closes #4248

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

Author: ChrisJBurns

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

@@ -109,21 +109,30 @@ type EmbeddingServerRef struct {
 
 // IncomingAuthConfig configures authentication for clients connecting to the Virtual MCP server
 //
-// +kubebuilder:validation:XValidation:rule="self.type == 'oidc' ? has(self.oidcConfig) : true",message="spec.incomingAuth.oidcConfig is required when type is oidc"
+// +kubebuilder:validation:XValidation:rule="self.type == 'oidc' ? (has(self.oidcConfig) || has(self.oidcConfigRef)) : true",message="spec.incomingAuth.oidcConfig or oidcConfigRef is required when type is oidc"
+// +kubebuilder:validation:XValidation:rule="!(has(self.oidcConfig) && has(self.oidcConfigRef))",message="oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig"
 //
-//nolint:lll // CEL validation rule exceeds line length limit
+//nolint:lll // CEL validation rules exceed line length limit
 type IncomingAuthConfig struct {
 	// Type defines the authentication type: anonymous or oidc
 	// When no authentication is required, explicitly set this to "anonymous"
 	// +kubebuilder:validation:Enum=anonymous;oidc
 	// +kubebuilder:validation:Required
 	Type string `json:"type"`
 
-	// OIDCConfig defines OIDC authentication configuration
-	// Reuses MCPServer OIDC patterns
+	// OIDCConfig defines OIDC authentication configuration.
+	// Deprecated: Use OIDCConfigRef to reference a shared MCPOIDCConfig resource instead.
+	// This field will be removed in v1beta1. OIDCConfig and OIDCConfigRef are mutually exclusive.
 	// +optional
 	OIDCConfig *OIDCConfigRef `json:"oidcConfig,omitempty"`
 
+	// OIDCConfigRef references a shared MCPOIDCConfig resource for OIDC authentication.
+	// The referenced MCPOIDCConfig must exist in the same namespace as this VirtualMCPServer.
+	// Per-server overrides (audience, scopes) are specified here; shared provider config
+	// lives in the MCPOIDCConfig resource. Mutually exclusive with oidcConfig.
+	// +optional
+	OIDCConfigRef *MCPOIDCConfigReference `json:"oidcConfigRef,omitempty"`
+
 	// AuthzConfig defines authorization policy configuration
 	// Reuses MCPServer authz patterns
 	// +optional
@@ -210,6 +219,11 @@ type VirtualMCPServerStatus struct {
 	// (excludes unavailable, degraded, and unknown backends)
 	// +optional
 	BackendCount int `json:"backendCount,omitempty"`
+
+	// OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection.
+	// Only populated when IncomingAuth.OIDCConfigRef is set.
+	// +optional
+	OIDCConfigHash string `json:"oidcConfigHash,omitempty"`
 }
 
 // VirtualMCPServerPhase represents the lifecycle phase of a VirtualMCPServer

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

@@ -46,6 +46,7 @@ type MCPOIDCConfigReconciler struct {
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs/finalizers,verbs=update
+// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpservers,verbs=get;list;watch
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -199,7 +200,7 @@ func (r *MCPOIDCConfigReconciler) handleDeletion(
 	return ctrl.Result{}, nil
 }
 
-// findReferencingWorkloads returns the workload resources (MCPServer)
+// findReferencingWorkloads returns the workload resources (MCPServer and VirtualMCPServer)
 // that reference this MCPOIDCConfig via their OIDCConfigRef field.
 func (r *MCPOIDCConfigReconciler) findReferencingWorkloads(
 	ctx context.Context,
@@ -216,11 +217,25 @@ func (r *MCPOIDCConfigReconciler) findReferencingWorkloads(
 			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
 		}
 	}
+
+	// Check VirtualMCPServers
+	vmcpList := &mcpv1alpha1.VirtualMCPServerList{}
+	if err := r.List(ctx, vmcpList, client.InNamespace(oidcConfig.Namespace)); err != nil {
+		return nil, fmt.Errorf("failed to list VirtualMCPServers: %w", err)
+	}
+	for _, vmcp := range vmcpList.Items {
+		if vmcp.Spec.IncomingAuth != nil &&
+			vmcp.Spec.IncomingAuth.OIDCConfigRef != nil &&
+			vmcp.Spec.IncomingAuth.OIDCConfigRef.Name == oidcConfig.Name {
+			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcp.Name})
+		}
+	}
+
 	return refs, nil
 }
 
 // SetupWithManager sets up the controller with the Manager.
-// Watches MCPServer changes to maintain accurate ReferencingWorkloads status.
+// Watches MCPServer and VirtualMCPServer changes to maintain accurate ReferencingWorkloads status.
 func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// Watch MCPServer changes to update ReferencingWorkloads on referenced MCPOIDCConfigs.
 	// This handler enqueues both the currently-referenced MCPOIDCConfig AND any
@@ -274,5 +289,56 @@ func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&mcpv1alpha1.MCPOIDCConfig{}).
 		Watches(&mcpv1alpha1.MCPServer{}, mcpServerHandler).
+		Watches(
+			&mcpv1alpha1.VirtualMCPServer{},
+			handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToOIDCConfig),
+		).
 		Complete(r)
 }
+
+// mapVirtualMCPServerToOIDCConfig maps VirtualMCPServer changes to MCPOIDCConfig reconciliation requests.
+// Enqueues both the currently-referenced config and any config that still lists this
+// VirtualMCPServer in ReferencingWorkloads (handles ref-removal / deletion).
+func (r *MCPOIDCConfigReconciler) mapVirtualMCPServerToOIDCConfig(
+	ctx context.Context, obj client.Object,
+) []reconcile.Request {
+	vmcp, ok := obj.(*mcpv1alpha1.VirtualMCPServer)
+	if !ok {
+		return nil
+	}
+
+	seen := make(map[types.NamespacedName]struct{})
+	var requests []reconcile.Request
+
+	// Enqueue the currently-referenced MCPOIDCConfig (if any)
+	if vmcp.Spec.IncomingAuth != nil && vmcp.Spec.IncomingAuth.OIDCConfigRef != nil {
+		nn := types.NamespacedName{
+			Name:      vmcp.Spec.IncomingAuth.OIDCConfigRef.Name,
+			Namespace: vmcp.Namespace,
+		}
+		seen[nn] = struct{}{}
+		requests = append(requests, reconcile.Request{NamespacedName: nn})
+	}
+
+	// Also enqueue any MCPOIDCConfig that still lists this VirtualMCPServer in
+	// ReferencingWorkloads — handles ref-removal and deletion cases.
+	oidcConfigList := &mcpv1alpha1.MCPOIDCConfigList{}
+	if err := r.List(ctx, oidcConfigList, client.InNamespace(vmcp.Namespace)); err != nil {
+		log.FromContext(ctx).Error(err, "Failed to list MCPOIDCConfigs for VirtualMCPServer watch")
+		return requests
+	}
+	for _, cfg := range oidcConfigList.Items {
+		nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace}
+		if _, already := seen[nn]; already {
+			continue
+		}
+		for _, ref := range cfg.Status.ReferencingWorkloads {
+			if ref.Kind == "VirtualMCPServer" && ref.Name == vmcp.Name {
+				requests = append(requests, reconcile.Request{NamespacedName: nn})
+				break
+			}
+		}
+	}
+
+	return requests
+}

cmd/thv-operator/controllers/mcpoidcconfig_controller.go

@@ -20,6 +20,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -118,6 +119,8 @@ type VirtualMCPServerReconciler struct {
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=create;get;list;watch
 // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;delete;get;list;patch;update;watch
 // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;delete;get;list;patch;update;watch
+// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs,verbs=get;list;watch
+// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=embeddingservers,verbs=get;list;watch
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=embeddingservers/status,verbs=get
 
@@ -151,6 +154,15 @@ func (r *VirtualMCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, nil
 	}
 
+	// Validate MCPOIDCConfigRef if referenced (before resource creation).
+	// handleOIDCConfig is a no-op when OIDCConfigRef is nil.
+	if err := r.handleOIDCConfig(ctx, vmcp, statusManager); err != nil {
+		if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
+			ctxLogger.Error(applyErr, "Failed to apply status updates after MCPOIDCConfig validation error")
+		}
+		return ctrl.Result{}, err
+	}
+
 	// Ensure all resources
 	if result, err := r.ensureAllResources(ctx, vmcp, statusManager); err != nil {
 		// Apply status changes before returning error
@@ -1268,7 +1280,10 @@ func (r *VirtualMCPServerReconciler) containerNeedsUpdate(
 	}
 
 	// Check if environment variables have changed
-	expectedEnv := r.buildEnvVarsForVmcp(ctx, vmcp, typedWorkloads)
+	expectedEnv, err := r.buildEnvVarsForVmcp(ctx, vmcp, typedWorkloads)
+	if err != nil {
+		return true // Trigger update to surface the error
+	}
 	if !reflect.DeepEqual(container.Env, expectedEnv) {
 		return true
 	}
@@ -2131,6 +2146,12 @@ func (r *VirtualMCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			&mcpv1alpha1.EmbeddingServer{},
 			handler.EnqueueRequestsFromMapFunc(r.mapEmbeddingServerToVirtualMCPServer),
 		).
+		// Watch referenced MCPOIDCConfigs so that validity/hash changes
+		// trigger VirtualMCPServer reconciliation.
+		Watches(
+			&mcpv1alpha1.MCPOIDCConfig{},
+			handler.EnqueueRequestsFromMapFunc(r.mapOIDCConfigToVirtualMCPServer),
+		).
 		Complete(r)
 }
 
@@ -2703,3 +2724,151 @@ func generateHMACSecret() (string, error) {
 	// Encode as base64 for safe storage and environment variable use
 	return base64.StdEncoding.EncodeToString(secret), nil
 }
+
+// handleOIDCConfig validates and tracks the hash of the referenced MCPOIDCConfig.
+// It sets the OIDCConfigRefValidated condition and triggers reconciliation when
+// the OIDC configuration changes.
+func (r *VirtualMCPServerReconciler) handleOIDCConfig(
+	ctx context.Context,
+	vmcp *mcpv1alpha1.VirtualMCPServer,
+	statusManager virtualmcpserverstatus.StatusManager,
+) error {
+	ctxLogger := log.FromContext(ctx)
+
+	if vmcp.Spec.IncomingAuth == nil || vmcp.Spec.IncomingAuth.OIDCConfigRef == nil {
+		// No MCPOIDCConfig referenced, clear any stored hash
+		if vmcp.Status.OIDCConfigHash != "" {
+			vmcp.Status.OIDCConfigHash = ""
+			if err := r.Status().Update(ctx, vmcp); err != nil {
+				return fmt.Errorf("failed to clear MCPOIDCConfig hash from status: %w", err)
+			}
+		}
+		return nil
+	}
+
+	ref := vmcp.Spec.IncomingAuth.OIDCConfigRef
+
+	// Get the referenced MCPOIDCConfig
+	oidcConfig, err := ctrlutil.GetOIDCConfigForServer(ctx, r.Client, vmcp.Namespace, ref)
+	if err != nil {
+		statusManager.SetCondition(
+			mcpv1alpha1.ConditionOIDCConfigRefValidated,
+			mcpv1alpha1.ConditionReasonOIDCConfigRefNotFound,
+			fmt.Sprintf("MCPOIDCConfig %s not found: %v", ref.Name, err),
+			metav1.ConditionFalse,
+		)
+		return err
+	}
+
+	if oidcConfig == nil {
+		statusManager.SetCondition(
+			mcpv1alpha1.ConditionOIDCConfigRefValidated,
+			mcpv1alpha1.ConditionReasonOIDCConfigRefNotFound,
+			fmt.Sprintf("MCPOIDCConfig %s not found", ref.Name),
+			metav1.ConditionFalse,
+		)
+		return fmt.Errorf("MCPOIDCConfig %s not found", ref.Name)
+	}
+
+	// Check that the MCPOIDCConfig is valid
+	validCondition := meta.FindStatusCondition(oidcConfig.Status.Conditions, mcpv1alpha1.ConditionTypeOIDCConfigReady)
+	if validCondition == nil || validCondition.Status != metav1.ConditionTrue {
+		msg := fmt.Sprintf("MCPOIDCConfig %s is not valid", ref.Name)
+		if validCondition != nil {
+			msg = fmt.Sprintf("MCPOIDCConfig %s is not valid: %s", ref.Name, validCondition.Message)
+		}
+		statusManager.SetCondition(
+			mcpv1alpha1.ConditionOIDCConfigRefValidated,
+			mcpv1alpha1.ConditionReasonOIDCConfigRefNotValid,
+			msg,
+			metav1.ConditionFalse,
+		)
+		return fmt.Errorf("%s", msg)
+	}
+
+	// Update ReferencingWorkloads on the MCPOIDCConfig status
+	if err := r.updateOIDCConfigReferencingWorkloads(ctx, oidcConfig, vmcp.Name); err != nil {
+		ctxLogger.Error(err, "Failed to update MCPOIDCConfig ReferencingWorkloads")
+		// Non-fatal: continue with reconciliation
+	}
+
+	// Set valid condition
+	statusManager.SetCondition(
+		mcpv1alpha1.ConditionOIDCConfigRefValidated,
+		mcpv1alpha1.ConditionReasonOIDCConfigRefValid,
+		fmt.Sprintf("MCPOIDCConfig %s is valid and ready", ref.Name),
+		metav1.ConditionTrue,
+	)
+
+	// Check if the MCPOIDCConfig hash has changed
+	if vmcp.Status.OIDCConfigHash != oidcConfig.Status.ConfigHash {
+		ctxLogger.Info("MCPOIDCConfig has changed, updating VirtualMCPServer",
+			"vmcp", vmcp.Name,
+			"oidcConfig", oidcConfig.Name,
+			"oldHash", vmcp.Status.OIDCConfigHash,
+			"newHash", oidcConfig.Status.ConfigHash)
+
+		vmcp.Status.OIDCConfigHash = oidcConfig.Status.ConfigHash
+		if err := r.Status().Update(ctx, vmcp); err != nil {
+			return fmt.Errorf("failed to update MCPOIDCConfig hash in status: %w", err)
+		}
+	}
+
+	return nil
+}
+
+// updateOIDCConfigReferencingWorkloads ensures the VirtualMCPServer is listed in
+// the MCPOIDCConfig's ReferencingWorkloads status field.
+func (r *VirtualMCPServerReconciler) updateOIDCConfigReferencingWorkloads(
+	ctx context.Context,
+	oidcConfig *mcpv1alpha1.MCPOIDCConfig,
+	vmcpName string,
+) error {
+	ref := mcpv1alpha1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName}
+	// Check if already listed
+	for _, entry := range oidcConfig.Status.ReferencingWorkloads {
+		if entry.Kind == ref.Kind && entry.Name == ref.Name {
+			return nil
+		}
+	}
+
+	// Add the workload reference
+	oidcConfig.Status.ReferencingWorkloads = append(oidcConfig.Status.ReferencingWorkloads, ref)
+	if err := r.Status().Update(ctx, oidcConfig); err != nil {
+		return fmt.Errorf("failed to update MCPOIDCConfig ReferencingWorkloads: %w", err)
+	}
+
+	return nil
+}
+
+// mapOIDCConfigToVirtualMCPServer maps MCPOIDCConfig changes to VirtualMCPServer reconciliation requests.
+func (r *VirtualMCPServerReconciler) mapOIDCConfigToVirtualMCPServer(
+	ctx context.Context, obj client.Object,
+) []reconcile.Request {
+	oidcConfig, ok := obj.(*mcpv1alpha1.MCPOIDCConfig)
+	if !ok {
+		return nil
+	}
+
+	vmcpList := &mcpv1alpha1.VirtualMCPServerList{}
+	if err := r.List(ctx, vmcpList, client.InNamespace(oidcConfig.Namespace)); err != nil {
+		log.FromContext(ctx).Error(err, "Failed to list VirtualMCPServers for MCPOIDCConfig watch")
+		return nil
+	}
+
+	var requests []reconcile.Request
+	for _, vmcp := range vmcpList.Items {
+		if vmcp.Spec.IncomingAuth != nil &&
+			vmcp.Spec.IncomingAuth.OIDCConfigRef != nil &&
+			vmcp.Spec.IncomingAuth.OIDCConfigRef.Name == oidcConfig.Name {
+			requests = append(requests, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      vmcp.Name,
+					Namespace: vmcp.Namespace,
+				},
+			})
+		}
+	}
+
+	return requests
+}