Replace ReferencingServers with ReferencingWorkloads on MCPToolConfig (stacklok#4506)

Replace ReferencingServers with structured ReferencingWorkloads on MCPToolConfig

Migrate MCPToolConfig from plain string server names to structured
WorkloadReference entries (kind + name) per RFC-0023, following the
pattern already implemented on MCPOIDCConfig in PR stacklok#4492.

- Replace ReferencingServers []string with ReferencingWorkloads []WorkloadReference
- Add findReferencingWorkloads returning []WorkloadReference
- Update handleDeletion to use DeletionBlocked condition with ReferencedByWorkloads reason
- Update watch handler to check ref.Kind and ref.Name for stale entry cleanup
- Update unit, edge-case, and integration tests for WorkloadReference assertions
- Regenerate deepcopy, CRD manifests, and Helm templates

Closes stacklok#4491

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

Author: 2 people

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

@@ -96,17 +96,17 @@ type MCPToolConfigStatus struct {
 	// +optional
 	ConfigHash string `json:"configHash,omitempty"`
 
-	// ReferencingServers is a list of MCPServer resources that reference this MCPToolConfig
-	// This helps track which servers need to be reconciled when this config changes
+	// ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig.
+	// Each entry identifies the workload by kind and name.
 	// +optional
-	ReferencingServers []string `json:"referencingServers,omitempty"`
+	ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"`
 }
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:shortName=tc;toolconfig,categories=toolhive
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
-// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingServers`
+// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingWorkloads`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
 // MCPToolConfig is the Schema for the mcptoolconfigs API.

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

@@ -98,6 +98,15 @@ func (r *ToolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return r.handleConfigHashChange(ctx, toolConfig, configHash)
 	}
 
+	// Refresh ReferencingWorkloads list
+	referencingWorkloads, err := r.findReferencingWorkloads(ctx, toolConfig)
+	if err != nil {
+		logger.Error(err, "Failed to find referencing workloads")
+	} else if !slices.Equal(toolConfig.Status.ReferencingWorkloads, referencingWorkloads) {
+		toolConfig.Status.ReferencingWorkloads = referencingWorkloads
+		conditionChanged = true
+	}
+
 	// Update condition if it changed (even without hash change)
 	if conditionChanged {
 		if err := r.Status().Update(ctx, toolConfig); err != nil {
@@ -106,9 +115,7 @@ func (r *ToolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 	}
 
-	// Even when hash hasn't changed, update referencing servers list.
-	// This ensures ReferencingServers is updated when MCPServers are created or deleted.
-	return r.updateReferencingServers(ctx, toolConfig)
+	return ctrl.Result{}, nil
 }
 
 // handleConfigHashChange handles the logic when the config hash changes
@@ -134,13 +141,12 @@ func (r *ToolConfigReconciler) handleConfigHashChange(
 	toolConfig.Status.ConfigHash = configHash
 	toolConfig.Status.ObservedGeneration = toolConfig.Generation
 
-	// Update the status with the list of referencing servers
-	serverNames := make([]string, 0, len(referencingServers))
+	// Update the status with the list of referencing workloads
+	refs := make([]mcpv1alpha1.WorkloadReference, 0, len(referencingServers))
 	for _, server := range referencingServers {
-		serverNames = append(serverNames, server.Name)
+		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
 	}
-	slices.Sort(serverNames)
-	toolConfig.Status.ReferencingServers = serverNames
+	toolConfig.Status.ReferencingWorkloads = refs
 
 	// Update the MCPToolConfig status
 	if err := r.Status().Update(ctx, toolConfig); err != nil {
@@ -166,46 +172,6 @@ func (r *ToolConfigReconciler) handleConfigHashChange(
 	return ctrl.Result{}, nil
 }
 
-// updateReferencingServers finds referencing MCPServers and updates the status if the list changed
-func (r *ToolConfigReconciler) updateReferencingServers(
-	ctx context.Context,
-	toolConfig *mcpv1alpha1.MCPToolConfig,
-) (ctrl.Result, error) {
-	referencingServers, err := r.findReferencingMCPServers(ctx, toolConfig)
-	if err != nil {
-		logger := log.FromContext(ctx)
-		logger.Error(err, "Failed to find referencing MCPServers")
-		meta.SetStatusCondition(&toolConfig.Status.Conditions, metav1.Condition{
-			Type:               mcpv1alpha1.ConditionToolConfigValid,
-			Status:             metav1.ConditionFalse,
-			Reason:             mcpv1alpha1.ConditionReasonToolConfigValidationFailed,
-			Message:            "Failed to find referencing MCPServers",
-			ObservedGeneration: toolConfig.Generation,
-		})
-		if updateErr := r.Status().Update(ctx, toolConfig); updateErr != nil {
-			logger.Error(updateErr, "Failed to update MCPToolConfig status after error")
-		}
-		return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err)
-	}
-
-	serverNames := make([]string, 0, len(referencingServers))
-	for _, server := range referencingServers {
-		serverNames = append(serverNames, server.Name)
-	}
-	slices.Sort(serverNames)
-
-	if !slices.Equal(toolConfig.Status.ReferencingServers, serverNames) {
-		toolConfig.Status.ReferencingServers = serverNames
-		if err := r.Status().Update(ctx, toolConfig); err != nil {
-			logger := log.FromContext(ctx)
-			logger.Error(err, "Failed to update MCPToolConfig status")
-			return ctrl.Result{}, err
-		}
-	}
-
-	return ctrl.Result{}, nil
-}
-
 // calculateConfigHash calculates a hash of the MCPToolConfig spec using Kubernetes utilities
 func (*ToolConfigReconciler) calculateConfigHash(spec mcpv1alpha1.MCPToolConfigSpec) string {
 	return ctrlutil.CalculateConfigHash(spec)
@@ -216,31 +182,32 @@ func (r *ToolConfigReconciler) handleDeletion(ctx context.Context, toolConfig *m
 	logger := log.FromContext(ctx)
 
 	if controllerutil.ContainsFinalizer(toolConfig, ToolConfigFinalizerName) {
-		// Check if any MCPServers are still referencing this MCPToolConfig
-		referencingServers, err := r.findReferencingMCPServers(ctx, toolConfig)
+		// Check if any workloads still reference this MCPToolConfig
+		referencingWorkloads, err := r.findReferencingWorkloads(ctx, toolConfig)
 		if err != nil {
-			logger.Error(err, "Failed to find referencing MCPServers during deletion")
+			logger.Error(err, "Failed to check referencing workloads during deletion")
 			return ctrl.Result{}, err
 		}
 
-		if len(referencingServers) > 0 {
-			// Cannot delete - still referenced
-			serverNames := make([]string, 0, len(referencingServers))
-			for _, server := range referencingServers {
-				serverNames = append(serverNames, server.Name)
+		if len(referencingWorkloads) > 0 {
+			logger.Info("MCPToolConfig is still referenced by workloads, blocking deletion",
+				"toolconfig", toolConfig.Name,
+				"referencingWorkloads", referencingWorkloads)
+
+			meta.SetStatusCondition(&toolConfig.Status.Conditions, metav1.Condition{
+				Type:               "DeletionBlocked",
+				Status:             metav1.ConditionTrue,
+				Reason:             "ReferencedByWorkloads",
+				Message:            fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads),
+				ObservedGeneration: toolConfig.Generation,
+			})
+			toolConfig.Status.ReferencingWorkloads = referencingWorkloads
+			if updateErr := r.Status().Update(ctx, toolConfig); updateErr != nil {
+				logger.Error(updateErr, "Failed to update status during deletion block")
 			}
-			logger.Info("Cannot delete MCPToolConfig - still referenced by MCPServers",
-				"toolconfig", toolConfig.Name, "referencingServers", serverNames)
 
-			// Update status to show it's still referenced
-			toolConfig.Status.ReferencingServers = serverNames
-			if err := r.Status().Update(ctx, toolConfig); err != nil {
-				logger.Error(err, "Failed to update MCPToolConfig status during deletion")
-			}
-
-			// Return an error to prevent deletion
-			return ctrl.Result{}, fmt.Errorf("MCPToolConfig %s is still referenced by MCPServers: %v",
-				toolConfig.Name, serverNames)
+			// Requeue to check again later
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 		}
 
 		// No references, safe to remove finalizer and allow deletion
@@ -255,7 +222,28 @@ func (r *ToolConfigReconciler) handleDeletion(ctx context.Context, toolConfig *m
 	return ctrl.Result{}, nil
 }
 
-// findReferencingMCPServers finds all MCPServers that reference the given MCPToolConfig
+// findReferencingWorkloads returns the workload resources (MCPServer)
+// that reference this MCPToolConfig via their ToolConfigRef field.
+func (r *ToolConfigReconciler) findReferencingWorkloads(
+	ctx context.Context,
+	toolConfig *mcpv1alpha1.MCPToolConfig,
+) ([]mcpv1alpha1.WorkloadReference, error) {
+	mcpServerList := &mcpv1alpha1.MCPServerList{}
+	if err := r.List(ctx, mcpServerList, client.InNamespace(toolConfig.Namespace)); err != nil {
+		return nil, fmt.Errorf("failed to list MCPServers: %w", err)
+	}
+
+	var refs []mcpv1alpha1.WorkloadReference
+	for _, server := range mcpServerList.Items {
+		if server.Spec.ToolConfigRef != nil && server.Spec.ToolConfigRef.Name == toolConfig.Name {
+			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
+		}
+	}
+	return refs, nil
+}
+
+// findReferencingMCPServers finds all MCPServers that reference the given MCPToolConfig.
+// Returns the full MCPServer objects, used by handleConfigHashChange to update server annotations.
 func (r *ToolConfigReconciler) findReferencingMCPServers(
 	ctx context.Context,
 	toolConfig *mcpv1alpha1.MCPToolConfig,
@@ -270,33 +258,59 @@ func (r *ToolConfigReconciler) findReferencingMCPServers(
 }
 
 // SetupWithManager sets up the controller with the Manager.
+// Watches MCPServer changes to maintain accurate ReferencingWorkloads status.
 func (r *ToolConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	// Create a handler that maps MCPServer changes to MCPToolConfig reconciliation requests.
-	// When an MCPServer is created/updated/deleted, we need to reconcile the MCPToolConfig
-	// it references so that the ReferencingServers status field stays up to date.
+	// Watch MCPServer changes to update ReferencingWorkloads on referenced MCPToolConfigs.
+	// This handler enqueues both the currently-referenced MCPToolConfig AND any
+	// MCPToolConfig that still lists this server in ReferencingWorkloads (covers the
+	// case where a server removes its toolConfigRef — the previously-referenced
+	// config needs to reconcile and clean up the stale entry).
 	toolConfigHandler := handler.EnqueueRequestsFromMapFunc(
-		func(_ context.Context, obj client.Object) []reconcile.Request {
-			mcpServer, ok := obj.(*mcpv1alpha1.MCPServer)
+		func(ctx context.Context, obj client.Object) []reconcile.Request {
+			server, ok := obj.(*mcpv1alpha1.MCPServer)
 			if !ok {
 				return nil
 			}
 
-			if mcpServer.Spec.ToolConfigRef == nil {
-				return nil
+			seen := make(map[types.NamespacedName]struct{})
+			var requests []reconcile.Request
+
+			// Enqueue the currently-referenced MCPToolConfig (if any)
+			if server.Spec.ToolConfigRef != nil {
+				nn := types.NamespacedName{
+					Name:      server.Spec.ToolConfigRef.Name,
+					Namespace: server.Namespace,
+				}
+				seen[nn] = struct{}{}
+				requests = append(requests, reconcile.Request{NamespacedName: nn})
+			}
+
+			// Also enqueue any MCPToolConfig that still lists this server in
+			// ReferencingWorkloads — handles ref-removal and server-deletion cases.
+			toolConfigList := &mcpv1alpha1.MCPToolConfigList{}
+			if err := r.List(ctx, toolConfigList, client.InNamespace(server.Namespace)); err != nil {
+				log.FromContext(ctx).Error(err, "Failed to list MCPToolConfigs for MCPServer watch")
+				return requests
+			}
+			for _, cfg := range toolConfigList.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 == "MCPServer" && ref.Name == server.Name {
+						requests = append(requests, reconcile.Request{NamespacedName: nn})
+						break
+					}
+				}
 			}
 
-			return []reconcile.Request{{
-				NamespacedName: types.NamespacedName{
-					Name:      mcpServer.Spec.ToolConfigRef.Name,
-					Namespace: mcpServer.Namespace,
-				},
-			}}
+			return requests
 		},
 	)
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&mcpv1alpha1.MCPToolConfig{}).
-		// Watch for MCPServers and reconcile the MCPToolConfig when they change
 		Watches(&mcpv1alpha1.MCPServer{}, toolConfigHandler).
 		Complete(r)
 }

cmd/thv-operator/controllers/toolconfig_controller.go

@@ -122,7 +122,8 @@ func TestToolConfigReconciler_EdgeCases(t *testing.T) {
 		err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
 		require.NoError(t, err)
 		assert.NotEmpty(t, updatedConfig.Status.ConfigHash)
-		assert.Contains(t, updatedConfig.Status.ReferencingServers, "test-server")
+		assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
+			mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "test-server"})
 	})
 
 	t.Run("reconcile with changed spec", func(t *testing.T) {
@@ -350,10 +351,13 @@ func TestToolConfigReconciler_ComplexScenarios(t *testing.T) {
 		var updatedConfig mcpv1alpha1.MCPToolConfig
 		err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
 		require.NoError(t, err)
-		assert.Len(t, updatedConfig.Status.ReferencingServers, 3)
-		assert.Contains(t, updatedConfig.Status.ReferencingServers, "server1")
-		assert.Contains(t, updatedConfig.Status.ReferencingServers, "server2")
-		assert.Contains(t, updatedConfig.Status.ReferencingServers, "server3")
+		assert.Len(t, updatedConfig.Status.ReferencingWorkloads, 3)
+		assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
+			mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server1"})
+		assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
+			mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server2"})
+		assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
+			mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server3"})
 	})
 
 	t.Run("empty MCPToolConfig spec", func(t *testing.T) {