From 48d2dca486dfb5d3d575ad4e8dd3804f608c4a63 Mon Sep 17 00:00:00 2001 From: Sanskarzz Date: Tue, 26 May 2026 02:53:22 +0530 Subject: [PATCH 1/3] Fixed VirtualMCPCompositeToolDefinition printer columns output Signed-off-by: Sanskarzz --- cmd/thv-operator/api/v1alpha1/types.go | 4 +- ...virtualmcpcompositetooldefinition_types.go | 12 +- cmd/thv-operator/app/app.go | 10 +- ...almcpcompositetooldefinition_controller.go | 138 ++++++++++++++++++ ...compositetooldefinition_controller_test.go | 109 ++++++++++++++ ...ev_virtualmcpcompositetooldefinitions.yaml | 32 +++- ...ev_virtualmcpcompositetooldefinitions.yaml | 32 +++- .../operator/templates/clusterrole/role.yaml | 1 + docs/operator/crd-api.md | 2 + 9 files changed, 323 insertions(+), 17 deletions(-) create mode 100644 cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go create mode 100644 cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go diff --git a/cmd/thv-operator/api/v1alpha1/types.go b/cmd/thv-operator/api/v1alpha1/types.go index ef84fd4cb8..22ae9dbdce 100644 --- a/cmd/thv-operator/api/v1alpha1/types.go +++ b/cmd/thv-operator/api/v1alpha1/types.go @@ -339,9 +339,9 @@ type MCPToolConfigList struct { //+kubebuilder:subresource:status //+kubebuilder:resource:shortName=vmcpctd;compositetool,categories=toolhive //+kubebuilder:printcolumn:name="Workflow",type="string",JSONPath=".spec.name",description="Workflow name" -//+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".spec.steps[*]",description="Number of steps" +//+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".status.stepCount",description="Number of steps" //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.validationStatus",description="Validation status" -//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.referencingVirtualServers[*]",description="Refs" +//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Number of referencing VirtualMCPServers" //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" diff --git a/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go b/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go index e2a4ec0da6..667e9f280a 100644 --- a/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go +++ b/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go @@ -18,6 +18,14 @@ type VirtualMCPCompositeToolDefinitionSpec struct { // VirtualMCPCompositeToolDefinitionStatus defines the observed state of VirtualMCPCompositeToolDefinition type VirtualMCPCompositeToolDefinitionStatus struct { + // StepCount is the number of steps in the composite tool workflow. + // +optional + StepCount int32 `json:"stepCount,omitempty"` + + // RefCount is the number of VirtualMCPServers that reference this workflow. + // +optional + RefCount int32 `json:"refCount,omitempty"` + // ValidationStatus indicates the validation state of the workflow // - Valid: Workflow structure is valid // - Invalid: Workflow has validation errors @@ -102,9 +110,9 @@ const ( //+kubebuilder:subresource:status //+kubebuilder:resource:shortName=vmcpctd;compositetool,categories=toolhive //+kubebuilder:printcolumn:name="Workflow",type="string",JSONPath=".spec.name",description="Workflow name" -//+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".spec.steps[*]",description="Number of steps" +//+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".status.stepCount",description="Number of steps" //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.validationStatus",description="Validation status" -//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.referencingVirtualServers[*]",description="Refs" +//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Number of referencing VirtualMCPServers" //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" diff --git a/cmd/thv-operator/app/app.go b/cmd/thv-operator/app/app.go index 4bfb1b52fc..83eeef856c 100644 --- a/cmd/thv-operator/app/app.go +++ b/cmd/thv-operator/app/app.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -316,7 +317,7 @@ func setupRegistryController(mgr ctrl.Manager, imagePullSecretsDefaults imagepul } // setupAggregationControllers sets up Virtual MCP-related controllers and webhooks -// (MCPGroup, VirtualMCPServer, and their webhooks). Must run after +// (MCPGroup, VirtualMCPServer, VirtualMCPCompositeToolDefinition, and their webhooks). Must run after // setupServerControllers, which creates the MCPServer.Spec.GroupRef field index // these controllers depend on. // imagePullSecretsDefaults are merged with vmcp.Spec.ImagePullSecrets when the @@ -340,6 +341,13 @@ func setupAggregationControllers(mgr ctrl.Manager, imagePullSecretsDefaults imag return fmt.Errorf("unable to create controller VirtualMCPServer: %w", err) } + // Set up VirtualMCPCompositeToolDefinition controller + if err := (&controllers.VirtualMCPCompositeToolDefinitionReconciler{ + Client: mgr.GetClient(), + }).SetupWithManager(mgr); err != nil { + return fmt.Errorf("unable to create controller VirtualMCPCompositeToolDefinition: %w", err) + } + return nil } diff --git a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go new file mode 100644 index 0000000000..7b46b34c03 --- /dev/null +++ b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go @@ -0,0 +1,138 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "context" + "sort" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "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" +) + +// VirtualMCPCompositeToolDefinitionReconciler maintains display-oriented status +// for a VirtualMCPCompositeToolDefinition. +type VirtualMCPCompositeToolDefinitionReconciler struct { + client.Client +} + +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpcompositetooldefinitions,verbs=get;list;watch +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpcompositetooldefinitions/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpservers,verbs=get;list;watch + +// Reconcile updates the scalar counts used by kubectl printer columns and +// maintains the existing referencingVirtualServers status list. +func (r *VirtualMCPCompositeToolDefinitionReconciler) Reconcile( + ctx context.Context, + req ctrl.Request, +) (ctrl.Result, error) { + compositeToolDefinition := &mcpv1beta1.VirtualMCPCompositeToolDefinition{} + if err := r.Get(ctx, req.NamespacedName, compositeToolDefinition); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, err + } + + referencingVirtualServers, err := r.findReferencingVirtualServers(ctx, compositeToolDefinition) + if err != nil { + return ctrl.Result{}, err + } + + stepCount := compositeToolDefinitionItemCount(len(compositeToolDefinition.Spec.Steps)) + refCount := compositeToolDefinitionItemCount(len(referencingVirtualServers)) + + if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, compositeToolDefinition, + func(definition *mcpv1beta1.VirtualMCPCompositeToolDefinition) { + definition.Status.StepCount = stepCount + definition.Status.RefCount = refCount + definition.Status.ReferencingVirtualServers = referencingVirtualServers + }); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil +} + +func (r *VirtualMCPCompositeToolDefinitionReconciler) findReferencingVirtualServers( + ctx context.Context, + compositeToolDefinition *mcpv1beta1.VirtualMCPCompositeToolDefinition, +) ([]string, error) { + virtualMCPServers := &mcpv1beta1.VirtualMCPServerList{} + if err := r.List(ctx, virtualMCPServers, client.InNamespace(compositeToolDefinition.Namespace)); err != nil { + return nil, err + } + + referencingVirtualServers := make([]string, 0) + for _, virtualMCPServer := range virtualMCPServers.Items { + for _, ref := range virtualMCPServer.Spec.Config.CompositeToolRefs { + if ref.Name == compositeToolDefinition.Name { + referencingVirtualServers = append(referencingVirtualServers, virtualMCPServer.Name) + break + } + } + } + sort.Strings(referencingVirtualServers) + return referencingVirtualServers, nil +} + +func compositeToolDefinitionItemCount(length int) int32 { + return int32(length) //nolint:gosec // Kubernetes object size limits keep CRD list lengths within int32. +} + +// SetupWithManager configures reconciliation of definitions and the virtual +// servers whose references determine the Refs column. +func (r *VirtualMCPCompositeToolDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error { + virtualMCPServerHandler := handler.EnqueueRequestsFromMapFunc( + func(ctx context.Context, obj client.Object) []reconcile.Request { + virtualMCPServer, ok := obj.(*mcpv1beta1.VirtualMCPServer) + if !ok { + return nil + } + + requests := make([]reconcile.Request, 0, len(virtualMCPServer.Spec.Config.CompositeToolRefs)) + seen := make(map[types.NamespacedName]struct{}) + for _, ref := range virtualMCPServer.Spec.Config.CompositeToolRefs { + name := types.NamespacedName{Name: ref.Name, Namespace: virtualMCPServer.Namespace} + if _, exists := seen[name]; exists { + continue + } + seen[name] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: name}) + } + + definitions := &mcpv1beta1.VirtualMCPCompositeToolDefinitionList{} + if err := r.List(ctx, definitions, client.InNamespace(virtualMCPServer.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list VirtualMCPCompositeToolDefinitions for VirtualMCPServer watch") + return requests + } + for _, definition := range definitions.Items { + name := types.NamespacedName{Name: definition.Name, Namespace: definition.Namespace} + if _, exists := seen[name]; exists { + continue + } + for _, referencingVirtualServer := range definition.Status.ReferencingVirtualServers { + if referencingVirtualServer == virtualMCPServer.Name { + requests = append(requests, reconcile.Request{NamespacedName: name}) + break + } + } + } + return requests + }, + ) + + return ctrl.NewControllerManagedBy(mgr). + For(&mcpv1beta1.VirtualMCPCompositeToolDefinition{}). + Watches(&mcpv1beta1.VirtualMCPServer{}, virtualMCPServerHandler). + Complete(r) +} diff --git a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go new file mode 100644 index 0000000000..2f952ee4ca --- /dev/null +++ b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config" +) + +func TestVirtualMCPCompositeToolDefinitionReconciler_Reconcile(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + definition *mcpv1beta1.VirtualMCPCompositeToolDefinition + virtualMCPServers []mcpv1beta1.VirtualMCPServer + expectedStepCount int32 + expectedRefCount int32 + expectedReferencingVMCPNames []string + }{ + { + name: "populates step and reference counts", + definition: &mcpv1beta1.VirtualMCPCompositeToolDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "workflow", Namespace: "default"}, + Spec: mcpv1beta1.VirtualMCPCompositeToolDefinitionSpec{ + CompositeToolConfig: vmcpconfig.CompositeToolConfig{ + Steps: []vmcpconfig.WorkflowStepConfig{{ID: "first"}, {ID: "second"}}, + }, + }, + }, + virtualMCPServers: []mcpv1beta1.VirtualMCPServer{ + virtualMCPServerWithCompositeToolRef("server-b", "workflow"), + virtualMCPServerWithCompositeToolRef("server-a", "workflow"), + virtualMCPServerWithCompositeToolRef("unrelated", "other-workflow"), + }, + expectedStepCount: 2, + expectedRefCount: 2, + expectedReferencingVMCPNames: []string{"server-a", "server-b"}, + }, + { + name: "clears references no longer present", + definition: &mcpv1beta1.VirtualMCPCompositeToolDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "workflow", Namespace: "default"}, + Status: mcpv1beta1.VirtualMCPCompositeToolDefinitionStatus{ + RefCount: 1, + ReferencingVirtualServers: []string{"removed-server"}, + }, + }, + expectedStepCount: 0, + expectedRefCount: 0, + expectedReferencingVMCPNames: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + + objects := make([]runtime.Object, 0, len(tt.virtualMCPServers)+1) + objects = append(objects, tt.definition) + for i := range tt.virtualMCPServers { + objects = append(objects, &tt.virtualMCPServers[i]) + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objects...). + WithStatusSubresource(&mcpv1beta1.VirtualMCPCompositeToolDefinition{}). + Build() + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{Client: fakeClient} + key := types.NamespacedName{Name: tt.definition.Name, Namespace: tt.definition.Namespace} + + _, err := reconciler.Reconcile(t.Context(), reconcile.Request{ + NamespacedName: key, + }) + require.NoError(t, err) + + updated := &mcpv1beta1.VirtualMCPCompositeToolDefinition{} + require.NoError(t, fakeClient.Get(t.Context(), key, updated)) + assert.Equal(t, tt.expectedStepCount, updated.Status.StepCount) + assert.Equal(t, tt.expectedRefCount, updated.Status.RefCount) + assert.Equal(t, tt.expectedReferencingVMCPNames, updated.Status.ReferencingVirtualServers) + }) + } +} + +func virtualMCPServerWithCompositeToolRef(name, definitionName string) mcpv1beta1.VirtualMCPServer { + return mcpv1beta1.VirtualMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: mcpv1beta1.VirtualMCPServerSpec{ + Config: vmcpconfig.Config{ + CompositeToolRefs: []vmcpconfig.CompositeToolRef{{Name: definitionName}}, + }, + }, + } +} diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml index a9c17deff5..12cd603380 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml @@ -25,15 +25,15 @@ spec: name: Workflow type: string - description: Number of steps - jsonPath: .spec.steps[*] + jsonPath: .status.stepCount name: Steps type: integer - description: Validation status jsonPath: .status.validationStatus name: Status type: string - - description: Refs - jsonPath: .status.referencingVirtualServers[*] + - description: Number of referencing VirtualMCPServers + jsonPath: .status.refCount name: Refs type: integer - description: Age @@ -395,6 +395,11 @@ spec: It corresponds to the resource's generation, which is updated on mutation by the API Server format: int64 type: integer + refCount: + description: RefCount is the number of VirtualMCPServers that reference + this workflow. + format: int32 + type: integer referencingVirtualServers: description: |- ReferencingVirtualServers lists VirtualMCPServer resources that reference this workflow @@ -403,6 +408,11 @@ spec: type: string type: array x-kubernetes-list-type: set + stepCount: + description: StepCount is the number of steps in the composite tool + workflow. + format: int32 + type: integer validationErrors: description: ValidationErrors contains validation error messages if ValidationStatus is Invalid @@ -432,15 +442,15 @@ spec: name: Workflow type: string - description: Number of steps - jsonPath: .spec.steps[*] + jsonPath: .status.stepCount name: Steps type: integer - description: Validation status jsonPath: .status.validationStatus name: Status type: string - - description: Refs - jsonPath: .status.referencingVirtualServers[*] + - description: Number of referencing VirtualMCPServers + jsonPath: .status.refCount name: Refs type: integer - description: Age @@ -802,6 +812,11 @@ spec: It corresponds to the resource's generation, which is updated on mutation by the API Server format: int64 type: integer + refCount: + description: RefCount is the number of VirtualMCPServers that reference + this workflow. + format: int32 + type: integer referencingVirtualServers: description: |- ReferencingVirtualServers lists VirtualMCPServer resources that reference this workflow @@ -810,6 +825,11 @@ spec: type: string type: array x-kubernetes-list-type: set + stepCount: + description: StepCount is the number of steps in the composite tool + workflow. + format: int32 + type: integer validationErrors: description: ValidationErrors contains validation error messages if ValidationStatus is Invalid diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml index 39fcdea5fc..4f2f88f084 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml @@ -28,15 +28,15 @@ spec: name: Workflow type: string - description: Number of steps - jsonPath: .spec.steps[*] + jsonPath: .status.stepCount name: Steps type: integer - description: Validation status jsonPath: .status.validationStatus name: Status type: string - - description: Refs - jsonPath: .status.referencingVirtualServers[*] + - description: Number of referencing VirtualMCPServers + jsonPath: .status.refCount name: Refs type: integer - description: Age @@ -398,6 +398,11 @@ spec: It corresponds to the resource's generation, which is updated on mutation by the API Server format: int64 type: integer + refCount: + description: RefCount is the number of VirtualMCPServers that reference + this workflow. + format: int32 + type: integer referencingVirtualServers: description: |- ReferencingVirtualServers lists VirtualMCPServer resources that reference this workflow @@ -406,6 +411,11 @@ spec: type: string type: array x-kubernetes-list-type: set + stepCount: + description: StepCount is the number of steps in the composite tool + workflow. + format: int32 + type: integer validationErrors: description: ValidationErrors contains validation error messages if ValidationStatus is Invalid @@ -435,15 +445,15 @@ spec: name: Workflow type: string - description: Number of steps - jsonPath: .spec.steps[*] + jsonPath: .status.stepCount name: Steps type: integer - description: Validation status jsonPath: .status.validationStatus name: Status type: string - - description: Refs - jsonPath: .status.referencingVirtualServers[*] + - description: Number of referencing VirtualMCPServers + jsonPath: .status.refCount name: Refs type: integer - description: Age @@ -805,6 +815,11 @@ spec: It corresponds to the resource's generation, which is updated on mutation by the API Server format: int64 type: integer + refCount: + description: RefCount is the number of VirtualMCPServers that reference + this workflow. + format: int32 + type: integer referencingVirtualServers: description: |- ReferencingVirtualServers lists VirtualMCPServer resources that reference this workflow @@ -813,6 +828,11 @@ spec: type: string type: array x-kubernetes-list-type: set + stepCount: + description: StepCount is the number of steps in the composite tool + workflow. + format: int32 + type: integer validationErrors: description: ValidationErrors contains validation error messages if ValidationStatus is Invalid diff --git a/deploy/charts/operator/templates/clusterrole/role.yaml b/deploy/charts/operator/templates/clusterrole/role.yaml index a7a8b3f6e4..488ccef1f3 100644 --- a/deploy/charts/operator/templates/clusterrole/role.yaml +++ b/deploy/charts/operator/templates/clusterrole/role.yaml @@ -144,6 +144,7 @@ rules: - mcptelemetryconfigs/status - mcptoolconfigs/status - mcpwebhookconfigs/status + - virtualmcpcompositetooldefinitions/status - virtualmcpservers/status verbs: - get diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index b8f04ff351..34423b9b37 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -3545,6 +3545,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | +| `stepCount` _integer_ | StepCount is the number of steps in the composite tool workflow. | | Optional: \{\}
| +| `refCount` _integer_ | RefCount is the number of VirtualMCPServers that reference this workflow. | | Optional: \{\}
| | `validationStatus` _[api.v1beta1.ValidationStatus](#apiv1beta1validationstatus)_ | ValidationStatus indicates the validation state of the workflow
- Valid: Workflow structure is valid
- Invalid: Workflow has validation errors | | Enum: [Valid Invalid Unknown]
Optional: \{\}
| | `validationErrors` _string array_ | ValidationErrors contains validation error messages if ValidationStatus is Invalid | | Optional: \{\}
| | `referencingVirtualServers` _string array_ | ReferencingVirtualServers lists VirtualMCPServer resources that reference this workflow
This helps track which servers need to be reconciled when this workflow changes | | Optional: \{\}
| From 145a068ebdbc859b0ba5d11009dba7746224362f Mon Sep 17 00:00:00 2001 From: Sanskarzz Date: Wed, 27 May 2026 00:51:48 +0530 Subject: [PATCH 2/3] Improved coverage and fixed CI lint error Signed-off-by: Sanskarzz --- cmd/thv-operator/api/v1alpha1/types.go | 2 +- ...virtualmcpcompositetooldefinition_types.go | 2 +- cmd/thv-operator/app/app.go | 1 - ...almcpcompositetooldefinition_controller.go | 79 +++++---- ...compositetooldefinition_controller_test.go | 166 ++++++++++++++++++ ...ev_virtualmcpcompositetooldefinitions.yaml | 4 +- ...ev_virtualmcpcompositetooldefinitions.yaml | 4 +- 7 files changed, 214 insertions(+), 44 deletions(-) diff --git a/cmd/thv-operator/api/v1alpha1/types.go b/cmd/thv-operator/api/v1alpha1/types.go index 22ae9dbdce..8b2c9e50ae 100644 --- a/cmd/thv-operator/api/v1alpha1/types.go +++ b/cmd/thv-operator/api/v1alpha1/types.go @@ -341,7 +341,7 @@ type MCPToolConfigList struct { //+kubebuilder:printcolumn:name="Workflow",type="string",JSONPath=".spec.name",description="Workflow name" //+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".status.stepCount",description="Number of steps" //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.validationStatus",description="Validation status" -//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Number of referencing VirtualMCPServers" +//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Referencing servers" //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" diff --git a/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go b/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go index 667e9f280a..d71aa89fbe 100644 --- a/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go +++ b/cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go @@ -112,7 +112,7 @@ const ( //+kubebuilder:printcolumn:name="Workflow",type="string",JSONPath=".spec.name",description="Workflow name" //+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".status.stepCount",description="Number of steps" //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.validationStatus",description="Validation status" -//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Number of referencing VirtualMCPServers" +//+kubebuilder:printcolumn:name="Refs",type="integer",JSONPath=".status.refCount",description="Referencing servers" //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" diff --git a/cmd/thv-operator/app/app.go b/cmd/thv-operator/app/app.go index 83eeef856c..0319208837 100644 --- a/cmd/thv-operator/app/app.go +++ b/cmd/thv-operator/app/app.go @@ -18,7 +18,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" diff --git a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go index 7b46b34c03..3c0f4579de 100644 --- a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller.go @@ -89,47 +89,52 @@ func compositeToolDefinitionItemCount(length int) int32 { return int32(length) //nolint:gosec // Kubernetes object size limits keep CRD list lengths within int32. } -// SetupWithManager configures reconciliation of definitions and the virtual -// servers whose references determine the Refs column. -func (r *VirtualMCPCompositeToolDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error { - virtualMCPServerHandler := handler.EnqueueRequestsFromMapFunc( - func(ctx context.Context, obj client.Object) []reconcile.Request { - virtualMCPServer, ok := obj.(*mcpv1beta1.VirtualMCPServer) - if !ok { - return nil - } +// mapVirtualMCPServerToCompositeToolDefinitions enqueues both definitions a server +// currently references and definitions that still contain a stale reference to it. +func (r *VirtualMCPCompositeToolDefinitionReconciler) mapVirtualMCPServerToCompositeToolDefinitions( + ctx context.Context, + obj client.Object, +) []reconcile.Request { + virtualMCPServer, ok := obj.(*mcpv1beta1.VirtualMCPServer) + if !ok { + return nil + } - requests := make([]reconcile.Request, 0, len(virtualMCPServer.Spec.Config.CompositeToolRefs)) - seen := make(map[types.NamespacedName]struct{}) - for _, ref := range virtualMCPServer.Spec.Config.CompositeToolRefs { - name := types.NamespacedName{Name: ref.Name, Namespace: virtualMCPServer.Namespace} - if _, exists := seen[name]; exists { - continue - } - seen[name] = struct{}{} + requests := make([]reconcile.Request, 0, len(virtualMCPServer.Spec.Config.CompositeToolRefs)) + seen := make(map[types.NamespacedName]struct{}) + for _, ref := range virtualMCPServer.Spec.Config.CompositeToolRefs { + name := types.NamespacedName{Name: ref.Name, Namespace: virtualMCPServer.Namespace} + if _, exists := seen[name]; exists { + continue + } + seen[name] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: name}) + } + + definitions := &mcpv1beta1.VirtualMCPCompositeToolDefinitionList{} + if err := r.List(ctx, definitions, client.InNamespace(virtualMCPServer.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list VirtualMCPCompositeToolDefinitions for VirtualMCPServer watch") + return requests + } + for _, definition := range definitions.Items { + name := types.NamespacedName{Name: definition.Name, Namespace: definition.Namespace} + if _, exists := seen[name]; exists { + continue + } + for _, referencingVirtualServer := range definition.Status.ReferencingVirtualServers { + if referencingVirtualServer == virtualMCPServer.Name { requests = append(requests, reconcile.Request{NamespacedName: name}) + break } + } + } + return requests +} - definitions := &mcpv1beta1.VirtualMCPCompositeToolDefinitionList{} - if err := r.List(ctx, definitions, client.InNamespace(virtualMCPServer.Namespace)); err != nil { - log.FromContext(ctx).Error(err, "Failed to list VirtualMCPCompositeToolDefinitions for VirtualMCPServer watch") - return requests - } - for _, definition := range definitions.Items { - name := types.NamespacedName{Name: definition.Name, Namespace: definition.Namespace} - if _, exists := seen[name]; exists { - continue - } - for _, referencingVirtualServer := range definition.Status.ReferencingVirtualServers { - if referencingVirtualServer == virtualMCPServer.Name { - requests = append(requests, reconcile.Request{NamespacedName: name}) - break - } - } - } - return requests - }, - ) +// SetupWithManager configures reconciliation of definitions and the virtual +// servers whose references determine the Refs column. +func (r *VirtualMCPCompositeToolDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error { + virtualMCPServerHandler := handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToCompositeToolDefinitions) return ctrl.NewControllerManagedBy(mgr). For(&mcpv1beta1.VirtualMCPCompositeToolDefinition{}). diff --git a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go index 2f952ee4ca..7ddf18c3bf 100644 --- a/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go +++ b/cmd/thv-operator/controllers/virtualmcpcompositetooldefinition_controller_test.go @@ -4,14 +4,19 @@ package controllers import ( + "context" + "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "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" @@ -97,6 +102,167 @@ func TestVirtualMCPCompositeToolDefinitionReconciler_Reconcile(t *testing.T) { } } +func TestVirtualMCPCompositeToolDefinitionReconciler_ReconcileNotFound(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{ + Client: fake.NewClientBuilder().WithScheme(scheme).Build(), + } + + result, err := reconciler.Reconcile(t.Context(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "missing", Namespace: "default"}, + }) + + require.NoError(t, err) + assert.Equal(t, reconcile.Result{}, result) +} + +func TestVirtualMCPCompositeToolDefinitionReconciler_ReconcileErrors(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + + definition := &mcpv1beta1.VirtualMCPCompositeToolDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "workflow", Namespace: "default"}, + } + getErr := errors.New("simulated get failure") + listErr := errors.New("simulated list failure") + + t.Run("returns get failure", func(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func( + _ context.Context, + _ client.WithWatch, + _ client.ObjectKey, + _ client.Object, + _ ...client.GetOption, + ) error { + return getErr + }, + }). + Build() + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{Client: fakeClient} + + _, err := reconciler.Reconcile(t.Context(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "workflow", Namespace: "default"}, + }) + require.ErrorIs(t, err, getErr) + }) + + t.Run("returns reference list failure", func(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(definition.DeepCopy()). + WithInterceptorFuncs(interceptor.Funcs{ + List: func( + _ context.Context, + _ client.WithWatch, + list client.ObjectList, + _ ...client.ListOption, + ) error { + if _, ok := list.(*mcpv1beta1.VirtualMCPServerList); ok { + return listErr + } + return nil + }, + }). + Build() + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{Client: fakeClient} + + _, err := reconciler.Reconcile(t.Context(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: definition.Name, Namespace: definition.Namespace}, + }) + require.ErrorIs(t, err, listErr) + }) +} + +func TestVirtualMCPCompositeToolDefinitionReconciler_MapVirtualMCPServer(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + server := virtualMCPServerWithCompositeToolRef("server", "current") + server.Spec.Config.CompositeToolRefs = append(server.Spec.Config.CompositeToolRefs, + vmcpconfig.CompositeToolRef{Name: "current"}) + current := &mcpv1beta1.VirtualMCPCompositeToolDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "current", Namespace: "default"}, + Status: mcpv1beta1.VirtualMCPCompositeToolDefinitionStatus{ + ReferencingVirtualServers: []string{"server"}, + }, + } + stale := &mcpv1beta1.VirtualMCPCompositeToolDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "stale", Namespace: "default"}, + Status: mcpv1beta1.VirtualMCPCompositeToolDefinitionStatus{ + ReferencingVirtualServers: []string{"server"}, + }, + } + + t.Run("returns current and stale definitions without duplicates", func(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(current.DeepCopy(), stale.DeepCopy()). + Build() + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{Client: fakeClient} + + assert.ElementsMatch(t, []reconcile.Request{ + {NamespacedName: types.NamespacedName{Name: "current", Namespace: "default"}}, + {NamespacedName: types.NamespacedName{Name: "stale", Namespace: "default"}}, + }, reconciler.mapVirtualMCPServerToCompositeToolDefinitions(t.Context(), server.DeepCopy())) + }) + + t.Run("returns nil for unrelated object type", func(t *testing.T) { + t.Parallel() + + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{ + Client: fake.NewClientBuilder().WithScheme(scheme).Build(), + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod", Namespace: "default"}} + + assert.Nil(t, reconciler.mapVirtualMCPServerToCompositeToolDefinitions(t.Context(), pod)) + }) + + t.Run("returns current definition if stale lookup fails", func(t *testing.T) { + t.Parallel() + + listErr := errors.New("simulated definition list failure") + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + List: func( + _ context.Context, + _ client.WithWatch, + list client.ObjectList, + _ ...client.ListOption, + ) error { + if _, ok := list.(*mcpv1beta1.VirtualMCPCompositeToolDefinitionList); ok { + return listErr + } + return nil + }, + }). + Build() + reconciler := &VirtualMCPCompositeToolDefinitionReconciler{Client: fakeClient} + + assert.Equal(t, []reconcile.Request{{ + NamespacedName: types.NamespacedName{Name: "current", Namespace: "default"}, + }}, reconciler.mapVirtualMCPServerToCompositeToolDefinitions(t.Context(), server.DeepCopy())) + }) +} + func virtualMCPServerWithCompositeToolRef(name, definitionName string) mcpv1beta1.VirtualMCPServer { return mcpv1beta1.VirtualMCPServer{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml index 12cd603380..3db50fa1e0 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml @@ -32,7 +32,7 @@ spec: jsonPath: .status.validationStatus name: Status type: string - - description: Number of referencing VirtualMCPServers + - description: Referencing servers jsonPath: .status.refCount name: Refs type: integer @@ -449,7 +449,7 @@ spec: jsonPath: .status.validationStatus name: Status type: string - - description: Number of referencing VirtualMCPServers + - description: Referencing servers jsonPath: .status.refCount name: Refs type: integer diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml index 4f2f88f084..0fc1bc32f2 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml @@ -35,7 +35,7 @@ spec: jsonPath: .status.validationStatus name: Status type: string - - description: Number of referencing VirtualMCPServers + - description: Referencing servers jsonPath: .status.refCount name: Refs type: integer @@ -452,7 +452,7 @@ spec: jsonPath: .status.validationStatus name: Status type: string - - description: Number of referencing VirtualMCPServers + - description: Referencing servers jsonPath: .status.refCount name: Refs type: integer From e7f4ca95437d63f09f0a6934c8e9c9e4d4a4ca17 Mon Sep 17 00:00:00 2001 From: Sanskarzz Date: Wed, 27 May 2026 01:35:32 +0530 Subject: [PATCH 3/3] Fixed Operator E2E CI failures Signed-off-by: Sanskarzz --- .../operator/multi-tenancy/setup/assert-rbac-clusterrole.yaml | 1 + .../operator/single-tenancy/setup/assert-rbac-clusterrole.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/test/e2e/chainsaw/operator/multi-tenancy/setup/assert-rbac-clusterrole.yaml b/test/e2e/chainsaw/operator/multi-tenancy/setup/assert-rbac-clusterrole.yaml index a7a8b3f6e4..488ccef1f3 100644 --- a/test/e2e/chainsaw/operator/multi-tenancy/setup/assert-rbac-clusterrole.yaml +++ b/test/e2e/chainsaw/operator/multi-tenancy/setup/assert-rbac-clusterrole.yaml @@ -144,6 +144,7 @@ rules: - mcptelemetryconfigs/status - mcptoolconfigs/status - mcpwebhookconfigs/status + - virtualmcpcompositetooldefinitions/status - virtualmcpservers/status verbs: - get diff --git a/test/e2e/chainsaw/operator/single-tenancy/setup/assert-rbac-clusterrole.yaml b/test/e2e/chainsaw/operator/single-tenancy/setup/assert-rbac-clusterrole.yaml index a7a8b3f6e4..488ccef1f3 100644 --- a/test/e2e/chainsaw/operator/single-tenancy/setup/assert-rbac-clusterrole.yaml +++ b/test/e2e/chainsaw/operator/single-tenancy/setup/assert-rbac-clusterrole.yaml @@ -144,6 +144,7 @@ rules: - mcptelemetryconfigs/status - mcptoolconfigs/status - mcpwebhookconfigs/status + - virtualmcpcompositetooldefinitions/status - virtualmcpservers/status verbs: - get