Enable MCPRemoteProxy discovery in MCPGroup and VirtualMCPServer (#2970)

* add MCPRemoteProxy support in MCPGroup and VirtualMCPServer * use list instead of get in loop * refactor and change the name * operator crds bump * fix unit tests * updated crd-ref gen * SetupWithManager add a Watches() call for MCPRemoteProxy resources * test coverage for findReferencingMCPRemoteProxies, findMCPGroupForMCPRemoteProxy, updateReferencingRemoteProxiesOnDeletion * consolidating status updates at the end of reconciliation * clear both status after entering failed status * removed GroupRefValidated condition when the groupRef is empty to remove any stale data * adding test cases for: MCPRemoteProxy discovery, Mixed groups (both MCPServer and MCPRemoteProxy), mcpRemoteProxyToBackend conversion * adding test cases with WorkloadTypeMCPRemoteProxy to verify the aggregator handles both workload types correctly * adding backend.Metadata[workload_type] = mcp_server in mcpServerToBackend as well. * unit tests to verify the adapter correctly converts workload types * consolidate the discoverRemoteProxyAuthConfig and discoverAuthConfig * bump version * operator-crds version bump --------- Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>

Author: amirejaz

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

@@ -18,14 +18,22 @@ type MCPGroupStatus struct {
 // +kubebuilder:default=Pending
 Phase MCPGroupPhase `json:"phase,omitempty"`
 
-// Servers lists server names in this group
+// Servers lists MCPServer names in this group
 // +optional
 Servers []string `json:"servers"`
 
-// ServerCount is the number of servers
+// ServerCount is the number of MCPServers
 // +optional
 ServerCount int `json:"serverCount"`
 
+// RemoteProxies lists MCPRemoteProxy names in this group
+// +optional
+RemoteProxies []string `json:"remoteProxies,omitempty"`
+
+// RemoteProxyCount is the number of MCPRemoteProxies
+// +optional
+RemoteProxyCount int `json:"remoteProxyCount,omitempty"`
+
 // Conditions represent observations
 // +optional
 Conditions []metav1.Condition `json:"conditions,omitempty"`

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

@@ -66,6 +66,11 @@ type MCPRemoteProxySpec struct {
 // ResourceOverrides allows overriding annotations and labels for resources created by the operator
 // +optional
 ResourceOverrides *ResourceOverrides `json:"resourceOverrides,omitempty"`
+
+// GroupRef is the name of the MCPGroup this proxy belongs to
+// Must reference an existing MCPGroup in the same namespace
+// +optional
+GroupRef string `json:"groupRef,omitempty"`
 }
 
 // MCPRemoteProxyStatus defines the observed state of MCPRemoteProxy
@@ -131,6 +136,9 @@ const (
 
 // ConditionTypeAuthConfigured indicates whether authentication is properly configured
 ConditionTypeAuthConfigured = "AuthConfigured"
+
+// ConditionTypeMCPRemoteProxyGroupRefValidated indicates whether the GroupRef is valid
+ConditionTypeMCPRemoteProxyGroupRefValidated = "GroupRefValidated"
 )
 
 // Condition reasons for MCPRemoteProxy
@@ -155,6 +163,15 @@ const (
 
 // ConditionReasonMissingOIDCConfig indicates OIDCConfig is not specified
 ConditionReasonMissingOIDCConfig = "MissingOIDCConfig"
+
+// ConditionReasonMCPRemoteProxyGroupRefValidated indicates the GroupRef is valid
+ConditionReasonMCPRemoteProxyGroupRefValidated = "GroupRefIsValid"
+
+// ConditionReasonMCPRemoteProxyGroupRefNotFound indicates the GroupRef is invalid
+ConditionReasonMCPRemoteProxyGroupRefNotFound = "GroupRefNotFound"
+
+// ConditionReasonMCPRemoteProxyGroupRefNotReady indicates the referenced MCPGroup is not in the Ready state
+ConditionReasonMCPRemoteProxyGroupRefNotReady = "GroupRefNotReady"
 )
 
 //+kubebuilder:object:root=true

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

@@ -33,6 +33,8 @@ type MCPGroupReconciler struct {
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpgroups/finalizers,verbs=update
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpservers,verbs=get;list;watch
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpservers/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies,verbs=get;list;watch
+// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies/status,verbs=get;update;patch
 
 // Reconcile is part of the main kubernetes reconciliation loop
 // which aims to move the current state of the cluster closer to the desired state.
@@ -71,68 +73,137 @@ func (r *MCPGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 return ctrl.Result{RequeueAfter: 500 * time.Millisecond}, nil
 }
 
+// Find and update status for MCPServers and MCPRemoteProxies
+result, err := r.updateGroupMemberStatus(ctx, mcpGroup)
+if err != nil || result.RequeueAfter > 0 {
+return result, err
+}
+
+ctxLogger.Info("Successfully reconciled MCPGroup",
+"serverCount", mcpGroup.Status.ServerCount,
+"remoteProxyCount", mcpGroup.Status.RemoteProxyCount)
+return ctrl.Result{}, nil
+}
+
+// updateGroupMemberStatus finds MCPServers and MCPRemoteProxies referencing the group
+// and updates the MCPGroup status accordingly.
+func (r *MCPGroupReconciler) updateGroupMemberStatus(
+ctx context.Context,
+mcpGroup *mcpv1alpha1.MCPGroup,
+) (ctrl.Result, error) {
+ctxLogger := log.FromContext(ctx)
+
 // Find MCPServers that reference this MCPGroup
 mcpServers, err := r.findReferencingMCPServers(ctx, mcpGroup)
 if err != nil {
-ctxLogger.Error(err, "Failed to list MCPServers")
-mcpGroup.Status.Phase = mcpv1alpha1.MCPGroupPhaseFailed
-meta.SetStatusCondition(&mcpGroup.Status.Conditions, metav1.Condition{
-Type: mcpv1alpha1.ConditionTypeMCPServersChecked,
-Status: metav1.ConditionFalse,
-Reason: mcpv1alpha1.ConditionReasonListMCPServersFailed,
-Message: "Failed to list MCPServers in namespace",
-ObservedGeneration: mcpGroup.Generation,
-})
-mcpGroup.Status.ServerCount = 0
-mcpGroup.Status.Servers = nil
-// Update the MCPGroup status to reflect the failure
-if updateErr := r.Status().Update(ctx, mcpGroup); updateErr != nil {
-if errors.IsConflict(err) {
-// Requeue to retry with fresh data
-return ctrl.Result{Requeue: true}, nil
-}
-ctxLogger.Error(updateErr, "Failed to update MCPGroup status after list failure")
-}
-return ctrl.Result{}, nil
+return r.handleListFailure(ctx, mcpGroup, err, "MCPServers")
+}
+
+// Find MCPRemoteProxies that reference this MCPGroup
+mcpRemoteProxies, err := r.findReferencingMCPRemoteProxies(ctx, mcpGroup)
+if err != nil {
+return r.handleListFailure(ctx, mcpGroup, err, "MCPRemoteProxies")
 }
 
 meta.SetStatusCondition(&mcpGroup.Status.Conditions, metav1.Condition{
 Type: mcpv1alpha1.ConditionTypeMCPServersChecked,
 Status: metav1.ConditionTrue,
 Reason: mcpv1alpha1.ConditionReasonListMCPServersSucceeded,
-Message: "Successfully listed MCPServers in namespace",
+Message: "Successfully listed MCPServers and MCPRemoteProxies in namespace",
 ObservedGeneration: mcpGroup.Generation,
 })
 
-// Set MCPGroup status fields
-mcpGroup.Status.ServerCount = len(mcpServers)
-if len(mcpServers) == 0 {
-mcpGroup.Status.Servers = []string{}
-} else {
-// If there are servers, extract their names
-mcpGroup.Status.Servers = make([]string, len(mcpServers))
-for i, server := range mcpServers {
-mcpGroup.Status.Servers[i] = server.Name
-}
-sort.Strings(mcpGroup.Status.Servers)
-}
+// Set MCPGroup status fields for MCPServers
+r.populateServerStatus(mcpGroup, mcpServers)
+
+// Set MCPGroup status fields for MCPRemoteProxies
+r.populateRemoteProxyStatus(mcpGroup, mcpRemoteProxies)
 
 mcpGroup.Status.Phase = mcpv1alpha1.MCPGroupPhaseReady
 
 // Update the MCPGroup status
 if err := r.Status().Update(ctx, mcpGroup); err != nil {
 if errors.IsConflict(err) {
-// Requeue to retry with fresh data
-return ctrl.Result{Requeue: true}, nil
+return ctrl.Result{RequeueAfter: 500 * time.Millisecond}, nil
 }
 ctxLogger.Error(err, "Failed to update MCPGroup status")
 return ctrl.Result{}, err
 }
 
-ctxLogger.Info("Successfully reconciled MCPGroup", "serverCount", mcpGroup.Status.ServerCount)
+ctxLogger.Info("Successfully reconciled MCPGroup",
+"serverCount", mcpGroup.Status.ServerCount,
+"remoteProxyCount", mcpGroup.Status.RemoteProxyCount)
+return ctrl.Result{}, nil
+}
+
+// handleListFailure handles the case when listing MCPServers or MCPRemoteProxies fails.
+func (r *MCPGroupReconciler) handleListFailure(
+ctx context.Context,
+mcpGroup *mcpv1alpha1.MCPGroup,
+listErr error,
+resourceType string,
+) (ctrl.Result, error) {
+ctxLogger := log.FromContext(ctx)
+ctxLogger.Error(listErr, "Failed to list "+resourceType)
+
+mcpGroup.Status.Phase = mcpv1alpha1.MCPGroupPhaseFailed
+meta.SetStatusCondition(&mcpGroup.Status.Conditions, metav1.Condition{
+Type: mcpv1alpha1.ConditionTypeMCPServersChecked,
+Status: metav1.ConditionFalse,
+Reason: mcpv1alpha1.ConditionReasonListMCPServersFailed,
+Message: "Failed to list " + resourceType + " in namespace",
+ObservedGeneration: mcpGroup.Generation,
+})
+
+// Clear both resource types' status fields to avoid stale data when entering Failed state
+mcpGroup.Status.ServerCount = 0
+mcpGroup.Status.Servers = nil
+mcpGroup.Status.RemoteProxyCount = 0
+mcpGroup.Status.RemoteProxies = nil
+
+if updateErr := r.Status().Update(ctx, mcpGroup); updateErr != nil {
+if errors.IsConflict(updateErr) {
+return ctrl.Result{RequeueAfter: 500 * time.Millisecond}, nil
+}
+ctxLogger.Error(updateErr, "Failed to update MCPGroup status after list failure")
+}
 return ctrl.Result{}, nil
 }
 
+// populateServerStatus populates the MCPGroup status with MCPServer information.
+func (*MCPGroupReconciler) populateServerStatus(
+mcpGroup *mcpv1alpha1.MCPGroup,
+mcpServers []mcpv1alpha1.MCPServer,
+) {
+mcpGroup.Status.ServerCount = len(mcpServers)
+if len(mcpServers) == 0 {
+mcpGroup.Status.Servers = []string{}
+return
+}
+mcpGroup.Status.Servers = make([]string, len(mcpServers))
+for i, server := range mcpServers {
+mcpGroup.Status.Servers[i] = server.Name
+}
+sort.Strings(mcpGroup.Status.Servers)
+}
+
+// populateRemoteProxyStatus populates the MCPGroup status with MCPRemoteProxy information.
+func (*MCPGroupReconciler) populateRemoteProxyStatus(
+mcpGroup *mcpv1alpha1.MCPGroup,
+mcpRemoteProxies []mcpv1alpha1.MCPRemoteProxy,
+) {
+mcpGroup.Status.RemoteProxyCount = len(mcpRemoteProxies)
+if len(mcpRemoteProxies) == 0 {
+mcpGroup.Status.RemoteProxies = []string{}
+return
+}
+mcpGroup.Status.RemoteProxies = make([]string, len(mcpRemoteProxies))
+for i, proxy := range mcpRemoteProxies {
+mcpGroup.Status.RemoteProxies[i] = proxy.Name
+}
+sort.Strings(mcpGroup.Status.RemoteProxies)
+}
+
 // handleDeletion handles the deletion of an MCPGroup
 func (r *MCPGroupReconciler) handleDeletion(ctx context.Context, mcpGroup *mcpv1alpha1.MCPGroup) (ctrl.Result, error) {
 ctxLogger := log.FromContext(ctx)
@@ -151,6 +222,19 @@ func (r *MCPGroupReconciler) handleDeletion(ctx context.Context, mcpGroup *mcpv1
 r.updateReferencingServersOnDeletion(ctx, referencingServers, mcpGroup.Name)
 }
 
+// Find all MCPRemoteProxies that reference this group
+referencingProxies, err := r.findReferencingMCPRemoteProxies(ctx, mcpGroup)
+if err != nil {
+ctxLogger.Error(err, "Failed to find referencing MCPRemoteProxies during deletion")
+return ctrl.Result{}, err
+}
+
+// Update conditions on all referencing MCPRemoteProxies to indicate the group is being deleted
+if len(referencingProxies) > 0 {
+ctxLogger.Info("Updating conditions on referencing MCPRemoteProxies", "count", len(referencingProxies))
+r.updateReferencingRemoteProxiesOnDeletion(ctx, referencingProxies, mcpGroup.Name)
+}
+
 // Remove the finalizer to allow deletion
 controllerutil.RemoveFinalizer(mcpGroup, MCPGroupFinalizerName)
 if err := r.Update(ctx, mcpGroup); err != nil {
@@ -183,6 +267,22 @@ func (r *MCPGroupReconciler) findReferencingMCPServers(
 return mcpServerList.Items, nil
 }
 
+// findReferencingMCPRemoteProxies finds all MCPRemoteProxies that reference the given MCPGroup
+func (r *MCPGroupReconciler) findReferencingMCPRemoteProxies(
+ctx context.Context, mcpGroup *mcpv1alpha1.MCPGroup) ([]mcpv1alpha1.MCPRemoteProxy, error) {
+
+mcpRemoteProxyList := &mcpv1alpha1.MCPRemoteProxyList{}
+listOpts := []client.ListOption{
+client.InNamespace(mcpGroup.Namespace),
+client.MatchingFields{"spec.groupRef": mcpGroup.Name},
+}
+if err := r.List(ctx, mcpRemoteProxyList, listOpts...); err != nil {
+return nil, err
+}
+
+return mcpRemoteProxyList.Items, nil
+}
+
 // updateReferencingServersOnDeletion updates the conditions on MCPServers to indicate their group is being deleted
 func (r *MCPGroupReconciler) updateReferencingServersOnDeletion(
 ctx context.Context, servers []mcpv1alpha1.MCPServer, groupName string) {
@@ -210,6 +310,33 @@ func (r *MCPGroupReconciler) updateReferencingServersOnDeletion(
 }
 }
 
+// updateReferencingRemoteProxiesOnDeletion updates the conditions on MCPRemoteProxies to indicate their group is being deleted
+func (r *MCPGroupReconciler) updateReferencingRemoteProxiesOnDeletion(
+ctx context.Context, proxies []mcpv1alpha1.MCPRemoteProxy, groupName string) {
+ctxLogger := log.FromContext(ctx)
+
+for _, proxy := range proxies {
+// Update the condition to indicate the group is being deleted
+meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{
+Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyGroupRefValidated,
+Status: metav1.ConditionFalse,
+Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyGroupRefNotFound,
+Message: "Referenced MCPGroup is being deleted",
+ObservedGeneration: proxy.Generation,
+})
+
+// Update the proxy status
+if err := r.Status().Update(ctx, &proxy); err != nil {
+ctxLogger.Error(err, "Failed to update MCPRemoteProxy condition during group deletion",
+"mcpremoteproxy", proxy.Name, "mcpgroup", groupName)
+// Continue with other proxies even if one fails
+continue
+}
+ctxLogger.Info("Updated MCPRemoteProxy condition for group deletion",
+"mcpremoteproxy", proxy.Name, "mcpgroup", groupName)
+}
+}
+
 func (r *MCPGroupReconciler) findMCPGroupForMCPServer(ctx context.Context, obj client.Object) []ctrl.Request {
 ctxLogger := log.FromContext(ctx)
 
@@ -248,12 +375,55 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPServer(ctx context.Context, obj c
 }
 }
 
+func (r *MCPGroupReconciler) findMCPGroupForMCPRemoteProxy(ctx context.Context, obj client.Object) []ctrl.Request {
+ctxLogger := log.FromContext(ctx)
+
+// Get the MCPRemoteProxy object
+mcpRemoteProxy, ok := obj.(*mcpv1alpha1.MCPRemoteProxy)
+if !ok {
+ctxLogger.Error(nil, "Object is not an MCPRemoteProxy", "object", obj.GetName())
+return []ctrl.Request{}
+}
+if mcpRemoteProxy.Spec.GroupRef == "" {
+// No MCPGroup reference, nothing to do
+return []ctrl.Request{}
+}
+
+// Find which MCPGroup this MCPRemoteProxy belongs to
+ctxLogger.Info(
+"Finding MCPGroup for MCPRemoteProxy",
+"namespace",
+obj.GetNamespace(),
+"mcpremoteproxy",
+obj.GetName(),
+"groupRef",
+mcpRemoteProxy.Spec.GroupRef)
+group := &mcpv1alpha1.MCPGroup{}
+groupKey := types.NamespacedName{Namespace: obj.GetNamespace(), Name: mcpRemoteProxy.Spec.GroupRef}
+if err := r.Get(ctx, groupKey, group); err != nil {
+ctxLogger.Error(err, "Failed to get MCPGroup for MCPRemoteProxy",
+"namespace", obj.GetNamespace(), "name", mcpRemoteProxy.Spec.GroupRef)
+return []ctrl.Request{}
+}
+return []ctrl.Request{
+{
+NamespacedName: types.NamespacedName{
+Namespace: obj.GetNamespace(),
+Name: group.Name,
+},
+},
+}
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *MCPGroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
 return ctrl.NewControllerManagedBy(mgr).
 For(&mcpv1alpha1.MCPGroup{}).
 Watches(
 &mcpv1alpha1.MCPServer{}, handler.EnqueueRequestsFromMapFunc(r.findMCPGroupForMCPServer),
 ).
+Watches(
+&mcpv1alpha1.MCPRemoteProxy{}, handler.EnqueueRequestsFromMapFunc(r.findMCPGroupForMCPRemoteProxy),
+).
 Complete(r)
 }