Skip to content

Commit d7d12b9

Browse files
ChrisJBurnsclaude
andcommitted
Replace ReferencingServers with structured ReferencingWorkloads on MCPExternalAuthConfig
RFC-0023 specifies a structured referencingWorkloads list with kind and name fields on shared configuration CRD status. This migrates MCPExternalAuthConfig from plain []string to []WorkloadReference, following the pattern already implemented for MCPOIDCConfig in #4492. The deletion handler now uses a DeletionBlocked condition with reason ReferencedByWorkloads and requeues instead of returning an error. The watch handler also enqueues configs with stale ReferencingWorkloads entries to handle ref-removal and server-deletion cases. Closes #4491 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a78d9bf commit d7d12b9

8 files changed

Lines changed: 199 additions & 109 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -712,18 +712,18 @@ type MCPExternalAuthConfigStatus struct {
712712
// +optional
713713
ConfigHash string `json:"configHash,omitempty"`
714714

715-
// ReferencingServers is a list of MCPServer resources that reference this MCPExternalAuthConfig
716-
// This helps track which servers need to be reconciled when this config changes
715+
// ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig.
716+
// Each entry identifies the workload by kind and name.
717717
// +optional
718-
ReferencingServers []string `json:"referencingServers,omitempty"`
718+
ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"`
719719
}
720720

721721
// +kubebuilder:object:root=true
722722
// +kubebuilder:subresource:status
723723
// +kubebuilder:resource:shortName=extauth;mcpextauth,categories=toolhive
724724
// +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type`
725725
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
726-
// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingServers`
726+
// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingWorkloads`
727727
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
728728

729729
// MCPExternalAuthConfig is the Schema for the mcpexternalauthconfigs API.

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

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go

Lines changed: 109 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr
123123
}
124124
}
125125

126-
// Even when hash hasn't changed, update referencing servers list.
127-
// This ensures ReferencingServers is updated when MCPServers are created or deleted.
128-
return r.updateReferencingServers(ctx, externalAuthConfig)
126+
// Even when hash hasn't changed, update referencing workloads list.
127+
// This ensures ReferencingWorkloads is updated when MCPServers are created or deleted.
128+
return r.updateReferencingWorkloads(ctx, externalAuthConfig)
129129
}
130130

131131
// calculateConfigHash calculates a hash of the MCPExternalAuthConfig spec using Kubernetes utilities
@@ -155,13 +155,27 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange(
155155
return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err)
156156
}
157157

158-
// Update the status with the list of referencing servers
159-
serverNames := make([]string, 0, len(referencingServers))
158+
// Update the status with the list of referencing workloads
159+
refs := make([]mcpv1alpha1.WorkloadReference, 0, len(referencingServers))
160160
for _, server := range referencingServers {
161-
serverNames = append(serverNames, server.Name)
161+
refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
162162
}
163-
slices.Sort(serverNames)
164-
externalAuthConfig.Status.ReferencingServers = serverNames
163+
slices.SortFunc(refs, func(a, b mcpv1alpha1.WorkloadReference) int {
164+
if a.Kind != b.Kind {
165+
if a.Kind < b.Kind {
166+
return -1
167+
}
168+
return 1
169+
}
170+
if a.Name < b.Name {
171+
return -1
172+
}
173+
if a.Name > b.Name {
174+
return 1
175+
}
176+
return 0
177+
})
178+
externalAuthConfig.Status.ReferencingWorkloads = refs
165179

166180
// Update the MCPExternalAuthConfig status
167181
if err := r.Status().Update(ctx, externalAuthConfig); err != nil {
@@ -197,31 +211,32 @@ func (r *MCPExternalAuthConfigReconciler) handleDeletion(
197211
logger := log.FromContext(ctx)
198212

199213
if controllerutil.ContainsFinalizer(externalAuthConfig, ExternalAuthConfigFinalizerName) {
200-
// Check if any MCPServers are still referencing this MCPExternalAuthConfig
201-
referencingServers, err := r.findReferencingMCPServers(ctx, externalAuthConfig)
214+
// Check if any workloads still reference this MCPExternalAuthConfig
215+
referencingWorkloads, err := r.findReferencingWorkloads(ctx, externalAuthConfig)
202216
if err != nil {
203-
logger.Error(err, "Failed to find referencing MCPServers during deletion")
217+
logger.Error(err, "Failed to check referencing workloads during deletion")
204218
return ctrl.Result{}, err
205219
}
206220

207-
if len(referencingServers) > 0 {
208-
// Cannot delete - still referenced
209-
serverNames := make([]string, 0, len(referencingServers))
210-
for _, server := range referencingServers {
211-
serverNames = append(serverNames, server.Name)
212-
}
213-
logger.Info("Cannot delete MCPExternalAuthConfig - still referenced by MCPServers",
214-
"externalAuthConfig", externalAuthConfig.Name, "referencingServers", serverNames)
215-
216-
// Update status to show it's still referenced
217-
externalAuthConfig.Status.ReferencingServers = serverNames
218-
if err := r.Status().Update(ctx, externalAuthConfig); err != nil {
219-
logger.Error(err, "Failed to update MCPExternalAuthConfig status during deletion")
221+
if len(referencingWorkloads) > 0 {
222+
logger.Info("MCPExternalAuthConfig is still referenced by workloads, blocking deletion",
223+
"externalAuthConfig", externalAuthConfig.Name,
224+
"referencingWorkloads", referencingWorkloads)
225+
226+
meta.SetStatusCondition(&externalAuthConfig.Status.Conditions, metav1.Condition{
227+
Type: "DeletionBlocked",
228+
Status: metav1.ConditionTrue,
229+
Reason: "ReferencedByWorkloads",
230+
Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads),
231+
ObservedGeneration: externalAuthConfig.Generation,
232+
})
233+
externalAuthConfig.Status.ReferencingWorkloads = referencingWorkloads
234+
if updateErr := r.Status().Update(ctx, externalAuthConfig); updateErr != nil {
235+
logger.Error(updateErr, "Failed to update status during deletion block")
220236
}
221237

222-
// Return an error to prevent deletion
223-
return ctrl.Result{}, fmt.Errorf("MCPExternalAuthConfig %s is still referenced by MCPServers: %v",
224-
externalAuthConfig.Name, serverNames)
238+
// Requeue to check again later
239+
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
225240
}
226241

227242
// No references, safe to remove finalizer and allow deletion
@@ -250,28 +265,75 @@ func (r *MCPExternalAuthConfigReconciler) findReferencingMCPServers(
250265
})
251266
}
252267

268+
// findReferencingWorkloads returns the workload resources (MCPServer)
269+
// that reference this MCPExternalAuthConfig via their ExternalAuthConfigRef field.
270+
func (r *MCPExternalAuthConfigReconciler) findReferencingWorkloads(
271+
ctx context.Context,
272+
externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig,
273+
) ([]mcpv1alpha1.WorkloadReference, error) {
274+
mcpServerList := &mcpv1alpha1.MCPServerList{}
275+
if err := r.List(ctx, mcpServerList, client.InNamespace(externalAuthConfig.Namespace)); err != nil {
276+
return nil, fmt.Errorf("failed to list MCPServers: %w", err)
277+
}
278+
279+
var refs []mcpv1alpha1.WorkloadReference
280+
for _, server := range mcpServerList.Items {
281+
if server.Spec.ExternalAuthConfigRef != nil && server.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name {
282+
refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
283+
}
284+
}
285+
return refs, nil
286+
}
287+
253288
// SetupWithManager sets up the controller with the Manager.
289+
// Watches MCPServer changes to maintain accurate ReferencingWorkloads status.
254290
func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
255-
// Create a handler that maps MCPServer changes to MCPExternalAuthConfig reconciliation requests.
256-
// When an MCPServer is created, updated, or deleted, we reconcile the MCPExternalAuthConfig
257-
// it references so that the ReferencingServers status field stays up to date.
291+
// Watch MCPServer changes to update ReferencingWorkloads on referenced MCPExternalAuthConfigs.
292+
// This handler enqueues both the currently-referenced MCPExternalAuthConfig AND any
293+
// MCPExternalAuthConfig that still lists this server in ReferencingWorkloads (covers the
294+
// case where a server removes its externalAuthConfigRef — the previously-referenced
295+
// config needs to reconcile and clean up the stale entry).
258296
mcpServerHandler := handler.EnqueueRequestsFromMapFunc(
259-
func(_ context.Context, obj client.Object) []reconcile.Request {
260-
mcpServer, ok := obj.(*mcpv1alpha1.MCPServer)
297+
func(ctx context.Context, obj client.Object) []reconcile.Request {
298+
server, ok := obj.(*mcpv1alpha1.MCPServer)
261299
if !ok {
262300
return nil
263301
}
264302

265-
if mcpServer.Spec.ExternalAuthConfigRef == nil {
266-
return nil
303+
seen := make(map[types.NamespacedName]struct{})
304+
var requests []reconcile.Request
305+
306+
// Enqueue the currently-referenced MCPExternalAuthConfig (if any)
307+
if server.Spec.ExternalAuthConfigRef != nil {
308+
nn := types.NamespacedName{
309+
Name: server.Spec.ExternalAuthConfigRef.Name,
310+
Namespace: server.Namespace,
311+
}
312+
seen[nn] = struct{}{}
313+
requests = append(requests, reconcile.Request{NamespacedName: nn})
314+
}
315+
316+
// Also enqueue any MCPExternalAuthConfig that still lists this server in
317+
// ReferencingWorkloads — handles ref-removal and server-deletion cases.
318+
extAuthConfigList := &mcpv1alpha1.MCPExternalAuthConfigList{}
319+
if err := r.List(ctx, extAuthConfigList, client.InNamespace(server.Namespace)); err != nil {
320+
log.FromContext(ctx).Error(err, "Failed to list MCPExternalAuthConfigs for MCPServer watch")
321+
return requests
322+
}
323+
for _, cfg := range extAuthConfigList.Items {
324+
nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace}
325+
if _, already := seen[nn]; already {
326+
continue
327+
}
328+
for _, ref := range cfg.Status.ReferencingWorkloads {
329+
if ref.Kind == "MCPServer" && ref.Name == server.Name {
330+
requests = append(requests, reconcile.Request{NamespacedName: nn})
331+
break
332+
}
333+
}
267334
}
268335

269-
return []reconcile.Request{{
270-
NamespacedName: types.NamespacedName{
271-
Name: mcpServer.Spec.ExternalAuthConfigRef.Name,
272-
Namespace: mcpServer.Namespace,
273-
},
274-
}}
336+
return requests
275337
},
276338
)
277339

@@ -282,26 +344,20 @@ func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) err
282344
Complete(r)
283345
}
284346

285-
// updateReferencingServers finds referencing MCPServers and updates the status if the list changed
286-
func (r *MCPExternalAuthConfigReconciler) updateReferencingServers(
347+
// updateReferencingWorkloads finds referencing workloads and updates the status if the list changed
348+
func (r *MCPExternalAuthConfigReconciler) updateReferencingWorkloads(
287349
ctx context.Context,
288350
externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig,
289351
) (ctrl.Result, error) {
290-
referencingServers, err := r.findReferencingMCPServers(ctx, externalAuthConfig)
352+
refs, err := r.findReferencingWorkloads(ctx, externalAuthConfig)
291353
if err != nil {
292354
logger := log.FromContext(ctx)
293-
logger.Error(err, "Failed to find referencing MCPServers")
294-
return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err)
295-
}
296-
297-
serverNames := make([]string, 0, len(referencingServers))
298-
for _, server := range referencingServers {
299-
serverNames = append(serverNames, server.Name)
355+
logger.Error(err, "Failed to find referencing workloads")
356+
return ctrl.Result{}, fmt.Errorf("failed to find referencing workloads: %w", err)
300357
}
301-
slices.Sort(serverNames)
302358

303-
if !slices.Equal(externalAuthConfig.Status.ReferencingServers, serverNames) {
304-
externalAuthConfig.Status.ReferencingServers = serverNames
359+
if !slices.Equal(externalAuthConfig.Status.ReferencingWorkloads, refs) {
360+
externalAuthConfig.Status.ReferencingWorkloads = refs
305361
if err := r.Status().Update(ctx, externalAuthConfig); err != nil {
306362
logger := log.FromContext(ctx)
307363
logger.Error(err, "Failed to update MCPExternalAuthConfig status")

0 commit comments

Comments
 (0)