Skip to content

Commit 1090e26

Browse files
onematchfoxonematchfox
committed
fix: ensure that tool headers are resolved within the tools namespace
Currently we default to using the agent namespace - but this no longer holds true so we need to explicitly look at the tools namespace instead. Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
1 parent ea46642 commit 1090e26

File tree

3 files changed

+82
-70
lines changed

3 files changed

+82
-70
lines changed

‎go/api/v1alpha2/remotemcpserver_types.go‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,21 +71,6 @@ func (t *RemoteMCPServerSpec) Scan(src any) error {
7171
return nil
7272
}
7373

74-
func (s *RemoteMCPServerSpec) ResolveHeaders(ctx context.Context, client client.Client, namespace string) (map[string]string, error) {
75-
result := map[string]string{}
76-
77-
for _, h := range s.HeadersFrom {
78-
k, v, err := h.Resolve(ctx, client, namespace)
79-
if err != nil {
80-
return nil, fmt.Errorf("failed to resolve header: %v", err)
81-
}
82-
83-
result[k] = v
84-
}
85-
86-
return result, nil
87-
}
88-
8974
var _ driver.Valuer = (*RemoteMCPServerSpec)(nil)
9075

9176
func (t RemoteMCPServerSpec) Value() (driver.Value, error) {
@@ -123,6 +108,22 @@ type RemoteMCPServer struct {
123108
Status RemoteMCPServerStatus `json:"status,omitempty"`
124109
}
125110

111+
// ResolveHeaders resolves all HeadersFrom entries using the object's namespace.
112+
func (r *RemoteMCPServer) ResolveHeaders(ctx context.Context, client client.Client) (map[string]string, error) {
113+
result := map[string]string{}
114+
115+
for _, h := range r.Spec.HeadersFrom {
116+
k, v, err := h.Resolve(ctx, client, r.Namespace)
117+
if err != nil {
118+
return nil, fmt.Errorf("failed to resolve header: %v", err)
119+
}
120+
121+
result[k] = v
122+
}
123+
124+
return result, nil
125+
}
126+
126127
// +kubebuilder:object:root=true
127128
// RemoteMCPServerList contains a list of RemoteMCPServer.
128129
type RemoteMCPServerList struct {

‎go/internal/controller/reconciler/reconciler.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func (a *kagentReconciler) ReconcileKagentMCPService(ctx context.Context, req ct
199199
if remoteService, err := agent_translator.ConvertServiceToRemoteMCPServer(service); err != nil {
200200
reconcileLog.Error(err, "failed to convert service to remote mcp service", "service", utils.GetObjectRef(service))
201201
} else {
202-
if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbService, remoteService, service.Namespace); err != nil {
202+
if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbService, remoteService); err != nil {
203203
return fmt.Errorf("failed to upsert tool server for mcp service %s: %v", utils.GetObjectRef(service), err)
204204
}
205205
}
@@ -365,7 +365,7 @@ func (a *kagentReconciler) ReconcileKagentMCPServer(ctx context.Context, req ctr
365365
if remoteSpec, err := agent_translator.ConvertMCPServerToRemoteMCPServer(mcpServer); err != nil {
366366
reconcileLog.Error(err, "failed to convert mcp server to remote mcp server", "mcpServer", utils.GetObjectRef(mcpServer))
367367
} else {
368-
if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, remoteSpec, mcpServer.Namespace); err != nil {
368+
if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, remoteSpec); err != nil {
369369
return fmt.Errorf("failed to upsert tool server for remote mcp server %s: %v", utils.GetObjectRef(mcpServer), err)
370370
}
371371
}
@@ -408,7 +408,7 @@ func (a *kagentReconciler) ReconcileKagentRemoteMCPServer(ctx context.Context, r
408408
GroupKind: server.GroupVersionKind().GroupKind().String(),
409409
}
410410

411-
tools, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, &server.Spec, server.Namespace)
411+
tools, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, server)
412412
if err != nil {
413413
l.Error(err, "failed to upsert tool server for remote mcp server")
414414

@@ -638,7 +638,7 @@ func (a *kagentReconciler) upsertAgent(ctx context.Context, agent *v1alpha2.Agen
638638
return nil
639639
}
640640

641-
func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Context, toolServer *database.ToolServer, remoteMcpServer *v1alpha2.RemoteMCPServerSpec, namespace string) ([]*v1alpha2.MCPTool, error) {
641+
func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Context, toolServer *database.ToolServer, remoteMcpServer *v1alpha2.RemoteMCPServer) ([]*v1alpha2.MCPTool, error) {
642642
// lock to prevent races
643643
a.upsertLock.Lock()
644644
defer a.upsertLock.Unlock()
@@ -647,7 +647,7 @@ func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Contex
647647
return nil, fmt.Errorf("failed to store toolServer %s: %v", toolServer.Name, err)
648648
}
649649

650-
tsp, err := a.createMcpTransport(ctx, remoteMcpServer, namespace)
650+
tsp, err := a.createMcpTransport(ctx, remoteMcpServer)
651651
if err != nil {
652652
return nil, fmt.Errorf("failed to create client for toolServer %s: %v", toolServer.Name, err)
653653
}
@@ -664,17 +664,17 @@ func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Contex
664664
return tools, nil
665665
}
666666

667-
func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.RemoteMCPServerSpec, namespace string) (transport.Interface, error) {
668-
headers, err := s.ResolveHeaders(ctx, a.kube, namespace)
667+
func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.RemoteMCPServer) (transport.Interface, error) {
668+
headers, err := s.ResolveHeaders(ctx, a.kube)
669669
if err != nil {
670670
return nil, err
671671
}
672672

673-
switch s.Protocol {
673+
switch s.Spec.Protocol {
674674
case v1alpha2.RemoteMCPServerProtocolSse:
675-
return transport.NewSSE(s.URL, transport.WithHeaders(headers))
675+
return transport.NewSSE(s.Spec.URL, transport.WithHeaders(headers))
676676
default:
677-
return transport.NewStreamableHTTP(s.URL, transport.WithHTTPHeaders(headers))
677+
return transport.NewStreamableHTTP(s.Spec.URL, transport.WithHTTPHeaders(headers))
678678
}
679679
}
680680

‎go/internal/controller/translator/agent/adk_api_translator.go‎

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -512,10 +512,15 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al
512512
}
513513

514514
for _, tool := range agent.Spec.Declarative.Tools {
515+
headers, err := tool.ResolveHeaders(ctx, a.kube, agent.Namespace)
516+
if err != nil {
517+
return nil, nil, nil, err
518+
}
519+
515520
// Skip tools that are not applicable to the model provider
516521
switch {
517522
case tool.McpServer != nil:
518-
err := a.translateMCPServerTarget(ctx, cfg, agent.Namespace, tool.McpServer, tool.HeadersFrom)
523+
err := a.translateMCPServerTarget(ctx, cfg, agent.Namespace, tool.McpServer, headers)
519524
if err != nil {
520525
return nil, nil, nil, err
521526
}
@@ -545,10 +550,6 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al
545550
switch toolAgent.Spec.Type {
546551
case v1alpha2.AgentType_BYO, v1alpha2.AgentType_Declarative:
547552
url := fmt.Sprintf("http://%s.%s:8080", toolAgent.Name, toolAgent.Namespace)
548-
headers, err := tool.ResolveHeaders(ctx, a.kube, agent.Namespace)
549-
if err != nil {
550-
return nil, nil, nil, err
551-
}
552553

553554
cfg.RemoteAgents = append(cfg.RemoteAgents, adk.RemoteAgentConfig{
554555
Name: utils.ConvertToPythonIdentifier(utils.GetObjectRef(toolAgent)),
@@ -910,48 +911,52 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
910911
return nil, nil, nil, fmt.Errorf("unknown model provider: %s", model.Spec.Provider)
911912
}
912913

913-
func (a *adkApiTranslator) translateStreamableHttpTool(ctx context.Context, tool *v1alpha2.RemoteMCPServerSpec, namespace string) (*adk.StreamableHTTPConnectionParams, error) {
914-
headers, err := tool.ResolveHeaders(ctx, a.kube, namespace)
914+
func (a *adkApiTranslator) translateStreamableHttpTool(ctx context.Context, server *v1alpha2.RemoteMCPServer, agentHeaders map[string]string) (*adk.StreamableHTTPConnectionParams, error) {
915+
headers, err := server.ResolveHeaders(ctx, a.kube)
915916
if err != nil {
916917
return nil, err
917918
}
919+
// Agent headers override tool headers
920+
maps.Copy(headers, agentHeaders)
918921

919922
params := &adk.StreamableHTTPConnectionParams{
920-
Url: tool.URL,
923+
Url: server.Spec.URL,
921924
Headers: headers,
922925
}
923-
if tool.Timeout != nil {
924-
params.Timeout = ptr.To(tool.Timeout.Seconds())
926+
if server.Spec.Timeout != nil {
927+
params.Timeout = ptr.To(server.Spec.Timeout.Seconds())
925928
}
926-
if tool.SseReadTimeout != nil {
927-
params.SseReadTimeout = ptr.To(tool.SseReadTimeout.Seconds())
929+
if server.Spec.SseReadTimeout != nil {
930+
params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds())
928931
}
929-
if tool.TerminateOnClose != nil {
930-
params.TerminateOnClose = tool.TerminateOnClose
932+
if server.Spec.TerminateOnClose != nil {
933+
params.TerminateOnClose = server.Spec.TerminateOnClose
931934
}
932935
return params, nil
933936
}
934937

935-
func (a *adkApiTranslator) translateSseHttpTool(ctx context.Context, tool *v1alpha2.RemoteMCPServerSpec, namespace string) (*adk.SseConnectionParams, error) {
936-
headers, err := tool.ResolveHeaders(ctx, a.kube, namespace)
938+
func (a *adkApiTranslator) translateSseHttpTool(ctx context.Context, server *v1alpha2.RemoteMCPServer, agentHeaders map[string]string) (*adk.SseConnectionParams, error) {
939+
headers, err := server.ResolveHeaders(ctx, a.kube)
937940
if err != nil {
938941
return nil, err
939942
}
943+
// Agent headers override tool headers
944+
maps.Copy(headers, agentHeaders)
940945

941946
params := &adk.SseConnectionParams{
942-
Url: tool.URL,
947+
Url: server.Spec.URL,
943948
Headers: headers,
944949
}
945-
if tool.Timeout != nil {
946-
params.Timeout = ptr.To(tool.Timeout.Seconds())
950+
if server.Spec.Timeout != nil {
951+
params.Timeout = ptr.To(server.Spec.Timeout.Seconds())
947952
}
948-
if tool.SseReadTimeout != nil {
949-
params.SseReadTimeout = ptr.To(tool.SseReadTimeout.Seconds())
953+
if server.Spec.SseReadTimeout != nil {
954+
params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds())
950955
}
951956
return params, nil
952957
}
953958

954-
func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, toolServer *v1alpha2.McpServerTool, toolHeaders []v1alpha2.ValueRef) error {
959+
func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, toolServer *v1alpha2.McpServerTool, agentHeaders map[string]string) error {
955960
gvk := toolServer.GroupKind()
956961

957962
switch gvk {
@@ -983,14 +988,12 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *
983988
return fmt.Errorf("cross-namespace reference to MCPServer %s is not allowed from namespace %s: MCPServer does not support cross-namespace references", mcpServerRef, agentNamespace)
984989
}
985990

986-
spec, err := ConvertMCPServerToRemoteMCPServer(mcpServer)
991+
remoteMcpServer, err := ConvertMCPServerToRemoteMCPServer(mcpServer)
987992
if err != nil {
988993
return err
989994
}
990995

991-
spec.HeadersFrom = append(spec.HeadersFrom, toolHeaders...)
992-
993-
return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, spec, toolServer.ToolNames)
996+
return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders)
994997
case schema.GroupKind{
995998
Group: "",
996999
Kind: "RemoteMCPServer",
@@ -1017,9 +1020,7 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *
10171020
return fmt.Errorf("cross-namespace reference to RemoteMCPServer %s is not allowed from namespace %s", remoteMcpServerRef, agentNamespace)
10181021
}
10191022

1020-
remoteMcpServer.Spec.HeadersFrom = append(remoteMcpServer.Spec.HeadersFrom, toolHeaders...)
1021-
1022-
return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, &remoteMcpServer.Spec, toolServer.ToolNames)
1023+
return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders)
10231024
case schema.GroupKind{
10241025
Group: "",
10251026
Kind: "Service",
@@ -1043,21 +1044,19 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *
10431044
return fmt.Errorf("cross-namespace reference to Service %s is not allowed from namespace %s: Service does not support cross-namespace references", svcRef, agentNamespace)
10441045
}
10451046

1046-
spec, err := ConvertServiceToRemoteMCPServer(svc)
1047+
remoteMcpServer, err := ConvertServiceToRemoteMCPServer(svc)
10471048
if err != nil {
10481049
return err
10491050
}
10501051

1051-
spec.HeadersFrom = append(spec.HeadersFrom, toolHeaders...)
1052-
1053-
return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, spec, toolServer.ToolNames)
1052+
return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders)
10541053

10551054
default:
10561055
return fmt.Errorf("unknown tool server type: %s", gvk)
10571056
}
10581057
}
10591058

1060-
func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPServerSpec, error) {
1059+
func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPServer, error) {
10611060
// Check wellknown annotations
10621061
port := int64(0)
10631062
protocol := string(MCPServiceProtocolDefault)
@@ -1098,27 +1097,39 @@ func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPSe
10981097
if port == 0 {
10991098
return nil, fmt.Errorf("no port found for service %s with protocol %s", svc.Name, protocol)
11001099
}
1101-
return &v1alpha2.RemoteMCPServerSpec{
1102-
URL: fmt.Sprintf("http://%s.%s:%d%s", svc.Name, svc.Namespace, port, path),
1103-
Protocol: v1alpha2.RemoteMCPServerProtocol(protocol),
1100+
return &v1alpha2.RemoteMCPServer{
1101+
ObjectMeta: metav1.ObjectMeta{
1102+
Name: svc.Name,
1103+
Namespace: svc.Namespace,
1104+
},
1105+
Spec: v1alpha2.RemoteMCPServerSpec{
1106+
URL: fmt.Sprintf("http://%s.%s:%d%s", svc.Name, svc.Namespace, port, path),
1107+
Protocol: v1alpha2.RemoteMCPServerProtocol(protocol),
1108+
},
11041109
}, nil
11051110
}
11061111

1107-
func ConvertMCPServerToRemoteMCPServer(mcpServer *v1alpha1.MCPServer) (*v1alpha2.RemoteMCPServerSpec, error) {
1112+
func ConvertMCPServerToRemoteMCPServer(mcpServer *v1alpha1.MCPServer) (*v1alpha2.RemoteMCPServer, error) {
11081113
if mcpServer.Spec.Deployment.Port == 0 {
11091114
return nil, fmt.Errorf("cannot determine port for MCP server %s", mcpServer.Name)
11101115
}
11111116

1112-
return &v1alpha2.RemoteMCPServerSpec{
1113-
URL: fmt.Sprintf("http://%s.%s:%d/mcp", mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Deployment.Port),
1114-
Protocol: v1alpha2.RemoteMCPServerProtocolStreamableHttp,
1117+
return &v1alpha2.RemoteMCPServer{
1118+
ObjectMeta: metav1.ObjectMeta{
1119+
Name: mcpServer.Name,
1120+
Namespace: mcpServer.Namespace,
1121+
},
1122+
Spec: v1alpha2.RemoteMCPServerSpec{
1123+
URL: fmt.Sprintf("http://%s.%s:%d/mcp", mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Deployment.Port),
1124+
Protocol: v1alpha2.RemoteMCPServerProtocolStreamableHttp,
1125+
},
11151126
}, nil
11161127
}
11171128

1118-
func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, remoteMcpServer *v1alpha2.RemoteMCPServerSpec, toolNames []string) error {
1119-
switch remoteMcpServer.Protocol {
1129+
func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, remoteMcpServer *v1alpha2.RemoteMCPServer, toolNames []string, agentHeaders map[string]string) error {
1130+
switch remoteMcpServer.Spec.Protocol {
11201131
case v1alpha2.RemoteMCPServerProtocolSse:
1121-
tool, err := a.translateSseHttpTool(ctx, remoteMcpServer, agentNamespace)
1132+
tool, err := a.translateSseHttpTool(ctx, remoteMcpServer, agentHeaders)
11221133
if err != nil {
11231134
return err
11241135
}
@@ -1127,7 +1138,7 @@ func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, a
11271138
Tools: toolNames,
11281139
})
11291140
default:
1130-
tool, err := a.translateStreamableHttpTool(ctx, remoteMcpServer, agentNamespace)
1141+
tool, err := a.translateStreamableHttpTool(ctx, remoteMcpServer, agentHeaders)
11311142
if err != nil {
11321143
return err
11331144
}

0 commit comments

Comments
 (0)