diff --git a/cmd/thv-operator/api/v1alpha1/types.go b/cmd/thv-operator/api/v1alpha1/types.go index ae1258ac9b..f3d16e6e6f 100644 --- a/cmd/thv-operator/api/v1alpha1/types.go +++ b/cmd/thv-operator/api/v1alpha1/types.go @@ -47,7 +47,6 @@ type EmbeddingServerList struct { //+kubebuilder:resource:shortName=extauth;mcpextauth,categories=toolhive //+kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -//+kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPExternalAuthConfig is the deprecated v1alpha1 version of the MCPExternalAuthConfig resource. @@ -105,7 +104,6 @@ type MCPGroupList struct { //+kubebuilder:resource:shortName=mcpoidc,categories=toolhive //+kubebuilder:printcolumn:name="Source",type=string,JSONPath=`.spec.type` //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -//+kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPOIDCConfig is the deprecated v1alpha1 version of the MCPOIDCConfig resource. @@ -134,7 +132,6 @@ type MCPOIDCConfigList struct { //+kubebuilder:resource:shortName=authzcfg,categories=toolhive //+kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -//+kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPAuthzConfig is the deprecated v1alpha1 version of the MCPAuthzConfig resource. @@ -318,7 +315,6 @@ type MCPTelemetryConfigList struct { //+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true //+kubebuilder:resource:shortName=mwc,categories=toolhive //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -//+kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingWorkloads` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPWebhookConfig is the Schema for the mcpwebhookconfigs API. @@ -346,7 +342,6 @@ type MCPWebhookConfigList struct { //+kubebuilder:subresource:status //+kubebuilder:resource:shortName=tc;toolconfig,categories=toolhive //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -//+kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPToolConfig is the deprecated v1alpha1 version of the MCPToolConfig resource. diff --git a/cmd/thv-operator/api/v1beta1/mcpauthzconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpauthzconfig_types.go index 22e8820476..0a4360c7bb 100644 --- a/cmd/thv-operator/api/v1beta1/mcpauthzconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpauthzconfig_types.go @@ -58,21 +58,6 @@ type MCPAuthzConfigStatus struct { // ConfigHash is a hash of the current configuration for change detection // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferenceCount is the number of workloads referencing this config. - // +optional - ReferenceCount int32 `json:"referenceCount,omitempty"` - - // ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. - // Each entry identifies the workload by kind and name. The map key is the - // (kind, name) pair so two workloads of different kinds that share a name - // (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - // entries rather than colliding under merge-patch semantics. - // +listType=map - // +listMapKey=kind - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } // +kubebuilder:object:root=true @@ -82,7 +67,6 @@ type MCPAuthzConfigStatus struct { // +kubebuilder:resource:shortName=authzcfg,categories=toolhive // +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -// +kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPAuthzConfig is the Schema for the mcpauthzconfigs API. diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index caa123ddb4..8b15e4100e 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -1317,21 +1317,6 @@ type MCPExternalAuthConfigStatus struct { // ConfigHash is a hash of the current configuration for change detection // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferenceCount is the number of workloads referencing this config. - // +optional - ReferenceCount int32 `json:"referenceCount,omitempty"` - - // ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig. - // Each entry identifies the workload by kind and name. The map key is the - // (kind, name) pair so two workloads of different kinds that share a name - // (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - // entries rather than colliding under merge-patch semantics. - // +listType=map - // +listMapKey=kind - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } // +kubebuilder:object:root=true @@ -1341,7 +1326,6 @@ type MCPExternalAuthConfigStatus struct { // +kubebuilder:resource:shortName=extauth;mcpextauth,categories=toolhive // +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -// +kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPExternalAuthConfig is the Schema for the mcpexternalauthconfigs API. diff --git a/cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go index 0cec132737..2b9eb79630 100644 --- a/cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go @@ -184,21 +184,6 @@ type MCPOIDCConfigStatus struct { // ConfigHash is a hash of the current configuration for change detection // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferenceCount is the number of workloads referencing this config. - // +optional - ReferenceCount int32 `json:"referenceCount,omitempty"` - - // ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig. - // Each entry identifies the workload by kind and name. The map key is the - // (kind, name) pair so two workloads of different kinds that share a name - // (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - // entries rather than colliding under merge-patch semantics. - // +listType=map - // +listMapKey=kind - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } // +kubebuilder:object:root=true @@ -208,7 +193,6 @@ type MCPOIDCConfigStatus struct { // +kubebuilder:resource:shortName=mcpoidc,categories=toolhive // +kubebuilder:printcolumn:name="Source",type=string,JSONPath=`.spec.type` // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -// +kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPOIDCConfig is the Schema for the mcpoidcconfigs API. diff --git a/cmd/thv-operator/api/v1beta1/mcptelemetryconfig_types.go b/cmd/thv-operator/api/v1beta1/mcptelemetryconfig_types.go index c35eb73de2..6286583184 100644 --- a/cmd/thv-operator/api/v1beta1/mcptelemetryconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcptelemetryconfig_types.go @@ -128,12 +128,6 @@ type MCPTelemetryConfigStatus struct { // ConfigHash is a hash of the current configuration for change detection // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferencingWorkloads lists workloads that reference this MCPTelemetryConfig - // +listType=map - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } // +kubebuilder:object:root=true diff --git a/cmd/thv-operator/api/v1beta1/mcpwebhookconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpwebhookconfig_types.go index 579e327a27..88bb8c8013 100644 --- a/cmd/thv-operator/api/v1beta1/mcpwebhookconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpwebhookconfig_types.go @@ -103,11 +103,4 @@ type MCPWebhookConfigStatus struct { // ConfigHash is a hash of the spec, used for detecting changes // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferencingWorkloads is a list of workload resources that reference this MCPWebhookConfig. - // Each entry identifies the workload by kind and name. - // +listType=map - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } diff --git a/cmd/thv-operator/api/v1beta1/toolconfig_types.go b/cmd/thv-operator/api/v1beta1/toolconfig_types.go index b28b37030a..ac709375f4 100644 --- a/cmd/thv-operator/api/v1beta1/toolconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/toolconfig_types.go @@ -96,17 +96,6 @@ type MCPToolConfigStatus struct { // ConfigHash is a hash of the current configuration for change detection // +optional ConfigHash string `json:"configHash,omitempty"` - - // ReferenceCount is the number of workloads referencing this config. - // +optional - ReferenceCount int32 `json:"referenceCount,omitempty"` - - // ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig. - // Each entry identifies the workload by kind and name. - // +listType=map - // +listMapKey=name - // +optional - ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` } // +kubebuilder:object:root=true @@ -115,7 +104,6 @@ type MCPToolConfigStatus struct { // +kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true // +kubebuilder:resource:shortName=tc;toolconfig,categories=toolhive // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` -// +kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // MCPToolConfig is the Schema for the mcptoolconfigs API. diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index 9704dd666c..1893779b80 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -799,11 +799,6 @@ func (in *MCPAuthzConfigStatus) DeepCopyInto(out *MCPAuthzConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfigStatus. @@ -935,11 +930,6 @@ func (in *MCPExternalAuthConfigStatus) DeepCopyInto(out *MCPExternalAuthConfigSt (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPExternalAuthConfigStatus. @@ -1192,11 +1182,6 @@ func (in *MCPOIDCConfigStatus) DeepCopyInto(out *MCPOIDCConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPOIDCConfigStatus. @@ -1933,11 +1918,6 @@ func (in *MCPTelemetryConfigStatus) DeepCopyInto(out *MCPTelemetryConfigStatus) (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPTelemetryConfigStatus. @@ -2095,11 +2075,6 @@ func (in *MCPToolConfigStatus) DeepCopyInto(out *MCPToolConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPToolConfigStatus. @@ -2151,11 +2126,6 @@ func (in *MCPWebhookConfigStatus) DeepCopyInto(out *MCPWebhookConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ReferencingWorkloads != nil { - in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads - *out = make([]WorkloadReference, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPWebhookConfigStatus. diff --git a/cmd/thv-operator/controllers/config_controller_events_test.go b/cmd/thv-operator/controllers/config_controller_events_test.go index 56f8df8fdc..f1cc31a0f2 100644 --- a/cmd/thv-operator/controllers/config_controller_events_test.go +++ b/cmd/thv-operator/controllers/config_controller_events_test.go @@ -237,7 +237,7 @@ func TestMCPExternalAuthConfigReconciler_EmitsDeletionBlockedEvent(t *testing.T) } // TestMCPExternalAuthConfigReconciler_EmitsRecoveryOnSteadyStatePath covers the -// recovery transition that flows through updateReferencingWorkloads rather than +// recovery transition that flows through updateSteadyStateStatus rather than // handleConfigHashChange: a config whose stored hash already matches the current // spec (no hash change) but whose Valid condition is still False from an earlier // failure. Reconcile must emit a single Normal ConfigValid event. diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go index 5b4685c331..9e68d03971 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go @@ -14,16 +14,11 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" @@ -41,7 +36,8 @@ const ( // // This controller manages the lifecycle of MCPAuthzConfig resources: validation // via the authorizer factory registry, config hash computation, finalizer management, -// reference tracking, and deletion protection when workloads reference this config. +// and deletion protection when workloads reference this config. The set of referencing +// workloads is recomputed on demand during deletion rather than tracked in status. type MCPAuthzConfigReconciler struct { client.Client Scheme *runtime.Scheme @@ -128,18 +124,6 @@ func (r *MCPAuthzConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque wasInvalid := conditionStatusIs(authzConfig.Status.Conditions, mcpv1beta1.ConditionTypeAuthzConfigValid, metav1.ConditionFalse) - // Refresh the referencing workloads list. If the lookup fails we must - // requeue with backoff rather than continuing with a stale slice — a - // silent swallow would let ReferencingWorkloads / ReferenceCount drift - // permanently out of sync with the cluster on a transient apiserver - // hiccup. - referencingWorkloads, err := r.findReferencingWorkloads(ctx, authzConfig) - if err != nil { - logger.Error(err, "Failed to find referencing workloads") - return ctrl.Result{}, err - } - newRefCount := workloadReferenceCount(referencingWorkloads) - // Calculate the hash of the current configuration. // The spec is canonicalized first so that two semantically-equal configs // that differ only in whitespace or JSON key order produce the same hash — @@ -156,18 +140,15 @@ func (r *MCPAuthzConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Single status patch covering the steady-state success path: Valid=True, - // fresh hash + observedGeneration, fresh references, and DeletionBlocked - // cleared (we only reach this path when DeletionTimestamp is zero). - // MutateAndPatchStatus short-circuits on an empty diff, so a no-op - // reconcile still produces no wire call (SteadyStateNoOp behaviour - // preserved). + // fresh hash + observedGeneration, and DeletionBlocked cleared (we only + // reach this path when DeletionTimestamp is zero). MutateAndPatchStatus + // short-circuits on an empty diff, so a no-op reconcile still produces no + // wire call (SteadyStateNoOp behaviour preserved). if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, authzConfig, func(c *mcpv1beta1.MCPAuthzConfig) { setValidTrueCondition(c) meta.RemoveStatusCondition(&c.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) c.Status.ConfigHash = configHash c.Status.ObservedGeneration = c.Generation - c.Status.ReferencingWorkloads = referencingWorkloads - c.Status.ReferenceCount = newRefCount }); err != nil { logger.Error(err, "Failed to update MCPAuthzConfig status") return ctrl.Result{}, err @@ -279,8 +260,6 @@ func (r *MCPAuthzConfigReconciler) handleDeletion( Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), ObservedGeneration: c.Generation, }) - c.Status.ReferencingWorkloads = referencingWorkloads - c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) }) if updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") @@ -394,8 +373,6 @@ func (r *MCPAuthzConfigReconciler) findReferencingWorkloads( } // SetupWithManager sets up the controller with the Manager. -// Watches MCPServer, VirtualMCPServer, and MCPRemoteProxy changes to maintain -// accurate ReferencingWorkloads status. func (r *MCPAuthzConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { // Field indexes backing findReferencingWorkloads: each lets the controller // query only the workloads referencing a given config rather than listing @@ -416,75 +393,7 @@ func (r *MCPAuthzConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to set up VirtualMCPServer authzConfigRef index: %w", err) } - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.MCPAuthzConfig{}). - Watches(&mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToAuthzConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Watches(&mcpv1beta1.VirtualMCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToAuthzConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Watches(&mcpv1beta1.MCPRemoteProxy{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToAuthzConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) } - -// mapMCPServerToAuthzConfig maps an MCPServer to the MCPAuthzConfig it currently references. -// EnqueueRequestsFromMapFunc invokes this on both the old and new object on update (and on the -// object for create/delete), so a ref change or deletion automatically enqueues both the -// previously- and newly-referenced config; the previously-referenced config then prunes the -// stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPAuthzConfigReconciler) mapMCPServerToAuthzConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok || server.Spec.AuthzConfigRef == nil || server.Spec.AuthzConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: server.Spec.AuthzConfigRef.Name, - Namespace: server.Namespace, - }}} -} - -// mapVirtualMCPServerToAuthzConfig maps a VirtualMCPServer to the MCPAuthzConfig it currently -// references via spec.incomingAuth. EnqueueRequestsFromMapFunc invokes this on both the old and -// new object on update (and on the object for create/delete), so a ref change or deletion -// automatically enqueues both the previously- and newly-referenced config; the previously- -// referenced config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPAuthzConfigReconciler) mapVirtualMCPServerToAuthzConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - vmcp, ok := obj.(*mcpv1beta1.VirtualMCPServer) - if !ok || vmcp.Spec.IncomingAuth == nil || - vmcp.Spec.IncomingAuth.AuthzConfigRef == nil || vmcp.Spec.IncomingAuth.AuthzConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: vmcp.Spec.IncomingAuth.AuthzConfigRef.Name, - Namespace: vmcp.Namespace, - }}} -} - -// mapMCPRemoteProxyToAuthzConfig maps an MCPRemoteProxy to the MCPAuthzConfig it currently -// references. EnqueueRequestsFromMapFunc invokes this on both the old and new object on update -// (and on the object for create/delete), so a ref change or deletion automatically enqueues both -// the previously- and newly-referenced config; the previously-referenced config then prunes the -// stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPAuthzConfigReconciler) mapMCPRemoteProxyToAuthzConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - proxy, ok := obj.(*mcpv1beta1.MCPRemoteProxy) - if !ok || proxy.Spec.AuthzConfigRef == nil || proxy.Spec.AuthzConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: proxy.Spec.AuthzConfigRef.Name, - Namespace: proxy.Namespace, - }}} -} diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go index d377efb0bc..681e26c7b6 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -488,7 +487,7 @@ func TestMCPAuthzConfigReconciler_ConfigChangeTriggersHashUpdate(t *testing.T) { assert.Equal(t, int64(2), finalConfig.Status.ObservedGeneration, "ObservedGeneration should be updated") } -func TestMCPAuthzConfigReconciler_HashAndRefsLandInOneReconcile(t *testing.T) { +func TestMCPAuthzConfigReconciler_HashAndGenerationLandInOneReconcile(t *testing.T) { t.Parallel() ctx := t.Context() @@ -512,14 +511,9 @@ func TestMCPAuthzConfigReconciler_HashAndRefsLandInOneReconcile(t *testing.T) { var after mcpv1beta1.MCPAuthzConfig require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &after)) - // F4 regression: hash, references, AND ObservedGeneration must all land - // in the same success-path reconcile. The previous shape returned early - // on hashChanged and left ReferenceCount at zero until the next event. + // The hash and ObservedGeneration must both land in the same success-path + // reconcile rather than the controller returning early on hashChanged. assert.NotEmpty(t, after.Status.ConfigHash, "ConfigHash should be set") - assert.Equal(t, int32(1), after.Status.ReferenceCount, - "ReferenceCount must match the referencing workload list in the same reconcile that wrote the hash") - require.Len(t, after.Status.ReferencingWorkloads, 1) - assert.Equal(t, "ref-server", after.Status.ReferencingWorkloads[0].Name) assert.Equal(t, int64(1), after.Status.ObservedGeneration, "ObservedGeneration must land in the same patch as the hash") } @@ -706,99 +700,6 @@ func TestMCPAuthzConfigReconciler_findReferencingWorkloads(t *testing.T) { } } -// TestMCPAuthzConfigReconciler_watchHandlers verifies that the workload watch map -// functions enqueue exactly the config the workload currently references (or nothing -// when the workload has no ref). The previously-referenced config is enqueued by -// EnqueueRequestsFromMapFunc, which runs the map function on both the old and new -// object on update — no manual stale-reference scan in the handler. -func TestMCPAuthzConfigReconciler_watchHandlers(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - obj client.Object - expected map[string]struct{} - }{ - { - name: "MCPServer with ref enqueues the current config", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithAuthzConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "VirtualMCPServer with ref enqueues the current config", - obj: v1beta1test.NewVirtualMCPServer("vmcp", "default", - v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{ - Type: "anonymous", - AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "current-config"}, - }), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPRemoteProxy with ref enqueues the current config", - obj: v1beta1test.NewMCPRemoteProxy("proxy", "default", - v1beta1test.WithRemoteProxyURL("https://example.com"), - v1beta1test.WithRemoteProxyAuthzConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPServer without ref enqueues nothing", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - ), - expected: map[string]struct{}{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - ctx := t.Context() - r, _ := newAuthzTestReconciler(t, tt.obj) - - requests := func() []reconcile.Request { - switch tt.obj.(type) { - case *mcpv1beta1.MCPServer: - return r.mapMCPServerToAuthzConfig(ctx, tt.obj) - case *mcpv1beta1.VirtualMCPServer: - return r.mapVirtualMCPServerToAuthzConfig(ctx, tt.obj) - case *mcpv1beta1.MCPRemoteProxy: - return r.mapMCPRemoteProxyToAuthzConfig(ctx, tt.obj) - default: - t.Fatalf("unexpected object type %T", tt.obj) - return nil - } - }() - - got := make(map[string]struct{}, len(requests)) - for _, req := range requests { - assert.Equal(t, "default", req.Namespace) - got[req.Name] = struct{}{} - } - assert.Equal(t, tt.expected, got) - }) - } -} - -// TestMCPAuthzConfigReconciler_watchHandlersWrongType verifies the map functions -// gracefully ignore objects of an unexpected type. -func TestMCPAuthzConfigReconciler_watchHandlersWrongType(t *testing.T) { - t.Parallel() - - ctx := t.Context() - r, _ := newAuthzTestReconciler(t) - - wrong := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "default"}} - assert.Nil(t, r.mapMCPServerToAuthzConfig(ctx, wrong)) - assert.Nil(t, r.mapVirtualMCPServerToAuthzConfig(ctx, wrong)) - assert.Nil(t, r.mapMCPRemoteProxyToAuthzConfig(ctx, wrong)) -} - // TestMCPAuthzConfigReconciler_DeletionWithoutFinalizer verifies that handleDeletion // is a no-op when the config never had the finalizer (the object is passed directly // rather than created in the fake client, which rejects a deletionTimestamp without a diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go index 94513419db..09e6a12c5c 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go @@ -17,13 +17,9 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" @@ -156,9 +152,8 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr } // Steady-state success path: ensure Valid=True and the IdentitySynthesized - // advisory are set, and refresh the referencing-workloads list, in a single - // status patch. - return r.updateReferencingWorkloads(ctx, externalAuthConfig) + // advisory are set in a single status patch. + return r.updateSteadyStateStatus(ctx, externalAuthConfig) } // setValidTrueAndSynthesized stamps ConditionTypeValid=True and refreshes the @@ -330,36 +325,28 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange( "oldHash", externalAuthConfig.Status.ConfigHash, "newHash", configHash) - // Find all MCPServers that reference this MCPExternalAuthConfig + // Find all MCPServers that reference this MCPExternalAuthConfig so we can + // trigger their reconciliation once the new hash is recorded. referencingServers, err := r.findReferencingMCPServers(ctx, externalAuthConfig) if err != nil { logger.Error(err, "Failed to find referencing MCPServers") return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err) } - // Build the list of referencing workloads - refs := make([]mcpv1beta1.WorkloadReference, 0, len(referencingServers)) - for _, server := range referencingServers { - refs = append(refs, mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPServer, Name: server.Name}) - } - ctrlutil.SortWorkloadRefs(refs) - // Capture the recovery transition before the patch mutates conditions in // place, so a single Normal event fires on the False->True transition. wasInvalid := conditionStatusIs(externalAuthConfig.Status.Conditions, mcpv1beta1.ConditionTypeValid, metav1.ConditionFalse) // Single status patch covering the hash-change success path: the new hash - // and generation, the refreshed reference list, and the Valid=True / - // IdentitySynthesized conditions. All mutations happen inside the closure so - // the pre-mutate snapshot stays clean (a MutateAndPatchStatus prerequisite). + // and generation, and the Valid=True / IdentitySynthesized conditions. All + // mutations happen inside the closure so the pre-mutate snapshot stays clean + // (a MutateAndPatchStatus prerequisite). if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, func(c *mcpv1beta1.MCPExternalAuthConfig) { r.setValidTrueAndSynthesized(c) c.Status.ConfigHash = configHash c.Status.ObservedGeneration = c.Generation - c.Status.ReferencingWorkloads = refs - c.Status.ReferenceCount = workloadReferenceCount(refs) }); err != nil { logger.Error(err, "Failed to update MCPExternalAuthConfig status") return ctrl.Result{}, err @@ -419,8 +406,6 @@ func (r *MCPExternalAuthConfigReconciler) handleDeletion( Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), ObservedGeneration: c.Generation, }) - c.Status.ReferencingWorkloads = referencingWorkloads - c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) }) if updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") @@ -588,7 +573,6 @@ func (r *MCPExternalAuthConfigReconciler) findReferencingWorkloads( } // SetupWithManager sets up the controller with the Manager. -// Watches MCPServer and MCPRemoteProxy changes to maintain accurate ReferencingWorkloads status. func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { // Field indexes backing findReferencingMCPServers / findReferencingMCPRemoteProxies. // Each is a combined index covering both spec.externalAuthConfigRef and @@ -607,135 +591,21 @@ func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) err return fmt.Errorf("failed to set up MCPRemoteProxy externalAuthConfigRef index: %w", err) } - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.MCPExternalAuthConfig{}). - Watches( - &mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToExternalAuthConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). - Watches( - &mcpv1beta1.MCPRemoteProxy{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToExternalAuthConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). Complete(r) } -// mapMCPServerToExternalAuthConfig maps an MCPServer to the MCPExternalAuthConfig(s) -// it currently references via spec.externalAuthConfigRef and/or spec.authServerRef. -// EnqueueRequestsFromMapFunc invokes this on both the old and new object on update -// (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -// -// A server may name the same config via both fields, so the two referenced names are -// deduplicated against a seen set before being turned into requests. -func (*MCPExternalAuthConfigReconciler) mapMCPServerToExternalAuthConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok { - return nil - } - - seen := make(map[types.NamespacedName]struct{}) - var requests []reconcile.Request - - // Enqueue the currently-referenced MCPExternalAuthConfig via externalAuthConfigRef (if any) - if server.Spec.ExternalAuthConfigRef != nil && server.Spec.ExternalAuthConfigRef.Name != "" { - nn := types.NamespacedName{ - Name: server.Spec.ExternalAuthConfigRef.Name, - Namespace: server.Namespace, - } - seen[nn] = struct{}{} - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - - // Enqueue the MCPExternalAuthConfig referenced via authServerRef (if any), deduped - if server.Spec.AuthServerRef != nil && - server.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig && - server.Spec.AuthServerRef.Name != "" { - nn := types.NamespacedName{ - Name: server.Spec.AuthServerRef.Name, - Namespace: server.Namespace, - } - if _, already := seen[nn]; !already { - seen[nn] = struct{}{} - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - } - - return requests -} - -// mapMCPRemoteProxyToExternalAuthConfig maps an MCPRemoteProxy to the MCPExternalAuthConfig(s) -// it currently references via spec.externalAuthConfigRef and/or spec.authServerRef. -// EnqueueRequestsFromMapFunc invokes this on both the old and new object on update -// (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -// -// A proxy may name the same config via both fields, so the two referenced names are -// deduplicated against a seen set before being turned into requests. -func (*MCPExternalAuthConfigReconciler) mapMCPRemoteProxyToExternalAuthConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - proxy, ok := obj.(*mcpv1beta1.MCPRemoteProxy) - if !ok { - return nil - } - - seen := make(map[types.NamespacedName]struct{}) - var requests []reconcile.Request - - // Enqueue the currently-referenced MCPExternalAuthConfig via externalAuthConfigRef (if any) - if proxy.Spec.ExternalAuthConfigRef != nil && proxy.Spec.ExternalAuthConfigRef.Name != "" { - nn := types.NamespacedName{ - Name: proxy.Spec.ExternalAuthConfigRef.Name, - Namespace: proxy.Namespace, - } - seen[nn] = struct{}{} - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - - // Enqueue the MCPExternalAuthConfig referenced via authServerRef (if any), deduped - if proxy.Spec.AuthServerRef != nil && - proxy.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig && - proxy.Spec.AuthServerRef.Name != "" { - nn := types.NamespacedName{ - Name: proxy.Spec.AuthServerRef.Name, - Namespace: proxy.Namespace, - } - if _, already := seen[nn]; !already { - seen[nn] = struct{}{} - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - } - - return requests -} - -// updateReferencingWorkloads writes the steady-state success status in a single -// patch: it ensures Valid=True and the IdentitySynthesized advisory are set and -// refreshes the referencing-workloads list. MutateAndPatchStatus short-circuits -// on an empty diff, so a no-op reconcile skips the wire call. -func (r *MCPExternalAuthConfigReconciler) updateReferencingWorkloads( +// updateSteadyStateStatus writes the steady-state success status in a single +// patch: it ensures Valid=True and the IdentitySynthesized advisory are set. +// MutateAndPatchStatus short-circuits on an empty diff, so a no-op reconcile +// skips the wire call. +func (r *MCPExternalAuthConfigReconciler) updateSteadyStateStatus( ctx context.Context, externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig, ) (ctrl.Result, error) { logger := log.FromContext(ctx) - refs, err := r.findReferencingWorkloads(ctx, externalAuthConfig) - if err != nil { - logger.Error(err, "Failed to find referencing workloads") - return ctrl.Result{}, fmt.Errorf("failed to find referencing workloads: %w", err) - } - // Capture the recovery transition before the patch mutates conditions in // place, so a single Normal event fires on the False->True transition. wasInvalid := conditionStatusIs(externalAuthConfig.Status.Conditions, @@ -744,11 +614,6 @@ func (r *MCPExternalAuthConfigReconciler) updateReferencingWorkloads( if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig, func(c *mcpv1beta1.MCPExternalAuthConfig) { r.setValidTrueAndSynthesized(c) - if !ctrlutil.WorkloadRefsEqual(c.Status.ReferencingWorkloads, refs) || - c.Status.ReferenceCount != workloadReferenceCount(refs) { - c.Status.ReferencingWorkloads = refs - c.Status.ReferenceCount = workloadReferenceCount(refs) - } }); err != nil { logger.Error(err, "Failed to update MCPExternalAuthConfig status") return ctrl.Result{}, err diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go index af7779b980..309d5799eb 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go @@ -238,13 +238,6 @@ func TestMCPExternalAuthConfigReconciler_Reconcile(t *testing.T) { assert.NotEmpty(t, updatedConfig.Status.ConfigHash, "MCPExternalAuthConfig status should have config hash") } - - // Check referencing workloads in status - if tt.existingMCPServer != nil { - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: tt.existingMCPServer.Name}, - "Status should contain referencing MCPServer as WorkloadReference") - } }) } } @@ -594,144 +587,6 @@ func TestMCPExternalAuthConfigReconciler_ConfigChangeTriggersReconciliation(t *t "MCPServer should have annotation with new config hash") } -func TestMCPExternalAuthConfigReconciler_ReferencingWorkloadsUpdatedWithoutHashChange(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - externalAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: "default", - Generation: 1, - }, - Spec: mcpv1beta1.MCPExternalAuthConfigSpec{ - Type: mcpv1beta1.ExternalAuthTypeTokenExchange, - TokenExchange: &mcpv1beta1.TokenExchangeConfig{ - TokenURL: "https://oauth.example.com/token", - ClientID: "test-client", - ClientSecretRef: &mcpv1beta1.SecretKeyRef{ - Name: "test-secret", - Key: "client-secret", - }, - Audience: "backend-service", - }, - }, - } - - r, fakeClient := newTestMCPExternalAuthConfigReconciler(t, externalAuthConfig) - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: externalAuthConfig.Name, - Namespace: externalAuthConfig.Namespace, - }, - } - - // First reconciliation - add finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - // Second reconciliation - sets hash, no servers yet - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - var updatedConfig mcpv1beta1.MCPExternalAuthConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.NotEmpty(t, updatedConfig.Status.ConfigHash) - assert.Empty(t, updatedConfig.Status.ReferencingWorkloads, "No workloads should be referencing yet") - assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) - - // Now add an MCPServer that references this config (without changing the config spec) - mcpServer := v1beta1test.NewMCPServer("new-server", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithExternalAuthConfigRef("test-config"), - ) - require.NoError(t, fakeClient.Create(ctx, mcpServer)) - - // Reconcile again - hash hasn't changed, but referencing servers should be updated - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "new-server"}, - "ReferencingWorkloads should be updated even without hash change") - assert.EqualValues(t, 1, updatedConfig.Status.ReferenceCount) -} - -func TestMCPExternalAuthConfigReconciler_ReferencingWorkloadsRemovedOnServerDeletion(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - externalAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: "default", - Generation: 1, - }, - Spec: mcpv1beta1.MCPExternalAuthConfigSpec{ - Type: mcpv1beta1.ExternalAuthTypeTokenExchange, - TokenExchange: &mcpv1beta1.TokenExchangeConfig{ - TokenURL: "https://oauth.example.com/token", - ClientID: "test-client", - ClientSecretRef: &mcpv1beta1.SecretKeyRef{ - Name: "test-secret", - Key: "client-secret", - }, - Audience: "backend-service", - }, - }, - } - - mcpServer := v1beta1test.NewMCPServer("server-to-delete", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithExternalAuthConfigRef("test-config"), - ) - - r, fakeClient := newTestMCPExternalAuthConfigReconciler(t, externalAuthConfig, mcpServer) - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: externalAuthConfig.Name, - Namespace: externalAuthConfig.Namespace, - }, - } - - // Add finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - // Set hash and referencing servers - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - var updatedConfig mcpv1beta1.MCPExternalAuthConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server-to-delete"}) - assert.EqualValues(t, 1, updatedConfig.Status.ReferenceCount) - - // Delete the MCPServer - require.NoError(t, fakeClient.Delete(ctx, mcpServer)) - - // Reconcile again - referencing servers should be empty now - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Empty(t, updatedConfig.Status.ReferencingWorkloads, - "ReferencingWorkloads should be empty after server deletion") - assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) -} - func TestMCPExternalAuthConfigReconciler_findReferencingWorkloads_authServerRef(t *testing.T) { t.Parallel() @@ -1561,116 +1416,3 @@ func TestMCPExternalAuthConfigReconciler_ReconcileKeepsExistingForeignCondition( require.NotNil(t, own, "controller-owned Valid condition must land") assert.Equal(t, metav1.ConditionTrue, own.Status) } - -// TestMCPExternalAuthConfigReconciler_watchHandlers verifies that the workload -// watch map functions enqueue exactly the config(s) the workload currently -// references via either externalAuthConfigRef or a MCPExternalAuthConfig-kind -// authServerRef, deduplicating when both name the same config. The -// previously-referenced config is enqueued by EnqueueRequestsFromMapFunc, which -// runs the map function on both the old and new object on update — no manual -// stale-reference scan in the handler. -func TestMCPExternalAuthConfigReconciler_watchHandlers(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - obj client.Object - expected map[string]struct{} - }{ - { - name: "MCPServer externalAuthConfigRef enqueues the current config", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithExternalAuthConfigRef("cfg-a"), - ), - expected: map[string]struct{}{"cfg-a": {}}, - }, - { - name: "MCPServer authServerRef of MCPExternalAuthConfig kind enqueues it", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithAuthServerRef(authServerRefKindMCPExternalAuthConfig, "cfg-b"), - ), - expected: map[string]struct{}{"cfg-b": {}}, - }, - { - name: "MCPServer with both refs to different configs enqueues both", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithExternalAuthConfigRef("cfg-a"), - v1beta1test.WithAuthServerRef(authServerRefKindMCPExternalAuthConfig, "cfg-b"), - ), - expected: map[string]struct{}{"cfg-a": {}, "cfg-b": {}}, - }, - { - name: "MCPServer with both refs to the same config enqueues it once", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithExternalAuthConfigRef("cfg-a"), - v1beta1test.WithAuthServerRef(authServerRefKindMCPExternalAuthConfig, "cfg-a"), - ), - expected: map[string]struct{}{"cfg-a": {}}, - }, - { - name: "MCPServer authServerRef of another kind is ignored", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithAuthServerRef("MCPOIDCConfig", "cfg-x"), - ), - expected: map[string]struct{}{}, - }, - { - name: "MCPServer without refs enqueues nothing", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - ), - expected: map[string]struct{}{}, - }, - { - name: "MCPRemoteProxy with both refs to different configs enqueues both", - obj: v1beta1test.NewMCPRemoteProxy("proxy", "default", - v1beta1test.WithRemoteProxyURL("https://example.com"), - v1beta1test.WithRemoteProxyExternalAuthConfigRef("cfg-a"), - v1beta1test.WithRemoteProxyAuthServerRef(authServerRefKindMCPExternalAuthConfig, "cfg-b"), - ), - expected: map[string]struct{}{"cfg-a": {}, "cfg-b": {}}, - }, - { - name: "MCPRemoteProxy with both refs to the same config enqueues it once", - obj: v1beta1test.NewMCPRemoteProxy("proxy", "default", - v1beta1test.WithRemoteProxyURL("https://example.com"), - v1beta1test.WithRemoteProxyExternalAuthConfigRef("cfg-a"), - v1beta1test.WithRemoteProxyAuthServerRef(authServerRefKindMCPExternalAuthConfig, "cfg-a"), - ), - expected: map[string]struct{}{"cfg-a": {}}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - ctx := t.Context() - r, _ := newTestMCPExternalAuthConfigReconciler(t) - - requests := func() []reconcile.Request { - switch tt.obj.(type) { - case *mcpv1beta1.MCPServer: - return r.mapMCPServerToExternalAuthConfig(ctx, tt.obj) - case *mcpv1beta1.MCPRemoteProxy: - return r.mapMCPRemoteProxyToExternalAuthConfig(ctx, tt.obj) - default: - t.Fatalf("unexpected object type %T", tt.obj) - return nil - } - }() - - got := make(map[string]struct{}, len(requests)) - for _, req := range requests { - assert.Equal(t, "default", req.Namespace) - got[req.Name] = struct{}{} - } - assert.Equal(t, tt.expected, got) - }) - } -} diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go index f8f377d5fc..1826b7278a 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller.go @@ -13,16 +13,11 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" @@ -39,8 +34,9 @@ const ( // MCPOIDCConfigReconciler reconciles a MCPOIDCConfig object. // // This controller manages the lifecycle of MCPOIDCConfig resources: validation, -// config hash computation, finalizer management, reference tracking, and -// deletion protection when MCPServer resources reference this config. +// config hash computation, finalizer management, and deletion protection when +// MCPServer resources reference this config. The set of referencing workloads is +// recomputed on demand during deletion rather than tracked in status. type MCPOIDCConfigReconciler struct { client.Client Scheme *runtime.Scheme @@ -124,27 +120,12 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } - // Refresh ReferencingWorkloads list. On error, fall through with the lookup - // result skipped: the status patch below is best-effort and still ensures - // the Valid=True condition is set even when the reference refresh fails. - referencingWorkloads, findErr := r.findReferencingWorkloads(ctx, oidcConfig) - if findErr != nil { - logger.Error(findErr, "Failed to find referencing workloads") - } - // Single status patch covering the steady-state success path: ensure the - // Valid=True condition is set, and refresh the references list when the - // lookup succeeded and the list changed. MutateAndPatchStatus short-circuits - // on an empty diff so the no-op case still skips the wire call - // (SteadyStateNoOp behaviour is preserved). + // Valid=True condition is set. MutateAndPatchStatus short-circuits on an + // empty diff so the no-op case still skips the wire call (SteadyStateNoOp + // behaviour is preserved). if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, oidcConfig, func(c *mcpv1beta1.MCPOIDCConfig) { setOIDCConfigValidTrueCondition(c) - if findErr == nil && - (!ctrlutil.WorkloadRefsEqual(c.Status.ReferencingWorkloads, referencingWorkloads) || - c.Status.ReferenceCount != workloadReferenceCount(referencingWorkloads)) { - c.Status.ReferencingWorkloads = referencingWorkloads - c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) - } }); err != nil { logger.Error(err, "Failed to update MCPOIDCConfig status") return ctrl.Result{}, err @@ -243,8 +224,6 @@ func (r *MCPOIDCConfigReconciler) handleDeletion( Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), ObservedGeneration: c.Generation, }) - c.Status.ReferencingWorkloads = referencingWorkloads - c.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) }) if updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") @@ -358,7 +337,6 @@ func (r *MCPOIDCConfigReconciler) findReferencingWorkloads( } // SetupWithManager sets up the controller with the Manager. -// Watches MCPServer, VirtualMCPServer, and MCPRemoteProxy changes to maintain accurate ReferencingWorkloads status. func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { // Field indexes backing findReferencingWorkloads: each lets the controller // query only the workloads referencing a given config rather than listing @@ -379,87 +357,7 @@ func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to set up MCPRemoteProxy oidcConfigRef index: %w", err) } - // Watch MCPServer changes to update ReferencingWorkloads on referenced MCPOIDCConfigs. - // The map function only returns the MCPOIDCConfig the server currently references. - // EnqueueRequestsFromMapFunc runs it on both the old and new object on update (and - // on the object for create/delete), so removing or changing the ref enqueues both - // the previously- and newly-referenced config — the previously-referenced config - // then reconciles and prunes the stale entry. No manual stale-reference scan needed. - // - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.MCPOIDCConfig{}). - Watches(&mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToOIDCConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Watches( - &mcpv1beta1.VirtualMCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToOIDCConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). - Watches( - &mcpv1beta1.MCPRemoteProxy{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToOIDCConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). Complete(r) } - -// mapMCPServerToOIDCConfig maps an MCPServer to the MCPOIDCConfig it currently -// references. EnqueueRequestsFromMapFunc invokes this on both the old and new object on -// update (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPOIDCConfigReconciler) mapMCPServerToOIDCConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok || server.Spec.OIDCConfigRef == nil || server.Spec.OIDCConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: server.Spec.OIDCConfigRef.Name, - Namespace: server.Namespace, - }}} -} - -// mapVirtualMCPServerToOIDCConfig maps a VirtualMCPServer to the MCPOIDCConfig it -// currently references via spec.incomingAuth. EnqueueRequestsFromMapFunc invokes this -// on both the old and new object on update (and on the object for create/delete), so a -// ref change or deletion automatically enqueues both the previously- and newly-referenced -// config; the previously-referenced config then prunes the stale entry on reconcile. No -// manual stale-reference scan needed. -func (*MCPOIDCConfigReconciler) mapVirtualMCPServerToOIDCConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - vmcp, ok := obj.(*mcpv1beta1.VirtualMCPServer) - if !ok || vmcp.Spec.IncomingAuth == nil || - vmcp.Spec.IncomingAuth.OIDCConfigRef == nil || vmcp.Spec.IncomingAuth.OIDCConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: vmcp.Spec.IncomingAuth.OIDCConfigRef.Name, - Namespace: vmcp.Namespace, - }}} -} - -// mapMCPRemoteProxyToOIDCConfig maps an MCPRemoteProxy to the MCPOIDCConfig it currently -// references. EnqueueRequestsFromMapFunc invokes this on both the old and new object on -// update (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPOIDCConfigReconciler) mapMCPRemoteProxyToOIDCConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - proxy, ok := obj.(*mcpv1beta1.MCPRemoteProxy) - if !ok || proxy.Spec.OIDCConfigRef == nil || proxy.Spec.OIDCConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: proxy.Spec.OIDCConfigRef.Name, - Namespace: proxy.Namespace, - }}} -} diff --git a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go index 598d49b535..213d6a32aa 100644 --- a/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go @@ -4,8 +4,6 @@ package controllers import ( - "context" - "sync/atomic" "testing" "time" @@ -16,11 +14,9 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" - "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1/v1beta1test" "github.com/stacklok/toolhive/cmd/thv-operator/internal/testutil" ) @@ -517,59 +513,6 @@ func TestMCPOIDCConfig_Validate(t *testing.T) { } } -func TestMCPOIDCConfigReconciler_ReferenceCountUpdatedWithWorkloads(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - oidcConfig := &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: "default", - Generation: 1, - }, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://issuer.example.com", - ClientID: "test-client", - }, - }, - } - mcpServer := v1beta1test.NewMCPServer("server-to-track", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithOIDCConfigRef("test-config", ""), - ) - - reconciler, fakeClient := newTestMCPOIDCConfigReconciler(t, oidcConfig, mcpServer) - req := reconcile.Request{NamespacedName: types.NamespacedName{Name: oidcConfig.Name, Namespace: oidcConfig.Namespace}} - - result, err := reconciler.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - _, err = reconciler.Reconcile(ctx, req) - require.NoError(t, err) - - _, err = reconciler.Reconcile(ctx, req) - require.NoError(t, err) - - var updatedConfig mcpv1beta1.MCPOIDCConfig - require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPServer, Name: "server-to-track"}) - assert.EqualValues(t, 1, updatedConfig.Status.ReferenceCount) - - require.NoError(t, fakeClient.Delete(ctx, mcpServer)) - - _, err = reconciler.Reconcile(ctx, req) - require.NoError(t, err) - - require.NoError(t, fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)) - assert.Empty(t, updatedConfig.Status.ReferencingWorkloads) - assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) -} - // TestMCPOIDCConfigReconciler_ReconcileKeepsExistingForeignCondition verifies // that when the controller observes a foreign-owned condition already on the // object and then writes its own Valid=True, it folds its condition into the @@ -635,195 +578,3 @@ func TestMCPOIDCConfigReconciler_ReconcileKeepsExistingForeignCondition(t *testi require.NotNil(t, own, "controller-owned Valid condition must land") assert.Equal(t, metav1.ConditionTrue, own.Status) } - -// TestMCPOIDCConfigReconciler_ConcurrentForeignConditionSurvivesMergePatch -// proves the property the MutateAndPatchStatus migration actually buys over a -// full r.Status().Update: a condition written by a disjoint owner that lands -// between the reconciler's Get and its status patch survives, because the -// reconciler sends a JSON merge-patch carrying only the fields it changed -// (here referencingWorkloads/referenceCount) rather than sending a full PUT of -// its stale view of the whole Status.Conditions array. -// -// The merge-patch-vs-PUT behaviour lives entirely in the shared -// ctrlutil.MutateAndPatchStatus helper that all three config controllers use, -// so this single end-to-end proof guards the mechanism for the OIDC, -// ExternalAuth, and Authz reconcilers alike. -// -// A WithInterceptorFuncs Get hook simulates the concurrent writer: once armed, -// it injects a foreign condition into the backing store immediately after the -// reconciler reads the object. The reconcile that follows changes only the -// reference list — the spec, and hence the generation and the Valid -// condition's ObservedGeneration, is unchanged — so the merge-patch body omits -// the conditions array and the foreign entry is preserved. A regression to -// r.Status().Update would PUT conditions=[Valid] and erase it. -func TestMCPOIDCConfigReconciler_ConcurrentForeignConditionSurvivesMergePatch(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - - oidcConfig := &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "concurrent-config", Namespace: "default", Generation: 1}, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://accounts.google.com", - ClientID: "test-client", - }, - }, - } - key := client.ObjectKeyFromObject(oidcConfig) - - foreign := metav1.Condition{ - Type: "ForeignControllerSays", - Status: metav1.ConditionTrue, - Reason: "ExternallySet", - Message: "written by a concurrent owner between Get and Patch", - LastTransitionTime: metav1.Now(), - } - - var armed atomic.Bool - inject := interceptor.Funcs{ - Get: func(ctx context.Context, cl client.WithWatch, k client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - if err := cl.Get(ctx, k, obj, opts...); err != nil { - return err - } - // Simulate a disjoint owner writing a condition into the store - // right after the reconciler's read of the config object. cl is the - // inner (non-intercepted) client, so this does not recurse. - if k == key && armed.CompareAndSwap(true, false) { - cur := &mcpv1beta1.MCPOIDCConfig{} - if err := cl.Get(ctx, k, cur); err != nil { - return err - } - meta.SetStatusCondition(&cur.Status.Conditions, foreign) - if err := cl.Status().Update(ctx, cur); err != nil { - return err - } - } - return nil - }, - } - - fakeClient := withOIDCConfigRefIndexes(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(oidcConfig). - WithStatusSubresource(&mcpv1beta1.MCPOIDCConfig{}). - WithInterceptorFuncs(inject). - Build() - r := &MCPOIDCConfigReconciler{Client: fakeClient, Scheme: scheme} - req := reconcile.Request{NamespacedName: key} - - // Reach steady state: finalizer added, then Valid=True + ConfigHash set. - _, err := r.Reconcile(ctx, req) - require.NoError(t, err) - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - // Introduce a referencing workload so the next reconcile patches only - // referencingWorkloads/referenceCount — not the conditions array, and - // without bumping the config's generation. - server := v1beta1test.NewMCPServer("referencing-server", "default", - v1beta1test.WithImage("img"), - v1beta1test.WithOIDCConfigRef(oidcConfig.Name, "aud"), - ) - require.NoError(t, fakeClient.Create(ctx, server)) - - // Arm the concurrent writer and run the reference-refresh reconcile. - armed.Store(true) - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - require.False(t, armed.Load(), "interceptor must have fired on the config Get") - - var after mcpv1beta1.MCPOIDCConfig - require.NoError(t, fakeClient.Get(ctx, key, &after)) - - foreignAfter := meta.FindStatusCondition(after.Status.Conditions, "ForeignControllerSays") - require.NotNil(t, foreignAfter, - "a concurrently-written foreign condition must survive the reconciler's merge-patch; a full PUT would erase it") - assert.Equal(t, metav1.ConditionTrue, foreignAfter.Status) - - // The reconciler's own work still landed in the same patch cycle. - own := meta.FindStatusCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeOIDCConfigValid) - require.NotNil(t, own, "controller-owned Valid condition must remain") - assert.Equal(t, metav1.ConditionTrue, own.Status) - assert.EqualValues(t, 1, after.Status.ReferenceCount, "reference refresh must have been applied") -} - -// TestMCPOIDCConfigReconciler_watchHandlers verifies that the workload watch map -// functions enqueue exactly the config the workload currently references (or nothing -// when the workload has no ref). The previously-referenced config is enqueued by -// EnqueueRequestsFromMapFunc, which runs the map function on both the old and new -// object on update — no manual stale-reference scan in the handler. -func TestMCPOIDCConfigReconciler_watchHandlers(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - obj client.Object - expected map[string]struct{} - }{ - { - name: "MCPServer with ref enqueues the current config", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithOIDCConfigRef("current-config", "aud"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "VirtualMCPServer with ref enqueues the current config", - obj: v1beta1test.NewVirtualMCPServer("vmcp", "default", - v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{ - Type: "oidc", - OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{Name: "current-config"}, - }), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPRemoteProxy with ref enqueues the current config", - obj: v1beta1test.NewMCPRemoteProxy("proxy", "default", - v1beta1test.WithRemoteProxyURL("https://example.com"), - v1beta1test.WithRemoteProxyOIDCConfigRef("current-config", "aud"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPServer without ref enqueues nothing", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - ), - expected: map[string]struct{}{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - ctx := t.Context() - r, _ := newTestMCPOIDCConfigReconciler(t, tt.obj) - - requests := func() []reconcile.Request { - switch tt.obj.(type) { - case *mcpv1beta1.MCPServer: - return r.mapMCPServerToOIDCConfig(ctx, tt.obj) - case *mcpv1beta1.VirtualMCPServer: - return r.mapVirtualMCPServerToOIDCConfig(ctx, tt.obj) - case *mcpv1beta1.MCPRemoteProxy: - return r.mapMCPRemoteProxyToOIDCConfig(ctx, tt.obj) - default: - t.Fatalf("unexpected object type %T", tt.obj) - return nil - } - }() - - got := make(map[string]struct{}, len(requests)) - for _, req := range requests { - assert.Equal(t, "default", req.Namespace) - got[req.Name] = struct{}{} - } - assert.Equal(t, tt.expected, got) - }) - } -} diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index ef52c2657e..58fef4ee8e 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -934,13 +934,11 @@ func (r *MCPRemoteProxyReconciler) handleOIDCConfig(ctx context.Context, proxy * return err } - // ReferencingWorkloads on the MCPOIDCConfig is maintained solely by the - // MCPOIDCConfig controller, which watches MCPServer/VirtualMCPServer/ - // MCPRemoteProxy and recomputes the full list (additions and removals). The - // MCPRemoteProxy controller must not write the config's status: a full - // r.Status().Update here would clobber conditions the config controller - // owns, and the previous append-only write never removed stale entries. - // See #5511. + // The MCPRemoteProxy controller must not write the MCPOIDCConfig's status: + // that status is owned by the MCPOIDCConfig controller, and a full + // r.Status().Update here would clobber conditions it owns. The config + // controller no longer tracks referencing workloads in its status; deletion + // protection recomputes referrers on demand in its finalizer. See #5511. // Detect whether the condition is transitioning to True (e.g. recovering from // a transient error). Without this check the status update is skipped when the @@ -1592,8 +1590,8 @@ func (r *MCPRemoteProxyReconciler) mapTelemetryConfigToMCPRemoteProxy( // handleAuthzConfig validates the referenced MCPAuthzConfig, tracks its hash on // the MCPRemoteProxy status, and sets the AuthzConfigRefValidated condition. When // the ref is cleared it removes both the hash and the condition so a stale "valid" -// signal does not linger. ReferencingWorkloads on the MCPAuthzConfig is owned by -// the MCPAuthzConfig controller (#5511); this controller never writes it. +// signal does not linger. The MCPAuthzConfig's status is owned by the +// MCPAuthzConfig controller (#5511); this controller never writes it. func (r *MCPRemoteProxyReconciler) handleAuthzConfig(ctx context.Context, proxy *mcpv1beta1.MCPRemoteProxy) error { if proxy.Spec.AuthzConfigRef == nil { // No MCPAuthzConfig referenced: clear any stored hash and remove the diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 1974d609f7..be464f8243 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -2342,13 +2342,11 @@ func (r *MCPServerReconciler) handleOIDCConfig(ctx context.Context, m *mcpv1beta return err } - // ReferencingWorkloads on the MCPOIDCConfig is maintained solely by the - // MCPOIDCConfig controller, which watches MCPServer/VirtualMCPServer/ - // MCPRemoteProxy and recomputes the full list (additions and removals). The - // MCPServer controller must not write the config's status: a full - // r.Status().Update here would clobber conditions the config controller - // owns, and the previous append-only write never removed stale entries. - // See #5511. + // The MCPServer controller must not write the MCPOIDCConfig's status: that + // status is owned by the MCPOIDCConfig controller, and a full + // r.Status().Update here would clobber conditions it owns. The config + // controller no longer tracks referencing workloads in its status; deletion + // protection recomputes referrers on demand in its finalizer. See #5511. // Detect whether the condition is transitioning to True (e.g. recovering from // a transient error). Without this check the status update is skipped when the @@ -2438,8 +2436,8 @@ func setOIDCConfigRefCondition(m *mcpv1beta1.MCPServer, status metav1.ConditionS // handleAuthzConfig validates the referenced MCPAuthzConfig, tracks its hash on // the MCPServer status, and sets the AuthzConfigRefValidated condition. When the // ref is cleared it removes both the hash and the condition so a stale "valid" -// signal does not linger. ReferencingWorkloads on the MCPAuthzConfig is owned by -// the MCPAuthzConfig controller (#5511); this controller never writes it. +// signal does not linger. The MCPAuthzConfig's status is owned by the +// MCPAuthzConfig controller (#5511); this controller never writes it. // // Revocation semantics (fail-stale, not fail-open): if a previously-valid ref // later becomes invalid or missing, this returns an error and Reconcile stops diff --git a/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go b/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go index 8f360d5ed4..fa0cc4fca1 100644 --- a/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go +++ b/cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go @@ -294,15 +294,13 @@ func TestMCPOIDCConfigReconciler_handleDeletion_BlocksWhenReferenced(t *testing. assert.Contains(t, cfg.Finalizers, OIDCConfigFinalizerName, "finalizer must remain") // The DeletionBlocked condition is written through MutateAndPatchStatus; - // re-fetch to confirm it (and the referencing-workload bookkeeping) was - // persisted rather than only mutated in memory. + // re-fetch to confirm it was persisted rather than only mutated in memory. var after mcpv1beta1.MCPOIDCConfig require.NoError(t, fc.Get(ctx, client.ObjectKeyFromObject(cfg), &after)) blocked := meta.FindStatusCondition(after.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) require.NotNil(t, blocked, "DeletionBlocked condition must be set while referenced") assert.Equal(t, metav1.ConditionTrue, blocked.Status) assert.Equal(t, "ReferencedByWorkloads", blocked.Reason) - assert.EqualValues(t, 1, after.Status.ReferenceCount, "referencing workload must be recorded") } func TestMCPOIDCConfigReconciler_handleDeletion_AllowsWhenNotReferenced(t *testing.T) { @@ -374,20 +372,20 @@ func conditionStatusPtr(s metav1.ConditionStatus) *metav1.ConditionStatus { // TestMCPServerReconciler_handleOIDCConfig_DoesNotWriteConfigStatus guards the // trust boundary consolidated in #5511: the MCPServer controller may read the // referenced MCPOIDCConfig but must never write its status. The MCPOIDCConfig -// controller is the sole owner of that status (conditions and -// ReferencingWorkloads). This unit test is the actual enforcement of that -// boundary — RBAC does not enforce it, because the operator runs as a single -// ServiceAccount whose aggregated role still grants mcpoidcconfigs/status write -// for the MCPOIDCConfig controller's own use. A future reintroduction of a -// cross-controller status write would flip the ResourceVersion and fail here. +// controller is the sole owner of that status. This unit test is the actual +// enforcement of that boundary — RBAC does not enforce it, because the operator +// runs as a single ServiceAccount whose aggregated role still grants +// mcpoidcconfigs/status write for the MCPOIDCConfig controller's own use. A +// future reintroduction of a cross-controller status write would flip the +// ResourceVersion and fail here. func TestMCPServerReconciler_handleOIDCConfig_DoesNotWriteConfigStatus(t *testing.T) { t.Parallel() ctx := t.Context() scheme := testutil.NewScheme(t) - // Seed a config whose status carries a condition and a ReferencingWorkloads - // entry owned by the MCPOIDCConfig controller — neither must be touched. + // Seed a config whose status carries conditions owned by the MCPOIDCConfig + // controller — none must be touched. oidcConfig := &mcpv1beta1.MCPOIDCConfig{ ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "default"}, Spec: mcpv1beta1.MCPOIDCConfigSpec{ @@ -406,8 +404,6 @@ func TestMCPServerReconciler_handleOIDCConfig_DoesNotWriteConfigStatus(t *testin Reason: "ExternallySet", LastTransitionTime: metav1.Now(), }, }, - ReferencingWorkloads: []mcpv1beta1.WorkloadReference{{Kind: "MCPServer", Name: "someone-else"}}, - ReferenceCount: 1, }, } mcpServer := &mcpv1beta1.MCPServer{ @@ -435,8 +431,6 @@ func TestMCPServerReconciler_handleOIDCConfig_DoesNotWriteConfigStatus(t *testin assert.Equal(t, before.ResourceVersion, after.ResourceVersion, "MCPServer reconcile must not write the MCPOIDCConfig — its status is owned by the MCPOIDCConfig controller") - assert.Equal(t, before.Status.ReferencingWorkloads, after.Status.ReferencingWorkloads, - "MCPServer reconcile must not touch the config's ReferencingWorkloads") assert.NotNil(t, meta.FindStatusCondition(after.Status.Conditions, "ForeignControllerSays"), "config-owned conditions must remain untouched") } diff --git a/cmd/thv-operator/controllers/mcptelemetryconfig_controller.go b/cmd/thv-operator/controllers/mcptelemetryconfig_controller.go index 02cb84f6af..2d579b4582 100644 --- a/cmd/thv-operator/controllers/mcptelemetryconfig_controller.go +++ b/cmd/thv-operator/controllers/mcptelemetryconfig_controller.go @@ -12,15 +12,10 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" @@ -37,7 +32,8 @@ const ( // MCPTelemetryConfigReconciler reconciles a MCPTelemetryConfig object. // // This controller manages the lifecycle of MCPTelemetryConfig resources: validation, -// config hash computation, finalizer management, reference tracking, and deletion protection. +// config hash computation, finalizer management, and deletion protection. The set of +// referencing workloads is recomputed on demand during deletion rather than tracked in status. type MCPTelemetryConfigReconciler struct { client.Client Scheme *runtime.Scheme @@ -109,17 +105,9 @@ func (r *MCPTelemetryConfigReconciler) Reconcile(ctx context.Context, req ctrl.R // Calculate the hash of the current configuration configHash := r.calculateConfigHash(telemetryConfig.Spec) - // Track referencing workloads - referencingWorkloads, err := r.findReferencingWorkloads(ctx, telemetryConfig) - if err != nil { - logger.Error(err, "Failed to find referencing workloads") - return ctrl.Result{}, err - } - // Check what changed hashChanged := telemetryConfig.Status.ConfigHash != configHash - refsChanged := !ctrlutil.WorkloadRefsEqual(telemetryConfig.Status.ReferencingWorkloads, referencingWorkloads) - needsUpdate := hashChanged || refsChanged || conditionChanged + needsUpdate := hashChanged || conditionChanged if hashChanged { logger.Info("MCPTelemetryConfig configuration changed", @@ -130,7 +118,6 @@ func (r *MCPTelemetryConfigReconciler) Reconcile(ctx context.Context, req ctrl.R if needsUpdate { telemetryConfig.Status.ConfigHash = configHash telemetryConfig.Status.ObservedGeneration = telemetryConfig.Generation - telemetryConfig.Status.ReferencingWorkloads = referencingWorkloads if err := r.Status().Update(ctx, telemetryConfig); err != nil { logger.Error(err, "Failed to update MCPTelemetryConfig status") @@ -178,7 +165,6 @@ func indexVirtualMCPServerByTelemetryConfigRef(obj client.Object) []string { } // SetupWithManager sets up the controller with the Manager. -// Watches MCPServer changes to maintain accurate ReferencingWorkloads status. func (r *MCPTelemetryConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { // Field indexes backing findReferencingWorkloads: each lets the controller // query only the workloads referencing a given config rather than listing @@ -199,91 +185,11 @@ func (r *MCPTelemetryConfigReconciler) SetupWithManager(mgr ctrl.Manager) error return fmt.Errorf("failed to set up VirtualMCPServer telemetryConfigRef index: %w", err) } - // Watch MCPServer changes to update ReferencingWorkloads on referenced MCPTelemetryConfigs. - // The map function only returns the MCPTelemetryConfig the server currently references. - // EnqueueRequestsFromMapFunc runs it on both the old and new object on update (and - // on the object for create/delete), so removing or changing the ref enqueues both - // the previously- and newly-referenced config — the previously-referenced config - // then reconciles and prunes the stale entry. No manual stale-reference scan needed. - // - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.MCPTelemetryConfig{}). - Watches(&mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToTelemetryConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Watches( - &mcpv1beta1.MCPRemoteProxy{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToTelemetryConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). - Watches( - &mcpv1beta1.VirtualMCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToTelemetryConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). Complete(r) } -// mapMCPServerToTelemetryConfig maps an MCPServer to the MCPTelemetryConfig it currently -// references. EnqueueRequestsFromMapFunc invokes this on both the old and new object on -// update (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -func (*MCPTelemetryConfigReconciler) mapMCPServerToTelemetryConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok || server.Spec.TelemetryConfigRef == nil || server.Spec.TelemetryConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: server.Spec.TelemetryConfigRef.Name, - Namespace: server.Namespace, - }}} -} - -// mapMCPRemoteProxyToTelemetryConfig maps an MCPRemoteProxy to the MCPTelemetryConfig it -// currently references. EnqueueRequestsFromMapFunc invokes this on both the old and new -// object on update (and on the object for create/delete), so a ref change or deletion -// automatically enqueues both the previously- and newly-referenced config; the -// previously-referenced config then prunes the stale entry on reconcile. No manual -// stale-reference scan needed. -func (*MCPTelemetryConfigReconciler) mapMCPRemoteProxyToTelemetryConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - proxy, ok := obj.(*mcpv1beta1.MCPRemoteProxy) - if !ok || proxy.Spec.TelemetryConfigRef == nil || proxy.Spec.TelemetryConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: proxy.Spec.TelemetryConfigRef.Name, - Namespace: proxy.Namespace, - }}} -} - -// mapVirtualMCPServerToTelemetryConfig maps a VirtualMCPServer to the MCPTelemetryConfig -// it currently references. EnqueueRequestsFromMapFunc invokes this on both the old and new -// object on update (and on the object for create/delete), so a ref change or deletion -// automatically enqueues both the previously- and newly-referenced config; the -// previously-referenced config then prunes the stale entry on reconcile. No manual -// stale-reference scan needed. -func (*MCPTelemetryConfigReconciler) mapVirtualMCPServerToTelemetryConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - vmcp, ok := obj.(*mcpv1beta1.VirtualMCPServer) - if !ok || vmcp.Spec.TelemetryConfigRef == nil || vmcp.Spec.TelemetryConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: vmcp.Spec.TelemetryConfigRef.Name, - Namespace: vmcp.Namespace, - }}} -} - // calculateConfigHash calculates a hash of the MCPTelemetryConfig spec using Kubernetes utilities func (*MCPTelemetryConfigReconciler) calculateConfigHash(spec mcpv1beta1.MCPTelemetryConfigSpec) string { return ctrlutil.CalculateConfigHash(spec) diff --git a/cmd/thv-operator/controllers/mcptelemetryconfig_controller_test.go b/cmd/thv-operator/controllers/mcptelemetryconfig_controller_test.go index 78cd099aae..c77440105a 100644 --- a/cmd/thv-operator/controllers/mcptelemetryconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcptelemetryconfig_controller_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -601,73 +600,6 @@ func TestMCPTelemetryConfigReconciler_ConditionOnlyUpdate(t *testing.T) { assert.True(t, foundValid, "Should have Valid=True condition after condition-only update") } -func TestMCPTelemetryConfigReconciler_ReferenceTracking(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - scheme := testutil.NewScheme(t) - - telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "shared-config", - Namespace: "default", - Generation: 1, - }, - Spec: newTelemetrySpec("https://otel-collector:4317", true, false), - } - - // Two MCPServers reference this config, one does not - server1 := v1beta1test.NewMCPServer("server-a", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithTelemetryConfigRef("shared-config"), - ) - server2 := v1beta1test.NewMCPServer("server-b", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithTelemetryConfigRef("shared-config"), - ) - server3 := v1beta1test.NewMCPServer("server-c", "default", - v1beta1test.WithImage("test-image"), - // No TelemetryConfigRef - ) - - fakeClient := withTelemetryConfigRefIndexes(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(telemetryConfig, server1, server2, server3). - WithStatusSubresource(&mcpv1beta1.MCPTelemetryConfig{}). - Build() - - r := &MCPTelemetryConfigReconciler{ - Client: fakeClient, - Scheme: scheme, - } - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, - } - - // First reconcile: add finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0), "Should requeue after adding finalizer") - - // Second reconcile: set hash, condition, and referencing servers - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - var updated mcpv1beta1.MCPTelemetryConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updated) - require.NoError(t, err) - - // ReferencingWorkloads should list server-a and server-b (sorted), but not server-c - assert.Equal(t, []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "server-a"}, - {Kind: "MCPServer", Name: "server-b"}, - }, updated.Status.ReferencingWorkloads) -} - func TestMCPTelemetryConfigReconciler_handleDeletion_BlocksWhenReferenced(t *testing.T) { t.Parallel() @@ -801,84 +733,3 @@ func newTelemetrySpec(endpoint string, tracing, metrics bool) mcpv1beta1.MCPTele }, } } - -// TestMCPTelemetryConfigReconciler_watchHandlers verifies that the workload watch map -// functions enqueue exactly the config the workload currently references (or nothing -// when the workload has no ref). The previously-referenced config is enqueued by -// EnqueueRequestsFromMapFunc, which runs the map function on both the old and new -// object on update — no manual stale-reference scan in the handler. -func TestMCPTelemetryConfigReconciler_watchHandlers(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - obj client.Object - expected map[string]struct{} - }{ - { - name: "MCPServer with ref enqueues the current config", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithTelemetryConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPRemoteProxy with ref enqueues the current config", - obj: v1beta1test.NewMCPRemoteProxy("proxy", "default", - v1beta1test.WithRemoteProxyURL("https://example.com"), - v1beta1test.WithRemoteProxyTelemetryConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "VirtualMCPServer with ref enqueues the current config", - obj: v1beta1test.NewVirtualMCPServer("vmcp", "default", - v1beta1test.WithVMCPTelemetryConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPServer without ref enqueues nothing", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - ), - expected: map[string]struct{}{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - fakeClient := withTelemetryConfigRefIndexes(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(tt.obj). - WithStatusSubresource(&mcpv1beta1.MCPTelemetryConfig{}). - Build() - r := &MCPTelemetryConfigReconciler{Client: fakeClient, Scheme: scheme} - - requests := func() []reconcile.Request { - switch tt.obj.(type) { - case *mcpv1beta1.MCPServer: - return r.mapMCPServerToTelemetryConfig(ctx, tt.obj) - case *mcpv1beta1.MCPRemoteProxy: - return r.mapMCPRemoteProxyToTelemetryConfig(ctx, tt.obj) - case *mcpv1beta1.VirtualMCPServer: - return r.mapVirtualMCPServerToTelemetryConfig(ctx, tt.obj) - default: - t.Fatalf("unexpected object type %T", tt.obj) - return nil - } - }() - - got := make(map[string]struct{}, len(requests)) - for _, req := range requests { - assert.Equal(t, "default", req.Namespace) - got[req.Name] = struct{}{} - } - assert.Equal(t, tt.expected, got) - }) - } -} diff --git a/cmd/thv-operator/controllers/mcpwebhookconfig_controller.go b/cmd/thv-operator/controllers/mcpwebhookconfig_controller.go index 97903170d6..485e60ddba 100644 --- a/cmd/thv-operator/controllers/mcpwebhookconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpwebhookconfig_controller.go @@ -14,13 +14,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" @@ -120,7 +116,7 @@ func (r *MCPWebhookConfigReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - return r.updateReferencingWorkloads(ctx, webhookConfig) + return ctrl.Result{}, nil } // calculateConfigHash calculates a hash of the MCPWebhookConfig spec @@ -145,12 +141,9 @@ func (r *MCPWebhookConfigReconciler) handleConfigHashChange( return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err) } - refs := workloadRefsFromMCPServers(referencingServers) - if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, webhookConfig, func(cfg *mcpv1alpha1.MCPWebhookConfig) { cfg.Status.ConfigHash = configHash cfg.Status.ObservedGeneration = cfg.Generation - cfg.Status.ReferencingWorkloads = refs meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ Type: mcpv1beta1.ConditionTypeValid, Status: metav1.ConditionTrue, @@ -224,7 +217,6 @@ func (r *MCPWebhookConfigReconciler) handleDeletion( Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", refs), ObservedGeneration: cfg.Generation, }) - cfg.Status.ReferencingWorkloads = refs }); err != nil { logger.Error(err, "Failed to update MCPWebhookConfig status during deletion") return ctrl.Result{}, err @@ -237,7 +229,6 @@ func (r *MCPWebhookConfigReconciler) handleDeletion( if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, webhookConfig, func(cfg *mcpv1alpha1.MCPWebhookConfig) { meta.RemoveStatusCondition(&cfg.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) - cfg.Status.ReferencingWorkloads = nil }); err != nil { logger.Error(err, "Failed to clear MCPWebhookConfig deletion block status") return ctrl.Result{}, err @@ -294,33 +285,6 @@ func (r *MCPWebhookConfigReconciler) findReferencingMCPServers( return serverList.Items, nil } -// updateReferencingWorkloads updates the list of workloads referencing this config -func (r *MCPWebhookConfigReconciler) updateReferencingWorkloads( - ctx context.Context, - webhookConfig *mcpv1alpha1.MCPWebhookConfig, -) (ctrl.Result, error) { - logger := log.FromContext(ctx) - - referencingServers, err := r.findReferencingMCPServers(ctx, webhookConfig) - if err != nil { - logger.Error(err, "Failed to find referencing MCPServers") - return ctrl.Result{}, err - } - - refs := workloadRefsFromMCPServers(referencingServers) - if !ctrlutil.WorkloadRefsEqual(webhookConfig.Status.ReferencingWorkloads, refs) { - if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, webhookConfig, - func(cfg *mcpv1alpha1.MCPWebhookConfig) { - cfg.Status.ReferencingWorkloads = refs - }); err != nil { - logger.Error(err, "Failed to update referencing workloads list") - return ctrl.Result{}, err - } - } - - return ctrl.Result{}, nil -} - func workloadRefsFromMCPServers(servers []mcpv1beta1.MCPServer) []mcpv1beta1.WorkloadReference { refs := make([]mcpv1beta1.WorkloadReference, 0, len(servers)) for _, server := range servers { @@ -346,34 +310,7 @@ func (r *MCPWebhookConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to set up MCPServer webhookConfigRef index: %w", err) } - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1alpha1.MCPWebhookConfig{}). - Watches( - &mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToWebhookConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{}), - ). Complete(r) } - -// mapMCPServerToWebhookConfig maps an MCPServer to the MCPWebhookConfig it currently references. -// EnqueueRequestsFromMapFunc invokes this on both the old and new object on update (and on the -// object for create/delete), so a ref change or deletion automatically enqueues both the -// previously- and newly-referenced config; the previously-referenced config then prunes the stale -// entry on reconcile. No manual stale-reference scan needed. -func (*MCPWebhookConfigReconciler) mapMCPServerToWebhookConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok || server.Spec.WebhookConfigRef == nil || server.Spec.WebhookConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: client.ObjectKey{ - Name: server.Spec.WebhookConfigRef.Name, - Namespace: server.Namespace, - }}} -} diff --git a/cmd/thv-operator/controllers/mcpwebhookconfig_controller_test.go b/cmd/thv-operator/controllers/mcpwebhookconfig_controller_test.go index c11aa4eec6..abcc117f28 100644 --- a/cmd/thv-operator/controllers/mcpwebhookconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpwebhookconfig_controller_test.go @@ -4,21 +4,17 @@ package controllers import ( - "context" - "errors" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" @@ -131,11 +127,6 @@ func TestMCPWebhookConfigReconciler_Reconcile(t *testing.T) { assert.NotEmpty(t, updatedConfig.Status.ConfigHash) } if tt.existingMCPServer != nil { - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, mcpv1beta1.WorkloadReference{ - Kind: mcpv1beta1.WorkloadKindMCPServer, - Name: tt.existingMCPServer.Name, - }) - var updatedServer mcpv1beta1.MCPServer require.NoError(t, fakeClient.Get(ctx, types.NamespacedName{ Name: tt.existingMCPServer.Name, @@ -252,7 +243,6 @@ func TestMCPWebhookConfigReconciler_handleDeletion(t *testing.T) { require.NotNil(t, cond) assert.Equal(t, metav1.ConditionTrue, cond.Status) assert.Equal(t, "ReferencedByWorkloads", cond.Reason) - assert.NotEmpty(t, blockedConfig.Status.ReferencingWorkloads) // Delete server and try again require.NoError(t, fakeClient.Delete(ctx, mcpServer)) @@ -268,7 +258,6 @@ func TestMCPWebhookConfigReconciler_handleDeletion(t *testing.T) { require.NoError(t, err) assert.Empty(t, deletedConfig.Finalizers) assert.Nil(t, meta.FindStatusCondition(deletedConfig.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked)) - assert.Empty(t, deletedConfig.Status.ReferencingWorkloads) } func TestMCPWebhookConfigReconciler_handleDeletionClearsBlockedCondition(t *testing.T) { @@ -283,10 +272,6 @@ func TestMCPWebhookConfigReconciler_handleDeletionClearsBlockedCondition(t *test Finalizers: []string{WebhookConfigFinalizerName}, }, Status: mcpv1beta1.MCPWebhookConfigStatus{ - ReferencingWorkloads: []mcpv1beta1.WorkloadReference{{ - Kind: mcpv1beta1.WorkloadKindMCPServer, - Name: "server1", - }}, Conditions: []metav1.Condition{{ Type: mcpv1beta1.ConditionTypeDeletionBlocked, Status: metav1.ConditionTrue, @@ -311,142 +296,6 @@ func TestMCPWebhookConfigReconciler_handleDeletionClearsBlockedCondition(t *test require.NoError(t, fakeClient.Get(ctx, client.ObjectKeyFromObject(webhookConfig), clearedConfig)) assert.Empty(t, clearedConfig.Finalizers) assert.Nil(t, meta.FindStatusCondition(clearedConfig.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked)) - assert.Empty(t, clearedConfig.Status.ReferencingWorkloads) -} - -// TestMCPWebhookConfigReconciler_mapMCPServerToWebhookConfig verifies the map function -// enqueues exactly the config the MCPServer currently references (or nothing when it has -// no ref). The previously-referenced config is enqueued by EnqueueRequestsFromMapFunc, -// which runs the map function on both the old and new object on update — no manual -// stale-reference scan in the handler. -func TestMCPWebhookConfigReconciler_mapMCPServerToWebhookConfig(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - - server := &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, - Spec: mcpv1beta1.MCPServerSpec{ - WebhookConfigRef: &mcpv1beta1.WebhookConfigRef{Name: "current-config"}, - }, - } - - t.Run("current WebhookConfigRef returns one request", func(t *testing.T) { - t.Parallel() - - serverCopy := server.DeepCopy() - fakeClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(serverCopy). - Build() - r := &MCPWebhookConfigReconciler{Client: fakeClient, Scheme: scheme} - - require.ElementsMatch(t, []reconcile.Request{{ - NamespacedName: types.NamespacedName{Name: "current-config", Namespace: "default"}, - }}, r.mapMCPServerToWebhookConfig(ctx, serverCopy)) - }) - - t.Run("no WebhookConfigRef returns nil", func(t *testing.T) { - t.Parallel() - - serverWithoutRef := server.DeepCopy() - serverWithoutRef.Spec.WebhookConfigRef = nil - fakeClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(serverWithoutRef). - Build() - r := &MCPWebhookConfigReconciler{Client: fakeClient, Scheme: scheme} - - assert.Nil(t, r.mapMCPServerToWebhookConfig(ctx, serverWithoutRef)) - }) - - t.Run("wrong object type returns nil", func(t *testing.T) { - t.Parallel() - - fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() - r := &MCPWebhookConfigReconciler{Client: fakeClient, Scheme: scheme} - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod", Namespace: "default"}} - - assert.Nil(t, r.mapMCPServerToWebhookConfig(ctx, pod)) - }) -} - -func TestMCPWebhookConfigReconciler_updateReferencingWorkloads(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - - webhookConfig := &mcpv1alpha1.MCPWebhookConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "webhook-config", Namespace: "default"}, - } - referencingServer := v1beta1test.NewMCPServer("server", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithWebhookConfigRef(webhookConfig.Name), - ) - otherServer := v1beta1test.NewMCPServer("other-server", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithWebhookConfigRef("other-config"), - ) - fakeClient := withWebhookConfigRefIndex(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(webhookConfig, referencingServer, otherServer). - WithStatusSubresource(&mcpv1alpha1.MCPWebhookConfig{}). - Build() - r := &MCPWebhookConfigReconciler{Client: fakeClient, Scheme: scheme} - - result, err := r.updateReferencingWorkloads(ctx, webhookConfig) - require.NoError(t, err) - assert.Equal(t, reconcile.Result{}, result) - - var updated mcpv1alpha1.MCPWebhookConfig - require.NoError(t, fakeClient.Get(ctx, client.ObjectKeyFromObject(webhookConfig), &updated)) - assert.Equal(t, []mcpv1beta1.WorkloadReference{{ - Kind: mcpv1beta1.WorkloadKindMCPServer, - Name: referencingServer.Name, - }}, updated.Status.ReferencingWorkloads) - - result, err = r.updateReferencingWorkloads(ctx, &updated) - require.NoError(t, err) - assert.Equal(t, reconcile.Result{}, result) - - var unchanged mcpv1alpha1.MCPWebhookConfig - require.NoError(t, fakeClient.Get(ctx, client.ObjectKeyFromObject(webhookConfig), &unchanged)) - assert.Equal(t, updated.Status.ReferencingWorkloads, unchanged.Status.ReferencingWorkloads) -} - -func TestMCPWebhookConfigReconciler_updateReferencingWorkloadsListError(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - - webhookConfig := &mcpv1alpha1.MCPWebhookConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "webhook-config", Namespace: "default"}, - } - listErr := errors.New("simulated MCPServer list failure") - fakeClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(webhookConfig). - WithInterceptorFuncs(interceptor.Funcs{ - List: func( - ctx context.Context, - c client.WithWatch, - list client.ObjectList, - opts ...client.ListOption, - ) error { - if _, ok := list.(*mcpv1beta1.MCPServerList); ok { - return listErr - } - return c.List(ctx, list, opts...) - }, - }). - Build() - r := &MCPWebhookConfigReconciler{Client: fakeClient, Scheme: scheme} - - result, err := r.updateReferencingWorkloads(ctx, webhookConfig) - require.ErrorIs(t, err, listErr) - assert.Equal(t, reconcile.Result{}, result) } func findWebhookConfigCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition { diff --git a/cmd/thv-operator/controllers/toolconfig_controller.go b/cmd/thv-operator/controllers/toolconfig_controller.go index 558259fa27..6bb53da4da 100644 --- a/cmd/thv-operator/controllers/toolconfig_controller.go +++ b/cmd/thv-operator/controllers/toolconfig_controller.go @@ -12,15 +12,10 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" @@ -99,17 +94,6 @@ 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 !ctrlutil.WorkloadRefsEqual(toolConfig.Status.ReferencingWorkloads, referencingWorkloads) || - toolConfig.Status.ReferenceCount != workloadReferenceCount(referencingWorkloads) { - toolConfig.Status.ReferencingWorkloads = referencingWorkloads - toolConfig.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) - conditionChanged = true - } - // Update condition if it changed (even without hash change) if conditionChanged { if err := r.Status().Update(ctx, toolConfig); err != nil { @@ -144,15 +128,6 @@ func (r *ToolConfigReconciler) handleConfigHashChange( toolConfig.Status.ConfigHash = configHash toolConfig.Status.ObservedGeneration = toolConfig.Generation - // Update the status with the list of referencing workloads - refs := make([]mcpv1beta1.WorkloadReference, 0, len(referencingServers)) - for _, server := range referencingServers { - refs = append(refs, mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPServer, Name: server.Name}) - } - ctrlutil.SortWorkloadRefs(refs) - toolConfig.Status.ReferencingWorkloads = refs - toolConfig.Status.ReferenceCount = workloadReferenceCount(refs) - // Update the MCPToolConfig status if err := r.Status().Update(ctx, toolConfig); err != nil { logger.Error(err, "Failed to update MCPToolConfig status") @@ -206,8 +181,6 @@ func (r *ToolConfigReconciler) handleDeletion(ctx context.Context, toolConfig *m Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), ObservedGeneration: toolConfig.Generation, }) - toolConfig.Status.ReferencingWorkloads = referencingWorkloads - toolConfig.Status.ReferenceCount = workloadReferenceCount(referencingWorkloads) if updateErr := r.Status().Update(ctx, toolConfig); updateErr != nil { logger.Error(updateErr, "Failed to update status during deletion block") } @@ -285,7 +258,6 @@ 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 { // Field index backing findReferencingWorkloads / findReferencingMCPServers: lets // the controller query only the MCPServers referencing a given config rather than @@ -296,39 +268,7 @@ func (r *ToolConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to set up MCPServer toolConfigRef index: %w", err) } - // Watch MCPServer changes to update ReferencingWorkloads on referenced MCPToolConfigs. - // The map function only returns the MCPToolConfig the server currently references. - // EnqueueRequestsFromMapFunc runs it on both the old and new object on update (and - // on the object for create/delete), so removing or changing the ref enqueues both - // the previously- and newly-referenced config — the previously-referenced config - // then reconciles and prunes the stale entry. No manual stale-reference scan needed. - // - // GenerationChangedPredicate also suppresses the workload-watch resync; the self-heal - // backstop for a stale ReferencingWorkloads entry (e.g. a workload deleted while the - // operator was down) is this config's own For() resync, which re-runs Reconcile and - // rebuilds ReferencingWorkloads from the index. return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.MCPToolConfig{}). - Watches(&mcpv1beta1.MCPServer{}, - handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToToolConfig), - builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) } - -// mapMCPServerToToolConfig maps an MCPServer to the MCPToolConfig it currently -// references. EnqueueRequestsFromMapFunc invokes this on both the old and new object on -// update (and on the object for create/delete), so a ref change or deletion automatically -// enqueues both the previously- and newly-referenced config; the previously-referenced -// config then prunes the stale entry on reconcile. No manual stale-reference scan needed. -func (*ToolConfigReconciler) mapMCPServerToToolConfig( - _ context.Context, obj client.Object, -) []reconcile.Request { - server, ok := obj.(*mcpv1beta1.MCPServer) - if !ok || server.Spec.ToolConfigRef == nil || server.Spec.ToolConfigRef.Name == "" { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{ - Name: server.Spec.ToolConfigRef.Name, - Namespace: server.Namespace, - }}} -} diff --git a/cmd/thv-operator/controllers/toolconfig_controller_edge_cases_test.go b/cmd/thv-operator/controllers/toolconfig_controller_edge_cases_test.go index 91c05e32ff..60fa75e292 100644 --- a/cmd/thv-operator/controllers/toolconfig_controller_edge_cases_test.go +++ b/cmd/thv-operator/controllers/toolconfig_controller_edge_cases_test.go @@ -112,8 +112,6 @@ 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.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "test-server"}) }) t.Run("reconcile with changed spec", func(t *testing.T) { @@ -238,89 +236,6 @@ func (c *errorClient) List(ctx context.Context, list client.ObjectList, opts ... func TestToolConfigReconciler_ComplexScenarios(t *testing.T) { t.Parallel() - t.Run("multiple MCPServers referencing same MCPToolConfig", func(t *testing.T) { - t.Parallel() - ctx := t.Context() - - scheme := testutil.NewScheme(t) - - toolConfig := &mcpv1beta1.MCPToolConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "shared-config", - Namespace: "default", - }, - Spec: mcpv1beta1.MCPToolConfigSpec{ - ToolsFilter: []string{"tool1", "tool2", "tool3"}, - ToolsOverride: map[string]mcpv1beta1.ToolOverride{ - "tool1": { - Name: "custom-tool1", - Description: "Customized tool 1", - }, - }, - }, - } - - // Create multiple MCPServers referencing the same MCPToolConfig - mcpServers := []*mcpv1beta1.MCPServer{ - v1beta1test.NewMCPServer("server1", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithToolConfigRef("shared-config"), - ), - v1beta1test.NewMCPServer("server2", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithToolConfigRef("shared-config"), - ), - v1beta1test.NewMCPServer("server3", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithToolConfigRef("shared-config"), - ), - } - - objs := []client.Object{toolConfig} - for _, server := range mcpServers { - objs = append(objs, server) - } - - fakeClient := withToolConfigRefIndex(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(objs...). - WithStatusSubresource(&mcpv1beta1.MCPToolConfig{}). - Build() - - r := &ToolConfigReconciler{ - Client: fakeClient, - Scheme: scheme, - } - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: toolConfig.Name, - Namespace: toolConfig.Namespace, - }, - } - - // First reconciliation adds finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - // Second reconciliation updates status - result, err = r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Equal(t, time.Duration(0), result.RequeueAfter) - - // Verify all servers are listed in status - var updatedConfig mcpv1beta1.MCPToolConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Len(t, updatedConfig.Status.ReferencingWorkloads, 3) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server1"}) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server2"}) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server3"}) - }) - t.Run("empty MCPToolConfig spec", func(t *testing.T) { t.Parallel() ctx := t.Context() diff --git a/cmd/thv-operator/controllers/toolconfig_controller_test.go b/cmd/thv-operator/controllers/toolconfig_controller_test.go index 7eb043ec04..0b38393f5a 100644 --- a/cmd/thv-operator/controllers/toolconfig_controller_test.go +++ b/cmd/thv-operator/controllers/toolconfig_controller_test.go @@ -210,13 +210,6 @@ func TestToolConfigReconciler_Reconcile(t *testing.T) { "MCPToolConfig status should have config hash") } - // Check referencing workloads in status - if tt.existingMCPServer != nil { - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: tt.existingMCPServer.Name}, - "Status should contain referencing MCPServer as WorkloadReference") - } - // Check Valid condition is set after successful reconciliation cond := k8smeta.FindStatusCondition(updatedConfig.Status.Conditions, mcpv1beta1.ConditionToolConfigValid) require.NotNil(t, cond, "Valid condition must be set after successful reconciliation") @@ -276,149 +269,6 @@ func TestToolConfigReconciler_findReferencingWorkloads(t *testing.T) { assert.NotContains(t, refs, mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server3"}) } -func TestToolConfigReconciler_ReferencingWorkloadsUpdatedWithoutHashChange(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - scheme := testutil.NewScheme(t) - - toolConfig := &mcpv1beta1.MCPToolConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: "default", - }, - Spec: mcpv1beta1.MCPToolConfigSpec{ - ToolsFilter: []string{"tool1"}, - }, - } - - fakeClient := withToolConfigRefIndex(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(toolConfig). - WithStatusSubresource(&mcpv1beta1.MCPToolConfig{}). - Build() - - r := &ToolConfigReconciler{ - Client: fakeClient, - Scheme: scheme, - } - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: toolConfig.Name, - Namespace: toolConfig.Namespace, - }, - } - - // First reconciliation - add finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - // Second reconciliation - sets hash, no servers yet - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - var updatedConfig mcpv1beta1.MCPToolConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.NotEmpty(t, updatedConfig.Status.ConfigHash) - assert.Empty(t, updatedConfig.Status.ReferencingWorkloads, "No servers should be referencing yet") - assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) - - // Verify Valid condition is set after initial reconciliation - cond := k8smeta.FindStatusCondition(updatedConfig.Status.Conditions, mcpv1beta1.ConditionToolConfigValid) - require.NotNil(t, cond, "Valid condition must be set after reconciliation") - assert.Equal(t, metav1.ConditionTrue, cond.Status) - - // Add an MCPServer that references this config (without changing the config spec) - mcpServer := v1beta1test.NewMCPServer("new-server", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithToolConfigRef("test-config"), - ) - require.NoError(t, fakeClient.Create(ctx, mcpServer)) - - // Reconcile again - hash hasn't changed, but referencing servers should be updated - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "new-server"}, - "ReferencingWorkloads should be updated even without hash change") - assert.EqualValues(t, 1, updatedConfig.Status.ReferenceCount) -} - -func TestToolConfigReconciler_ReferencingWorkloadsRemovedOnServerDeletion(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - scheme := testutil.NewScheme(t) - - toolConfig := &mcpv1beta1.MCPToolConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: "default", - }, - Spec: mcpv1beta1.MCPToolConfigSpec{ - ToolsFilter: []string{"tool1"}, - }, - } - - mcpServer := v1beta1test.NewMCPServer("server-to-delete", "default", - v1beta1test.WithImage("test-image"), - v1beta1test.WithToolConfigRef("test-config"), - ) - - fakeClient := withToolConfigRefIndex(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(toolConfig, mcpServer). - WithStatusSubresource(&mcpv1beta1.MCPToolConfig{}). - Build() - - r := &ToolConfigReconciler{ - Client: fakeClient, - Scheme: scheme, - } - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: toolConfig.Name, - Namespace: toolConfig.Namespace, - }, - } - - // Add finalizer - result, err := r.Reconcile(ctx, req) - require.NoError(t, err) - assert.Greater(t, result.RequeueAfter, time.Duration(0)) - - // Set hash and referencing servers - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - var updatedConfig mcpv1beta1.MCPToolConfig - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Contains(t, updatedConfig.Status.ReferencingWorkloads, - mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: "server-to-delete"}) - assert.EqualValues(t, 1, updatedConfig.Status.ReferenceCount) - - // Delete the MCPServer - require.NoError(t, fakeClient.Delete(ctx, mcpServer)) - - // Reconcile again - referencing servers should be empty now - _, err = r.Reconcile(ctx, req) - require.NoError(t, err) - - err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) - require.NoError(t, err) - assert.Empty(t, updatedConfig.Status.ReferencingWorkloads, - "ReferencingWorkloads should be empty after server deletion") - assert.EqualValues(t, 0, updatedConfig.Status.ReferenceCount) -} - func TestToolConfigReconciler_ValidConditionObservedGeneration(t *testing.T) { t.Parallel() @@ -497,57 +347,3 @@ func TestToolConfigReconciler_ValidConditionObservedGeneration(t *testing.T) { assert.Equal(t, int64(2), cond.ObservedGeneration, "ObservedGeneration should be updated to match new Generation") } - -// TestMCPToolConfigReconciler_watchHandlers verifies that the MCPServer watch map -// function enqueues exactly the config the server currently references (or nothing -// when the server has no ref). The previously-referenced config is enqueued by -// EnqueueRequestsFromMapFunc, which runs the map function on both the old and new -// object on update — no manual stale-reference scan in the handler. -func TestMCPToolConfigReconciler_watchHandlers(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - obj client.Object - expected map[string]struct{} - }{ - { - name: "MCPServer with ref enqueues the current config", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - v1beta1test.WithToolConfigRef("current-config"), - ), - expected: map[string]struct{}{"current-config": {}}, - }, - { - name: "MCPServer without ref enqueues nothing", - obj: v1beta1test.NewMCPServer("srv", "default", - v1beta1test.WithImage("example/mcp:latest"), - ), - expected: map[string]struct{}{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - ctx := t.Context() - scheme := testutil.NewScheme(t) - fakeClient := withToolConfigRefIndex(fake.NewClientBuilder().WithScheme(scheme)). - WithObjects(tt.obj). - WithStatusSubresource(&mcpv1beta1.MCPToolConfig{}). - Build() - r := &ToolConfigReconciler{Client: fakeClient, Scheme: scheme} - - requests := r.mapMCPServerToToolConfig(ctx, tt.obj) - - got := make(map[string]struct{}, len(requests)) - for _, req := range requests { - assert.Equal(t, "default", req.Namespace) - got[req.Name] = struct{}{} - } - assert.Equal(t, tt.expected, got) - }) - } -} diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller.go b/cmd/thv-operator/controllers/virtualmcpserver_controller.go index ea94504456..4eed3f1a38 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller.go @@ -3389,8 +3389,8 @@ func (r *VirtualMCPServerReconciler) handleConfigRefs( // (spec.incomingAuth.authzConfigRef), tracks its hash on the VirtualMCPServer // status, and sets the AuthzConfigRefValidated condition. When the ref is // cleared it removes both the hash and the condition so a stale "valid" signal -// does not linger. ReferencingWorkloads on the MCPAuthzConfig is owned by the -// MCPAuthzConfig controller (#5511); this controller never writes it. +// does not linger. The MCPAuthzConfig's status is owned by the MCPAuthzConfig +// controller (#5511); this controller never writes it. // // Revocation semantics (fail-stale, not fail-open): if a previously-valid ref // later becomes invalid or missing, this returns an error and Reconcile stops @@ -3575,13 +3575,11 @@ func (r *VirtualMCPServerReconciler) handleOIDCConfig( return fmt.Errorf("%s", msg) } - // ReferencingWorkloads on the MCPOIDCConfig is maintained solely by the - // MCPOIDCConfig controller, which watches MCPServer/VirtualMCPServer/ - // MCPRemoteProxy and recomputes the full list (additions and removals). The - // VirtualMCPServer controller must not write the config's status: a full - // r.Status().Update here would clobber conditions the config controller - // owns, and the previous append-only write never removed stale entries. - // See #5511. + // The VirtualMCPServer controller must not write the MCPOIDCConfig's status: + // that status is owned by the MCPOIDCConfig controller, and a full + // r.Status().Update here would clobber conditions it owns. The config + // controller no longer tracks referencing workloads in its status; deletion + // protection recomputes referrers on demand in its finalizer. See #5511. // Set valid condition statusManager.SetCondition( diff --git a/cmd/thv-operator/controllers/workload_reference_count.go b/cmd/thv-operator/controllers/workload_reference_count.go deleted file mode 100644 index aeb36b6c26..0000000000 --- a/cmd/thv-operator/controllers/workload_reference_count.go +++ /dev/null @@ -1,22 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package controllers - -import ( - "math" - - mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" -) - -func workloadReferenceCount(refs []mcpv1beta1.WorkloadReference) int32 { - return workloadReferenceCountFromLen(len(refs)) -} - -func workloadReferenceCountFromLen(length int) int32 { - if length > math.MaxInt32 { - return math.MaxInt32 - } - - return int32(length) //nolint:gosec // guarded above against int32 overflow -} diff --git a/cmd/thv-operator/controllers/workload_reference_count_test.go b/cmd/thv-operator/controllers/workload_reference_count_test.go deleted file mode 100644 index 7afa8c489e..0000000000 --- a/cmd/thv-operator/controllers/workload_reference_count_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package controllers - -import ( - "math" - "testing" - - "github.com/stretchr/testify/assert" - - mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" -) - -func TestWorkloadReferenceCount(t *testing.T) { - t.Parallel() - - assert.EqualValues(t, 0, workloadReferenceCount(nil)) - assert.EqualValues(t, 0, workloadReferenceCount([]mcpv1beta1.WorkloadReference{})) - - refs := []mcpv1beta1.WorkloadReference{ - {Kind: mcpv1beta1.WorkloadKindMCPServer, Name: "server-1"}, - {Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: "proxy-1"}, - } - - assert.EqualValues(t, 2, workloadReferenceCount(refs)) - assert.Equal(t, int32(math.MaxInt32), workloadReferenceCountFromLen(math.MaxInt32+1)) -} diff --git a/cmd/thv-operator/pkg/controllerutil/config.go b/cmd/thv-operator/pkg/controllerutil/config.go index ff8f127d41..e3736a7251 100644 --- a/cmd/thv-operator/pkg/controllerutil/config.go +++ b/cmd/thv-operator/pkg/controllerutil/config.go @@ -54,14 +54,6 @@ func SortWorkloadRefs(refs []mcpv1beta1.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 []mcpv1beta1.WorkloadReference) bool { - return slices.EqualFunc(a, b, func(x, y mcpv1beta1.WorkloadReference) bool { - return x.Kind == y.Kind && x.Name == y.Name - }) -} - // GetToolConfigForMCPRemoteProxy fetches MCPToolConfig referenced by MCPRemoteProxy func GetToolConfigForMCPRemoteProxy( ctx context.Context, diff --git a/cmd/thv-operator/pkg/controllerutil/config_test.go b/cmd/thv-operator/pkg/controllerutil/config_test.go index 2a6bbb793d..ef06f64227 100644 --- a/cmd/thv-operator/pkg/controllerutil/config_test.go +++ b/cmd/thv-operator/pkg/controllerutil/config_test.go @@ -128,56 +128,6 @@ func TestSortWorkloadRefs(t *testing.T) { }) } -func TestWorkloadRefsEqual(t *testing.T) { - t.Parallel() - - t.Run("equal slices", func(t *testing.T) { - t.Parallel() - a := []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "alpha"}, - {Kind: "MCPServer", Name: "beta"}, - } - b := []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "alpha"}, - {Kind: "MCPServer", Name: "beta"}, - } - assert.True(t, WorkloadRefsEqual(a, b)) - }) - - t.Run("different order is not equal", func(t *testing.T) { - t.Parallel() - a := []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "alpha"}, - {Kind: "MCPServer", Name: "beta"}, - } - b := []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "beta"}, - {Kind: "MCPServer", Name: "alpha"}, - } - assert.False(t, WorkloadRefsEqual(a, b)) - }) - - t.Run("different lengths", func(t *testing.T) { - t.Parallel() - a := []mcpv1beta1.WorkloadReference{{Kind: "MCPServer", Name: "alpha"}} - b := []mcpv1beta1.WorkloadReference{ - {Kind: "MCPServer", Name: "alpha"}, - {Kind: "MCPServer", Name: "beta"}, - } - assert.False(t, WorkloadRefsEqual(a, b)) - }) - - t.Run("both nil", func(t *testing.T) { - t.Parallel() - assert.True(t, WorkloadRefsEqual(nil, nil)) - }) - - t.Run("nil vs empty", func(t *testing.T) { - t.Parallel() - assert.True(t, WorkloadRefsEqual(nil, []mcpv1beta1.WorkloadReference{})) - }) -} - func TestGetTelemetryConfigForMCPRemoteProxy(t *testing.T) { t.Parallel() diff --git a/cmd/thv-operator/test-integration/mcp-external-auth/mcpexternalauthconfig_controller_integration_test.go b/cmd/thv-operator/test-integration/mcp-external-auth/mcpexternalauthconfig_controller_integration_test.go index a0eda3135c..4c3e81d690 100644 --- a/cmd/thv-operator/test-integration/mcp-external-auth/mcpexternalauthconfig_controller_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-external-auth/mcpexternalauthconfig_controller_integration_test.go @@ -11,6 +11,8 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -242,27 +244,6 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { }, timeout, interval).Should(BeTrue()) }) - It("Should update MCPExternalAuthConfig status with referencing workload", func() { - // Wait for the auth config status to be updated with the referencing workload - Eventually(func() bool { - updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: authConfigName, - Namespace: namespace, - }, updatedAuthConfig) - if err != nil { - return false - } - // Check if the server is in the referencing workloads list - for _, ref := range updatedAuthConfig.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" && ref.Name == mcpServerName { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - It("Should create ConfigMap with token exchange configuration", func() { // Wait for ConfigMap to be created configMapName := mcpServerName + "-runconfig" @@ -323,7 +304,7 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { }) }) - Context("When a referencing MCPServer is deleted", Ordered, func() { + Context("When updating an MCPExternalAuthConfig", Ordered, func() { var ( namespace string authConfigName string @@ -332,13 +313,14 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { mcpServer *mcpv1beta1.MCPServer oauthSecret *corev1.Secret oauthSecretName string + originalHash string ) BeforeAll(func() { namespace = defaultNamespace - authConfigName = "test-external-auth-prune" - mcpServerName = "external-auth-prune-server" - oauthSecretName = "oauth-test-secret-prune" + authConfigName = "test-external-auth-update" + mcpServerName = "external-auth-update-server" + oauthSecretName = "oauth-test-secret-update" // Create namespace if it doesn't exist ns := &corev1.Namespace{ @@ -355,7 +337,7 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { Namespace: namespace, }, StringData: map[string]string{ - "client-secret": "test-secret-value-prune", + "client-secret": "original-secret", }, } Expect(k8sClient.Create(ctx, oauthSecret)).Should(Succeed()) @@ -370,12 +352,12 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { Type: "tokenExchange", TokenExchange: &mcpv1beta1.TokenExchangeConfig{ TokenURL: "https://oauth.example.com/token", - ClientID: "test-client-id-prune", + ClientID: "original-client-id", ClientSecretRef: &mcpv1beta1.SecretKeyRef{ Name: oauthSecretName, Key: "client-secret", }, - Audience: "mcp-backend-prune", + Audience: "original-audience", Scopes: []string{"read"}, }, }, @@ -389,7 +371,11 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { Name: authConfigName, Namespace: namespace, }, updatedAuthConfig) - return err == nil && updatedAuthConfig.Status.ConfigHash != "" + if err != nil { + return false + } + originalHash = updatedAuthConfig.Status.ConfigHash + return originalHash != "" }, timeout, interval).Should(BeTrue()) // Create MCPServer with external auth reference @@ -409,40 +395,45 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { } Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - // Wait until the server is tracked in ReferencingWorkloads. + // Wait for the MCPServer to have the original hash Eventually(func() bool { - updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} + updatedMCPServer := &mcpv1beta1.MCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: authConfigName, + Name: mcpServerName, Namespace: namespace, - }, updatedAuthConfig) + }, updatedMCPServer) if err != nil { return false } - for _, ref := range updatedAuthConfig.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" && ref.Name == mcpServerName { - return true - } - } - return false + return updatedMCPServer.Status.ExternalAuthConfigHash == originalHash }, timeout, interval).Should(BeTrue()) }) AfterAll(func() { - // mcpServer is already deleted by the spec; ignore NotFound on cleanup. - _ = k8sClient.Delete(ctx, mcpServer) + // Clean up resources + Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) Expect(k8sClient.Delete(ctx, authConfig)).Should(Succeed()) Expect(k8sClient.Delete(ctx, oauthSecret)).Should(Succeed()) }) - It("Should prune the workload from ReferencingWorkloads and decrement ReferenceCount", func() { - // Delete the referencing MCPServer. The controller-runtime watch - // enqueues the referenced MCPExternalAuthConfig (via the old-object - // map function), and the level-triggered reconcile must prune the - // now-stale reference. - Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) + It("Should update config hash when auth config is modified", func() { + // Update the auth config + Eventually(func() error { + updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} + if err := k8sClient.Get(ctx, types.NamespacedName{ + Name: authConfigName, + Namespace: namespace, + }, updatedAuthConfig); err != nil { + return err + } - // Eventually the reference is pruned and the count drops to zero. + // Modify the audience + updatedAuthConfig.Spec.TokenExchange.Audience = "updated-audience" + return k8sClient.Update(ctx, updatedAuthConfig) + }, timeout, interval).Should(Succeed()) + + // Wait for the config hash to change + var newHash string Eventually(func() bool { updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -452,17 +443,37 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { if err != nil { return false } - for _, ref := range updatedAuthConfig.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" && ref.Name == mcpServerName { - return false - } + newHash = updatedAuthConfig.Status.ConfigHash + return newHash != "" && newHash != originalHash + }, timeout, interval).Should(BeTrue()) + + // Verify the new hash is different + Expect(newHash).NotTo(Equal(originalHash)) + }) + + It("Should trigger MCPServer reconciliation with updated hash", func() { + // Wait for the MCPServer to get the updated hash + Eventually(func() bool { + updatedMCPServer := &mcpv1beta1.MCPServer{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: mcpServerName, + Namespace: namespace, + }, updatedMCPServer) + if err != nil { + return false } - return updatedAuthConfig.Status.ReferenceCount == 0 + // Check if the hash has been updated + return updatedMCPServer.Status.ExternalAuthConfigHash != originalHash }, timeout, interval).Should(BeTrue()) }) }) - Context("When updating an MCPExternalAuthConfig", Ordered, func() { + Context("When deleting an MCPExternalAuthConfig with active references", Ordered, func() { + // The block->unblock deletion transition is driven purely by the + // controller's periodic requeue (the workload watches were removed), so + // the unblock assertion below must tolerate the up-to-30s requeue delay. + const deletionRequeueTimeout = time.Second * 45 + var ( namespace string authConfigName string @@ -471,14 +482,13 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { mcpServer *mcpv1beta1.MCPServer oauthSecret *corev1.Secret oauthSecretName string - originalHash string ) BeforeAll(func() { namespace = defaultNamespace - authConfigName = "test-external-auth-update" - mcpServerName = "external-auth-update-server" - oauthSecretName = "oauth-test-secret-update" + authConfigName = "test-external-auth-deletion" + mcpServerName = "external-auth-deletion-server" + oauthSecretName = "oauth-test-secret-deletion" // Create namespace if it doesn't exist ns := &corev1.Namespace{ @@ -495,7 +505,7 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { Namespace: namespace, }, StringData: map[string]string{ - "client-secret": "original-secret", + "client-secret": "deletion-secret-value", }, } Expect(k8sClient.Create(ctx, oauthSecret)).Should(Succeed()) @@ -510,33 +520,20 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { Type: "tokenExchange", TokenExchange: &mcpv1beta1.TokenExchangeConfig{ TokenURL: "https://oauth.example.com/token", - ClientID: "original-client-id", + ClientID: "deletion-client-id", ClientSecretRef: &mcpv1beta1.SecretKeyRef{ Name: oauthSecretName, Key: "client-secret", }, - Audience: "original-audience", + Audience: "mcp-backend-deletion", Scopes: []string{"read"}, }, }, } Expect(k8sClient.Create(ctx, authConfig)).Should(Succeed()) - // Wait for the auth config to have a hash - Eventually(func() bool { - updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: authConfigName, - Namespace: namespace, - }, updatedAuthConfig) - if err != nil { - return false - } - originalHash = updatedAuthConfig.Status.ConfigHash - return originalHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create MCPServer with external auth reference + // Create MCPServer referencing the auth config so it is observably + // referenced before we attempt deletion. mcpServer = &mcpv1beta1.MCPServer{ ObjectMeta: metav1.ObjectMeta{ Name: mcpServerName, @@ -553,7 +550,9 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { } Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - // Wait for the MCPServer to have the original hash + // Wait for the MCPServer to be wired to the config (the external auth + // config hash propagates to its status) so the reference is + // observable before deletion. Eventually(func() bool { updatedMCPServer := &mcpv1beta1.MCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -563,66 +562,87 @@ var _ = Describe("MCPExternalAuthConfig Controller Integration Tests", func() { if err != nil { return false } - return updatedMCPServer.Status.ExternalAuthConfigHash == originalHash + return updatedMCPServer.Status.ExternalAuthConfigHash != "" }, timeout, interval).Should(BeTrue()) }) AfterAll(func() { - // Clean up resources - Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) - Expect(k8sClient.Delete(ctx, authConfig)).Should(Succeed()) - Expect(k8sClient.Delete(ctx, oauthSecret)).Should(Succeed()) + // Best-effort cleanup; the specs may have already removed these. + _ = k8sClient.Delete(ctx, mcpServer) + _ = k8sClient.Delete(ctx, oauthSecret) + + // Ensure the MCPExternalAuthConfig is fully gone before the suite + // reuses the shared namespace. + Eventually(func() bool { + updated := &mcpv1beta1.MCPExternalAuthConfig{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: authConfigName, + Namespace: namespace, + }, updated) + return errors.IsNotFound(err) + }, deletionRequeueTimeout, interval).Should(BeTrue()) }) - It("Should update config hash when auth config is modified", func() { - // Update the auth config - Eventually(func() error { - updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} + It("Should have the finalizer once it is being tracked", func() { + Eventually(func() []string { + updated := &mcpv1beta1.MCPExternalAuthConfig{} if err := k8sClient.Get(ctx, types.NamespacedName{ Name: authConfigName, Namespace: namespace, - }, updatedAuthConfig); err != nil { - return err + }, updated); err != nil { + return nil } + return updated.Finalizers + }, timeout, interval).Should(ContainElement("mcpexternalauthconfig.toolhive.stacklok.dev/finalizer")) + }) - // Modify the audience - updatedAuthConfig.Spec.TokenExchange.Audience = "updated-audience" - return k8sClient.Update(ctx, updatedAuthConfig) - }, timeout, interval).Should(Succeed()) + It("Should not be deleted while referenced and should set DeletionBlocked condition", func() { + // Request deletion; the finalizer must keep the object alive. + Expect(k8sClient.Delete(ctx, authConfig)).Should(Succeed()) - // Wait for the config hash to change - var newHash string + // The object should still exist (finalizer blocks deletion) with a + // pending deletion timestamp and a DeletionBlocked=True condition. Eventually(func() bool { - updatedAuthConfig := &mcpv1beta1.MCPExternalAuthConfig{} + updated := &mcpv1beta1.MCPExternalAuthConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ Name: authConfigName, Namespace: namespace, - }, updatedAuthConfig) + }, updated) if err != nil { return false } - newHash = updatedAuthConfig.Status.ConfigHash - return newHash != "" && newHash != originalHash + if updated.DeletionTimestamp.IsZero() { + return false + } + cond := meta.FindStatusCondition(updated.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + return cond != nil && cond.Status == metav1.ConditionTrue }, timeout, interval).Should(BeTrue()) - // Verify the new hash is different - Expect(newHash).NotTo(Equal(originalHash)) + // Confirm it stays blocked (does not vanish) while still referenced. + Consistently(func() bool { + updated := &mcpv1beta1.MCPExternalAuthConfig{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: authConfigName, + Namespace: namespace, + }, updated) + return err == nil + }, time.Second*2, interval).Should(BeTrue()) }) - It("Should trigger MCPServer reconciliation with updated hash", func() { - // Wait for the MCPServer to get the updated hash + It("Should be deleted after the referencing MCPServer is removed", func() { + // Remove the reference. + Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) + + // With the workload watches removed, the unblock relies solely on the + // controller's periodic (up-to-30s) requeue, so allow extra time. Eventually(func() bool { - updatedMCPServer := &mcpv1beta1.MCPServer{} + updated := &mcpv1beta1.MCPExternalAuthConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: mcpServerName, + Name: authConfigName, Namespace: namespace, - }, updatedMCPServer) - if err != nil { - return false - } - // Check if the hash has been updated - return updatedMCPServer.Status.ExternalAuthConfigHash != originalHash - }, timeout, interval).Should(BeTrue()) + }, updated) + return errors.IsNotFound(err) + }, deletionRequeueTimeout, interval).Should(BeTrue()) }) }) }) diff --git a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpremoteproxy_integration_test.go b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpremoteproxy_integration_test.go index 3e08a884d3..1b8fc8aa39 100644 --- a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpremoteproxy_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpremoteproxy_integration_test.go @@ -145,25 +145,6 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te }, timeout, interval).Should(BeTrue()) }) - It("should track MCPRemoteProxy in MCPOIDCConfig ReferencingWorkloads", func() { - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) }) Context("When MCPRemoteProxy references non-existent MCPOIDCConfig (fail-closed on missing)", Ordered, func() { @@ -414,23 +395,18 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te proxy = newTestMCPRemoteProxy(proxyName, namespace, configName) Expect(k8sClient.Create(ctx, proxy)).Should(Succeed()) - // Wait for ReferencingWorkloads to be populated + // Wait for the proxy to be wired to the config (OIDCConfigHash populated) + // so the config is observably referenced before we attempt deletion. Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} + updated := &mcpv1beta1.MCPRemoteProxy{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, + Name: proxyName, Namespace: namespace, }, updated) if err != nil { return false } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false + return updated.Status.OIDCConfigHash != "" }, timeout, interval).Should(BeTrue()) // Attempt to delete the MCPOIDCConfig (should be blocked by finalizer) @@ -455,8 +431,9 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("should not be deleted while referenced by MCPRemoteProxy", func() { - // The object should still exist because the finalizer blocks deletion + It("should not be deleted while referenced and should set DeletionBlocked condition", func() { + // The object should still exist (finalizer blocks deletion) with a + // pending deletion timestamp and a DeletionBlocked=True condition. Eventually(func() bool { updated := &mcpv1beta1.MCPOIDCConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -466,7 +443,11 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te if err != nil { return false } - return !updated.DeletionTimestamp.IsZero() + if updated.DeletionTimestamp.IsZero() { + return false + } + cond := meta.FindStatusCondition(updated.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + return cond != nil && cond.Status == metav1.ConditionTrue }, timeout, interval).Should(BeTrue()) }) @@ -549,25 +530,6 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te proxy = newTestMCPRemoteProxy(proxyName, namespace, configName) Expect(k8sClient.Create(ctx, proxy)).Should(Succeed()) - // Wait for ReferencingWorkloads to contain the proxy - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - // Wait for the proxy OIDCConfigHash to be populated Eventually(func() bool { updated := &mcpv1beta1.MCPRemoteProxy{} @@ -588,7 +550,7 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("should clean up ReferencingWorkloads and clear OIDCConfigHash after ref removal", func() { + It("should clear OIDCConfigHash and remove condition after ref removal", func() { // Remove the OIDCConfigRef from the MCPRemoteProxy updated := &mcpv1beta1.MCPRemoteProxy{} Expect(k8sClient.Get(ctx, types.NamespacedName{ @@ -600,25 +562,6 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te updated.Spec.OIDCConfigRef = nil Expect(k8sClient.Update(ctx, updated)).Should(Succeed()) - // MCPOIDCConfig should no longer list MCPRemoteProxy in ReferencingWorkloads - Eventually(func() bool { - cfg := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, cfg) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range cfg.Status.ReferencingWorkloads { - if ref == expectedRef { - return false - } - } - return true - }, timeout, interval).Should(BeTrue()) - // MCPRemoteProxy OIDCConfigHash should be cleared and condition removed Eventually(func() bool { proxyUpdated := &mcpv1beta1.MCPRemoteProxy{} @@ -638,109 +581,4 @@ var _ = Describe("MCPOIDCConfig and MCPRemoteProxy Cross-Resource Integration Te }, timeout, interval).Should(BeTrue()) }) }) - - Context("When MCPRemoteProxy is deleted, should clean up ReferencingWorkloads", Ordered, func() { - var ( - namespace string - configName string - proxyName string - oidcConfig *mcpv1beta1.MCPOIDCConfig - proxy *mcpv1beta1.MCPRemoteProxy - ns *corev1.Namespace - ) - - BeforeAll(func() { - ns = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-proxy-oidcref-cleanup-", - }, - } - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - namespace = ns.Name - - configName = testOIDCConfigName - proxyName = testRemoteProxyName - - // Create MCPOIDCConfig - oidcConfig = &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://accounts.google.com", - ClientID: "test-client", - }, - }, - } - Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed()) - - // Wait for ready - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return updated.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create MCPRemoteProxy with OIDCConfigRef - proxy = newTestMCPRemoteProxy(proxyName, namespace, configName) - Expect(k8sClient.Create(ctx, proxy)).Should(Succeed()) - - // Wait for ReferencingWorkloads to contain the proxy - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - - AfterAll(func() { - _ = k8sClient.Delete(ctx, oidcConfig) - Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) - }) - - It("should remove MCPRemoteProxy from ReferencingWorkloads after deletion", func() { - // Delete the MCPRemoteProxy - Expect(k8sClient.Delete(ctx, proxy)).Should(Succeed()) - - // Eventually the referencing workloads list should not contain the proxy - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: mcpv1beta1.WorkloadKindMCPRemoteProxy, Name: proxyName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return false - } - } - return true - }, timeout, interval).Should(BeTrue()) - }) - }) }) diff --git a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpserver_integration_test.go b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpserver_integration_test.go index b61cfa66ac..d8be7502c9 100644 --- a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpserver_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpserver_integration_test.go @@ -139,138 +139,6 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests", }, timeout, interval).Should(BeTrue()) }) - It("should track MCPServer in MCPOIDCConfig ReferencingWorkloads", func() { - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: serverName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("When MCPServer is deleted, should clean up ReferencingWorkloads", Ordered, func() { - var ( - namespace string - configName string - serverName string - oidcConfig *mcpv1beta1.MCPOIDCConfig - mcpServer *mcpv1beta1.MCPServer - ns *corev1.Namespace - ) - - BeforeAll(func() { - // Create a unique namespace for this test context - ns = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-oidcref-cleanup-", - }, - } - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - namespace = ns.Name - - configName = testOIDCConfigName - serverName = testServerName - - // Create MCPOIDCConfig - oidcConfig = &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://accounts.google.com", - ClientID: "test-client", - }, - }, - } - Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed()) - - // Wait for ready - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return updated.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create MCPServer with OIDCConfigRef - mcpServer = &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPServerSpec{ - Image: testServerImage, - OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{ - Name: configName, - Audience: "test-audience", - Scopes: []string{"openid"}, - }, - }, - } - Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - - // Wait for ReferencingWorkloads to contain the server - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: serverName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - - AfterAll(func() { - _ = k8sClient.Delete(ctx, oidcConfig) - Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) - }) - - It("should remove server from ReferencingWorkloads after MCPServer deletion", func() { - // Delete the MCPServer - Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) - - // Eventually the referencing workloads list should be empty - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return len(updated.Status.ReferencingWorkloads) == 0 - }, timeout, interval).Should(BeTrue()) - }) }) Context("When deleting MCPOIDCConfig with active references", Ordered, func() { @@ -342,23 +210,18 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests", } Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - // Wait for ReferencingWorkloads to be populated + // Wait for the server to be wired to the config (OIDCConfigHash populated) + // so the config is observably referenced before we attempt deletion. Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} + updated := &mcpv1beta1.MCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, + Name: serverName, Namespace: namespace, }, updated) if err != nil { return false } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: serverName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false + return updated.Status.OIDCConfigHash != "" }, timeout, interval).Should(BeTrue()) // Attempt to delete the MCPOIDCConfig (should be blocked by finalizer) @@ -383,8 +246,9 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests", Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("should not be deleted while referenced", func() { - // The object should still exist because the finalizer blocks deletion + It("should not be deleted while referenced and should set DeletionBlocked condition", func() { + // The object should still exist (finalizer blocks deletion) with a + // pending deletion timestamp and a DeletionBlocked=True condition. Eventually(func() bool { updated := &mcpv1beta1.MCPOIDCConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -394,7 +258,11 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests", if err != nil { return false } - return !updated.DeletionTimestamp.IsZero() + if updated.DeletionTimestamp.IsZero() { + return false + } + cond := meta.FindStatusCondition(updated.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + return cond != nil && cond.Status == metav1.ConditionTrue }, timeout, interval).Should(BeTrue()) }) diff --git a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go index c598e9e5c4..6cdc7c0981 100644 --- a/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_virtualmcpserver_integration_test.go @@ -183,155 +183,6 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration "resource should be the explicit resourceUrl, not the internal service URL") }) - It("should track VirtualMCPServer in MCPOIDCConfig ReferencingWorkloads", func() { - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("When VirtualMCPServer is deleted, should clean up ReferencingWorkloads", Ordered, func() { - var ( - namespace string - configName string - vmcpName string - groupName string - oidcConfig *mcpv1beta1.MCPOIDCConfig - vmcpServer *mcpv1beta1.VirtualMCPServer - mcpGroup *mcpv1beta1.MCPGroup - ns *corev1.Namespace - ) - - BeforeAll(func() { - // Create a unique namespace for this test context - ns = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-vmcp-oidcref-cleanup-", - }, - } - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - namespace = ns.Name - - configName = testOIDCConfigName - vmcpName = testVMCPServerName - groupName = testVMCPGroupName - - // Create MCPGroup (required by VirtualMCPServer) - mcpGroup = &mcpv1beta1.MCPGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: groupName, - Namespace: namespace, - }, - } - Expect(k8sClient.Create(ctx, mcpGroup)).Should(Succeed()) - - // Create MCPOIDCConfig - oidcConfig = &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://accounts.google.com", - ClientID: "test-client", - }, - }, - } - Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed()) - - // Wait for ready - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return updated.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create VirtualMCPServer with OIDCConfigRef - vmcpServer = v1beta1test.NewVirtualMCPServer(vmcpName, namespace, - v1beta1test.WithVMCPGroupRef(groupName), - v1beta1test.WithVMCPConfig(vmcpconfig.Config{Group: groupName}), - v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{ - Type: "oidc", - OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{ - Name: configName, - Audience: "test-vmcp-audience", - Scopes: []string{"openid"}, - }, - }), - ) - Expect(k8sClient.Create(ctx, vmcpServer)).Should(Succeed()) - - // Wait for ReferencingWorkloads to contain the VirtualMCPServer - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - - AfterAll(func() { - _ = k8sClient.Delete(ctx, oidcConfig) - _ = k8sClient.Delete(ctx, mcpGroup) - Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) - }) - - It("should remove VirtualMCPServer from ReferencingWorkloads after deletion", func() { - // Delete the VirtualMCPServer - Expect(k8sClient.Delete(ctx, vmcpServer)).Should(Succeed()) - - // Eventually the referencing workloads list should not contain the vmcp entry - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return false - } - } - return true - }, timeout, interval).Should(BeTrue()) - }) }) Context("When deleting MCPOIDCConfig with active VirtualMCPServer references", Ordered, func() { @@ -413,23 +264,18 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration ) Expect(k8sClient.Create(ctx, vmcpServer)).Should(Succeed()) - // Wait for ReferencingWorkloads to be populated + // Wait for the VirtualMCPServer to be wired to the config (OIDCConfigHash + // populated) so the config is observably referenced before deletion. Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} + updated := &mcpv1beta1.VirtualMCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, + Name: vmcpName, Namespace: namespace, }, updated) if err != nil { return false } - expectedRef := mcpv1beta1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName} - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == expectedRef { - return true - } - } - return false + return updated.Status.OIDCConfigHash != "" }, timeout, interval).Should(BeTrue()) // Attempt to delete the MCPOIDCConfig (should be blocked by finalizer) @@ -455,8 +301,9 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("should not be deleted while referenced by VirtualMCPServer", func() { - // The object should still exist because the finalizer blocks deletion + It("should not be deleted while referenced and should set DeletionBlocked condition", func() { + // The object should still exist (finalizer blocks deletion) with a + // pending deletion timestamp and a DeletionBlocked=True condition. Eventually(func() bool { updated := &mcpv1beta1.MCPOIDCConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -466,7 +313,11 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration if err != nil { return false } - return !updated.DeletionTimestamp.IsZero() + if updated.DeletionTimestamp.IsZero() { + return false + } + cond := meta.FindStatusCondition(updated.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + return cond != nil && cond.Status == metav1.ConditionTrue }, timeout, interval).Should(BeTrue()) }) @@ -560,146 +411,4 @@ var _ = Describe("MCPOIDCConfig and VirtualMCPServer Cross-Resource Integration }) }) - Context("When both MCPServer and VirtualMCPServer reference same MCPOIDCConfig", Ordered, func() { - var ( - namespace string - configName string - serverName string - vmcpName string - groupName string - oidcConfig *mcpv1beta1.MCPOIDCConfig - mcpServer *mcpv1beta1.MCPServer - vmcpServer *mcpv1beta1.VirtualMCPServer - mcpGroup *mcpv1beta1.MCPGroup - ns *corev1.Namespace - ) - - BeforeAll(func() { - // Create a unique namespace for this test context - ns = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-vmcp-oidcref-both-", - }, - } - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - namespace = ns.Name - - configName = testOIDCConfigName - serverName = testServerName - vmcpName = testVMCPServerName - groupName = testVMCPGroupName - - // Create MCPGroup (required by VirtualMCPServer) - mcpGroup = &mcpv1beta1.MCPGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: groupName, - Namespace: namespace, - }, - } - Expect(k8sClient.Create(ctx, mcpGroup)).Should(Succeed()) - - // Create MCPOIDCConfig - oidcConfig = &mcpv1beta1.MCPOIDCConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPOIDCConfigSpec{ - Type: mcpv1beta1.MCPOIDCConfigTypeInline, - Inline: &mcpv1beta1.InlineOIDCSharedConfig{ - Issuer: "https://accounts.google.com", - ClientID: "test-client", - }, - }, - } - Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed()) - - // Wait for Valid condition and ConfigHash to be set - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - if updated.Status.ConfigHash == "" { - return false - } - for _, cond := range updated.Status.Conditions { - if cond.Type == mcpv1beta1.ConditionTypeOIDCConfigValid && cond.Status == metav1.ConditionTrue { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - - // Create MCPServer with OIDCConfigRef - mcpServer = &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: serverName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPServerSpec{ - Image: testServerImage, - OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{ - Name: configName, - Audience: "test-audience", - Scopes: []string{"openid"}, - }, - }, - } - Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - - // Create VirtualMCPServer with OIDCConfigRef - vmcpServer = v1beta1test.NewVirtualMCPServer(vmcpName, namespace, - v1beta1test.WithVMCPGroupRef(groupName), - v1beta1test.WithVMCPConfig(vmcpconfig.Config{Group: groupName}), - v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{ - Type: "oidc", - OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{ - Name: configName, - Audience: "test-vmcp-audience", - Scopes: []string{"openid"}, - }, - }), - ) - Expect(k8sClient.Create(ctx, vmcpServer)).Should(Succeed()) - }) - - AfterAll(func() { - _ = k8sClient.Delete(ctx, vmcpServer) - _ = k8sClient.Delete(ctx, mcpServer) - _ = k8sClient.Delete(ctx, oidcConfig) - _ = k8sClient.Delete(ctx, mcpGroup) - Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) - }) - - It("should track both workloads in ReferencingWorkloads", func() { - Eventually(func() bool { - updated := &mcpv1beta1.MCPOIDCConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - mcpServerRef := mcpv1beta1.WorkloadReference{Kind: "MCPServer", Name: serverName} - vmcpServerRef := mcpv1beta1.WorkloadReference{Kind: "VirtualMCPServer", Name: vmcpName} - hasMCPServer := false - hasVMCPServer := false - for _, ref := range updated.Status.ReferencingWorkloads { - if ref == mcpServerRef { - hasMCPServer = true - } - if ref == vmcpServerRef { - hasVMCPServer = true - } - } - return hasMCPServer && hasVMCPServer - }, timeout, interval).Should(BeTrue()) - }) - }) }) diff --git a/cmd/thv-operator/test-integration/mcp-telemetry-config/mcptelemetryconfig_controller_integration_test.go b/cmd/thv-operator/test-integration/mcp-telemetry-config/mcptelemetryconfig_controller_integration_test.go index 1700aee0d4..08f0c8f945 100644 --- a/cmd/thv-operator/test-integration/mcp-telemetry-config/mcptelemetryconfig_controller_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-telemetry-config/mcptelemetryconfig_controller_integration_test.go @@ -172,147 +172,6 @@ var _ = Describe("MCPTelemetryConfig Controller", func() { }, timeout, interval).Should(BeTrue()) }) - It("should track referencing MCPServers in status", func() { - // Create a telemetry config - telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ref-tracking", - Namespace: "default", - }, - } - telemetryConfig.Spec.OpenTelemetry = &mcpv1beta1.MCPTelemetryOTelConfig{ - Enabled: true, - Endpoint: testEndpoint, - Tracing: &mcpv1beta1.OpenTelemetryTracingConfig{Enabled: true}, - } - - Expect(k8sClient.Create(ctx, telemetryConfig)).To(Succeed()) - - // Wait for initial reconciliation (finalizer + hash) - Eventually(func() bool { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - return err == nil && fetched.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create an MCPServer that references this config - server := &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "server-ref-tracking", - Namespace: "default", - }, - Spec: mcpv1beta1.MCPServerSpec{ - Image: "example/mcp-server:latest", - TelemetryConfigRef: &mcpv1beta1.MCPTelemetryConfigReference{ - Name: "test-ref-tracking", - }, - }, - } - Expect(k8sClient.Create(ctx, server)).To(Succeed()) - - // The MCPServer watch should trigger a reconciliation of the MCPTelemetryConfig. - // Verify ReferencingWorkloads is updated to include our server. - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Name) - } - return names - }, timeout, interval).Should(ContainElement("server-ref-tracking")) - }) - - It("should prune referencing MCPServer from status after the MCPServer is deleted", func() { - // Create a telemetry config - telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ref-prune", - Namespace: "default", - }, - } - telemetryConfig.Spec.OpenTelemetry = &mcpv1beta1.MCPTelemetryOTelConfig{ - Enabled: true, - Endpoint: testEndpoint, - Tracing: &mcpv1beta1.OpenTelemetryTracingConfig{Enabled: true}, - } - - Expect(k8sClient.Create(ctx, telemetryConfig)).To(Succeed()) - - // Wait for initial reconciliation (finalizer + hash) - Eventually(func() bool { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - return err == nil && fetched.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create an MCPServer that references this config - server := &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "server-ref-prune", - Namespace: "default", - }, - Spec: mcpv1beta1.MCPServerSpec{ - Image: "example/mcp-server:latest", - TelemetryConfigRef: &mcpv1beta1.MCPTelemetryConfigReference{ - Name: "test-ref-prune", - }, - }, - } - Expect(k8sClient.Create(ctx, server)).To(Succeed()) - - // Wait until the server is tracked in ReferencingWorkloads. - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Name) - } - return names - }, timeout, interval).Should(ContainElement("server-ref-prune")) - - // Delete the MCPServer. The controller-runtime watch enqueues the - // referenced MCPTelemetryConfig (via the old-object map function), and - // the level-triggered reconcile must prune the now-stale reference. - Expect(k8sClient.Delete(ctx, server)).To(Succeed()) - - // Eventually the reference is pruned from status. - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Name) - } - return names - }, timeout, interval).ShouldNot(ContainElement("server-ref-prune")) - }) - It("should block deletion when MCPServers reference the config", func() { // Create a telemetry config telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ @@ -362,25 +221,20 @@ var _ = Describe("MCPTelemetryConfig Controller", func() { } Expect(k8sClient.Create(ctx, server)).To(Succeed()) - // Wait for ReferencingWorkloads to be populated - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} + // Ensure the referencing MCPServer is observable before attempting deletion. + Eventually(func() bool { + fetched := &mcpv1beta1.MCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, + Name: server.Name, + Namespace: server.Namespace, }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Name) - } - return names - }, timeout, interval).Should(ContainElement("server-deletion-blocker")) + return err == nil + }, timeout, interval).Should(BeTrue()) // Attempt to delete the config — the API call succeeds (sets DeletionTimestamp) - // but the finalizer blocks actual removal + // but the finalizer blocks actual removal because the config is still + // referenced. The controller recomputes referencing workloads on demand + // (via its field index) during the deletion reconcile. Expect(k8sClient.Delete(ctx, telemetryConfig)).To(Succeed()) // Verify the object still exists (finalizer prevents deletion) @@ -407,58 +261,6 @@ var _ = Describe("MCPTelemetryConfig Controller", func() { }, timeout, interval).Should(BeTrue(), "Config should be deleted after references are removed") }) - It("should track MCPRemoteProxy in ReferencingWorkloads", func() { - telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-proxy-ref-tracking", - Namespace: "default", - }, - } - telemetryConfig.Spec.OpenTelemetry = &mcpv1beta1.MCPTelemetryOTelConfig{ - Enabled: true, - Endpoint: testEndpoint, - Tracing: &mcpv1beta1.OpenTelemetryTracingConfig{Enabled: true}, - } - - Expect(k8sClient.Create(ctx, telemetryConfig)).To(Succeed()) - - // Wait for config to be ready - Eventually(func() bool { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - return err == nil && fetched.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create an MCPRemoteProxy that references this config - proxy := v1beta1test.NewMCPRemoteProxy("proxy-ref-tracking", "default", - v1beta1test.WithRemoteProxyURL("https://example.com/mcp"), - v1beta1test.WithRemoteProxyPort(0), - v1beta1test.WithRemoteProxyTelemetryConfigRef("test-proxy-ref-tracking"), - ) - Expect(k8sClient.Create(ctx, proxy)).To(Succeed()) - - // The MCPRemoteProxy watch should trigger reconciliation of MCPTelemetryConfig. - // Verify ReferencingWorkloads includes the proxy. - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, - }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Kind+"/"+ref.Name) - } - return names - }, timeout, interval).Should(ContainElement("MCPRemoteProxy/proxy-ref-tracking")) - }) - It("should block deletion when MCPRemoteProxy references the config", func() { telemetryConfig := &mcpv1beta1.MCPTelemetryConfig{ ObjectMeta: metav1.ObjectMeta{ @@ -500,24 +302,19 @@ var _ = Describe("MCPTelemetryConfig Controller", func() { ) Expect(k8sClient.Create(ctx, proxy)).To(Succeed()) - // Wait for ReferencingWorkloads to include the proxy - Eventually(func() []string { - fetched := &mcpv1beta1.MCPTelemetryConfig{} + // Ensure the referencing MCPRemoteProxy is observable before attempting deletion. + Eventually(func() bool { + fetched := &mcpv1beta1.MCPRemoteProxy{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: telemetryConfig.Name, - Namespace: telemetryConfig.Namespace, + Name: proxy.Name, + Namespace: proxy.Namespace, }, fetched) - if err != nil { - return nil - } - names := make([]string, 0, len(fetched.Status.ReferencingWorkloads)) - for _, ref := range fetched.Status.ReferencingWorkloads { - names = append(names, ref.Name) - } - return names - }, timeout, interval).Should(ContainElement("proxy-deletion-blocker")) + return err == nil + }, timeout, interval).Should(BeTrue()) - // Attempt to delete — finalizer blocks removal + // Attempt to delete — finalizer blocks removal because the config is still + // referenced. The controller recomputes referencing workloads on demand + // (via its field index) during the deletion reconcile. Expect(k8sClient.Delete(ctx, telemetryConfig)).To(Succeed()) // Verify object still exists diff --git a/cmd/thv-operator/test-integration/mcp-toolconfig/mcptoolconfig_controller_integration_test.go b/cmd/thv-operator/test-integration/mcp-toolconfig/mcptoolconfig_controller_integration_test.go index 10cb957e86..7bb06d5f19 100644 --- a/cmd/thv-operator/test-integration/mcp-toolconfig/mcptoolconfig_controller_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-toolconfig/mcptoolconfig_controller_integration_test.go @@ -234,115 +234,6 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { }) }) - Context("When MCPServers reference the MCPToolConfig", Ordered, func() { - var ( - namespace string - configName string - toolConfig *mcpv1beta1.MCPToolConfig - mcpServerName string - mcpServer *mcpv1beta1.MCPServer - ns *corev1.Namespace - ) - - BeforeAll(func() { - // Create a unique namespace for this test context - ns = &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-toolconfig-", - }, - } - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - namespace = ns.Name - - configName = testConfigName - mcpServerName = testServerName - - // Create MCPToolConfig - toolConfig = &mcpv1beta1.MCPToolConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPToolConfigSpec{ - ToolsFilter: []string{"tool1", "tool2"}, - }, - } - Expect(k8sClient.Create(ctx, toolConfig)).Should(Succeed()) - - // Wait for hash to be set before creating the MCPServer - Eventually(func() bool { - updated := &mcpv1beta1.MCPToolConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return updated.Status.ConfigHash != "" - }, timeout, interval).Should(BeTrue()) - - // Create MCPServer with ToolConfigRef - mcpServer = &mcpv1beta1.MCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: mcpServerName, - Namespace: namespace, - }, - Spec: mcpv1beta1.MCPServerSpec{ - Image: testImage, - ToolConfigRef: &mcpv1beta1.ToolConfigRef{ - Name: configName, - }, - }, - } - Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - }) - - AfterAll(func() { - // Ignore errors on cleanup since some tests may have already deleted these - _ = k8sClient.Delete(ctx, mcpServer) - Expect(k8sClient.Delete(ctx, toolConfig)).Should(Succeed()) - Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) - }) - - It("should track referencing workloads in status", func() { - Eventually(func() bool { - updated := &mcpv1beta1.MCPToolConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - for _, ref := range updated.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" && ref.Name == mcpServerName { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - - It("should remove server from status when MCPServer is deleted", func() { - // Delete the MCPServer - Expect(k8sClient.Delete(ctx, mcpServer)).Should(Succeed()) - - // Eventually the referencing workloads list should be empty - Eventually(func() bool { - updated := &mcpv1beta1.MCPToolConfig{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated) - if err != nil { - return false - } - return len(updated.Status.ReferencingWorkloads) == 0 - }, timeout, interval).Should(BeTrue()) - }) - }) - Context("When deleting MCPToolConfig with active references", Ordered, func() { var ( namespace string @@ -406,22 +297,18 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { } Expect(k8sClient.Create(ctx, mcpServer)).Should(Succeed()) - // Wait for ReferencingWorkloads to be populated + // Wait for the MCPServer to be wired to the config (ToolConfigHash + // populated) so the config is observably referenced before deletion. Eventually(func() bool { - updated := &mcpv1beta1.MCPToolConfig{} + updated := &mcpv1beta1.MCPServer{} err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, + Name: mcpServerName, Namespace: namespace, }, updated) if err != nil { return false } - for _, ref := range updated.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" && ref.Name == mcpServerName { - return true - } - } - return false + return updated.Status.ToolConfigHash != "" }, timeout, interval).Should(BeTrue()) // Attempt to delete the MCPToolConfig (should be blocked by finalizer) @@ -446,8 +333,9 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("should not be deleted while referenced", func() { - // The object should still exist because the finalizer blocks deletion + It("should not be deleted while referenced and should set DeletionBlocked condition", func() { + // The object should still exist (finalizer blocks deletion) with a + // pending deletion timestamp and a DeletionBlocked=True condition. Eventually(func() bool { updated := &mcpv1beta1.MCPToolConfig{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -457,7 +345,11 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { if err != nil { return false } - return !updated.DeletionTimestamp.IsZero() + if updated.DeletionTimestamp.IsZero() { + return false + } + cond := meta.FindStatusCondition(updated.Status.Conditions, mcpv1beta1.ConditionTypeDeletionBlocked) + return cond != nil && cond.Status == metav1.ConditionTrue }, timeout, interval).Should(BeTrue()) }) @@ -477,11 +369,11 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { }) }) - // These tests assert that an MCPToolConfig's ToolsFilter and ToolsOverride are - // not merely tracked via Status.ReferencingWorkloads, but actually propagate - // into the referencing MCPServer's rendered RunConfig ConfigMap. Both ride the - // same code path in createRunConfigFromMCPServer (WithToolsFilter / - // WithToolsOverride), so this single context covers filtering and renaming. + // These tests assert that an MCPToolConfig's ToolsFilter and ToolsOverride + // actually propagate into the referencing MCPServer's rendered RunConfig + // ConfigMap. Both ride the same code path in createRunConfigFromMCPServer + // (WithToolsFilter / WithToolsOverride), so this single context covers + // filtering and renaming. Context("When a referencing MCPServer renders its RunConfig", Ordered, func() { var ( namespace string @@ -656,25 +548,6 @@ var _ = Describe("MCPToolConfig Controller Integration Tests", func() { Expect(k8sClient.Delete(ctx, ns)).Should(Succeed()) }) - It("tracks every referencing MCPServer in status", func() { - Eventually(func() []string { - updated := &mcpv1beta1.MCPToolConfig{} - if err := k8sClient.Get(ctx, types.NamespacedName{ - Name: configName, - Namespace: namespace, - }, updated); err != nil { - return nil - } - var names []string - for _, ref := range updated.Status.ReferencingWorkloads { - if ref.Kind == "MCPServer" { - names = append(names, ref.Name) - } - } - return names - }, timeout, interval).Should(ConsistOf(serverNames)) - }) - It("propagates a config change to every referencing MCPServer", func() { // Capture the initial hash the servers converged on. initial := &mcpv1beta1.MCPToolConfig{} diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml index 518eb26970..eafdf71d39 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml @@ -27,9 +27,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -155,43 +152,6 @@ spec: for this MCPAuthzConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -205,9 +165,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -334,43 +291,6 @@ spec: for this MCPAuthzConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index d88668e8dd..c75ce0505f 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -28,9 +28,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -1457,43 +1454,6 @@ spec: It corresponds to the MCPExternalAuthConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -1507,9 +1467,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -2937,43 +2894,6 @@ spec: It corresponds to the MCPExternalAuthConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpoidcconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpoidcconfigs.yaml index 78be4fe997..b30459b312 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpoidcconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpoidcconfigs.yaml @@ -27,9 +27,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -281,43 +278,6 @@ spec: for this MCPOIDCConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -331,9 +291,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -586,43 +543,6 @@ spec: for this MCPOIDCConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptelemetryconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptelemetryconfigs.yaml index 2262801a88..a53f7b4926 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptelemetryconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptelemetryconfigs.yaml @@ -287,33 +287,6 @@ spec: for this MCPTelemetryConfig. format: int64 type: integer - referencingWorkloads: - description: ReferencingWorkloads lists workloads that reference this - MCPTelemetryConfig - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -588,33 +561,6 @@ spec: for this MCPTelemetryConfig. format: int64 type: integer - referencingWorkloads: - description: ReferencingWorkloads lists workloads that reference this - MCPTelemetryConfig - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptoolconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptoolconfigs.yaml index bdcfc3a3ca..681bd01ae1 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptoolconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptoolconfigs.yaml @@ -25,9 +25,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -189,39 +186,6 @@ spec: It corresponds to the MCPToolConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -232,9 +196,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -397,39 +358,6 @@ spec: It corresponds to the MCPToolConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpwebhookconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpwebhookconfigs.yaml index f5ea07646a..b6a802683e 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpwebhookconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpwebhookconfigs.yaml @@ -24,9 +24,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referencingWorkloads - name: References - type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -348,34 +345,6 @@ spec: to the current status format: int64 type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPWebhookConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml index 8a6d899874..ace9054aff 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml @@ -30,9 +30,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -158,43 +155,6 @@ spec: for this MCPAuthzConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -208,9 +168,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -337,43 +294,6 @@ spec: for this MCPAuthzConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 27ec4c33c4..008300c2e9 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -31,9 +31,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -1460,43 +1457,6 @@ spec: It corresponds to the MCPExternalAuthConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -1510,9 +1470,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -2940,43 +2897,6 @@ spec: It corresponds to the MCPExternalAuthConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpoidcconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpoidcconfigs.yaml index 2018b9bbe6..ed087f74c5 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpoidcconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpoidcconfigs.yaml @@ -30,9 +30,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -284,43 +281,6 @@ spec: for this MCPOIDCConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -334,9 +294,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -589,43 +546,6 @@ spec: for this MCPOIDCConfig. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig. - Each entry identifies the workload by kind and name. The map key is the - (kind, name) pair so two workloads of different kinds that share a name - (e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct - entries rather than colliding under merge-patch semantics. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - kind - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptelemetryconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptelemetryconfigs.yaml index 3377fe0408..d140312bcc 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptelemetryconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptelemetryconfigs.yaml @@ -290,33 +290,6 @@ spec: for this MCPTelemetryConfig. format: int64 type: integer - referencingWorkloads: - description: ReferencingWorkloads lists workloads that reference this - MCPTelemetryConfig - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -591,33 +564,6 @@ spec: for this MCPTelemetryConfig. format: int64 type: integer - referencingWorkloads: - description: ReferencingWorkloads lists workloads that reference this - MCPTelemetryConfig - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptoolconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptoolconfigs.yaml index d0b232ae59..de7120a4f2 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptoolconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptoolconfigs.yaml @@ -28,9 +28,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -192,39 +189,6 @@ spec: It corresponds to the MCPToolConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true @@ -235,9 +199,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referenceCount - name: References - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -400,39 +361,6 @@ spec: It corresponds to the MCPToolConfig's generation, which is updated on mutation by the API Server. format: int64 type: integer - referenceCount: - description: ReferenceCount is the number of workloads referencing - this config. - format: int32 - type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpwebhookconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpwebhookconfigs.yaml index 0640f21bc8..29c76d0901 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpwebhookconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpwebhookconfigs.yaml @@ -27,9 +27,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Valid')].status name: Valid type: string - - jsonPath: .status.referencingWorkloads - name: References - type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -351,34 +348,6 @@ spec: to the current status format: int64 type: integer - referencingWorkloads: - description: |- - ReferencingWorkloads is a list of workload resources that reference this MCPWebhookConfig. - Each entry identifies the workload by kind and name. - items: - description: |- - WorkloadReference identifies a workload that references a shared configuration resource. - Namespace is implicit — cross-namespace references are not supported. - properties: - kind: - description: Kind is the type of workload resource - enum: - - MCPServer - - VirtualMCPServer - - MCPRemoteProxy - type: string - name: - description: Name is the name of the workload resource - minLength: 1 - type: string - required: - - kind - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map type: object type: object served: true diff --git a/docs/arch/09-operator-architecture.md b/docs/arch/09-operator-architecture.md index 38ce8001fc..30535715ec 100644 --- a/docs/arch/09-operator-architecture.md +++ b/docs/arch/09-operator-architecture.md @@ -249,7 +249,7 @@ MCPOIDCConfig eliminates OIDC configuration duplication — define an identity p - `scopes` (optional) — Defaults to `["openid"]` - `resourceUrl` (optional) — Public URL for OAuth protected resource metadata (RFC 9728); defaults to internal service URL -**Status fields** include a `Ready` condition, `configHash` for change detection, and `referencingWorkloads` tracking which resources reference this config. Deletion is blocked while references exist (finalizer pattern). +**Status fields** include a `Ready` condition and `configHash` for change detection. Deletion is blocked while references exist (finalizer pattern): the controller recomputes the referencing workloads on demand at deletion time (a field-indexed lookup), rather than maintaining a stored list in status. **Referenced by**: MCPServer and MCPRemoteProxy (via `spec.oidcConfigRef`); VirtualMCPServer (via `spec.incomingAuth.oidcConfigRef`) @@ -270,7 +270,7 @@ MCPTelemetryConfig centralises telemetry infrastructure settings (collector endp - CEL validation prevents header name overlap between `headers` and `sensitiveHeaders` - Per-server `serviceName` override in the workload's `telemetryConfigRef` (since `service.name` must be unique per server) -**Status fields** include a `Ready` condition, `configHash` for change detection, and `referencingWorkloads` tracking. +**Status fields** include a `Ready` condition and `configHash` for change detection. Referencing workloads are recomputed on demand at deletion time (finalizer + field-indexed lookup), not stored in status. **Referenced by**: MCPServer, VirtualMCPServer, MCPRemoteProxy (via `telemetryConfigRef`) @@ -291,7 +291,7 @@ MCPWebhookConfig declares one or more `validating` and/or `mutating` webhooks (U - Optional `tlsConfig` with CA bundle, mTLS client cert, or `insecureSkipVerify` - Optional `hmacSecretRef` adds the `X-Toolhive-Signature` header to outgoing payloads -**Status fields** include a `Ready` condition, `configHash` for change detection, and `referencingWorkloads`. +**Status fields** include a `Ready` condition and `configHash` for change detection. Referencing workloads are recomputed on demand at deletion time (finalizer + field-indexed lookup), not stored in status. **Referenced by**: MCPServer (via `webhookConfigRef`) diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index a1cc135267..f382755f22 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -1766,8 +1766,6 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPAuthzConfig's state | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPAuthzConfig. | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| -| `referenceCount` _integer_ | ReferenceCount is the number of workloads referencing this config. | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig.
Each entry identifies the workload by kind and name. The map key is the
(kind, name) pair so two workloads of different kinds that share a name
(e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct
entries rather than colliding under merge-patch semantics. | | Optional: \{\}
| #### api.v1beta1.MCPExternalAuthConfig @@ -1856,8 +1854,6 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPExternalAuthConfig's state | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPExternalAuthConfig.
It corresponds to the MCPExternalAuthConfig's generation, which is updated on mutation by the API Server. | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| -| `referenceCount` _integer_ | ReferenceCount is the number of workloads referencing this config. | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPExternalAuthConfig.
Each entry identifies the workload by kind and name. The map key is the
(kind, name) pair so two workloads of different kinds that share a name
(e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct
entries rather than colliding under merge-patch semantics. | | Optional: \{\}
| #### api.v1beta1.MCPGroup @@ -2101,8 +2097,6 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPOIDCConfig's state | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPOIDCConfig. | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| -| `referenceCount` _integer_ | ReferenceCount is the number of workloads referencing this config. | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig.
Each entry identifies the workload by kind and name. The map key is the
(kind, name) pair so two workloads of different kinds that share a name
(e.g., an MCPServer "foo" and a VirtualMCPServer "foo") are distinct
entries rather than colliding under merge-patch semantics. | | Optional: \{\}
| #### api.v1beta1.MCPRegistry @@ -2682,7 +2676,6 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPTelemetryConfig's state | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPTelemetryConfig. | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads lists workloads that reference this MCPTelemetryConfig | | Optional: \{\}
| #### api.v1beta1.MCPTelemetryOTelConfig @@ -2795,8 +2788,6 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPToolConfig's state | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPToolConfig.
It corresponds to the MCPToolConfig's generation, which is updated on mutation by the API Server. | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| -| `referenceCount` _integer_ | ReferenceCount is the number of workloads referencing this config. | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig.
Each entry identifies the workload by kind and name. | | Optional: \{\}
| #### api.v1beta1.MCPWebhookConfigSpec @@ -2830,7 +2821,6 @@ MCPWebhookConfigStatus defines the observed state of MCPWebhookConfig | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the last observed generation corresponding to the current status | | Optional: \{\}
| | `configHash` _string_ | ConfigHash is a hash of the spec, used for detecting changes | | Optional: \{\}
| -| `referencingWorkloads` _[api.v1beta1.WorkloadReference](#apiv1beta1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPWebhookConfig.
Each entry identifies the workload by kind and name. | | Optional: \{\}
| #### api.v1beta1.ModelCacheConfig @@ -3973,26 +3963,5 @@ _Appears in:_ | `insecureSkipVerify` _boolean_ | InsecureSkipVerify disables server certificate verification.
WARNING: This should only be used for development/testing and not in production environments. | | Optional: \{\}
| -#### api.v1beta1.WorkloadReference - - - -WorkloadReference identifies a workload that references a shared configuration resource. -Namespace is implicit — cross-namespace references are not supported. - - - -_Appears in:_ -- [api.v1beta1.MCPAuthzConfigStatus](#apiv1beta1mcpauthzconfigstatus) -- [api.v1beta1.MCPExternalAuthConfigStatus](#apiv1beta1mcpexternalauthconfigstatus) -- [api.v1beta1.MCPOIDCConfigStatus](#apiv1beta1mcpoidcconfigstatus) -- [api.v1beta1.MCPTelemetryConfigStatus](#apiv1beta1mcptelemetryconfigstatus) -- [api.v1beta1.MCPToolConfigStatus](#apiv1beta1mcptoolconfigstatus) -- [api.v1beta1.MCPWebhookConfigStatus](#apiv1beta1mcpwebhookconfigstatus) - -| Field | Description | Default | Validation | -| --- | --- | --- | --- | -| `kind` _string_ | Kind is the type of workload resource | | Enum: [MCPServer VirtualMCPServer MCPRemoteProxy]
Required: \{\}
| -| `name` _string_ | Name is the name of the workload resource | | MinLength: 1
Required: \{\}
|