Extract shared WorkloadReference helpers into controllerutil (#4520)

* Extract shared WorkloadReference helpers into controllerutil

The ReferencingWorkloads pattern was duplicated across four config
controllers with inconsistent sorting behavior: MCPTelemetryConfig and
MCPExternalAuthConfig sorted refs, but MCPToolConfig and MCPOIDCConfig
did not. This caused unnecessary API server writes when the same set of
workloads was discovered in a different list order across reconcile runs.

Extract SortWorkloadRefs, WorkloadRefsEqual, and
FindWorkloadRefsFromMCPServers into the shared controllerutil package
so all controllers use deterministic, consistent ordering. Each
controller's findReferencingWorkloads now delegates to the shared
helper, removing ~50 lines of duplicated list-filter-build logic.

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

* Add WorkloadKind constants to replace string literals

Define WorkloadKindMCPServer, WorkloadKindVirtualMCPServer, and
WorkloadKindMCPRemoteProxy constants in the API types and use them
across all controllers. This fixes a goconst lint violation where
"MCPServer" appeared as a raw string literal 4+ times.

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: ChrisJBurns

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

@@ -144,6 +144,13 @@ type InlineOIDCSharedConfig struct {
 	InsecureAllowHTTP bool `json:"insecureAllowHTTP"`
 }
 
+// Well-known WorkloadReference Kind values.
+const (
+	WorkloadKindMCPServer        = "MCPServer"
+	WorkloadKindVirtualMCPServer = "VirtualMCPServer"
+	WorkloadKindMCPRemoteProxy   = "MCPRemoteProxy"
+)
+
 // WorkloadReference identifies a workload that references a shared configuration resource.
 // Namespace is implicit — cross-namespace references are not supported.
 type WorkloadReference struct {

cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go

@@ -6,7 +6,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"slices"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -158,23 +157,9 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange(
 	// Update the status with the list of referencing workloads
 	refs := make([]mcpv1alpha1.WorkloadReference, 0, len(referencingServers))
 	for _, server := range referencingServers {
-		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
+		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPServer, Name: server.Name})
 	}
-	slices.SortFunc(refs, func(a, b mcpv1alpha1.WorkloadReference) int {
-		if a.Kind != b.Kind {
-			if a.Kind < b.Kind {
-				return -1
-			}
-			return 1
-		}
-		if a.Name < b.Name {
-			return -1
-		}
-		if a.Name > b.Name {
-			return 1
-		}
-		return 0
-	})
+	ctrlutil.SortWorkloadRefs(refs)
 	externalAuthConfig.Status.ReferencingWorkloads = refs
 
 	// Update the MCPExternalAuthConfig status
@@ -271,18 +256,13 @@ func (r *MCPExternalAuthConfigReconciler) findReferencingWorkloads(
 	ctx context.Context,
 	externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig,
 ) ([]mcpv1alpha1.WorkloadReference, error) {
-	mcpServerList := &mcpv1alpha1.MCPServerList{}
-	if err := r.List(ctx, mcpServerList, client.InNamespace(externalAuthConfig.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.ExternalAuthConfigRef != nil && server.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name {
-			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
-		}
-	}
-	return refs, nil
+	return ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name,
+		func(server *mcpv1alpha1.MCPServer) *string {
+			if server.Spec.ExternalAuthConfigRef != nil {
+				return &server.Spec.ExternalAuthConfigRef.Name
+			}
+			return nil
+		})
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -326,7 +306,7 @@ func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) err
 					continue
 				}
 				for _, ref := range cfg.Status.ReferencingWorkloads {
-					if ref.Kind == "MCPServer" && ref.Name == server.Name {
+					if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name {
 						requests = append(requests, reconcile.Request{NamespacedName: nn})
 						break
 					}
@@ -356,7 +336,7 @@ func (r *MCPExternalAuthConfigReconciler) updateReferencingWorkloads(
 		return ctrl.Result{}, fmt.Errorf("failed to find referencing workloads: %w", err)
 	}
 
-	if !slices.Equal(externalAuthConfig.Status.ReferencingWorkloads, refs) {
+	if !ctrlutil.WorkloadRefsEqual(externalAuthConfig.Status.ReferencingWorkloads, refs) {
 		externalAuthConfig.Status.ReferencingWorkloads = refs
 		if err := r.Status().Update(ctx, externalAuthConfig); err != nil {
 			logger := log.FromContext(ctx)

cmd/thv-operator/controllers/mcpoidcconfig_controller.go

@@ -6,7 +6,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"slices"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -129,7 +128,7 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	referencingWorkloads, err := r.findReferencingWorkloads(ctx, oidcConfig)
 	if err != nil {
 		logger.Error(err, "Failed to find referencing workloads")
-	} else if !slices.Equal(oidcConfig.Status.ReferencingWorkloads, referencingWorkloads) {
+	} else if !ctrlutil.WorkloadRefsEqual(oidcConfig.Status.ReferencingWorkloads, referencingWorkloads) {
 		oidcConfig.Status.ReferencingWorkloads = referencingWorkloads
 		conditionChanged = true
 	}
@@ -206,19 +205,19 @@ func (r *MCPOIDCConfigReconciler) findReferencingWorkloads(
 	ctx context.Context,
 	oidcConfig *mcpv1alpha1.MCPOIDCConfig,
 ) ([]mcpv1alpha1.WorkloadReference, error) {
-	mcpServerList := &mcpv1alpha1.MCPServerList{}
-	if err := r.List(ctx, mcpServerList, client.InNamespace(oidcConfig.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.OIDCConfigRef != nil && server.Spec.OIDCConfigRef.Name == oidcConfig.Name {
-			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
-		}
+	// Find referencing MCPServers
+	refs, err := ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, oidcConfig.Namespace, oidcConfig.Name,
+		func(server *mcpv1alpha1.MCPServer) *string {
+			if server.Spec.OIDCConfigRef != nil {
+				return &server.Spec.OIDCConfigRef.Name
+			}
+			return nil
+		})
+	if err != nil {
+		return nil, err
 	}
 
-	// Check VirtualMCPServers
+	// Also 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)
@@ -227,10 +226,11 @@ func (r *MCPOIDCConfigReconciler) findReferencingWorkloads(
 		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})
+			refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindVirtualMCPServer, Name: vmcp.Name})
 		}
 	}
 
+	ctrlutil.SortWorkloadRefs(refs)
 	return refs, nil
 }
 
@@ -275,7 +275,7 @@ func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 					continue
 				}
 				for _, ref := range cfg.Status.ReferencingWorkloads {
-					if ref.Kind == "MCPServer" && ref.Name == server.Name {
+					if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name {
 						requests = append(requests, reconcile.Request{NamespacedName: nn})
 						break
 					}
@@ -333,7 +333,7 @@ func (r *MCPOIDCConfigReconciler) mapVirtualMCPServerToOIDCConfig(
 			continue
 		}
 		for _, ref := range cfg.Status.ReferencingWorkloads {
-			if ref.Kind == "VirtualMCPServer" && ref.Name == vmcp.Name {
+			if ref.Kind == mcpv1alpha1.WorkloadKindVirtualMCPServer && ref.Name == vmcp.Name {
 				requests = append(requests, reconcile.Request{NamespacedName: nn})
 				break
 			}

cmd/thv-operator/controllers/mcpserver_controller.go

@@ -2132,7 +2132,7 @@ func (r *MCPServerReconciler) updateOIDCConfigReferencingWorkloads(
 	serverName string,
 ) error {
 	ref := mcpv1alpha1.WorkloadReference{
-		Kind: "MCPServer",
+		Kind: mcpv1alpha1.WorkloadKindMCPServer,
 		Name: serverName,
 	}
 

cmd/thv-operator/controllers/mcptelemetryconfig_controller.go

@@ -6,7 +6,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"slices"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -116,7 +115,7 @@ func (r *MCPTelemetryConfigReconciler) Reconcile(ctx context.Context, req ctrl.R
 
 	// Check what changed
 	hashChanged := telemetryConfig.Status.ConfigHash != configHash
-	refsChanged := !workloadRefsEqual(telemetryConfig.Status.ReferencingWorkloads, referencingWorkloads)
+	refsChanged := !ctrlutil.WorkloadRefsEqual(telemetryConfig.Status.ReferencingWorkloads, referencingWorkloads)
 	needsUpdate := hashChanged || refsChanged || conditionChanged
 
 	if hashChanged {
@@ -180,7 +179,7 @@ func (r *MCPTelemetryConfigReconciler) SetupWithManager(mgr ctrl.Manager) error
 					continue
 				}
 				for _, ref := range cfg.Status.ReferencingWorkloads {
-					if ref.Kind == "MCPServer" && ref.Name == server.Name {
+					if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name {
 						requests = append(requests, reconcile.Request{NamespacedName: nn})
 						break
 					}
@@ -257,36 +256,11 @@ func (r *MCPTelemetryConfigReconciler) findReferencingWorkloads(
 	ctx context.Context,
 	telemetryConfig *mcpv1alpha1.MCPTelemetryConfig,
 ) ([]mcpv1alpha1.WorkloadReference, error) {
-	mcpServerList := &mcpv1alpha1.MCPServerList{}
-	if err := r.List(ctx, mcpServerList, client.InNamespace(telemetryConfig.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.TelemetryConfigRef != nil &&
-			server.Spec.TelemetryConfigRef.Name == telemetryConfig.Name {
-			refs = append(refs, mcpv1alpha1.WorkloadReference{
-				Kind: "MCPServer",
-				Name: server.Name,
-			})
-		}
-	}
-	slices.SortFunc(refs, func(a, b mcpv1alpha1.WorkloadReference) int {
-		if a.Name < b.Name {
-			return -1
-		}
-		if a.Name > b.Name {
-			return 1
-		}
-		return 0
-	})
-	return refs, nil
-}
-
-// workloadRefsEqual compares two WorkloadReference slices for equality.
-func workloadRefsEqual(a, b []mcpv1alpha1.WorkloadReference) bool {
-	return slices.EqualFunc(a, b, func(x, y mcpv1alpha1.WorkloadReference) bool {
-		return x.Kind == y.Kind && x.Name == y.Name
-	})
+	return ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, telemetryConfig.Namespace, telemetryConfig.Name,
+		func(server *mcpv1alpha1.MCPServer) *string {
+			if server.Spec.TelemetryConfigRef != nil {
+				return &server.Spec.TelemetryConfigRef.Name
+			}
+			return nil
+		})
 }

cmd/thv-operator/controllers/toolconfig_controller.go

@@ -6,7 +6,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"slices"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -102,7 +101,7 @@ func (r *ToolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	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) {
+	} else if !ctrlutil.WorkloadRefsEqual(toolConfig.Status.ReferencingWorkloads, referencingWorkloads) {
 		toolConfig.Status.ReferencingWorkloads = referencingWorkloads
 		conditionChanged = true
 	}
@@ -144,8 +143,9 @@ func (r *ToolConfigReconciler) handleConfigHashChange(
 	// Update the status with the list of referencing workloads
 	refs := make([]mcpv1alpha1.WorkloadReference, 0, len(referencingServers))
 	for _, server := range referencingServers {
-		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
+		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPServer, Name: server.Name})
 	}
+	ctrlutil.SortWorkloadRefs(refs)
 	toolConfig.Status.ReferencingWorkloads = refs
 
 	// Update the MCPToolConfig status
@@ -228,18 +228,13 @@ 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
+	return ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, toolConfig.Namespace, toolConfig.Name,
+		func(server *mcpv1alpha1.MCPServer) *string {
+			if server.Spec.ToolConfigRef != nil {
+				return &server.Spec.ToolConfigRef.Name
+			}
+			return nil
+		})
 }
 
 // findReferencingMCPServers finds all MCPServers that reference the given MCPToolConfig.
@@ -298,7 +293,7 @@ func (r *ToolConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 					continue
 				}
 				for _, ref := range cfg.Status.ReferencingWorkloads {
-					if ref.Kind == "MCPServer" && ref.Name == server.Name {
+					if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name {
 						requests = append(requests, reconcile.Request{NamespacedName: nn})
 						break
 					}

cmd/thv-operator/controllers/virtualmcpserver_controller.go

@@ -2824,7 +2824,7 @@ func (r *VirtualMCPServerReconciler) updateOIDCConfigReferencingWorkloads(
 	oidcConfig *mcpv1alpha1.MCPOIDCConfig,
 	vmcpName string,
 ) error {
-	ref := mcpv1alpha1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName}
+	ref := mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindVirtualMCPServer, Name: vmcpName}
 	// Check if already listed
 	for _, entry := range oidcConfig.Status.ReferencingWorkloads {
 		if entry.Kind == ref.Kind && entry.Name == ref.Name {

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

@@ -7,6 +7,8 @@ import (
 	"context"
 	"fmt"
 	"hash/fnv"
+	"slices"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/dump"
@@ -72,6 +74,52 @@ func FindReferencingMCPServers(
 	return referencingServers, nil
 }
 
+// CompareWorkloadRefs compares two WorkloadReference values by Kind then Name.
+// Suitable for use with slices.SortFunc.
+func CompareWorkloadRefs(a, b mcpv1alpha1.WorkloadReference) int {
+	if a.Kind != b.Kind {
+		return strings.Compare(a.Kind, b.Kind)
+	}
+	return strings.Compare(a.Name, b.Name)
+}
+
+// SortWorkloadRefs sorts a WorkloadReference slice by Kind then Name for deterministic ordering.
+// This prevents unnecessary API server writes when the same set of workloads is discovered
+// in a different list order across reconcile runs.
+func SortWorkloadRefs(refs []mcpv1alpha1.WorkloadReference) {
+	slices.SortFunc(refs, CompareWorkloadRefs)
+}
+
+// WorkloadRefsEqual reports whether two WorkloadReference slices contain the same entries.
+// Both slices must already be sorted (use SortWorkloadRefs) for correct results.
+func WorkloadRefsEqual(a, b []mcpv1alpha1.WorkloadReference) bool {
+	return slices.EqualFunc(a, b, func(x, y mcpv1alpha1.WorkloadReference) bool {
+		return x.Kind == y.Kind && x.Name == y.Name
+	})
+}
+
+// FindWorkloadRefsFromMCPServers returns a sorted list of WorkloadReference for MCPServers
+// in the given namespace that reference a config identified by configName.
+// The refExtractor determines which spec field contains the config reference name.
+func FindWorkloadRefsFromMCPServers(
+	ctx context.Context,
+	c client.Client,
+	namespace string,
+	configName string,
+	refExtractor func(*mcpv1alpha1.MCPServer) *string,
+) ([]mcpv1alpha1.WorkloadReference, error) {
+	servers, err := FindReferencingMCPServers(ctx, c, namespace, configName, refExtractor)
+	if err != nil {
+		return nil, err
+	}
+	refs := make([]mcpv1alpha1.WorkloadReference, 0, len(servers))
+	for _, server := range servers {
+		refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPServer, Name: server.Name})
+	}
+	SortWorkloadRefs(refs)
+	return refs, nil
+}
+
 // GetToolConfigForMCPRemoteProxy fetches MCPToolConfig referenced by MCPRemoteProxy
 func GetToolConfigForMCPRemoteProxy(
 	ctx context.Context,