Fix VirtualMCPServer reconciliation

cmd/thv-operator/controllers/virtualmcpserver_authz_configmap.go

@@ -18,6 +18,44 @@ import (
 	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
 )
 
+const virtualMCPServerConfigMapIndex = "toolhive.stacklok.dev/virtualmcpserver-configmap"
+
+// indexVirtualMCPServerConfigMaps returns namespace-local ConfigMap references
+// whose data changes require the VirtualMCPServer to reconcile.
+func indexVirtualMCPServerConfigMaps(obj client.Object) []string {
+	vmcp, ok := obj.(*mcpv1beta1.VirtualMCPServer)
+	if !ok {
+		return nil
+	}
+
+	names := make(map[string]struct{})
+	if vmcp.Spec.IncomingAuth != nil && vmcp.Spec.IncomingAuth.AuthzConfig != nil &&
+		vmcp.Spec.IncomingAuth.AuthzConfig.Type == mcpv1beta1.AuthzConfigTypeConfigMap &&
+		vmcp.Spec.IncomingAuth.AuthzConfig.ConfigMap != nil &&
+		vmcp.Spec.IncomingAuth.AuthzConfig.ConfigMap.Name != "" {
+		names[vmcp.Spec.IncomingAuth.AuthzConfig.ConfigMap.Name] = struct{}{}
+	}
+	if vmcp.Spec.AuthServerConfig != nil {
+		for _, provider := range vmcp.Spec.AuthServerConfig.UpstreamProviders {
+			if name := providerCABundleConfigMapName(provider); name != "" {
+				names[name] = struct{}{}
+			}
+		}
+		for i := range vmcp.Spec.AuthServerConfig.TrustedIssuers {
+			ref := vmcp.Spec.AuthServerConfig.TrustedIssuers[i].CABundleRef
+			if ref != nil && ref.ConfigMapRef != nil && ref.ConfigMapRef.Name != "" {
+				names[ref.ConfigMapRef.Name] = struct{}{}
+			}
+		}
+	}
+
+	result := make([]string, 0, len(names))
+	for name := range names {
+		result = append(result, name)
+	}
+	return result
+}
+
 // mapAuthzConfigMapToVirtualMCPServer maps ConfigMap changes to VirtualMCPServer reconciliation
 // requests. Used by SetupWithManager to trigger reconciliation when a ConfigMap referenced via
 // spec.incomingAuth.authzConfig.configMap is updated, so the converter can re-resolve policies
@@ -35,61 +73,27 @@ func (r *VirtualMCPServerReconciler) mapAuthzConfigMapToVirtualMCPServer(
 	}
 
 	vmcpList := &mcpv1beta1.VirtualMCPServerList{}
-	if err := r.List(ctx, vmcpList, client.InNamespace(cm.Namespace)); err != nil {
-		log.FromContext(ctx).Error(err, "Failed to list VirtualMCPServers for authz ConfigMap watch")
+	if err := r.List(ctx, vmcpList,
+		client.InNamespace(cm.Namespace),
+		client.MatchingFields{virtualMCPServerConfigMapIndex: cm.Name},
+	); err != nil {
+		log.FromContext(ctx).Error(err, "Failed to list VirtualMCPServers for ConfigMap watch")
 		return nil
 	}
 
-	var requests []reconcile.Request
-	for _, vmcp := range vmcpList.Items {
-		if !vmcpReferencesAuthzConfigMap(&vmcp, cm.Name) && !vmcpReferencesAuthServerCABundle(&vmcp, cm.Name) {
-			continue
-		}
+	requests := make([]reconcile.Request, 0, len(vmcpList.Items))
+	for i := range vmcpList.Items {
 		requests = append(requests, reconcile.Request{
 			NamespacedName: types.NamespacedName{
-				Name:      vmcp.Name,
-				Namespace: vmcp.Namespace,
+				Name:      vmcpList.Items[i].Name,
+				Namespace: vmcpList.Items[i].Namespace,
 			},
 		})
 	}
 
 	return requests
 }
 
-// vmcpReferencesAuthzConfigMap reports whether the VirtualMCPServer references the named
-// ConfigMap via spec.incomingAuth.authzConfig.
-func vmcpReferencesAuthzConfigMap(vmcp *mcpv1beta1.VirtualMCPServer, configMapName string) bool {
-	if vmcp.Spec.IncomingAuth == nil ||
-		vmcp.Spec.IncomingAuth.AuthzConfig == nil ||
-		vmcp.Spec.IncomingAuth.AuthzConfig.Type != mcpv1beta1.AuthzConfigTypeConfigMap ||
-		vmcp.Spec.IncomingAuth.AuthzConfig.ConfigMap == nil {
-		return false
-	}
-	return vmcp.Spec.IncomingAuth.AuthzConfig.ConfigMap.Name == configMapName
-}
-
-// vmcpReferencesAuthServerCABundle reports whether an inline auth-server
-// configuration selects the named ConfigMap for a CA bundle.
-func vmcpReferencesAuthServerCABundle(vmcp *mcpv1beta1.VirtualMCPServer, configMapName string) bool {
-	if vmcp.Spec.AuthServerConfig == nil {
-		return false
-	}
-	for i := range vmcp.Spec.AuthServerConfig.UpstreamProviders {
-		provider := &vmcp.Spec.AuthServerConfig.UpstreamProviders[i]
-		ref := provider.CABundleRef()
-		if ref != nil && ref.ConfigMapRef != nil && ref.ConfigMapRef.Name == configMapName {
-			return true
-		}
-	}
-	for i := range vmcp.Spec.AuthServerConfig.TrustedIssuers {
-		ref := vmcp.Spec.AuthServerConfig.TrustedIssuers[i].CABundleRef
-		if ref != nil && ref.ConfigMapRef != nil && ref.ConfigMapRef.Name == configMapName {
-			return true
-		}
-	}
-	return false
-}
-
 // resolved authz config. Update events are admitted only when .Data or .BinaryData actually
 // change, so metadata-only updates (labels, annotations, resourceVersion bumps) do not trigger
 // reconciliation. Create and Delete events are passed through so the controller can pick up a

cmd/thv-operator/controllers/virtualmcpserver_authz_configmap_test.go

@@ -80,6 +80,7 @@ func TestMapAuthzConfigMapToVirtualMCPServer(t *testing.T) {
 			vmcpNoIncomingAuth,
 			vmcpInOtherNamespace,
 		).
+		WithIndex(&mcpv1beta1.VirtualMCPServer{}, virtualMCPServerConfigMapIndex, indexVirtualMCPServerConfigMaps).
 		Build()
 
 	reconciler := &VirtualMCPServerReconciler{
@@ -143,7 +144,8 @@ func TestMapAuthzConfigMapToVirtualMCPServer_InlineCABundle(t *testing.T) {
 		OIDCConfig: &mcpv1beta1.OIDCUpstreamConfig{CABundleRef: &mcpv1beta1.CABundleSource{ConfigMapRef: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "ca-map"}}}},
 	}}}
 	scheme := testutil.NewScheme(t)
-	r := &VirtualMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(vmcp).Build()}
+	r := &VirtualMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(vmcp).
+		WithIndex(&mcpv1beta1.VirtualMCPServer{}, virtualMCPServerConfigMapIndex, indexVirtualMCPServerConfigMaps).Build()}
 	requests := r.mapAuthzConfigMapToVirtualMCPServer(t.Context(), &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "ca-map", Namespace: ns}})
 	require.Equal(t, []types.NamespacedName{{Name: "vmcp", Namespace: ns}}, []types.NamespacedName{requests[0].NamespacedName})
 }

cmd/thv-operator/controllers/virtualmcpserver_controller.go

@@ -467,8 +467,6 @@ func (r *VirtualMCPServerReconciler) runAuthValidations(
 	vmcp *mcpv1beta1.VirtualMCPServer,
 	statusManager virtualmcpserverstatus.StatusManager,
 ) (bool, error) {
-	ctxLogger := log.FromContext(ctx)
-
 	// Validate inline AuthServerConfig (when specified).
 	if vmcp.Spec.AuthServerConfig != nil {
 		// Surface the IdentitySynthesized advisory upfront, before validation.
@@ -482,13 +480,13 @@ func (r *VirtualMCPServerReconciler) runAuthValidations(
 		r.applyAuthServerIdentitySynthesizedCondition(vmcp, statusManager)
 		if err := r.validateAuthServerConfig(vmcp, statusManager); err != nil {
 			if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
-				ctxLogger.Error(applyErr, "Failed to apply status updates after AuthServerConfig validation error")
+				return false, applyErr
 			}
 			return false, nil
 		}
 		if terminal, err := r.validateAuthServerConfigCABundles(ctx, vmcp, statusManager); err != nil {
 			if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
-				ctxLogger.Error(applyErr, "Failed to apply status updates after CA bundle validation error")
+				return false, applyErr
 			}
 			if terminal {
 				return false, nil
@@ -507,7 +505,7 @@ func (r *VirtualMCPServerReconciler) runAuthValidations(
 	// RemoveConditionsWithPrefix call above when AuthServerConfig is nil.
 	if err := r.validateAuthzUpstreamAvailable(ctx, vmcp, statusManager); err != nil {
 		if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil {
-			ctxLogger.Error(applyErr, "Failed to apply status updates after AuthzUpstreamAvailable validation error")
+			return false, applyErr
 		}
 		return false, nil
 	}
@@ -2899,6 +2897,13 @@ func (r *VirtualMCPServerReconciler) mapEmbeddingServerToVirtualMCPServer(
 
 // SetupWithManager sets up the controller with the Manager
 func (r *VirtualMCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	if err := mgr.GetFieldIndexer().IndexField(
+		context.Background(), &mcpv1beta1.VirtualMCPServer{},
+		virtualMCPServerConfigMapIndex, indexVirtualMCPServerConfigMaps,
+	); err != nil {
+		return fmt.Errorf("failed to set up VirtualMCPServer ConfigMap reference index: %w", err)
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&mcpv1beta1.VirtualMCPServer{}).
 		Owns(&appsv1.Deployment{}).

cmd/thv-operator/controllers/virtualmcpserver_controller_test.go

@@ -4465,3 +4465,45 @@ func TestVirtualMCPServer_AuthServerConfigCABundleGetErrorIsTransient(t *testing
 	assert.NotEqual(t, mcpv1beta1.VirtualMCPServerPhaseFailed, vmcp.Status.Phase,
 		"a transient read failure must not stamp a terminal phase")
 }
+
+// TestVirtualMCPServer_RunAuthValidations_StatusWriteFailurePropagates verifies
+// that a failed status write inside runAuthValidations surfaces as an error
+// (so the caller requeues with backoff) instead of being logged and swallowed
+// as if the terminal validation path had completed cleanly.
+func TestVirtualMCPServer_RunAuthValidations_StatusWriteFailurePropagates(t *testing.T) {
+	t.Parallel()
+
+	// An empty Issuer fails validateAuthServerConfig on its first check,
+	// driving runAuthValidations into the applyStatusUpdates call whose
+	// error propagation this test targets.
+	vmcp := v1beta1test.NewVirtualMCPServer(testVmcpName, "default",
+		v1beta1test.WithVMCPAuthServerConfig(&mcpv1beta1.EmbeddedAuthServerConfig{}),
+		v1beta1test.MutateVMCP(func(v *mcpv1beta1.VirtualMCPServer) {
+			v.Generation = 1
+		}),
+	)
+
+	scheme := testutil.NewScheme(t)
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
+		WithObjects(vmcp).
+		WithStatusSubresource(&mcpv1beta1.VirtualMCPServer{}).
+		WithInterceptorFuncs(interceptor.Funcs{
+			SubResourceUpdate: func(_ context.Context, _ client.Client, _ string, _ client.Object,
+				_ ...client.SubResourceUpdateOption) error {
+				return apierrors.NewServiceUnavailable("apiserver is having a moment")
+			},
+		}).
+		Build()
+
+	reconciler := &VirtualMCPServerReconciler{
+		Client:           fakeClient,
+		Scheme:           scheme,
+		PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
+	}
+	statusManager := virtualmcpserverstatus.NewStatusManager(vmcp)
+
+	ok, err := reconciler.runAuthValidations(t.Context(), vmcp, statusManager)
+
+	require.Error(t, err, "a failed status write must surface so the caller requeues, not be swallowed")
+	assert.False(t, ok)
+}