Skip to content

Commit 97051d8

Browse files
JAORMXclaude
JAORMX
and
claude
authored
Add MCPServerEntry backend discovery to VirtualMCPServer (#4698)
* Add MCPServerEntry backend discovery to VirtualMCPServer controller VirtualMCPServer needs to discover MCPServerEntry resources (zero-infrastructure catalog entries) and include them as backends. This enables vMCP to route traffic to remote MCP servers declared as MCPServerEntry without proxy pods. - Add WorkloadTypeMCPServerEntry constant and discoverer logic that lists MCPServerEntries by groupRef, converts them to vmcp.Backend using the remote URL directly from the spec (no K8s Service needed) - Extend ConfigMap generation to include entry-type backends with remoteURL, transport, auth config, and CA bundle mount paths - Mount CA bundle ConfigMaps as read-only volumes at /etc/toolhive/ca-bundles/<entry-name>/ when caBundleRef is configured - Add mcpserverentries to VirtualMCPServer RBAC rules (get/list/watch) - Add MCPServerEntry watch with optimized mapper (via MCPGroup Status.Entries) - Add CABundlePath field to StaticBackendConfig for TLS verification - Fix pre-existing missing mcpremoteproxies RBAC marker on controller - Extract metadata key constants to fix goconst lint violations Closes #4657 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Regenerate CRD manifests and API docs for CABundlePath field Run task operator-manifests, operator-generate, and crdref-gen to pick up the new CABundlePath field on StaticBackendConfig and the updated RBAC markers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review feedback for MCPServerEntry discovery - Rename metadataKeyWorkloadStat to metadataKeyWorkloadStatus (F4) - Add nil guard to caBundleMountPath for defensive safety (F5) - Fix volume name truncation with hash suffix to avoid collisions and trailing hyphens in DNS labels (F2) - Add url.Parse validation for RemoteURL as defense-in-depth (F6) - Propagate errors from buildCABundlePathMap and buildCABundleVolumesForEntries instead of silently swallowing (F7) - Add explicit phase check to skip non-Valid MCPServerEntry backends - Use path.Join instead of fmt.Sprintf for path construction - Add tests for volume name truncation, collision avoidance, and phase-based filtering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix undefined assert.NotHasSuffix in volume name tests testify does not provide NotHasSuffix; use assert.False with strings.HasSuffix instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix import-shadowing lint errors in k8s workload backends Rename local `url` variables to `serverURL` and `proxyURL` to avoid shadowing the `net/url` import, which revive flags as import-shadowing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d3479a9 commit 97051d8

13 files changed

Lines changed: 1813 additions & 27 deletions

‎cmd/thv-operator/controllers/virtualmcpserver_controller.go‎

Lines changed: 120 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ type VirtualMCPServerReconciler struct {
109109
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpservers/status,verbs=get;update;patch
110110
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpgroups,verbs=get;list;watch
111111
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpservers,verbs=get;list;watch
112+
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies,verbs=get;list;watch
113+
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpserverentries,verbs=get;list;watch
112114
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpexternalauthconfigs,verbs=get;list;watch
113115
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcptoolconfigs,verbs=get;list;watch
114116
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpcompositetooldefinitions,verbs=get;list;watch
@@ -1837,6 +1839,22 @@ func (r *VirtualMCPServerReconciler) listMCPRemoteProxiesAsMap(
18371839
return mcpRemoteProxyMap, nil
18381840
}
18391841

1842+
// listMCPServerEntriesAsMap lists all MCPServerEntries in the namespace and returns a map by name.
1843+
func (r *VirtualMCPServerReconciler) listMCPServerEntriesAsMap(
1844+
ctx context.Context,
1845+
namespace string,
1846+
) (map[string]*mcpv1alpha1.MCPServerEntry, error) {
1847+
mcpServerEntryList := &mcpv1alpha1.MCPServerEntryList{}
1848+
if err := r.List(ctx, mcpServerEntryList, client.InNamespace(namespace)); err != nil {
1849+
return nil, err
1850+
}
1851+
mcpServerEntryMap := make(map[string]*mcpv1alpha1.MCPServerEntry, len(mcpServerEntryList.Items))
1852+
for i := range mcpServerEntryList.Items {
1853+
mcpServerEntryMap[mcpServerEntryList.Items[i].Name] = &mcpServerEntryList.Items[i]
1854+
}
1855+
return mcpServerEntryMap, nil
1856+
}
1857+
18401858
// discoverExternalAuthConfigs discovers ExternalAuthConfig from workloads and adds them to the outgoing config.
18411859
// Returns a list of non-fatal errors that should be reported via status conditions.
18421860
// The controller should continue in degraded mode even if some auth configs fail.
@@ -1862,9 +1880,15 @@ func (r *VirtualMCPServerReconciler) discoverExternalAuthConfigs(
18621880
return backendsWithAuthConfig, authErrors
18631881
}
18641882

1883+
mcpServerEntryMap, err := r.listMCPServerEntriesAsMap(ctx, vmcp.Namespace)
1884+
if err != nil {
1885+
ctxLogger.Error(err, "Failed to list MCPServerEntries")
1886+
return backendsWithAuthConfig, authErrors
1887+
}
1888+
18651889
for _, workloadInfo := range typedWorkloads {
18661890
externalAuthConfigName := r.getExternalAuthConfigNameFromWorkload(
1867-
workloadInfo, mcpServerMap, mcpRemoteProxyMap)
1891+
workloadInfo, mcpServerMap, mcpRemoteProxyMap, mcpServerEntryMap)
18681892
if externalAuthConfigName == "" {
18691893
continue
18701894
}
@@ -1920,6 +1944,7 @@ func (*VirtualMCPServerReconciler) getExternalAuthConfigNameFromWorkload(
19201944
workloadInfo workloads.TypedWorkload,
19211945
mcpServerMap map[string]*mcpv1alpha1.MCPServer,
19221946
mcpRemoteProxyMap map[string]*mcpv1alpha1.MCPRemoteProxy,
1947+
mcpServerEntryMap map[string]*mcpv1alpha1.MCPServerEntry,
19231948
) string {
19241949
switch workloadInfo.Type {
19251950
case workloads.WorkloadTypeMCPServer:
@@ -1936,6 +1961,13 @@ func (*VirtualMCPServerReconciler) getExternalAuthConfigNameFromWorkload(
19361961
}
19371962
return mcpRemoteProxy.Spec.ExternalAuthConfigRef.Name
19381963

1964+
case workloads.WorkloadTypeMCPServerEntry:
1965+
mcpServerEntry, found := mcpServerEntryMap[workloadInfo.Name]
1966+
if !found || mcpServerEntry.Spec.ExternalAuthConfigRef == nil {
1967+
return ""
1968+
}
1969+
return mcpServerEntry.Spec.ExternalAuthConfigRef.Name
1970+
19391971
default:
19401972
return ""
19411973
}
@@ -2048,10 +2080,12 @@ func injectSubjectProviderIfNeeded(
20482080
// convertBackendsToStaticBackends converts Backend objects to StaticBackendConfig for ConfigMap embedding.
20492081
// Preserves metadata and uses transport types from workload Specs.
20502082
// Logs warnings when backends are skipped due to missing URL or transport information.
2083+
// caBundlePathMap maps backend names to their CA bundle mount paths (populated for MCPServerEntry backends).
20512084
func convertBackendsToStaticBackends(
20522085
ctx context.Context,
20532086
backends []vmcptypes.Backend,
20542087
transportMap map[string]string,
2088+
caBundlePathMap map[string]string,
20552089
) []vmcpconfig.StaticBackendConfig {
20562090
logger := log.FromContext(ctx)
20572091
static := make([]vmcpconfig.StaticBackendConfig, 0, len(backends))
@@ -2069,12 +2103,18 @@ func convertBackendsToStaticBackends(
20692103
continue
20702104
}
20712105

2072-
static = append(static, vmcpconfig.StaticBackendConfig{
2106+
cfg := vmcpconfig.StaticBackendConfig{
20732107
Name: backend.Name,
20742108
URL: backend.BaseURL,
20752109
Transport: transport,
20762110
Metadata: backend.Metadata,
2077-
})
2111+
}
2112+
2113+
if caBundlePath, ok := caBundlePathMap[backend.Name]; ok {
2114+
cfg.CABundlePath = caBundlePath
2115+
}
2116+
2117+
static = append(static, cfg)
20782118
}
20792119
return static
20802120
}
@@ -2168,6 +2208,7 @@ func (r *VirtualMCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
21682208
Watches(&mcpv1alpha1.MCPGroup{}, handler.EnqueueRequestsFromMapFunc(r.mapMCPGroupToVirtualMCPServer)).
21692209
Watches(&mcpv1alpha1.MCPServer{}, handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToVirtualMCPServer)).
21702210
Watches(&mcpv1alpha1.MCPRemoteProxy{}, handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToVirtualMCPServer)).
2211+
Watches(&mcpv1alpha1.MCPServerEntry{}, handler.EnqueueRequestsFromMapFunc(r.mapMCPServerEntryToVirtualMCPServer)).
21712212
Watches(&mcpv1alpha1.MCPExternalAuthConfig{}, handler.EnqueueRequestsFromMapFunc(r.mapExternalAuthConfigToVirtualMCPServer)).
21722213
Watches(&mcpv1alpha1.MCPToolConfig{}, handler.EnqueueRequestsFromMapFunc(r.mapToolConfigToVirtualMCPServer)).
21732214
Watches(
@@ -2378,6 +2419,82 @@ func (r *VirtualMCPServerReconciler) mapMCPRemoteProxyToVirtualMCPServer(
23782419
return requests
23792420
}
23802421

2422+
// mapMCPServerEntryToVirtualMCPServer maps MCPServerEntry changes to VirtualMCPServer reconciliation requests.
2423+
// This function implements the same optimization as mapMCPServerToVirtualMCPServer to only reconcile
2424+
// VirtualMCPServers that are actually affected by the MCPServerEntry change.
2425+
//
2426+
// The optimization works by:
2427+
// 1. Finding all MCPGroups that include the changed MCPServerEntry (via Status.Entries)
2428+
// 2. Finding all VirtualMCPServers that reference those MCPGroups
2429+
// 3. Only reconciling those specific VirtualMCPServers
2430+
func (r *VirtualMCPServerReconciler) mapMCPServerEntryToVirtualMCPServer(
2431+
ctx context.Context,
2432+
obj client.Object,
2433+
) []reconcile.Request {
2434+
mcpServerEntry, ok := obj.(*mcpv1alpha1.MCPServerEntry)
2435+
if !ok {
2436+
return nil
2437+
}
2438+
2439+
ctxLogger := log.FromContext(ctx)
2440+
2441+
// Step 1: Find all MCPGroups that include this MCPServerEntry
2442+
mcpGroupList := &mcpv1alpha1.MCPGroupList{}
2443+
if err := r.List(ctx, mcpGroupList, client.InNamespace(mcpServerEntry.Namespace)); err != nil {
2444+
ctxLogger.Error(err, "Failed to list MCPGroups for MCPServerEntry watch")
2445+
return nil
2446+
}
2447+
2448+
affectedGroups := make(map[string]bool)
2449+
for _, group := range mcpGroupList.Items {
2450+
for _, entryName := range group.Status.Entries {
2451+
if entryName == mcpServerEntry.Name {
2452+
affectedGroups[group.Name] = true
2453+
ctxLogger.V(1).Info("MCPServerEntry is member of MCPGroup",
2454+
"mcpServerEntry", mcpServerEntry.Name,
2455+
"mcpGroup", group.Name)
2456+
break
2457+
}
2458+
}
2459+
}
2460+
2461+
if len(affectedGroups) == 0 {
2462+
ctxLogger.V(1).Info("MCPServerEntry not a member of any MCPGroup, skipping VirtualMCPServer reconciliation",
2463+
"mcpServerEntry", mcpServerEntry.Name)
2464+
return nil
2465+
}
2466+
2467+
// Step 2: Find VirtualMCPServers that reference the affected MCPGroups
2468+
vmcpList := &mcpv1alpha1.VirtualMCPServerList{}
2469+
if err := r.List(ctx, vmcpList, client.InNamespace(mcpServerEntry.Namespace)); err != nil {
2470+
ctxLogger.Error(err, "Failed to list VirtualMCPServers for MCPServerEntry watch")
2471+
return nil
2472+
}
2473+
2474+
var requests []reconcile.Request
2475+
for _, vmcp := range vmcpList.Items {
2476+
if affectedGroups[vmcp.Spec.Config.Group] {
2477+
requests = append(requests, reconcile.Request{
2478+
NamespacedName: types.NamespacedName{
2479+
Name: vmcp.Name,
2480+
Namespace: vmcp.Namespace,
2481+
},
2482+
})
2483+
ctxLogger.V(1).Info("Queuing VirtualMCPServer for reconciliation due to MCPServerEntry change",
2484+
"virtualMCPServer", vmcp.Name,
2485+
"mcpGroup", vmcp.Spec.Config.Group,
2486+
"mcpServerEntry", mcpServerEntry.Name)
2487+
}
2488+
}
2489+
2490+
ctxLogger.V(1).Info("Mapped MCPServerEntry to VirtualMCPServers",
2491+
"mcpServerEntry", mcpServerEntry.Name,
2492+
"affectedGroups", len(affectedGroups),
2493+
"virtualMCPServers", len(requests))
2494+
2495+
return requests
2496+
}
2497+
23812498
// mapExternalAuthConfigToVirtualMCPServer maps MCPExternalAuthConfig changes to VirtualMCPServer reconciliation requests
23822499
func (r *VirtualMCPServerReconciler) mapExternalAuthConfigToVirtualMCPServer(
23832500
ctx context.Context,

‎cmd/thv-operator/controllers/virtualmcpserver_controller_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ func TestVirtualMCPServerEnsureRBACResources(t *testing.T) {
272272
assert.Contains(t, toolhiveRule.Resources, "mcpgroups", "Role should allow listing mcpgroups")
273273
assert.Contains(t, toolhiveRule.Resources, "mcpservers", "Role should allow listing mcpservers")
274274
assert.Contains(t, toolhiveRule.Resources, "mcpremoteproxies", "Role should allow listing mcpremoteproxies")
275+
assert.Contains(t, toolhiveRule.Resources, "mcpserverentries", "Role should allow listing mcpserverentries")
275276
assert.Contains(t, toolhiveRule.Resources, "mcpexternalauthconfigs", "Role should allow listing mcpexternalauthconfigs")
276277

277278
// Verify RoleBinding was created
@@ -3317,3 +3318,150 @@ func mustBuildEnvVarsForVmcp(r *VirtualMCPServerReconciler, vmcp *mcpv1alpha1.Vi
33173318
}
33183319
return env
33193320
}
3321+
3322+
// TestGetExternalAuthConfigNameFromWorkload tests auth config ref extraction from all workload types
3323+
func TestGetExternalAuthConfigNameFromWorkload(t *testing.T) {
3324+
t.Parallel()
3325+
3326+
mcpServerMap := map[string]*mcpv1alpha1.MCPServer{
3327+
"server-with-auth": {
3328+
Spec: mcpv1alpha1.MCPServerSpec{
3329+
ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{
3330+
Name: "server-auth-config",
3331+
},
3332+
},
3333+
},
3334+
"server-no-auth": {
3335+
Spec: mcpv1alpha1.MCPServerSpec{},
3336+
},
3337+
}
3338+
3339+
mcpRemoteProxyMap := map[string]*mcpv1alpha1.MCPRemoteProxy{
3340+
"proxy-with-auth": {
3341+
Spec: mcpv1alpha1.MCPRemoteProxySpec{
3342+
ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{
3343+
Name: "proxy-auth-config",
3344+
},
3345+
},
3346+
},
3347+
}
3348+
3349+
mcpServerEntryMap := map[string]*mcpv1alpha1.MCPServerEntry{
3350+
"entry-with-auth": {
3351+
Spec: mcpv1alpha1.MCPServerEntrySpec{
3352+
ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{
3353+
Name: "entry-auth-config",
3354+
},
3355+
},
3356+
},
3357+
"entry-no-auth": {
3358+
Spec: mcpv1alpha1.MCPServerEntrySpec{},
3359+
},
3360+
}
3361+
3362+
tests := []struct {
3363+
name string
3364+
workload workloads.TypedWorkload
3365+
expectedName string
3366+
}{
3367+
{
3368+
name: "MCPServer with auth config ref",
3369+
workload: workloads.TypedWorkload{
3370+
Name: "server-with-auth",
3371+
Type: workloads.WorkloadTypeMCPServer,
3372+
},
3373+
expectedName: "server-auth-config",
3374+
},
3375+
{
3376+
name: "MCPServer without auth config ref",
3377+
workload: workloads.TypedWorkload{
3378+
Name: "server-no-auth",
3379+
Type: workloads.WorkloadTypeMCPServer,
3380+
},
3381+
expectedName: "",
3382+
},
3383+
{
3384+
name: "MCPServer not found in map",
3385+
workload: workloads.TypedWorkload{
3386+
Name: "non-existent",
3387+
Type: workloads.WorkloadTypeMCPServer,
3388+
},
3389+
expectedName: "",
3390+
},
3391+
{
3392+
name: "MCPRemoteProxy with auth config ref",
3393+
workload: workloads.TypedWorkload{
3394+
Name: "proxy-with-auth",
3395+
Type: workloads.WorkloadTypeMCPRemoteProxy,
3396+
},
3397+
expectedName: "proxy-auth-config",
3398+
},
3399+
{
3400+
name: "MCPServerEntry with auth config ref",
3401+
workload: workloads.TypedWorkload{
3402+
Name: "entry-with-auth",
3403+
Type: workloads.WorkloadTypeMCPServerEntry,
3404+
},
3405+
expectedName: "entry-auth-config",
3406+
},
3407+
{
3408+
name: "MCPServerEntry without auth config ref",
3409+
workload: workloads.TypedWorkload{
3410+
Name: "entry-no-auth",
3411+
Type: workloads.WorkloadTypeMCPServerEntry,
3412+
},
3413+
expectedName: "",
3414+
},
3415+
{
3416+
name: "MCPServerEntry not found in map",
3417+
workload: workloads.TypedWorkload{
3418+
Name: "non-existent-entry",
3419+
Type: workloads.WorkloadTypeMCPServerEntry,
3420+
},
3421+
expectedName: "",
3422+
},
3423+
{
3424+
name: "unknown workload type",
3425+
workload: workloads.TypedWorkload{
3426+
Name: "unknown",
3427+
Type: workloads.WorkloadType("UnknownType"),
3428+
},
3429+
expectedName: "",
3430+
},
3431+
}
3432+
3433+
for _, tt := range tests {
3434+
t.Run(tt.name, func(t *testing.T) {
3435+
t.Parallel()
3436+
3437+
r := &VirtualMCPServerReconciler{}
3438+
result := r.getExternalAuthConfigNameFromWorkload(
3439+
tt.workload,
3440+
mcpServerMap,
3441+
mcpRemoteProxyMap,
3442+
mcpServerEntryMap,
3443+
)
3444+
assert.Equal(t, tt.expectedName, result)
3445+
})
3446+
}
3447+
}
3448+
3449+
// TestDiscoveredRBACRulesIncludeMCPServerEntries verifies that the RBAC rules
3450+
// for discovered mode include mcpserverentries as an allowed resource
3451+
func TestDiscoveredRBACRulesIncludeMCPServerEntries(t *testing.T) {
3452+
t.Parallel()
3453+
3454+
foundMCPServerEntries := false
3455+
for _, rule := range vmcpDiscoveredRBACRules {
3456+
for _, apiGroup := range rule.APIGroups {
3457+
if apiGroup == "toolhive.stacklok.dev" {
3458+
for _, resource := range rule.Resources {
3459+
if resource == "mcpserverentries" {
3460+
foundMCPServerEntries = true
3461+
}
3462+
}
3463+
}
3464+
}
3465+
}
3466+
assert.True(t, foundMCPServerEntries, "vmcpDiscoveredRBACRules should include mcpserverentries")
3467+
}

0 commit comments

Comments
 (0)