From 0ac6866964c307789968913e66a4c7460cfbbe20 Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Wed, 26 Nov 2025 11:55:44 +0100 Subject: [PATCH 1/7] feat(controller): enable cross-namespace tool references Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- go/api/v1alpha2/agent_types.go | 19 +++++++++++++--- go/config/crd/bases/kagent.dev_agents.yaml | 4 ++++ go/internal/controller/agent_controller.go | 22 ++++++------------- .../translator/agent/adk_api_translator.go | 22 +++++++++---------- .../templates/kagent.dev_agents.yaml | 4 ++++ 5 files changed, 42 insertions(+), 29 deletions(-) diff --git a/go/api/v1alpha2/agent_types.go b/go/api/v1alpha2/agent_types.go index f28c3f01d..f9c958c5f 100644 --- a/go/api/v1alpha2/agent_types.go +++ b/go/api/v1alpha2/agent_types.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "trpc.group/trpc-go/trpc-a2a-go/server" @@ -208,7 +209,6 @@ func (s *Tool) ResolveHeaders(ctx context.Context, client client.Client, namespa type McpServerTool struct { // The reference to the ToolServer that provides the tool. - // Can either be a reference to the name of a ToolServer in the same namespace as the referencing Agent, or a reference to the name of an ToolServer in a different namespace in the form / // +optional TypedLocalReference `json:",inline"` @@ -222,8 +222,9 @@ type TypedLocalReference struct { // +optional Kind string `json:"kind"` // +optional - ApiGroup string `json:"apiGroup"` - Name string `json:"name"` + ApiGroup string `json:"apiGroup"` + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` } func (t *TypedLocalReference) GroupKind() schema.GroupKind { @@ -233,6 +234,18 @@ func (t *TypedLocalReference) GroupKind() schema.GroupKind { } } +func (t *TypedLocalReference) NamespacedName(defaultNamespace string) types.NamespacedName { + namespace := t.Namespace + if namespace == "" { + namespace = defaultNamespace + } + + return types.NamespacedName{ + Namespace: namespace, + Name: t.Name, + } +} + type A2AConfig struct { // +kubebuilder:validation:MinItems=1 Skills []AgentSkill `json:"skills,omitempty"` diff --git a/go/config/crd/bases/kagent.dev_agents.yaml b/go/config/crd/bases/kagent.dev_agents.yaml index 7b6bbc1ea..aaf5b8262 100644 --- a/go/config/crd/bases/kagent.dev_agents.yaml +++ b/go/config/crd/bases/kagent.dev_agents.yaml @@ -8865,6 +8865,8 @@ spec: type: string name: type: string + namespace: + type: string required: - name type: object @@ -8918,6 +8920,8 @@ spec: type: string name: type: string + namespace: + type: string toolNames: description: |- The names of the tools to be provided by the ToolServer diff --git a/go/internal/controller/agent_controller.go b/go/internal/controller/agent_controller.go index 18a3e31c1..e188058f4 100644 --- a/go/internal/controller/agent_controller.go +++ b/go/internal/controller/agent_controller.go @@ -181,10 +181,6 @@ func (r *AgentController) findAgentsUsingMCPServer(ctx context.Context, cl clien var agents []*v1alpha2.Agent for _, agent := range agentsList.Items { - if agent.Namespace != obj.Namespace { - continue - } - if agent.Spec.Type != v1alpha2.AgentType_Declarative { continue } @@ -198,7 +194,8 @@ func (r *AgentController) findAgentsUsingMCPServer(ctx context.Context, cl clien continue } - if tool.McpServer.Name == obj.Name { + mcpServerRef := tool.McpServer.NamespacedName(agent.Namespace) + if mcpServerRef == obj { agents = append(agents, &agent) } } @@ -229,11 +226,8 @@ func (r *AgentController) findAgentsUsingRemoteMCPServer(ctx context.Context, cl return } - if agent.Namespace != obj.Namespace { - continue - } - - if tool.McpServer.Name == obj.Name { + mcpServerRef := tool.McpServer.NamespacedName(agent.Namespace) + if mcpServerRef == obj { agents = append(agents, agent) return } @@ -249,6 +243,7 @@ func (r *AgentController) findAgentsUsingRemoteMCPServer(ctx context.Context, cl func (r *AgentController) findAgentsUsingMCPService(ctx context.Context, cl client.Client, obj types.NamespacedName) []*v1alpha2.Agent { var agentsList v1alpha2.AgentList + if err := cl.List( ctx, &agentsList, @@ -259,10 +254,6 @@ func (r *AgentController) findAgentsUsingMCPService(ctx context.Context, cl clie var agents []*v1alpha2.Agent for _, agent := range agentsList.Items { - if agent.Namespace != obj.Namespace { - continue - } - if agent.Spec.Type != v1alpha2.AgentType_Declarative { continue } @@ -276,7 +267,8 @@ func (r *AgentController) findAgentsUsingMCPService(ctx context.Context, cl clie continue } - if tool.McpServer.Name == obj.Name { + mcpServerRef := tool.McpServer.NamespacedName(agent.Namespace) + if mcpServerRef == obj { agents = append(agents, &agent) } } diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index a215ad383..636f3cbae 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -193,10 +193,7 @@ func (a *adkApiTranslator) validateAgent(ctx context.Context, agent *v1alpha2.Ag return fmt.Errorf("tool must have an agent reference") } - agentRef := types.NamespacedName{ - Namespace: agent.Namespace, - Name: tool.Agent.Name, - } + agentRef := tool.Agent.NamespacedName(agent.Namespace) if agentRef.Namespace == agent.Namespace && agentRef.Name == agent.Name { return fmt.Errorf("agent tool cannot be used to reference itself, %s", agentRef) @@ -523,10 +520,7 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al return nil, nil, nil, err } case tool.Agent != nil: - agentRef := types.NamespacedName{ - Namespace: agent.Namespace, - Name: tool.Agent.Name, - } + agentRef := tool.Agent.NamespacedName(agent.Namespace) if agentRef.Namespace == agent.Namespace && agentRef.Name == agent.Name { return nil, nil, nil, fmt.Errorf("agent tool cannot be used to reference itself, %s", agentRef) @@ -967,7 +961,9 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * Kind: "MCPServer", }: mcpServer := &v1alpha1.MCPServer{} - err := a.kube.Get(ctx, types.NamespacedName{Namespace: agentNamespace, Name: toolServer.Name}, mcpServer) + mcpServerRef := toolServer.NamespacedName(agentNamespace) + + err := a.kube.Get(ctx, mcpServerRef, mcpServer) if err != nil { return err } @@ -990,7 +986,9 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * Kind: "RemoteMCPServer", }: remoteMcpServer := &v1alpha2.RemoteMCPServer{} - err := a.kube.Get(ctx, types.NamespacedName{Namespace: agentNamespace, Name: toolServer.Name}, remoteMcpServer) + remoteMcpServerRef := toolServer.NamespacedName(agentNamespace) + + err := a.kube.Get(ctx, remoteMcpServerRef, remoteMcpServer) if err != nil { return err } @@ -1008,7 +1006,9 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * Kind: "Service", }: svc := &corev1.Service{} - err := a.kube.Get(ctx, types.NamespacedName{Namespace: agentNamespace, Name: toolServer.Name}, svc) + svcRef := toolServer.NamespacedName(agentNamespace) + + err := a.kube.Get(ctx, svcRef, svc) if err != nil { return err } diff --git a/helm/kagent-crds/templates/kagent.dev_agents.yaml b/helm/kagent-crds/templates/kagent.dev_agents.yaml index 7b6bbc1ea..aaf5b8262 100644 --- a/helm/kagent-crds/templates/kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/kagent.dev_agents.yaml @@ -8865,6 +8865,8 @@ spec: type: string name: type: string + namespace: + type: string required: - name type: object @@ -8918,6 +8920,8 @@ spec: type: string name: type: string + namespace: + type: string toolNames: description: |- The names of the tools to be provided by the ToolServer From 13410354c9bf632aafa7afd38a8153ecfc3b55ca Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Wed, 26 Nov 2025 12:08:40 +0100 Subject: [PATCH 2/7] feat(ui): add support for tool namespaces Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- ui/src/app/actions/agents.ts | 44 ++++++-- ui/src/app/agents/new/page.tsx | 1 + .../components/create/SelectToolsDialog.tsx | 48 +++++++-- ui/src/components/create/ToolsSection.tsx | 13 ++- .../sidebars/AgentDetailsSidebar.tsx | 8 +- ui/src/lib/__tests__/toolUtils.test.ts | 100 ++++++++++++++---- ui/src/lib/toolUtils.ts | 34 ++++-- ui/src/types/index.ts | 1 + 8 files changed, 198 insertions(+), 51 deletions(-) diff --git a/ui/src/app/actions/agents.ts b/ui/src/app/actions/agents.ts index 05e7dccbe..96c4936b0 100644 --- a/ui/src/app/actions/agents.ts +++ b/ui/src/app/actions/agents.ts @@ -19,6 +19,7 @@ function fromAgentFormDataToAgent(agentFormData: AgentFormData): Agent { : agentFormData.modelName; const type = agentFormData.type || "Declarative"; + const agentNamespace = agentFormData.namespace || ""; const convertTools = (tools: Tool[]) => tools.map((tool) => { @@ -27,10 +28,19 @@ function fromAgentFormDataToAgent(agentFormData: AgentFormData): Agent { if (!mcpServer) { throw new Error("MCP server not found"); } - // Ensure TypedLocalReference fields are only the name (no namespace) + let name = mcpServer.name; + let namespace: string | undefined = mcpServer.namespace; + if (k8sRefUtils.isValidRef(mcpServer.name)) { - name = k8sRefUtils.fromRef(mcpServer.name).name; + const parsed = k8sRefUtils.fromRef(mcpServer.name); + name = parsed.name; + // Ignore namespace on the name ref if one is set - using namespace/name format is legacy behavior + } + + // If no namespace is set, default to the agent's namespace + if (!namespace) { + namespace = agentNamespace; } let kind = mcpServer.kind; @@ -44,6 +54,7 @@ function fromAgentFormDataToAgent(agentFormData: AgentFormData): Agent { type: "McpServer", mcpServer: { name, + namespace, kind, apiGroup: mcpServer.apiGroup, toolNames: mcpServer.toolNames, @@ -52,15 +63,32 @@ function fromAgentFormDataToAgent(agentFormData: AgentFormData): Agent { } if (tool.type === "Agent") { - const agentObj = tool.agent as { ref?: string; name?: string; kind?: string; apiGroup?: string }; - const refOrName = agentObj.ref || agentObj.name || ""; - const nameOnly = k8sRefUtils.isValidRef(refOrName) ? k8sRefUtils.fromRef(refOrName).name : refOrName; + const agent = tool.agent; + if (!agent) { + throw new Error("Agent not found"); + } + + let name = agent.name; + let namespace: string | undefined = agent.namespace; + + if (k8sRefUtils.isValidRef(name)) { + const parsed = k8sRefUtils.fromRef(name); + name = parsed.name; + // Ignore namespace on the name ref if one is set - using namespace/name format is legacy behavior + } + + // If no namespace is set, default to the agent's namespace + if (!namespace) { + namespace = agentNamespace; + } + return { type: "Agent", agent: { - name: nameOnly, - kind: agentObj.kind || "Agent", - apiGroup: agentObj.apiGroup || "kagent.dev", + name, + namespace, + kind: agent.kind || "Agent", + apiGroup: agent.apiGroup || "kagent.dev", }, } as Tool; } diff --git a/ui/src/app/agents/new/page.tsx b/ui/src/app/agents/new/page.tsx index 6a3ee0259..7e5555798 100644 --- a/ui/src/app/agents/new/page.tsx +++ b/ui/src/app/agents/new/page.tsx @@ -610,6 +610,7 @@ function AgentPageContent({ isEditMode, agentName, agentNamespace }: AgentPageCo isSubmitting={state.isSubmitting || state.isLoading} onBlur={() => validateField('tools', state.selectedTools)} currentAgentName={state.name} + currentAgentNamespace={state.namespace} /> diff --git a/ui/src/components/create/SelectToolsDialog.tsx b/ui/src/components/create/SelectToolsDialog.tsx index 295e5a11d..166018ae5 100644 --- a/ui/src/components/create/SelectToolsDialog.tsx +++ b/ui/src/components/create/SelectToolsDialog.tsx @@ -24,6 +24,7 @@ interface SelectToolsDialogProps { onToolsSelected: (tools: Tool[]) => void; availableAgents: AgentResponse[]; loadingAgents: boolean; + currentAgentNamespace: string; } @@ -65,7 +66,7 @@ const getItemDisplayInfo = (item: ToolsResponse | AgentResponse): { } }; -export const SelectToolsDialog: React.FC = ({ open, onOpenChange, availableTools, selectedTools, onToolsSelected, availableAgents, loadingAgents }) => { +export const SelectToolsDialog: React.FC = ({ open, onOpenChange, availableTools, selectedTools, onToolsSelected, availableAgents, loadingAgents, currentAgentNamespace }) => { const [searchTerm, setSearchTerm] = useState(""); const [localSelectedTools, setLocalSelectedTools] = useState([]); const [categories, setCategories] = useState>(new Set()); @@ -174,11 +175,25 @@ export const SelectToolsDialog: React.FC = ({ open, onOp const isItemSelected = (item: ToolsResponse | AgentResponse): boolean => { if (isAgentResponse(item)) { const agentResp = item as AgentResponse; - const agentRef = k8sRefUtils.toRef(agentResp.agent.metadata.namespace || "", agentResp.agent.metadata.name); + + // "item" is an agent but called item to here so as not to confuse + // variables with the agent to which the tool is being added + const itemNamespace = agentResp.agent.metadata.namespace || ""; + const itemName = agentResp.agent.metadata.name; return localSelectedTools.some(tool => { if (!isAgentTool(tool)) return false; - return tool.agent?.name === agentRef || tool.agent?.name === agentResp.agent.metadata.name; + + const toolName = tool.agent?.name; + const toolNamespace = tool.agent?.namespace; + + // Match by name and namespace + if (toolNamespace) { + return toolNamespace === itemNamespace && toolName === itemName; + } + + // If no namespace in tool, match by name only + return toolName === itemName; }); } else { const toolItem = item as ToolsResponse; @@ -208,11 +223,14 @@ export const SelectToolsDialog: React.FC = ({ open, onOp if (isAgentResponse(item)) { const agentResp = item as AgentResponse; - const agentRef = k8sRefUtils.toRef(agentResp.agent.metadata.namespace || "", agentResp.agent.metadata.name); + const agentNamespace = agentResp.agent.metadata.namespace || ""; + const agentName = agentResp.agent.metadata.name; + toolToAdd = { type: "Agent", agent: { - name: agentRef, + name: agentName, + namespace: agentNamespace, kind: "Agent", apiGroup: "kagent.dev", } @@ -480,12 +498,18 @@ export const SelectToolsDialog: React.FC = ({ open, onOp ); const specificDescription = foundTool?.description || "Description not available"; + // Show server name with namespace for consistency + const serverName = tool.mcpServer?.name || ""; + const serverNamespace = tool.mcpServer?.namespace || currentAgentNamespace; + const serverDisplayName = `${serverNamespace}/${serverName}`; + const displayName = `${toolName} (${serverDisplayName})`; + return (
-

{toolName}

+

{displayName}

{specificDescription}

@@ -519,8 +543,16 @@ export const SelectToolsDialog: React.FC = ({ open, onOp } else { const matchedAgent = isAgentTool(tool) ? availableAgents.find(a => { - const ref = k8sRefUtils.toRef(a.agent.metadata.namespace || "", a.agent.metadata.name); - return ref === tool.agent?.name || a.agent.metadata.name === tool.agent?.name; + const agentName = tool.agent?.name; + const agentNamespace = tool.agent?.namespace; + + // Match by name and namespace (if namespace is specified) + if (agentNamespace) { + return a.agent.metadata.namespace === agentNamespace && + a.agent.metadata.name === agentName; + } + // If no namespace specified, match by name only + return a.agent.metadata.name === agentName; }) : undefined; diff --git a/ui/src/components/create/ToolsSection.tsx b/ui/src/components/create/ToolsSection.tsx index 31357338e..1bd12ae0c 100644 --- a/ui/src/components/create/ToolsSection.tsx +++ b/ui/src/components/create/ToolsSection.tsx @@ -16,9 +16,10 @@ interface ToolsSectionProps { isSubmitting: boolean; onBlur?: () => void; currentAgentName: string; + currentAgentNamespace: string; } -export const ToolsSection = ({ selectedTools, setSelectedTools, isSubmitting, onBlur, currentAgentName }: ToolsSectionProps) => { +export const ToolsSection = ({ selectedTools, setSelectedTools, isSubmitting, onBlur, currentAgentName, currentAgentNamespace }: ToolsSectionProps) => { const [showToolSelector, setShowToolSelector] = useState(false); const [availableAgents, setAvailableAgents] = useState([]); const [loadingAgents, setLoadingAgents] = useState(true); @@ -99,7 +100,12 @@ export const ToolsSection = ({ selectedTools, setSelectedTools, isSubmitting, on const mcpTool = agentTool as Tool; return mcpTool.mcpServer?.toolNames.map((mcpToolName: string) => { const toolIdentifierForDisplay = `${parentToolIdentifier}::${mcpToolName}`; - const displayName = mcpToolName; + + // Show server name with namespace for consistency + const serverName = mcpTool.mcpServer?.name || ""; + const serverNamespace = mcpTool.mcpServer?.namespace || currentAgentNamespace; + const serverDisplayName = `${serverNamespace}/${serverName}`; + const displayName = `${mcpToolName} (${serverDisplayName})`; // The tools on agent resource don't have descriptions, so we need to // get the descriptions from the db @@ -143,7 +149,7 @@ export const ToolsSection = ({ selectedTools, setSelectedTools, isSubmitting, on ); }); } else { - const displayName = getToolDisplayName(agentTool); + const displayName = getToolDisplayName(agentTool, currentAgentNamespace); const displayDescription = getToolDescription(agentTool, availableTools); let CurrentIcon: React.ElementType; @@ -236,6 +242,7 @@ export const ToolsSection = ({ selectedTools, setSelectedTools, isSubmitting, on selectedTools={selectedTools} onToolsSelected={handleToolSelect} loadingAgents={loadingAgents} + currentAgentNamespace={currentAgentNamespace} />
); diff --git a/ui/src/components/sidebars/AgentDetailsSidebar.tsx b/ui/src/components/sidebars/AgentDetailsSidebar.tsx index 8ad9f8125..158b4669d 100644 --- a/ui/src/components/sidebars/AgentDetailsSidebar.tsx +++ b/ui/src/components/sidebars/AgentDetailsSidebar.tsx @@ -160,6 +160,8 @@ export function AgentDetailsSidebar({ selectedAgentName, currentAgent, allTools ); } + const agentNamespace = currentAgent.agent.metadata.namespace || ""; + return ( {tools.flatMap((tool) => { @@ -169,17 +171,19 @@ export function AgentDetailsSidebar({ selectedAgentName, currentAgent, allTools const mcpProvider = tool.mcpServer.name || "mcp_server"; const mcpProviderParts = mcpProvider.split("."); const mcpProviderNameTooltip = mcpProviderParts[mcpProviderParts.length - 1]; + const serverDisplayName = `${tool.mcpServer.namespace || agentNamespace}/${tool.mcpServer.name || ""}`; return tool.mcpServer.toolNames.map((mcpToolName) => { const subToolIdentifier = `${baseToolIdentifier}::${mcpToolName}`; const description = toolDescriptions[subToolIdentifier] || "Description loading or unavailable"; const isExpanded = expandedTools[subToolIdentifier] || false; + const displayName = `${mcpToolName} (${serverDisplayName})`; return ( { }); describe('groupMcpToolsByServer', () => { - it('should group multiple MCP tools from same server into single entry', () => { + it('should group multiple MCP tools from same server into single entry and preserve namespace', () => { const githubServerRef = k8sRefUtils.toRef("default", "github-server"); const tools: Tool[] = [ { type: "McpServer", mcpServer: { name: githubServerRef, + namespace: "kagent", apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["create_pull_request"] @@ -95,6 +96,7 @@ describe('Tool Utility Functions', () => { type: "McpServer", mcpServer: { name: githubServerRef, + namespace: "kagent", apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["create_repository"] @@ -104,6 +106,7 @@ describe('Tool Utility Functions', () => { type: "McpServer", mcpServer: { name: githubServerRef, + namespace: "kagent", apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["fork_repository"] @@ -117,6 +120,7 @@ describe('Tool Utility Functions', () => { expect(result.groupedTools).toHaveLength(1); expect(result.groupedTools[0].type).toBe("McpServer"); expect(result.groupedTools[0].mcpServer?.name).toBe(githubServerRef); + expect(result.groupedTools[0].mcpServer?.namespace).toBe("kagent"); expect(result.groupedTools[0].mcpServer?.toolNames).toEqual([ "create_pull_request", "create_repository", @@ -447,30 +451,55 @@ describe('Tool Utility Functions', () => { }); describe('getToolDisplayName', () => { - it('should return agent ref for Agent tools', () => { + it('should return namespaced ref for Agent tools with explicit namespace', () => { + const agentTool: Tool = { + type: "Agent", + agent: { + name: "test-agent", + namespace: "kagent" + } + }; + expect(getToolDisplayName(agentTool, "default")).toBe("kagent/test-agent"); + }); + + it('should return namespaced ref for Agent tools using default namespace', () => { const agentTool: Tool = { type: "Agent", agent: { name: "test-agent" } }; - expect(getToolDisplayName(agentTool)).toBe("test-agent"); + expect(getToolDisplayName(agentTool, "default")).toBe("default/test-agent"); }); - it('should return server name for MCP tools', () => { + it('should return namespaced ref for MCP tools with explicit namespace', () => { const mcpTool: Tool = { type: "McpServer", mcpServer: { - name: "default/github-server", + name: "github-server", + namespace: "tools", + apiGroup: "kagent.dev", + kind: "MCPServer", + toolNames: ["create_pull_request"] + } + }; + expect(getToolDisplayName(mcpTool, "default")).toBe("tools/github-server"); + }); + + it('should return namespaced ref for MCP tools using default namespace', () => { + const mcpTool: Tool = { + type: "McpServer", + mcpServer: { + name: "github-server", apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["create_pull_request"] } }; - expect(getToolDisplayName(mcpTool)).toBe("default/github-server"); + expect(getToolDisplayName(mcpTool, "default")).toBe("default/github-server"); }); - it('should return "No name" for MCP tools with missing name', () => { + it('should return "Unknown Tool" for MCP tools with missing name', () => { const mcpTool: Tool = { type: "McpServer", mcpServer: { @@ -480,12 +509,12 @@ describe('Tool Utility Functions', () => { toolNames: ["create_pull_request"] } }; - expect(getToolDisplayName(mcpTool)).toBe("No name"); + expect(getToolDisplayName(mcpTool, "default")).toBe("Unknown Tool"); }); it('should return "Unknown Tool" for unknown tool types', () => { const unknownTool = { type: "Unknown" as any } as Tool; - expect(getToolDisplayName(unknownTool)).toBe("Unknown Tool"); + expect(getToolDisplayName(unknownTool, "default")).toBe("Unknown Tool"); }); }); @@ -707,7 +736,7 @@ describe('Tool Utility Functions', () => { }); describe('toolResponseToAgentTool', () => { - it('should use groupKind to set correct apiGroup and kind', () => { + it('should use groupKind to set correct apiGroup and kind, and extract namespace from serverRef', () => { const tool: ToolsResponse = { id: "create_pull_request", server_name: "default/ragflow-mcp-server", @@ -723,7 +752,8 @@ describe('Tool Utility Functions', () => { expect(result).toEqual({ type: "McpServer", mcpServer: { - name: "default/ragflow-mcp-server", + name: "ragflow-mcp-server", + namespace: "default", apiGroup: "kagent.dev", kind: "RemoteMCPServer", toolNames: ["create_pull_request"] @@ -731,7 +761,7 @@ describe('Tool Utility Functions', () => { }); }); - it('should handle special case for kagent-querydoc with empty apiGroup', () => { + it('should handle special case for kagent-querydoc with empty apiGroup and extract namespace', () => { const tool: ToolsResponse = { id: "query_documentation", server_name: "kagent/kagent-querydoc", @@ -747,7 +777,8 @@ describe('Tool Utility Functions', () => { expect(result).toEqual({ type: "McpServer", mcpServer: { - name: "kagent/kagent-querydoc", + name: "kagent-querydoc", + namespace: "kagent", apiGroup: "", kind: "Service", toolNames: ["query_documentation"] @@ -755,7 +786,7 @@ describe('Tool Utility Functions', () => { }); }); - it('should handle fallback when groupKind is empty', () => { + it('should handle fallback when groupKind is empty and extract namespace', () => { const tool: ToolsResponse = { id: "some_tool", server_name: "default/some-server", @@ -771,7 +802,8 @@ describe('Tool Utility Functions', () => { expect(result).toEqual({ type: "McpServer", mcpServer: { - name: "default/some-server", + name: "some-server", + namespace: "default", apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["some_tool"] @@ -779,7 +811,7 @@ describe('Tool Utility Functions', () => { }); }); - it('should handle groupKind with only kind', () => { + it('should handle groupKind with only kind and extract namespace', () => { const tool: ToolsResponse = { id: "some_tool", server_name: "default/some-server", @@ -795,7 +827,33 @@ describe('Tool Utility Functions', () => { expect(result).toEqual({ type: "McpServer", mcpServer: { - name: "default/some-server", + name: "some-server", + namespace: "default", + apiGroup: "kagent.dev", + kind: "MCPServer", + toolNames: ["some_tool"] + } + }); + }); + + it('should handle serverRef without namespace (no slash)', () => { + const tool: ToolsResponse = { + id: "some_tool", + server_name: "some-server", + description: "Some tool", + created_at: "2023-01-01T00:00:00Z", + updated_at: "2023-01-01T00:00:00Z", + deleted_at: "", + group_kind: "MCPServer" + }; + + const result = toolResponseToAgentTool(tool, tool.server_name); + + expect(result).toEqual({ + type: "McpServer", + mcpServer: { + name: "some-server", + namespace: undefined, apiGroup: "kagent.dev", kind: "MCPServer", toolNames: ["some_tool"] @@ -905,8 +963,8 @@ describe('Tool Utility Functions', () => { }); it('should handle null/undefined inputs for getToolDisplayName', () => { - expect(getToolDisplayName(null as any)).toBe("Unknown Tool"); - expect(getToolDisplayName(undefined as any)).toBe("Unknown Tool"); + expect(getToolDisplayName(null as any, "default")).toBe("Unknown Tool"); + expect(getToolDisplayName(undefined as any, "default")).toBe("Unknown Tool"); }); it('should handle null/undefined inputs for getToolDescription', () => { @@ -917,14 +975,14 @@ describe('Tool Utility Functions', () => { it('should handle malformed tool objects', () => { const malformedTool = { type: "Agent" } as Tool; // Missing agent property expect(getToolIdentifier(malformedTool)).toMatch(/^unknown-tool-[a-z0-9]+$/); - expect(getToolDisplayName(malformedTool)).toBe("Unknown Tool"); + expect(getToolDisplayName(malformedTool, "default")).toBe("Unknown Tool"); expect(getToolDescription(malformedTool, [])).toBe("No description available"); }); it('should handle MCP tools with null/undefined mcpServer', () => { const malformedMcpTool = { type: "McpServer" } as Tool; // Missing mcpServer property expect(getToolIdentifier(malformedMcpTool)).toMatch(/^unknown-tool-[a-z0-9]+$/); - expect(getToolDisplayName(malformedMcpTool)).toBe("Unknown Tool"); + expect(getToolDisplayName(malformedMcpTool, "default")).toBe("Unknown Tool"); expect(getToolDescription(malformedMcpTool, [])).toBe("No description available"); }); }); diff --git a/ui/src/lib/toolUtils.ts b/ui/src/lib/toolUtils.ts index 9b7a4b2e3..fefe13bd4 100644 --- a/ui/src/lib/toolUtils.ts +++ b/ui/src/lib/toolUtils.ts @@ -89,9 +89,9 @@ export const groupMcpToolsByServer = (tools: Tool[]): { } }); - // Convert to Tool objects- preserve original kind and apiGroup from the first tool of each server + // Convert to Tool objects- preserve original kind, apiGroup, and namespace from the first tool of each server const groupedMcpTools = Array.from(mcpToolsByServer.entries()).map(([serverNameRef, toolNamesSet]) => { - // Find the first tool from this server to get its kind and apiGroup + // Find the first tool from this server to get its kind, apiGroup, and namespace const originalTool = tools.find(tool => isMcpTool(tool) && tool.mcpServer?.name === serverNameRef ); @@ -102,6 +102,7 @@ export const groupMcpToolsByServer = (tools: Tool[]): { type: MCP_SERVER_TYPE, mcpServer: { name: serverNameRef, + namespace: originalMcpServer?.namespace, apiGroup: originalMcpServer?.apiGroup || DEFAULT_API_GROUP, kind: originalMcpServer?.kind || DEFAULT_MCP_KIND, toolNames: Array.from(toolNamesSet) @@ -147,16 +148,20 @@ export const parseGroupKind = (groupKind: string): { apiGroup: string; kind: str return { apiGroup, kind }; }; -export const getToolDisplayName = (tool: Tool): string => { +export const getToolDisplayName = (tool: Tool, defaultNamespace: string): string => { if (isAgentTool(tool) && tool.agent) { - try { - return tool.agent.name; - } catch { - return "Agent"; + const name = tool.agent?.name; + if (!name || name.trim() === "") { + return "Unknown Agent"; } + return k8sRefUtils.toRef(tool.agent.namespace || defaultNamespace, name); } else if (isMcpTool(tool)) { const mcpTool = tool as Tool; - return mcpTool.mcpServer?.name || "No name"; + const name = mcpTool.mcpServer?.name; + if (!name || name.trim() === "") { + return "Unknown Tool"; + } + return k8sRefUtils.toRef(mcpTool.mcpServer?.namespace || defaultNamespace, name); } return "Unknown Tool"; }; @@ -210,6 +215,16 @@ export const getToolResponseIdentifier = (tool: ToolsResponse | undefined | null export const toolResponseToAgentTool = (tool: ToolsResponse, serverRef: string): Tool => { let { apiGroup, kind } = parseGroupKind(tool.group_kind); + // Parse namespace and name from serverRef if it's in namespace/name format + let name = serverRef; + let namespace: string | undefined; + + if (k8sRefUtils.isValidRef(serverRef)) { + const parsed = k8sRefUtils.fromRef(serverRef); + name = parsed.name; + namespace = parsed.namespace; + } + // Special handling for kagent-querydoc - must have empty apiGroup for Kubernetes Service if (serverRef.toLocaleLowerCase().includes(QUERYDOC_SERVER_NAME)) { apiGroup = ""; @@ -219,7 +234,8 @@ export const toolResponseToAgentTool = (tool: ToolsResponse, serverRef: string): return { type: MCP_SERVER_TYPE, mcpServer: { - name: serverRef, + name, + namespace, apiGroup, kind, toolNames: [tool.id] diff --git a/ui/src/types/index.ts b/ui/src/types/index.ts index 075ae5d88..982bad21f 100644 --- a/ui/src/types/index.ts +++ b/ui/src/types/index.ts @@ -207,6 +207,7 @@ export interface TypedLocalReference { kind?: string; apiGroup?: string; name: string; + namespace?: string; } export interface McpServerTool extends TypedLocalReference { From a06c110aecd59d8d817f8cef5fd33fa67c5cfbde Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Wed, 3 Dec 2025 12:34:30 +0100 Subject: [PATCH 3/7] 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> --- go/api/v1alpha2/remotemcpserver_types.go | 31 +++--- .../controller/reconciler/reconciler.go | 20 ++-- .../translator/agent/adk_api_translator.go | 101 ++++++++++-------- 3 files changed, 82 insertions(+), 70 deletions(-) diff --git a/go/api/v1alpha2/remotemcpserver_types.go b/go/api/v1alpha2/remotemcpserver_types.go index 265224373..f50efb843 100644 --- a/go/api/v1alpha2/remotemcpserver_types.go +++ b/go/api/v1alpha2/remotemcpserver_types.go @@ -64,21 +64,6 @@ func (t *RemoteMCPServerSpec) Scan(src any) error { return nil } -func (s *RemoteMCPServerSpec) ResolveHeaders(ctx context.Context, client client.Client, namespace string) (map[string]string, error) { - result := map[string]string{} - - for _, h := range s.HeadersFrom { - k, v, err := h.Resolve(ctx, client, namespace) - if err != nil { - return nil, fmt.Errorf("failed to resolve header: %v", err) - } - - result[k] = v - } - - return result, nil -} - var _ driver.Valuer = (*RemoteMCPServerSpec)(nil) func (t RemoteMCPServerSpec) Value() (driver.Value, error) { @@ -116,6 +101,22 @@ type RemoteMCPServer struct { Status RemoteMCPServerStatus `json:"status,omitempty"` } +// ResolveHeaders resolves all HeadersFrom entries using the object's namespace. +func (r *RemoteMCPServer) ResolveHeaders(ctx context.Context, client client.Client) (map[string]string, error) { + result := map[string]string{} + + for _, h := range r.Spec.HeadersFrom { + k, v, err := h.Resolve(ctx, client, r.Namespace) + if err != nil { + return nil, fmt.Errorf("failed to resolve header: %v", err) + } + + result[k] = v + } + + return result, nil +} + // +kubebuilder:object:root=true // RemoteMCPServerList contains a list of RemoteMCPServer. type RemoteMCPServerList struct { diff --git a/go/internal/controller/reconciler/reconciler.go b/go/internal/controller/reconciler/reconciler.go index 3c5c76578..5bb1338ec 100644 --- a/go/internal/controller/reconciler/reconciler.go +++ b/go/internal/controller/reconciler/reconciler.go @@ -199,7 +199,7 @@ func (a *kagentReconciler) ReconcileKagentMCPService(ctx context.Context, req ct if remoteService, err := agent_translator.ConvertServiceToRemoteMCPServer(service); err != nil { reconcileLog.Error(err, "failed to convert service to remote mcp service", "service", utils.GetObjectRef(service)) } else { - if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbService, remoteService, service.Namespace); err != nil { + if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbService, remoteService); err != nil { return fmt.Errorf("failed to upsert tool server for mcp service %s: %v", utils.GetObjectRef(service), err) } } @@ -365,7 +365,7 @@ func (a *kagentReconciler) ReconcileKagentMCPServer(ctx context.Context, req ctr if remoteSpec, err := agent_translator.ConvertMCPServerToRemoteMCPServer(mcpServer); err != nil { reconcileLog.Error(err, "failed to convert mcp server to remote mcp server", "mcpServer", utils.GetObjectRef(mcpServer)) } else { - if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, remoteSpec, mcpServer.Namespace); err != nil { + if _, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, remoteSpec); err != nil { return fmt.Errorf("failed to upsert tool server for remote mcp server %s: %v", utils.GetObjectRef(mcpServer), err) } } @@ -408,7 +408,7 @@ func (a *kagentReconciler) ReconcileKagentRemoteMCPServer(ctx context.Context, r GroupKind: server.GroupVersionKind().GroupKind().String(), } - tools, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, &server.Spec, server.Namespace) + tools, err := a.upsertToolServerForRemoteMCPServer(ctx, dbServer, server) if err != nil { l.Error(err, "failed to upsert tool server for remote mcp server") @@ -638,7 +638,7 @@ func (a *kagentReconciler) upsertAgent(ctx context.Context, agent *v1alpha2.Agen return nil } -func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Context, toolServer *database.ToolServer, remoteMcpServer *v1alpha2.RemoteMCPServerSpec, namespace string) ([]*v1alpha2.MCPTool, error) { +func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Context, toolServer *database.ToolServer, remoteMcpServer *v1alpha2.RemoteMCPServer) ([]*v1alpha2.MCPTool, error) { // lock to prevent races a.upsertLock.Lock() defer a.upsertLock.Unlock() @@ -647,7 +647,7 @@ func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Contex return nil, fmt.Errorf("failed to store toolServer %s: %v", toolServer.Name, err) } - tsp, err := a.createMcpTransport(ctx, remoteMcpServer, namespace) + tsp, err := a.createMcpTransport(ctx, remoteMcpServer) if err != nil { return nil, fmt.Errorf("failed to create client for toolServer %s: %v", toolServer.Name, err) } @@ -664,17 +664,17 @@ func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Contex return tools, nil } -func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.RemoteMCPServerSpec, namespace string) (transport.Interface, error) { - headers, err := s.ResolveHeaders(ctx, a.kube, namespace) +func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.RemoteMCPServer) (transport.Interface, error) { + headers, err := s.ResolveHeaders(ctx, a.kube) if err != nil { return nil, err } - switch s.Protocol { + switch s.Spec.Protocol { case v1alpha2.RemoteMCPServerProtocolSse: - return transport.NewSSE(s.URL, transport.WithHeaders(headers)) + return transport.NewSSE(s.Spec.URL, transport.WithHeaders(headers)) default: - return transport.NewStreamableHTTP(s.URL, transport.WithHTTPHeaders(headers)) + return transport.NewStreamableHTTP(s.Spec.URL, transport.WithHTTPHeaders(headers)) } } diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index 636f3cbae..c36ad6691 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -512,10 +512,15 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al } for _, tool := range agent.Spec.Declarative.Tools { + headers, err := tool.ResolveHeaders(ctx, a.kube, agent.Namespace) + if err != nil { + return nil, nil, nil, err + } + // Skip tools that are not applicable to the model provider switch { case tool.McpServer != nil: - err := a.translateMCPServerTarget(ctx, cfg, agent.Namespace, tool.McpServer, tool.HeadersFrom) + err := a.translateMCPServerTarget(ctx, cfg, agent.Namespace, tool.McpServer, headers) if err != nil { return nil, nil, nil, err } @@ -536,10 +541,6 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al switch toolAgent.Spec.Type { case v1alpha2.AgentType_BYO, v1alpha2.AgentType_Declarative: url := fmt.Sprintf("http://%s.%s:8080", toolAgent.Name, toolAgent.Namespace) - headers, err := tool.ResolveHeaders(ctx, a.kube, agent.Namespace) - if err != nil { - return nil, nil, nil, err - } cfg.RemoteAgents = append(cfg.RemoteAgents, adk.RemoteAgentConfig{ Name: utils.ConvertToPythonIdentifier(utils.GetObjectRef(toolAgent)), @@ -901,48 +902,52 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC return nil, nil, nil, fmt.Errorf("unknown model provider: %s", model.Spec.Provider) } -func (a *adkApiTranslator) translateStreamableHttpTool(ctx context.Context, tool *v1alpha2.RemoteMCPServerSpec, namespace string) (*adk.StreamableHTTPConnectionParams, error) { - headers, err := tool.ResolveHeaders(ctx, a.kube, namespace) +func (a *adkApiTranslator) translateStreamableHttpTool(ctx context.Context, server *v1alpha2.RemoteMCPServer, agentHeaders map[string]string) (*adk.StreamableHTTPConnectionParams, error) { + headers, err := server.ResolveHeaders(ctx, a.kube) if err != nil { return nil, err } + // Agent headers override tool headers + maps.Copy(headers, agentHeaders) params := &adk.StreamableHTTPConnectionParams{ - Url: tool.URL, + Url: server.Spec.URL, Headers: headers, } - if tool.Timeout != nil { - params.Timeout = ptr.To(tool.Timeout.Seconds()) + if server.Spec.Timeout != nil { + params.Timeout = ptr.To(server.Spec.Timeout.Seconds()) } - if tool.SseReadTimeout != nil { - params.SseReadTimeout = ptr.To(tool.SseReadTimeout.Seconds()) + if server.Spec.SseReadTimeout != nil { + params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds()) } - if tool.TerminateOnClose != nil { - params.TerminateOnClose = tool.TerminateOnClose + if server.Spec.TerminateOnClose != nil { + params.TerminateOnClose = server.Spec.TerminateOnClose } return params, nil } -func (a *adkApiTranslator) translateSseHttpTool(ctx context.Context, tool *v1alpha2.RemoteMCPServerSpec, namespace string) (*adk.SseConnectionParams, error) { - headers, err := tool.ResolveHeaders(ctx, a.kube, namespace) +func (a *adkApiTranslator) translateSseHttpTool(ctx context.Context, server *v1alpha2.RemoteMCPServer, agentHeaders map[string]string) (*adk.SseConnectionParams, error) { + headers, err := server.ResolveHeaders(ctx, a.kube) if err != nil { return nil, err } + // Agent headers override tool headers + maps.Copy(headers, agentHeaders) params := &adk.SseConnectionParams{ - Url: tool.URL, + Url: server.Spec.URL, Headers: headers, } - if tool.Timeout != nil { - params.Timeout = ptr.To(tool.Timeout.Seconds()) + if server.Spec.Timeout != nil { + params.Timeout = ptr.To(server.Spec.Timeout.Seconds()) } - if tool.SseReadTimeout != nil { - params.SseReadTimeout = ptr.To(tool.SseReadTimeout.Seconds()) + if server.Spec.SseReadTimeout != nil { + params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds()) } return params, nil } -func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, toolServer *v1alpha2.McpServerTool, toolHeaders []v1alpha2.ValueRef) error { +func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, toolServer *v1alpha2.McpServerTool, agentHeaders map[string]string) error { gvk := toolServer.GroupKind() switch gvk { @@ -968,14 +973,12 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - spec, err := ConvertMCPServerToRemoteMCPServer(mcpServer) + remoteMcpServer, err := ConvertMCPServerToRemoteMCPServer(mcpServer) if err != nil { return err } - spec.HeadersFrom = append(spec.HeadersFrom, toolHeaders...) - - return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, spec, toolServer.ToolNames) + return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders) case schema.GroupKind{ Group: "", Kind: "RemoteMCPServer", @@ -993,9 +996,7 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - remoteMcpServer.Spec.HeadersFrom = append(remoteMcpServer.Spec.HeadersFrom, toolHeaders...) - - return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, &remoteMcpServer.Spec, toolServer.ToolNames) + return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders) case schema.GroupKind{ Group: "", Kind: "Service", @@ -1013,21 +1014,19 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - spec, err := ConvertServiceToRemoteMCPServer(svc) + remoteMcpServer, err := ConvertServiceToRemoteMCPServer(svc) if err != nil { return err } - spec.HeadersFrom = append(spec.HeadersFrom, toolHeaders...) - - return a.translateRemoteMCPServerTarget(ctx, agent, agentNamespace, spec, toolServer.ToolNames) + return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders) default: return fmt.Errorf("unknown tool server type: %s", gvk) } } -func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPServerSpec, error) { +func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPServer, error) { // Check wellknown annotations port := int64(0) protocol := string(MCPServiceProtocolDefault) @@ -1068,27 +1067,39 @@ func ConvertServiceToRemoteMCPServer(svc *corev1.Service) (*v1alpha2.RemoteMCPSe if port == 0 { return nil, fmt.Errorf("no port found for service %s with protocol %s", svc.Name, protocol) } - return &v1alpha2.RemoteMCPServerSpec{ - URL: fmt.Sprintf("http://%s.%s:%d%s", svc.Name, svc.Namespace, port, path), - Protocol: v1alpha2.RemoteMCPServerProtocol(protocol), + return &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: svc.Name, + Namespace: svc.Namespace, + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + URL: fmt.Sprintf("http://%s.%s:%d%s", svc.Name, svc.Namespace, port, path), + Protocol: v1alpha2.RemoteMCPServerProtocol(protocol), + }, }, nil } -func ConvertMCPServerToRemoteMCPServer(mcpServer *v1alpha1.MCPServer) (*v1alpha2.RemoteMCPServerSpec, error) { +func ConvertMCPServerToRemoteMCPServer(mcpServer *v1alpha1.MCPServer) (*v1alpha2.RemoteMCPServer, error) { if mcpServer.Spec.Deployment.Port == 0 { return nil, fmt.Errorf("cannot determine port for MCP server %s", mcpServer.Name) } - return &v1alpha2.RemoteMCPServerSpec{ - URL: fmt.Sprintf("http://%s.%s:%d/mcp", mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Deployment.Port), - Protocol: v1alpha2.RemoteMCPServerProtocolStreamableHttp, + return &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: mcpServer.Name, + Namespace: mcpServer.Namespace, + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + URL: fmt.Sprintf("http://%s.%s:%d/mcp", mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Deployment.Port), + Protocol: v1alpha2.RemoteMCPServerProtocolStreamableHttp, + }, }, nil } -func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, agentNamespace string, remoteMcpServer *v1alpha2.RemoteMCPServerSpec, toolNames []string) error { - switch remoteMcpServer.Protocol { +func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, remoteMcpServer *v1alpha2.RemoteMCPServer, toolNames []string, agentHeaders map[string]string) error { + switch remoteMcpServer.Spec.Protocol { case v1alpha2.RemoteMCPServerProtocolSse: - tool, err := a.translateSseHttpTool(ctx, remoteMcpServer, agentNamespace) + tool, err := a.translateSseHttpTool(ctx, remoteMcpServer, agentHeaders) if err != nil { return err } @@ -1097,7 +1108,7 @@ func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, a Tools: toolNames, }) default: - tool, err := a.translateStreamableHttpTool(ctx, remoteMcpServer, agentNamespace) + tool, err := a.translateStreamableHttpTool(ctx, remoteMcpServer, agentHeaders) if err != nil { return err } From 20686747ec757a67b6a028c45bc73f9fd2512cc4 Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Wed, 3 Dec 2025 11:48:45 +0100 Subject: [PATCH 4/7] tests: add golden test for cross-namespace tool use Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- .../agent_with_cross_namespace_tools.yaml | 104 ++++++ .../agent_with_cross_namespace_tools.json | 311 ++++++++++++++++++ 2 files changed, 415 insertions(+) create mode 100644 go/internal/controller/translator/agent/testdata/inputs/agent_with_cross_namespace_tools.yaml create mode 100644 go/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json diff --git a/go/internal/controller/translator/agent/testdata/inputs/agent_with_cross_namespace_tools.yaml b/go/internal/controller/translator/agent/testdata/inputs/agent_with_cross_namespace_tools.yaml new file mode 100644 index 000000000..8fc6ed66b --- /dev/null +++ b/go/internal/controller/translator/agent/testdata/inputs/agent_with_cross_namespace_tools.yaml @@ -0,0 +1,104 @@ +operation: translateAgent +targetObject: source-agent +namespace: source-ns +objects: + # Namespace objects for cross-namespace selector checks + - apiVersion: v1 + kind: Namespace + metadata: + name: source-ns + labels: + tools-access: "true" + - apiVersion: v1 + kind: Namespace + metadata: + name: tools-ns + # Secret for the agent's model config (in source namespace) + - apiVersion: v1 + kind: Secret + metadata: + name: openai-secret + namespace: source-ns + data: + api-key: c2stdGVzdC1hcGkta2V5 # base64 encoded "sk-test-api-key" + # Secret for the RemoteMCPServer headers (in tools namespace) + # RemoteMCPServer's headers are resolved in the tool's namespace + - apiVersion: v1 + kind: Secret + metadata: + name: tool-auth-secret + namespace: tools-ns + data: + auth-token: dG9vbC1zZWNyZXQtdG9rZW4= # base64 encoded "tool-secret-token" + # Model config for the source agent + - apiVersion: kagent.dev/v1alpha2 + kind: ModelConfig + metadata: + name: model-config + namespace: source-ns + spec: + provider: OpenAI + model: gpt-4o + apiKeySecret: openai-secret + apiKeySecretKey: api-key + # RemoteMCPServer in tools namespace - allows cross-namespace access from all namespaces + # Has its own auth headers that reference secrets in its own namespace + - apiVersion: kagent.dev/v1alpha2 + kind: RemoteMCPServer + metadata: + name: shared-tools + namespace: tools-ns + spec: + description: "Shared tool server accessible from multiple namespaces" + url: http://tools.tools-ns.svc:8080/mcp + timeout: 30s + # Headers are resolved from secrets in the RemoteMCPServer's namespace (tools-ns) + headersFrom: + - name: Authorization + valueFrom: + type: Secret + name: tool-auth-secret + key: auth-token + allowedNamespaces: + from: All + # Agent in tools namespace - allows cross-namespace access from all namespaces + - apiVersion: kagent.dev/v1alpha2 + kind: Agent + metadata: + name: tools-agent + namespace: tools-ns + spec: + type: Declarative + description: An agent that can be used as a cross-namespace tool + declarative: + systemMessage: You are an assistant that can be used as a cross-namespace tool. + modelConfig: model-config + allowedNamespaces: + from: All + # Agent in source namespace that references the cross-namespace tool + - apiVersion: kagent.dev/v1alpha2 + kind: Agent + metadata: + name: source-agent + namespace: source-ns + spec: + type: Declarative + description: An agent that uses cross-namespace tools + declarative: + systemMessage: You are an assistant with access to shared tools. + modelConfig: model-config + tools: + - type: McpServer + mcpServer: + name: shared-tools + namespace: tools-ns + kind: RemoteMCPServer + apiGroup: kagent.dev + toolNames: + - list_resources + - get_resource + - type: Agent + agent: + name: tools-agent + namespace: tools-ns + diff --git a/go/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json b/go/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json new file mode 100644 index 000000000..7c6fd0b56 --- /dev/null +++ b/go/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json @@ -0,0 +1,311 @@ +{ + "agentCard": { + "capabilities": { + "pushNotifications": false, + "stateTransitionHistory": true, + "streaming": true + }, + "defaultInputModes": [ + "text" + ], + "defaultOutputModes": [ + "text" + ], + "description": "An agent that uses cross-namespace tools", + "name": "source_agent", + "skills": null, + "url": "http://source-agent.source-ns:8080", + "version": "" + }, + "config": { + "description": "An agent that uses cross-namespace tools", + "http_tools": [ + { + "params": { + "headers": { + "Authorization": "tool-secret-token" + }, + "timeout": 30, + "url": "http://tools.tools-ns.svc:8080/mcp" + }, + "tools": [ + "list_resources", + "get_resource" + ] + } + ], + "instruction": "You are an assistant with access to shared tools.", + "model": { + "base_url": "", + "model": "gpt-4o", + "type": "openai" + }, + "remote_agents": [ + { + "description": "An agent that can be used as a cross-namespace tool", + "name": "tools_ns__NS__tools_agent", + "url": "http://tools-agent.tools-ns:8080" + } + ], + "sse_tools": null + }, + "manifest": [ + { + "apiVersion": "v1", + "kind": "Secret", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "source-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "source-agent" + }, + "name": "source-agent", + "namespace": "source-ns", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "source-agent", + "uid": "" + } + ] + }, + "stringData": { + "agent-card.json": "{\"name\":\"source_agent\",\"description\":\"An agent that uses cross-namespace tools\",\"url\":\"http://source-agent.source-ns:8080\",\"version\":\"\",\"capabilities\":{\"streaming\":true,\"pushNotifications\":false,\"stateTransitionHistory\":true},\"defaultInputModes\":[\"text\"],\"defaultOutputModes\":[\"text\"],\"skills\":[]}", + "config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"An agent that uses cross-namespace tools\",\"instruction\":\"You are an assistant with access to shared tools.\",\"http_tools\":[{\"params\":{\"url\":\"http://tools.tools-ns.svc:8080/mcp\",\"headers\":{\"Authorization\":\"tool-secret-token\"},\"timeout\":30},\"tools\":[\"list_resources\",\"get_resource\"]}],\"sse_tools\":null,\"remote_agents\":[{\"name\":\"tools_ns__NS__tools_agent\",\"url\":\"http://tools-agent.tools-ns:8080\",\"description\":\"An agent that can be used as a cross-namespace tool\"}]}" + } + }, + { + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "source-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "source-agent" + }, + "name": "source-agent", + "namespace": "source-ns", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "source-agent", + "uid": "" + } + ] + } + }, + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "source-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "source-agent" + }, + "name": "source-agent", + "namespace": "source-ns", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "source-agent", + "uid": "" + } + ] + }, + "spec": { + "selector": { + "matchLabels": { + "app": "kagent", + "kagent": "source-agent" + } + }, + "strategy": { + "rollingUpdate": { + "maxSurge": 1, + "maxUnavailable": 0 + }, + "type": "RollingUpdate" + }, + "template": { + "metadata": { + "annotations": { + "kagent.dev/config-hash": "5077803691472142283" + }, + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "source-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "source-agent" + } + }, + "spec": { + "containers": [ + { + "args": [ + "--host", + "0.0.0.0", + "--port", + "8080", + "--filepath", + "/config" + ], + "env": [ + { + "name": "OPENAI_API_KEY", + "valueFrom": { + "secretKeyRef": { + "key": "api-key", + "name": "openai-secret" + } + } + }, + { + "name": "KAGENT_NAMESPACE", + "valueFrom": { + "fieldRef": { + "fieldPath": "metadata.namespace" + } + } + }, + { + "name": "KAGENT_NAME", + "valueFrom": { + "fieldRef": { + "fieldPath": "spec.serviceAccountName" + } + } + }, + { + "name": "KAGENT_URL", + "value": "http://kagent-controller.kagent:8083" + } + ], + "image": "cr.kagent.dev/kagent-dev/kagent/app:dev", + "imagePullPolicy": "IfNotPresent", + "name": "kagent", + "ports": [ + { + "containerPort": 8080, + "name": "http" + } + ], + "readinessProbe": { + "httpGet": { + "path": "/health", + "port": "http" + }, + "initialDelaySeconds": 15, + "periodSeconds": 15, + "timeoutSeconds": 15 + }, + "resources": { + "limits": { + "cpu": "2", + "memory": "1Gi" + }, + "requests": { + "cpu": "100m", + "memory": "384Mi" + } + }, + "volumeMounts": [ + { + "mountPath": "/config", + "name": "config" + }, + { + "mountPath": "/var/run/secrets/tokens", + "name": "kagent-token" + } + ] + } + ], + "serviceAccountName": "source-agent", + "volumes": [ + { + "name": "config", + "secret": { + "secretName": "source-agent" + } + }, + { + "name": "kagent-token", + "projected": { + "sources": [ + { + "serviceAccountToken": { + "audience": "kagent", + "expirationSeconds": 3600, + "path": "kagent-token" + } + } + ] + } + } + ] + } + } + }, + "status": {} + }, + { + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "source-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "source-agent" + }, + "name": "source-agent", + "namespace": "source-ns", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "source-agent", + "uid": "" + } + ] + }, + "spec": { + "ports": [ + { + "name": "http", + "port": 8080, + "targetPort": 8080 + } + ], + "selector": { + "app": "kagent", + "kagent": "source-agent" + }, + "type": "ClusterIP" + }, + "status": { + "loadBalancer": {} + } + } + ] +} \ No newline at end of file From 9107df4c8e7af0abe14a81ee264a08d78f404695 Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Wed, 3 Dec 2025 11:48:24 +0100 Subject: [PATCH 5/7] feat(controller): restrict cross-namespace access to agents/tools Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- go/api/v1alpha2/agent_types.go | 8 + go/api/v1alpha2/common_types.go | 76 +++ go/api/v1alpha2/common_types_test.go | 347 ++++++++++ go/api/v1alpha2/remotemcpserver_types.go | 7 + go/api/v1alpha2/zz_generated.deepcopy.go | 30 + go/config/crd/bases/kagent.dev_agents.yaml | 73 +++ .../bases/kagent.dev_remotemcpservers.yaml | 72 +++ .../translator/agent/adk_api_translator.go | 30 + .../agent/adk_api_translator_test.go | 595 ++++++++++++++++++ .../templates/kagent.dev_agents.yaml | 73 +++ .../kagent.dev_remotemcpservers.yaml | 72 +++ 11 files changed, 1383 insertions(+) create mode 100644 go/api/v1alpha2/common_types_test.go create mode 100644 go/internal/controller/translator/agent/adk_api_translator_test.go diff --git a/go/api/v1alpha2/agent_types.go b/go/api/v1alpha2/agent_types.go index f9c958c5f..5fdb9f199 100644 --- a/go/api/v1alpha2/agent_types.go +++ b/go/api/v1alpha2/agent_types.go @@ -59,6 +59,14 @@ type AgentSpec struct { // and made available to the agent under the `/skills` folder. // +optional Skills *SkillForAgent `json:"skills,omitempty"` + + // AllowedNamespaces defines which namespaces are allowed to reference this Agent as a tool. + // This follows the Gateway API pattern for cross-namespace route attachments. + // If not specified, only Agents in the same namespace can reference this Agent as a tool. + // This field only applies when this Agent is used as a tool by another Agent. + // See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + // +optional + AllowedNamespaces *AllowedNamespaces `json:"allowedNamespaces,omitempty"` } type SkillForAgent struct { diff --git a/go/api/v1alpha2/common_types.go b/go/api/v1alpha2/common_types.go index fd5103f03..efdc6a62c 100644 --- a/go/api/v1alpha2/common_types.go +++ b/go/api/v1alpha2/common_types.go @@ -21,10 +21,86 @@ import ( "fmt" "github.com/kagent-dev/kagent/go/internal/utils" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) +// FromNamespaces specifies namespace from which references to this resource are allowed. +// This follows the same pattern as Gateway API's cross-namespace route attachment. +// See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing +// +kubebuilder:validation:Enum=All;Same;Selector +type FromNamespaces string + +const ( + // NamespacesFromAll allows references from all namespaces. + NamespacesFromAll FromNamespaces = "All" + // NamespacesFromSame only allows references from the same namespace as the target resource (default). + NamespacesFromSame FromNamespaces = "Same" + // NamespacesFromSelector allows references from namespaces matching the selector. + NamespacesFromSelector FromNamespaces = "Selector" +) + +// AllowedNamespaces defines which namespaces are allowed to reference this resource. +// This mechanism provides a bidirectional handshake for cross-namespace references, +// following the pattern used by Gateway API for cross-namespace route attachments. +// +// By default (when not specified), only references from the same namespace are allowed. +// +kubebuilder:validation:XValidation:rule="!(self.from == 'Selector' && !has(self.selector))",message="selector must be specified when from is Selector" +type AllowedNamespaces struct { + // From indicates where references to this resource can originate. + // Possible values are: + // * All: References from all namespaces are allowed. + // * Same: Only references from the same namespace are allowed (default). + // * Selector: References from namespaces matching the selector are allowed. + // +kubebuilder:default=Same + // +optional + From FromNamespaces `json:"from,omitempty"` + + // Selector is a label selector for namespaces that are allowed to reference this resource. + // Only used when From is set to "Selector". + // +optional + Selector *metav1.LabelSelector `json:"selector,omitempty"` +} + +// AllowsNamespace checks if a reference from the given namespace is allowed. +// The targetNamespace is the namespace where the resource being referenced lives. +// The sourceNamespace is the namespace where the referencing resource lives. +func (a *AllowedNamespaces) AllowsNamespace(ctx context.Context, c client.Client, sourceNamespace, targetNamespace string) (bool, error) { + // If AllowedNamespaces is nil, default to same namespace only + if a == nil { + return sourceNamespace == targetNamespace, nil + } + + switch a.From { + case NamespacesFromAll: + return true, nil + case NamespacesFromSame, "": + return sourceNamespace == targetNamespace, nil + case NamespacesFromSelector: + if a.Selector == nil { + return false, fmt.Errorf("selector must be specified when from is Selector") + } + + // Get the source namespace to check its labels + ns := &corev1.Namespace{} + if err := c.Get(ctx, types.NamespacedName{Name: sourceNamespace}, ns); err != nil { + return false, fmt.Errorf("failed to get namespace %s: %w", sourceNamespace, err) + } + + selector, err := metav1.LabelSelectorAsSelector(a.Selector) + if err != nil { + return false, fmt.Errorf("invalid label selector: %w", err) + } + + return selector.Matches(labels.Set(ns.Labels)), nil + default: + return false, fmt.Errorf("unknown from value: %s", a.From) + } +} + type ValueSourceType string const ( diff --git a/go/api/v1alpha2/common_types_test.go b/go/api/v1alpha2/common_types_test.go new file mode 100644 index 000000000..c2b1aba09 --- /dev/null +++ b/go/api/v1alpha2/common_types_test.go @@ -0,0 +1,347 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha2 + +import ( + "context" + "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" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestAllowedNamespaces_AllowsNamespace(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + + // Create test namespaces + namespaces := []runtime.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "target-ns", + }, + }, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-ns", + }, + }, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labeled-ns", + Labels: map[string]string{ + "shared-access": "true", + "team": "platform", + }, + }, + }, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-labels-ns", + Labels: map[string]string{}, + }, + }, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted-ns", + Labels: map[string]string{ + "restricted": "true", + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(namespaces...). + Build() + + ctx := context.Background() + + tests := []struct { + name string + allowedNs *AllowedNamespaces + sourceNamespace string + targetNamespace string + wantAllowed bool + wantErr bool + errContains string + }{ + { + name: "nil AllowedNamespaces defaults to same namespace only - same ns", + allowedNs: nil, + sourceNamespace: "target-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "nil AllowedNamespaces defaults to same namespace only - different ns", + allowedNs: nil, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "From=Same allows same namespace", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSame, + }, + sourceNamespace: "target-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Same denies different namespace", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSame, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "Empty From defaults to Same - same ns", + allowedNs: &AllowedNamespaces{ + From: "", + }, + sourceNamespace: "target-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "Empty From defaults to Same - different ns", + allowedNs: &AllowedNamespaces{ + From: "", + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "From=All allows any namespace", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromAll, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=All allows same namespace", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromAll, + }, + sourceNamespace: "target-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector allows namespace with matching label", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + }, + }, + }, + sourceNamespace: "labeled-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector denies namespace without matching label", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + }, + }, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "From=Selector with multiple labels - all match", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + "team": "platform", + }, + }, + }, + sourceNamespace: "labeled-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector with multiple labels - partial match fails", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + "team": "other-team", + }, + }, + }, + sourceNamespace: "labeled-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "From=Selector with MatchExpressions - In operator", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "team", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"platform", "infra"}, + }, + }, + }, + }, + sourceNamespace: "labeled-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector with MatchExpressions - Exists operator", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "shared-access", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + sourceNamespace: "labeled-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector with empty selector matches all namespaces", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{}, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector with MatchExpressions - DoesNotExist operator allows unrestricted", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "restricted", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantAllowed: true, + }, + { + name: "From=Selector with MatchExpressions - DoesNotExist operator denies restricted", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "restricted", + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + }, + }, + sourceNamespace: "restricted-ns", + targetNamespace: "target-ns", + wantAllowed: false, + }, + { + name: "From=Selector without selector returns error", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: nil, + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantErr: true, + errContains: "selector must be specified", + }, + { + name: "From=Selector with non-existent namespace returns error", + allowedNs: &AllowedNamespaces{ + From: NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + }, + }, + }, + sourceNamespace: "non-existent-ns", + targetNamespace: "target-ns", + wantErr: true, + errContains: "failed to get namespace", + }, + { + name: "Unknown From value returns error", + allowedNs: &AllowedNamespaces{ + From: "Unknown", + }, + sourceNamespace: "source-ns", + targetNamespace: "target-ns", + wantErr: true, + errContains: "unknown from value", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allowed, err := tt.allowedNs.AllowsNamespace(ctx, fakeClient, tt.sourceNamespace, tt.targetNamespace) + + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantAllowed, allowed) + }) + } +} diff --git a/go/api/v1alpha2/remotemcpserver_types.go b/go/api/v1alpha2/remotemcpserver_types.go index f50efb843..f8a355894 100644 --- a/go/api/v1alpha2/remotemcpserver_types.go +++ b/go/api/v1alpha2/remotemcpserver_types.go @@ -52,6 +52,13 @@ type RemoteMCPServerSpec struct { // +optional // +kubebuilder:default=true TerminateOnClose *bool `json:"terminateOnClose,omitempty"` + + // AllowedNamespaces defines which namespaces are allowed to reference this RemoteMCPServer. + // This follows the Gateway API pattern for cross-namespace route attachments. + // If not specified, only Agents in the same namespace can reference this RemoteMCPServer. + // See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + // +optional + AllowedNamespaces *AllowedNamespaces `json:"allowedNamespaces,omitempty"` } var _ sql.Scanner = (*RemoteMCPServerSpec)(nil) diff --git a/go/api/v1alpha2/zz_generated.deepcopy.go b/go/api/v1alpha2/zz_generated.deepcopy.go index 87e49ab21..eebc11d17 100644 --- a/go/api/v1alpha2/zz_generated.deepcopy.go +++ b/go/api/v1alpha2/zz_generated.deepcopy.go @@ -165,6 +165,11 @@ func (in *AgentSpec) DeepCopyInto(out *AgentSpec) { *out = new(SkillForAgent) (*in).DeepCopyInto(*out) } + if in.AllowedNamespaces != nil { + in, out := &in.AllowedNamespaces, &out.AllowedNamespaces + *out = new(AllowedNamespaces) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AgentSpec. @@ -199,6 +204,26 @@ func (in *AgentStatus) DeepCopy() *AgentStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AllowedNamespaces) DeepCopyInto(out *AllowedNamespaces) { + *out = *in + if in.Selector != nil { + in, out := &in.Selector, &out.Selector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllowedNamespaces. +func (in *AllowedNamespaces) DeepCopy() *AllowedNamespaces { + if in == nil { + return nil + } + out := new(AllowedNamespaces) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AnthropicConfig) DeepCopyInto(out *AnthropicConfig) { *out = *in @@ -734,6 +759,11 @@ func (in *RemoteMCPServerSpec) DeepCopyInto(out *RemoteMCPServerSpec) { *out = new(bool) **out = **in } + if in.AllowedNamespaces != nil { + in, out := &in.AllowedNamespaces, &out.AllowedNamespaces + *out = new(AllowedNamespaces) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RemoteMCPServerSpec. diff --git a/go/config/crd/bases/kagent.dev_agents.yaml b/go/config/crd/bases/kagent.dev_agents.yaml index aaf5b8262..f490530e4 100644 --- a/go/config/crd/bases/kagent.dev_agents.yaml +++ b/go/config/crd/bases/kagent.dev_agents.yaml @@ -2377,6 +2377,79 @@ spec: spec: description: AgentSpec defines the desired state of Agent. properties: + allowedNamespaces: + description: |- + AllowedNamespaces defines which namespaces are allowed to reference this Agent as a tool. + This follows the Gateway API pattern for cross-namespace route attachments. + If not specified, only Agents in the same namespace can reference this Agent as a tool. + This field only applies when this Agent is used as a tool by another Agent. + See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + properties: + from: + default: Same + description: |- + From indicates where references to this resource can originate. + Possible values are: + * All: References from all namespaces are allowed. + * Same: Only references from the same namespace are allowed (default). + * Selector: References from namespaces matching the selector are allowed. + enum: + - All + - Same + - Selector + type: string + selector: + description: |- + Selector is a label selector for namespaces that are allowed to reference this resource. + Only used when From is set to "Selector". + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: selector must be specified when from is Selector + rule: '!(self.from == ''Selector'' && !has(self.selector))' byo: properties: deployment: diff --git a/go/config/crd/bases/kagent.dev_remotemcpservers.yaml b/go/config/crd/bases/kagent.dev_remotemcpservers.yaml index e83b4935a..534c27b35 100644 --- a/go/config/crd/bases/kagent.dev_remotemcpservers.yaml +++ b/go/config/crd/bases/kagent.dev_remotemcpservers.yaml @@ -53,6 +53,78 @@ spec: spec: description: RemoteMCPServerSpec defines the desired state of RemoteMCPServer. properties: + allowedNamespaces: + description: |- + AllowedNamespaces defines which namespaces are allowed to reference this RemoteMCPServer. + This follows the Gateway API pattern for cross-namespace route attachments. + If not specified, only Agents in the same namespace can reference this RemoteMCPServer. + See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + properties: + from: + default: Same + description: |- + From indicates where references to this resource can originate. + Possible values are: + * All: References from all namespaces are allowed. + * Same: Only references from the same namespace are allowed (default). + * Selector: References from namespaces matching the selector are allowed. + enum: + - All + - Same + - Selector + type: string + selector: + description: |- + Selector is a label selector for namespaces that are allowed to reference this resource. + Only used when From is set to "Selector". + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: selector must be specified when from is Selector + rule: '!(self.from == ''Selector'' && !has(self.selector))' description: type: string headersFrom: diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index c36ad6691..662efbdac 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -538,6 +538,15 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al return nil, nil, nil, err } + // Check cross-namespace reference is allowed + allowed, err := toolAgent.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, agent.Namespace, toolAgent.Namespace) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to check cross-namespace reference for agent %s: %w", agentRef, err) + } + if !allowed { + return nil, nil, nil, fmt.Errorf("cross-namespace reference to agent %s is not allowed from namespace %s", agentRef, agent.Namespace) + } + switch toolAgent.Spec.Type { case v1alpha2.AgentType_BYO, v1alpha2.AgentType_Declarative: url := fmt.Sprintf("http://%s.%s:8080", toolAgent.Name, toolAgent.Namespace) @@ -973,6 +982,12 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } + // MCPServer type doesn't support cross-namespace references (external type) + // Only same-namespace references are allowed + if mcpServerRef.Namespace != agentNamespace { + return fmt.Errorf("cross-namespace reference to MCPServer %s is not allowed from namespace %s: MCPServer does not support cross-namespace references", mcpServerRef, agentNamespace) + } + remoteMcpServer, err := ConvertMCPServerToRemoteMCPServer(mcpServer) if err != nil { return err @@ -996,6 +1011,15 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } + // Check cross-namespace reference is allowed + allowed, err := remoteMcpServer.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, agentNamespace, remoteMcpServer.Namespace) + if err != nil { + return fmt.Errorf("failed to check cross-namespace reference for RemoteMCPServer %s: %w", remoteMcpServerRef, err) + } + if !allowed { + return fmt.Errorf("cross-namespace reference to RemoteMCPServer %s is not allowed from namespace %s", remoteMcpServerRef, agentNamespace) + } + return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders) case schema.GroupKind{ Group: "", @@ -1014,6 +1038,12 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } + // Service type doesn't support cross-namespace references (external type) + // Only same-namespace references are allowed + if svcRef.Namespace != agentNamespace { + return fmt.Errorf("cross-namespace reference to Service %s is not allowed from namespace %s: Service does not support cross-namespace references", svcRef, agentNamespace) + } + remoteMcpServer, err := ConvertServiceToRemoteMCPServer(svc) if err != nil { return err diff --git a/go/internal/controller/translator/agent/adk_api_translator_test.go b/go/internal/controller/translator/agent/adk_api_translator_test.go new file mode 100644 index 000000000..be8daa691 --- /dev/null +++ b/go/internal/controller/translator/agent/adk_api_translator_test.go @@ -0,0 +1,595 @@ +package agent_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kagent-dev/kagent/go/api/v1alpha2" + translator "github.com/kagent-dev/kagent/go/internal/controller/translator/agent" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + schemev1 "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { + scheme := schemev1.Scheme + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + // Create test namespaces + sourceNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-ns", + Labels: map[string]string{ + "shared-agent-access": "true", + }, + }, + } + targetNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "target-ns", + }, + } + unlabeledNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unlabeled-ns", + }, + } + + // Create model config in source namespace + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-model", + Namespace: "source-ns", + }, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-4", + Provider: v1alpha2.ModelProviderOpenAI, + }, + } + + tests := []struct { + name string + toolAgent *v1alpha2.Agent + sourceAgent *v1alpha2.Agent + wantErr bool + errContains string + }{ + { + name: "Same namespace reference - allowed by default", + toolAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Tool agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a tool agent", + ModelConfig: "test-model", + }, + // No AllowedNamespaces = same namespace only + }, + }, + sourceAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-agent", + Namespace: "source-ns", // Same namespace as tool agent + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Source agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a source agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "source-ns", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace reference - denied by default (no AllowedNamespaces)", + toolAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Tool agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a tool agent", + ModelConfig: "test-model", + }, + // No AllowedNamespaces = same namespace only + }, + }, + sourceAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-agent", + Namespace: "source-ns", // Different namespace from tool agent + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Source agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a source agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "cross-namespace reference to agent", + }, + { + name: "Cross-namespace reference - allowed with From=All", + toolAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Tool agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a tool agent", + ModelConfig: "test-model", + }, + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromAll, + }, + }, + }, + sourceAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-agent", + Namespace: "source-ns", // Different namespace as tool agent + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Source agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a source agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace reference - allowed with matching selector", + toolAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Tool agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a tool agent", + ModelConfig: "test-model", + }, + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-agent-access": "true", + }, + }, + }, + }, + }, + sourceAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-agent", + Namespace: "source-ns", // Has label "shared-agent-access": "true" + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Source agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a source agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace reference - denied with non-matching selector", + toolAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Tool agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a tool agent", + ModelConfig: "test-model", + }, + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-agent-access": "true", + }, + }, + }, + }, + }, + sourceAgent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-agent", + Namespace: "unlabeled-ns", // Does NOT have the required label `shared-agent-access` + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Source agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are a source agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "cross-namespace reference to agent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientBuilder := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects( + sourceNs, + targetNs, + unlabeledNs, + tt.toolAgent, + tt.sourceAgent, + ) + + // Create model config in source agent namespace + modelConfigSource := modelConfig.DeepCopy() + modelConfigSource.Namespace = tt.sourceAgent.Namespace + clientBuilder = clientBuilder.WithObjects(modelConfigSource) + + // Also need model config in tool agent namespace for the tool agent to be valid (if different) + if tt.toolAgent.Namespace != tt.sourceAgent.Namespace { + toolModelConfig := modelConfig.DeepCopy() + toolModelConfig.Namespace = tt.toolAgent.Namespace + clientBuilder = clientBuilder.WithObjects(toolModelConfig) + } + + kubeClient := clientBuilder.Build() + + defaultModel := types.NamespacedName{ + Namespace: tt.sourceAgent.Namespace, + Name: "test-model", + } + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil) + + _, err := trans.TranslateAgent(context.Background(), tt.sourceAgent) + + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + }) + } +} + +func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { + scheme := schemev1.Scheme + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + // Create test namespaces + sourceNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-ns", + Labels: map[string]string{ + "shared-tools-access": "true", + }, + }, + } + targetNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "target-ns", + }, + } + + // Create model config + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-model", + Namespace: "source-ns", + }, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-4", + Provider: v1alpha2.ModelProviderOpenAI, + }, + } + + tests := []struct { + name string + remoteMCPServer *v1alpha2.RemoteMCPServer + agent *v1alpha2.Agent + wantErr bool + errContains string + }{ + { + name: "Same namespace reference - allowed by default", + remoteMCPServer: &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "source-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + Description: "Tools server", + URL: "http://tools.example.com/mcp", + // No AllowedNamespaces = same namespace only + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are an agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "source-ns", + }, + ToolNames: []string{"tool1"}, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace reference - denied by default", + remoteMCPServer: &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "target-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + Description: "Tools server", + URL: "http://tools.example.com/mcp", + // No AllowedNamespaces = same namespace only + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are an agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "target-ns", + }, + ToolNames: []string{"tool1"}, + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "cross-namespace reference to RemoteMCPServer", + }, + { + name: "Cross-namespace reference - allowed with From=All", + remoteMCPServer: &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "target-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + Description: "Tools server", + URL: "http://tools.example.com/mcp", + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromAll, + }, + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are an agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "target-ns", + }, + ToolNames: []string{"tool1"}, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace reference - allowed with matching selector", + remoteMCPServer: &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "target-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + Description: "Tools server", + URL: "http://tools.example.com/mcp", + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-tools-access": "true", + }, + }, + }, + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent", + Namespace: "source-ns", // Has label "shared-tools-access": "true" + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are an agent", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "target-ns", + }, + ToolNames: []string{"tool1"}, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects( + sourceNs, + targetNs, + modelConfig, + tt.remoteMCPServer, + tt.agent, + ). + Build() + + defaultModel := types.NamespacedName{ + Namespace: tt.agent.Namespace, + Name: "test-model", + } + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil) + + _, err := trans.TranslateAgent(context.Background(), tt.agent) + + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + }) + } +} diff --git a/helm/kagent-crds/templates/kagent.dev_agents.yaml b/helm/kagent-crds/templates/kagent.dev_agents.yaml index aaf5b8262..f490530e4 100644 --- a/helm/kagent-crds/templates/kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/kagent.dev_agents.yaml @@ -2377,6 +2377,79 @@ spec: spec: description: AgentSpec defines the desired state of Agent. properties: + allowedNamespaces: + description: |- + AllowedNamespaces defines which namespaces are allowed to reference this Agent as a tool. + This follows the Gateway API pattern for cross-namespace route attachments. + If not specified, only Agents in the same namespace can reference this Agent as a tool. + This field only applies when this Agent is used as a tool by another Agent. + See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + properties: + from: + default: Same + description: |- + From indicates where references to this resource can originate. + Possible values are: + * All: References from all namespaces are allowed. + * Same: Only references from the same namespace are allowed (default). + * Selector: References from namespaces matching the selector are allowed. + enum: + - All + - Same + - Selector + type: string + selector: + description: |- + Selector is a label selector for namespaces that are allowed to reference this resource. + Only used when From is set to "Selector". + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: selector must be specified when from is Selector + rule: '!(self.from == ''Selector'' && !has(self.selector))' byo: properties: deployment: diff --git a/helm/kagent-crds/templates/kagent.dev_remotemcpservers.yaml b/helm/kagent-crds/templates/kagent.dev_remotemcpservers.yaml index e83b4935a..534c27b35 100644 --- a/helm/kagent-crds/templates/kagent.dev_remotemcpservers.yaml +++ b/helm/kagent-crds/templates/kagent.dev_remotemcpservers.yaml @@ -53,6 +53,78 @@ spec: spec: description: RemoteMCPServerSpec defines the desired state of RemoteMCPServer. properties: + allowedNamespaces: + description: |- + AllowedNamespaces defines which namespaces are allowed to reference this RemoteMCPServer. + This follows the Gateway API pattern for cross-namespace route attachments. + If not specified, only Agents in the same namespace can reference this RemoteMCPServer. + See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-routing + properties: + from: + default: Same + description: |- + From indicates where references to this resource can originate. + Possible values are: + * All: References from all namespaces are allowed. + * Same: Only references from the same namespace are allowed (default). + * Selector: References from namespaces matching the selector are allowed. + enum: + - All + - Same + - Selector + type: string + selector: + description: |- + Selector is a label selector for namespaces that are allowed to reference this resource. + Only used when From is set to "Selector". + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + x-kubernetes-validations: + - message: selector must be specified when from is Selector + rule: '!(self.from == ''Selector'' && !has(self.selector))' description: type: string headersFrom: From 491d8172866a40d37c54af8bc0366c027253f706 Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:44:59 +0100 Subject: [PATCH 6/7] fix: ensure agents can't reference tools in unwatched namespaces Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- .../controller/reconciler/reconciler.go | 63 +++++ .../controller/reconciler/reconciler_test.go | 267 ++++++++++++++++++ go/pkg/app/app.go | 1 + 3 files changed, 331 insertions(+) diff --git a/go/internal/controller/reconciler/reconciler.go b/go/internal/controller/reconciler/reconciler.go index 5bb1338ec..f39ff65ec 100644 --- a/go/internal/controller/reconciler/reconciler.go +++ b/go/internal/controller/reconciler/reconciler.go @@ -58,6 +58,10 @@ type kagentReconciler struct { defaultModelConfig types.NamespacedName + // watchedNamespaces is the list of namespaces the controller watches. + // An empty list means watching all namespaces. + watchedNamespaces []string + // TODO: Remove this lock since we have a DB which we can batch anyway upsertLock sync.Mutex } @@ -67,12 +71,14 @@ func NewKagentReconciler( kube client.Client, dbClient database.Client, defaultModelConfig types.NamespacedName, + watchedNamespaces []string, ) KagentReconciler { return &kagentReconciler{ adkTranslator: translator, kube: kube, dbClient: dbClient, defaultModelConfig: defaultModelConfig, + watchedNamespaces: watchedNamespaces, } } @@ -477,7 +483,57 @@ func (a *kagentReconciler) reconcileRemoteMCPServerStatus( return nil } +// validateCrossNamespaceReferences validates that all cross-namespace references +// in the agent's tools target namespaces that are watched by the controller. +// This prevents agents from referencing tools or agents in namespaces that the +// controller cannot access. +func (a *kagentReconciler) validateCrossNamespaceReferences(agent *v1alpha2.Agent) error { + if agent.Spec.Type != v1alpha2.AgentType_Declarative || agent.Spec.Declarative == nil { + return nil + } + + for _, tool := range agent.Spec.Declarative.Tools { + var targetNamespace string + var refKind string + var refName string + + switch { + case tool.McpServer != nil: + targetNamespace = tool.McpServer.Namespace + if targetNamespace == "" { + targetNamespace = agent.Namespace + } + refKind = tool.McpServer.Kind + if refKind == "" { + refKind = "MCPServer" + } + refName = tool.McpServer.Name + case tool.Agent != nil: + targetNamespace = tool.Agent.Namespace + if targetNamespace == "" { + targetNamespace = agent.Namespace + } + refKind = "Agent" + refName = tool.Agent.Name + default: + continue + } + + if !a.isNamespaceWatched(targetNamespace) { + return fmt.Errorf("cannot reference %s %s/%s: namespace %q is not watched by the controller", + refKind, targetNamespace, refName, targetNamespace) + } + } + + return nil +} + func (a *kagentReconciler) reconcileAgent(ctx context.Context, agent *v1alpha2.Agent) error { + // Validate that all cross-namespace references target watched namespaces + if err := a.validateCrossNamespaceReferences(agent); err != nil { + return err + } + agentOutputs, err := a.adkTranslator.TranslateAgent(ctx, agent) if err != nil { return fmt.Errorf("failed to translate agent %s/%s: %v", agent.Namespace, agent.Name, err) @@ -664,6 +720,13 @@ func (a *kagentReconciler) upsertToolServerForRemoteMCPServer(ctx context.Contex return tools, nil } +func (a *kagentReconciler) isNamespaceWatched(namespace string) bool { + if len(a.watchedNamespaces) == 0 { + return true + } + return slices.Contains(a.watchedNamespaces, namespace) +} + func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.RemoteMCPServer) (transport.Interface, error) { headers, err := s.ResolveHeaders(ctx, a.kube) if err != nil { diff --git a/go/internal/controller/reconciler/reconciler_test.go b/go/internal/controller/reconciler/reconciler_test.go index 92f04882f..3bc81ea7c 100644 --- a/go/internal/controller/reconciler/reconciler_test.go +++ b/go/internal/controller/reconciler/reconciler_test.go @@ -3,8 +3,10 @@ package reconciler import ( "testing" + "github.com/kagent-dev/kagent/go/api/v1alpha2" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -191,3 +193,268 @@ func TestComputeStatusSecretHash_Deterministic(t *testing.T) { }) } } + +func TestValidateCrossNamespaceReferences(t *testing.T) { + tests := []struct { + name string + watchedNamespaces []string + agent *v1alpha2.Agent + wantErr bool + errContains string + }{ + { + name: "BYO agent - no validation needed", + watchedNamespaces: []string{"ns1"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_BYO, + }, + }, + wantErr: false, + }, + { + name: "Declarative agent with no tools - passes", + watchedNamespaces: []string{"ns1"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + }, + }, + }, + wantErr: false, + }, + { + name: "Watch all namespaces (empty list) - allows any namespace", + watchedNamespaces: []string{}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "other-agent", + Namespace: "any-namespace", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Agent tool in watched namespace - passes", + watchedNamespaces: []string{"ns1", "ns2"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "other-agent", + Namespace: "ns2", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Agent tool in unwatched namespace - fails", + watchedNamespaces: []string{"ns1", "ns2"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "other-agent", + Namespace: "ns3", + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "namespace \"ns3\" is not watched by the controller", + }, + { + name: "McpServer tool in watched namespace - passes", + watchedNamespaces: []string{"ns1", "tools-ns"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "tools-ns", + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "McpServer tool in unwatched namespace - fails", + watchedNamespaces: []string{"ns1"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "tools-ns", + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "namespace \"tools-ns\" is not watched by the controller", + }, + { + name: "Tool with empty namespace defaults to agent namespace - passes", + watchedNamespaces: []string{"ns1"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "other-agent", + Namespace: "", // defaults to agent's namespace + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Multiple tools - one in unwatched namespace - fails", + watchedNamespaces: []string{"ns1", "ns2"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "ns1", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "agent-in-ns2", + Namespace: "ns2", + }, + }, + { + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "RemoteMCPServer", + ApiGroup: "kagent.dev", + Name: "tools-server", + Namespace: "unwatched-ns", + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + errContains: "namespace \"unwatched-ns\" is not watched by the controller", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reconciler := &kagentReconciler{ + watchedNamespaces: tt.watchedNamespaces, + } + + err := reconciler.validateCrossNamespaceReferences(tt.agent) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/go/pkg/app/app.go b/go/pkg/app/app.go index 8ee3b9589..312d0392d 100644 --- a/go/pkg/app/app.go +++ b/go/pkg/app/app.go @@ -379,6 +379,7 @@ func Start(getExtensionConfig GetExtensionConfig) { mgr.GetClient(), dbClient, cfg.DefaultModelConfig, + watchNamespacesList, ) if err := (&controller.ServiceController{ From b28a116bb43cc88cfa3361dc01ba7e096deafe76 Mon Sep 17 00:00:00 2001 From: Brian Fox <878612+onematchfox@users.noreply.github.com> Date: Fri, 12 Dec 2025 10:06:13 +0100 Subject: [PATCH 7/7] refactor: consolidate cross-namespace validation in controller Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com> --- .../controller/reconciler/reconciler.go | 133 ++++++-- .../controller/reconciler/reconciler_test.go | 308 ++++++++++++++---- .../translator/agent/adk_api_translator.go | 31 -- .../agent/adk_api_translator_test.go | 266 +-------------- 4 files changed, 369 insertions(+), 369 deletions(-) diff --git a/go/internal/controller/reconciler/reconciler.go b/go/internal/controller/reconciler/reconciler.go index f39ff65ec..ba859f2f1 100644 --- a/go/internal/controller/reconciler/reconciler.go +++ b/go/internal/controller/reconciler/reconciler.go @@ -483,54 +483,129 @@ func (a *kagentReconciler) reconcileRemoteMCPServerStatus( return nil } -// validateCrossNamespaceReferences validates that all cross-namespace references -// in the agent's tools target namespaces that are watched by the controller. -// This prevents agents from referencing tools or agents in namespaces that the -// controller cannot access. -func (a *kagentReconciler) validateCrossNamespaceReferences(agent *v1alpha2.Agent) error { +// validateCrossNamespaceReferences validates that any cross-namespace +// references in the agent's tools target namespaces that are watched by the +// controller. This prevents agents from referencing tools or agents in +// namespaces that the controller cannot access. +func (a *kagentReconciler) validateCrossNamespaceReferences(ctx context.Context, agent *v1alpha2.Agent) error { if agent.Spec.Type != v1alpha2.AgentType_Declarative || agent.Spec.Declarative == nil { return nil } for _, tool := range agent.Spec.Declarative.Tools { - var targetNamespace string - var refKind string - var refName string - switch { case tool.McpServer != nil: - targetNamespace = tool.McpServer.Namespace - if targetNamespace == "" { - targetNamespace = agent.Namespace - } - refKind = tool.McpServer.Kind - if refKind == "" { - refKind = "MCPServer" + if err := a.validateMcpServerReference(ctx, agent.Namespace, tool.McpServer); err != nil { + return err } - refName = tool.McpServer.Name case tool.Agent != nil: - targetNamespace = tool.Agent.Namespace - if targetNamespace == "" { - targetNamespace = agent.Namespace + if err := a.validateAgentReference(ctx, agent.Namespace, tool.Agent); err != nil { + return err } - refKind = "Agent" - refName = tool.Agent.Name - default: - continue } + } + + return nil +} + +// validateAgentReference validates a reference to an Agent as a tool. +// This includes: +// 1. Checking that target namespaces are watched by the controller +// 2. Checking that the target Agent allows references from the agent's namespace +func (a *kagentReconciler) validateAgentReference(ctx context.Context, sourceNamespace string, ref *v1alpha2.TypedLocalReference) error { + agentRef := ref.NamespacedName(sourceNamespace) + + // Same namespace references are always allowed + if agentRef.Namespace == sourceNamespace { + return nil + } - if !a.isNamespaceWatched(targetNamespace) { - return fmt.Errorf("cannot reference %s %s/%s: namespace %q is not watched by the controller", - refKind, targetNamespace, refName, targetNamespace) + // Check if the target namespace is watched by the controller + if !a.isNamespaceWatched(agentRef.Namespace) { + return fmt.Errorf("cannot reference Agent %s: namespace %q is not watched by the controller", + agentRef, agentRef.Namespace) + } + + // For cross-namespace references, check AllowedNamespaces on the target agent + targetAgent := &v1alpha2.Agent{} + if err := a.kube.Get(ctx, agentRef, targetAgent); err != nil { + return fmt.Errorf("failed to get agent %s: %w", agentRef, err) + } + + allowed, err := targetAgent.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, sourceNamespace, targetAgent.Namespace) + if err != nil { + return fmt.Errorf("failed to check cross-namespace reference for agent %s: %w", agentRef, err) + } + if !allowed { + return fmt.Errorf("cross-namespace reference to agent %s is not allowed from namespace %s", agentRef, sourceNamespace) + } + + return nil +} + +// validateMcpServerReference validates a reference to an MCP server tool. This +// includes: +// 1. Enforcing same-namespace-only for MCPServer and Service (external types) +// 2. Checking that target namespaces are watched by the controller +// 3. Checking that the target resource allows references from the agent's namespace +func (a *kagentReconciler) validateMcpServerReference(ctx context.Context, sourceNamespace string, ref *v1alpha2.McpServerTool) error { + gk := ref.GroupKind() + targetRef := ref.NamespacedName(sourceNamespace) + + // Same namespace references are always allowed + if targetRef.Namespace == sourceNamespace { + return nil + } + + // Handle based on the type of MCP server + switch gk { + case schema.GroupKind{Group: "", Kind: ""}, // TODO: This matches the translator's current fallthrough logic which defaults to MCPServer. That logic is likely a legacy of the inline KMCP support and should probably be adjusted to default to the first-class RemoteMCPServer CRD instead. + schema.GroupKind{Group: "", Kind: "MCPServer"}, + schema.GroupKind{Group: "kagent.dev", Kind: "MCPServer"}: + // MCPServer type doesn't support cross-namespace references (external type) + return fmt.Errorf("cross-namespace reference to MCPServer %s is not allowed from namespace %s: MCPServer does not support cross-namespace references", + targetRef, sourceNamespace) + + case schema.GroupKind{Group: "", Kind: "RemoteMCPServer"}, + schema.GroupKind{Group: "kagent.dev", Kind: "RemoteMCPServer"}: + + // Check if the target namespace is watched by the controller + if !a.isNamespaceWatched(targetRef.Namespace) { + kind := ref.Kind + if kind == "" { + kind = "MCPServer" + } + return fmt.Errorf("cannot reference %s %s: namespace %q is not watched by the controller", + kind, targetRef, targetRef.Namespace) + } + + // For RemoteMCPServer, check AllowedNamespaces + remoteMcpServer := &v1alpha2.RemoteMCPServer{} + if err := a.kube.Get(ctx, targetRef, remoteMcpServer); err != nil { + return fmt.Errorf("failed to get RemoteMCPServer %s: %w", targetRef, err) + } + + allowed, err := remoteMcpServer.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, sourceNamespace, remoteMcpServer.Namespace) + if err != nil { + return fmt.Errorf("failed to check cross-namespace reference for RemoteMCPServer %s: %w", targetRef, err) } + if !allowed { + return fmt.Errorf("cross-namespace reference to RemoteMCPServer %s is not allowed from namespace %s", targetRef, sourceNamespace) + } + + case schema.GroupKind{Group: "", Kind: "Service"}, + schema.GroupKind{Group: "core", Kind: "Service"}: + // Service type doesn't support cross-namespace references (external type) + return fmt.Errorf("cross-namespace reference to Service %s is not allowed from namespace %s: Service does not support cross-namespace references", + targetRef, sourceNamespace) } return nil } func (a *kagentReconciler) reconcileAgent(ctx context.Context, agent *v1alpha2.Agent) error { - // Validate that all cross-namespace references target watched namespaces - if err := a.validateCrossNamespaceReferences(agent); err != nil { + // Validate that any cross-namespace references are allowed + if err := a.validateCrossNamespaceReferences(ctx, agent); err != nil { return err } diff --git a/go/internal/controller/reconciler/reconciler_test.go b/go/internal/controller/reconciler/reconciler_test.go index 3bc81ea7c..f7e838b0a 100644 --- a/go/internal/controller/reconciler/reconciler_test.go +++ b/go/internal/controller/reconciler/reconciler_test.go @@ -1,13 +1,19 @@ package reconciler import ( + "context" "testing" "github.com/kagent-dev/kagent/go/api/v1alpha2" "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" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) // TestComputeStatusSecretHash_Output verifies the output of the hash function @@ -195,20 +201,40 @@ func TestComputeStatusSecretHash_Deterministic(t *testing.T) { } func TestValidateCrossNamespaceReferences(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + // Create test namespaces + sourceNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-ns", + Labels: map[string]string{ + "shared-access": "true", + }, + }, + } + targetNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "target-ns", + }, + } + tests := []struct { name string watchedNamespaces []string + objects []client.Object // Additional objects to create in fake client agent *v1alpha2.Agent wantErr bool errContains string }{ { name: "BYO agent - no validation needed", - watchedNamespaces: []string{"ns1"}, + watchedNamespaces: []string{"source-ns"}, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_BYO, @@ -218,11 +244,11 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { }, { name: "Declarative agent with no tools - passes", - watchedNamespaces: []string{"ns1"}, + watchedNamespaces: []string{"source-ns"}, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -234,12 +260,12 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { wantErr: false, }, { - name: "Watch all namespaces (empty list) - allows any namespace", - watchedNamespaces: []string{}, + name: "Agent tool in unwatched namespace - fails", + watchedNamespaces: []string{"source-ns"}, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -250,22 +276,35 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { Type: v1alpha2.ToolProviderType_Agent, Agent: &v1alpha2.TypedLocalReference{ Name: "other-agent", - Namespace: "any-namespace", + Namespace: "unwatched-ns", }, }, }, }, }, }, - wantErr: false, + wantErr: true, + errContains: "namespace \"unwatched-ns\" is not watched by the controller", }, { - name: "Agent tool in watched namespace - passes", - watchedNamespaces: []string{"ns1", "ns2"}, + name: "Same namespace agent tool - always allowed", + watchedNamespaces: []string{"source-ns"}, + objects: []client.Object{ + &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + // No AllowedNamespaces needed for same namespace + }, + }, + }, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -275,8 +314,8 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { { Type: v1alpha2.ToolProviderType_Agent, Agent: &v1alpha2.TypedLocalReference{ - Name: "other-agent", - Namespace: "ns2", + Name: "tool-agent", + Namespace: "source-ns", }, }, }, @@ -286,12 +325,24 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { wantErr: false, }, { - name: "Agent tool in unwatched namespace - fails", - watchedNamespaces: []string{"ns1", "ns2"}, + name: "Cross-namespace agent tool - denied without AllowedNamespaces", + watchedNamespaces: []string{"source-ns", "target-ns"}, + objects: []client.Object{ + &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + // No AllowedNamespaces = same namespace only + }, + }, + }, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -301,8 +352,8 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { { Type: v1alpha2.ToolProviderType_Agent, Agent: &v1alpha2.TypedLocalReference{ - Name: "other-agent", - Namespace: "ns3", + Name: "tool-agent", + Namespace: "target-ns", }, }, }, @@ -310,15 +361,112 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { }, }, wantErr: true, - errContains: "namespace \"ns3\" is not watched by the controller", + errContains: "cross-namespace reference to agent target-ns/tool-agent is not allowed from namespace source-ns", + }, + { + name: "Cross-namespace agent tool - allowed with From=All", + watchedNamespaces: []string{"source-ns", "target-ns"}, + objects: []client.Object{ + &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromAll, + }, + }, + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Cross-namespace agent tool - allowed with matching selector", + watchedNamespaces: []string{"source-ns", "target-ns"}, + objects: []client.Object{ + &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tool-agent", + Namespace: "target-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromSelector, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "shared-access": "true", + }, + }, + }, + }, + }, + }, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "source-ns", // Has label "shared-access": "true" + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "tool-agent", + Namespace: "target-ns", + }, + }, + }, + }, + }, + }, + wantErr: false, }, { - name: "McpServer tool in watched namespace - passes", - watchedNamespaces: []string{"ns1", "tools-ns"}, + name: "Cross-namespace RemoteMCPServer - denied without AllowedNamespaces", + watchedNamespaces: []string{"source-ns", "target-ns"}, + objects: []client.Object{ + &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "target-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + URL: "http://tools.example.com/mcp", + // No AllowedNamespaces = same namespace only + }, + }, + }, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -332,7 +480,7 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { Kind: "RemoteMCPServer", ApiGroup: "kagent.dev", Name: "tools-server", - Namespace: "tools-ns", + Namespace: "target-ns", }, }, }, @@ -340,15 +488,30 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { }, }, }, - wantErr: false, + wantErr: true, + errContains: "cross-namespace reference to RemoteMCPServer target-ns/tools-server is not allowed from namespace source-ns", }, { - name: "McpServer tool in unwatched namespace - fails", - watchedNamespaces: []string{"ns1"}, + name: "Cross-namespace RemoteMCPServer - allowed with From=All", + watchedNamespaces: []string{"source-ns", "target-ns"}, + objects: []client.Object{ + &v1alpha2.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tools-server", + Namespace: "target-ns", + }, + Spec: v1alpha2.RemoteMCPServerSpec{ + URL: "http://tools.example.com/mcp", + AllowedNamespaces: &v1alpha2.AllowedNamespaces{ + From: v1alpha2.NamespacesFromAll, + }, + }, + }, + }, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -362,7 +525,7 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { Kind: "RemoteMCPServer", ApiGroup: "kagent.dev", Name: "tools-server", - Namespace: "tools-ns", + Namespace: "target-ns", }, }, }, @@ -370,16 +533,15 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { }, }, }, - wantErr: true, - errContains: "namespace \"tools-ns\" is not watched by the controller", + wantErr: false, }, { - name: "Tool with empty namespace defaults to agent namespace - passes", - watchedNamespaces: []string{"ns1"}, + name: "Cross-namespace MCPServer - always denied (external type)", + watchedNamespaces: []string{"source-ns", "target-ns"}, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, @@ -387,46 +549,44 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { SystemMessage: "test", Tools: []*v1alpha2.Tool{ { - Type: v1alpha2.ToolProviderType_Agent, - Agent: &v1alpha2.TypedLocalReference{ - Name: "other-agent", - Namespace: "", // defaults to agent's namespace + Type: v1alpha2.ToolProviderType_McpServer, + McpServer: &v1alpha2.McpServerTool{ + TypedLocalReference: v1alpha2.TypedLocalReference{ + Kind: "MCPServer", + ApiGroup: "kagent.dev", + Name: "mcp-server", + Namespace: "target-ns", + }, }, }, }, }, }, }, - wantErr: false, + wantErr: true, + errContains: "MCPServer does not support cross-namespace references", }, { - name: "Multiple tools - one in unwatched namespace - fails", - watchedNamespaces: []string{"ns1", "ns2"}, + name: "Cross-namespace Service - always denied (external type)", + watchedNamespaces: []string{"source-ns", "target-ns"}, agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "test-agent", - Namespace: "ns1", + Namespace: "source-ns", }, Spec: v1alpha2.AgentSpec{ Type: v1alpha2.AgentType_Declarative, Declarative: &v1alpha2.DeclarativeAgentSpec{ SystemMessage: "test", Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_Agent, - Agent: &v1alpha2.TypedLocalReference{ - Name: "agent-in-ns2", - Namespace: "ns2", - }, - }, { Type: v1alpha2.ToolProviderType_McpServer, McpServer: &v1alpha2.McpServerTool{ TypedLocalReference: v1alpha2.TypedLocalReference{ - Kind: "RemoteMCPServer", - ApiGroup: "kagent.dev", - Name: "tools-server", - Namespace: "unwatched-ns", + Kind: "Service", + ApiGroup: "", + Name: "my-service", + Namespace: "target-ns", }, }, }, @@ -435,17 +595,55 @@ func TestValidateCrossNamespaceReferences(t *testing.T) { }, }, wantErr: true, - errContains: "namespace \"unwatched-ns\" is not watched by the controller", + errContains: "Service does not support cross-namespace references", + }, + { + name: "Tool with empty namespace defaults to agent namespace - passes", + watchedNamespaces: []string{"source-ns"}, + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-agent", + Namespace: "source-ns", + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "other-agent", + Namespace: "", // defaults to agent's namespace + }, + }, + }, + }, + }, + }, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Build fake client with test objects + clientBuilder := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(sourceNs, targetNs) + + for _, obj := range tt.objects { + clientBuilder = clientBuilder.WithObjects(obj) + } + + kubeClient := clientBuilder.Build() + reconciler := &kagentReconciler{ + kube: kubeClient, watchedNamespaces: tt.watchedNamespaces, } - err := reconciler.validateCrossNamespaceReferences(tt.agent) + err := reconciler.validateCrossNamespaceReferences(context.Background(), tt.agent) if tt.wantErr { assert.Error(t, err) diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index 662efbdac..aa2fe8a7c 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -531,22 +531,12 @@ func (a *adkApiTranslator) translateInlineAgent(ctx context.Context, agent *v1al return nil, nil, nil, fmt.Errorf("agent tool cannot be used to reference itself, %s", agentRef) } - // Translate a nested tool toolAgent := &v1alpha2.Agent{} err := a.kube.Get(ctx, agentRef, toolAgent) if err != nil { return nil, nil, nil, err } - // Check cross-namespace reference is allowed - allowed, err := toolAgent.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, agent.Namespace, toolAgent.Namespace) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to check cross-namespace reference for agent %s: %w", agentRef, err) - } - if !allowed { - return nil, nil, nil, fmt.Errorf("cross-namespace reference to agent %s is not allowed from namespace %s", agentRef, agent.Namespace) - } - switch toolAgent.Spec.Type { case v1alpha2.AgentType_BYO, v1alpha2.AgentType_Declarative: url := fmt.Sprintf("http://%s.%s:8080", toolAgent.Name, toolAgent.Namespace) @@ -982,12 +972,6 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - // MCPServer type doesn't support cross-namespace references (external type) - // Only same-namespace references are allowed - if mcpServerRef.Namespace != agentNamespace { - return fmt.Errorf("cross-namespace reference to MCPServer %s is not allowed from namespace %s: MCPServer does not support cross-namespace references", mcpServerRef, agentNamespace) - } - remoteMcpServer, err := ConvertMCPServerToRemoteMCPServer(mcpServer) if err != nil { return err @@ -1011,15 +995,6 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - // Check cross-namespace reference is allowed - allowed, err := remoteMcpServer.Spec.AllowedNamespaces.AllowsNamespace(ctx, a.kube, agentNamespace, remoteMcpServer.Namespace) - if err != nil { - return fmt.Errorf("failed to check cross-namespace reference for RemoteMCPServer %s: %w", remoteMcpServerRef, err) - } - if !allowed { - return fmt.Errorf("cross-namespace reference to RemoteMCPServer %s is not allowed from namespace %s", remoteMcpServerRef, agentNamespace) - } - return a.translateRemoteMCPServerTarget(ctx, agent, remoteMcpServer, toolServer.ToolNames, agentHeaders) case schema.GroupKind{ Group: "", @@ -1038,12 +1013,6 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent * return err } - // Service type doesn't support cross-namespace references (external type) - // Only same-namespace references are allowed - if svcRef.Namespace != agentNamespace { - return fmt.Errorf("cross-namespace reference to Service %s is not allowed from namespace %s: Service does not support cross-namespace references", svcRef, agentNamespace) - } - remoteMcpServer, err := ConvertServiceToRemoteMCPServer(svc) if err != nil { return err diff --git a/go/internal/controller/translator/agent/adk_api_translator_test.go b/go/internal/controller/translator/agent/adk_api_translator_test.go index be8daa691..3733b43f9 100644 --- a/go/internal/controller/translator/agent/adk_api_translator_test.go +++ b/go/internal/controller/translator/agent/adk_api_translator_test.go @@ -16,6 +16,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +// Test_AdkApiTranslator_CrossNamespaceAgentTool tests that the translator can +// handle cross-namespace agent references. Note that cross-namespace validation +// (e.g. AllowedNamespaces checks) is a concern of the reconciler, not the +// translator; the translator just performs the translation. func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { scheme := schemev1.Scheme require.NoError(t, v1alpha2.AddToScheme(scheme)) @@ -34,11 +38,6 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { Name: "target-ns", }, } - unlabeledNs := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "unlabeled-ns", - }, - } // Create model config in source namespace modelConfig := &v1alpha2.ModelConfig{ @@ -60,7 +59,7 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { errContains string }{ { - name: "Same namespace reference - allowed by default", + name: "Same namespace reference - works", toolAgent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "tool-agent", @@ -73,7 +72,6 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { SystemMessage: "You are a tool agent", ModelConfig: "test-model", }, - // No AllowedNamespaces = same namespace only }, }, sourceAgent: &v1alpha2.Agent{ @@ -102,7 +100,7 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { wantErr: false, }, { - name: "Cross-namespace reference - denied by default (no AllowedNamespaces)", + name: "Cross-namespace reference - translates successfully (validation is in reconciler)", toolAgent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ Name: "tool-agent", @@ -115,7 +113,6 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { SystemMessage: "You are a tool agent", ModelConfig: "test-model", }, - // No AllowedNamespaces = same namespace only }, }, sourceAgent: &v1alpha2.Agent{ @@ -141,152 +138,8 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { }, }, }, - wantErr: true, - errContains: "cross-namespace reference to agent", - }, - { - name: "Cross-namespace reference - allowed with From=All", - toolAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tool-agent", - Namespace: "target-ns", - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Tool agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a tool agent", - ModelConfig: "test-model", - }, - AllowedNamespaces: &v1alpha2.AllowedNamespaces{ - From: v1alpha2.NamespacesFromAll, - }, - }, - }, - sourceAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-agent", - Namespace: "source-ns", // Different namespace as tool agent - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Source agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a source agent", - ModelConfig: "test-model", - Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_Agent, - Agent: &v1alpha2.TypedLocalReference{ - Name: "tool-agent", - Namespace: "target-ns", - }, - }, - }, - }, - }, - }, wantErr: false, }, - { - name: "Cross-namespace reference - allowed with matching selector", - toolAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tool-agent", - Namespace: "target-ns", - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Tool agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a tool agent", - ModelConfig: "test-model", - }, - AllowedNamespaces: &v1alpha2.AllowedNamespaces{ - From: v1alpha2.NamespacesFromSelector, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "shared-agent-access": "true", - }, - }, - }, - }, - }, - sourceAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-agent", - Namespace: "source-ns", // Has label "shared-agent-access": "true" - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Source agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a source agent", - ModelConfig: "test-model", - Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_Agent, - Agent: &v1alpha2.TypedLocalReference{ - Name: "tool-agent", - Namespace: "target-ns", - }, - }, - }, - }, - }, - }, - wantErr: false, - }, - { - name: "Cross-namespace reference - denied with non-matching selector", - toolAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tool-agent", - Namespace: "target-ns", - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Tool agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a tool agent", - ModelConfig: "test-model", - }, - AllowedNamespaces: &v1alpha2.AllowedNamespaces{ - From: v1alpha2.NamespacesFromSelector, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "shared-agent-access": "true", - }, - }, - }, - }, - }, - sourceAgent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "source-agent", - Namespace: "unlabeled-ns", // Does NOT have the required label `shared-agent-access` - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Source agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are a source agent", - ModelConfig: "test-model", - Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_Agent, - Agent: &v1alpha2.TypedLocalReference{ - Name: "tool-agent", - Namespace: "target-ns", - }, - }, - }, - }, - }, - }, - wantErr: true, - errContains: "cross-namespace reference to agent", - }, } for _, tt := range tests { @@ -296,7 +149,6 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { WithObjects( sourceNs, targetNs, - unlabeledNs, tt.toolAgent, tt.sourceAgent, ) @@ -337,6 +189,10 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) { } } +// Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer tests that the translator +// can handle cross-namespace RemoteMCPServer references. Note that cross-namespace +// validation (AllowedNamespaces checks) is now done in the reconciler, +// not the translator. The translator just performs the translation. func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { scheme := schemev1.Scheme require.NoError(t, v1alpha2.AddToScheme(scheme)) @@ -376,7 +232,7 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { errContains string }{ { - name: "Same namespace reference - allowed by default", + name: "Same namespace reference - works", remoteMCPServer: &v1alpha2.RemoteMCPServer{ ObjectMeta: metav1.ObjectMeta{ Name: "tools-server", @@ -385,7 +241,6 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { Spec: v1alpha2.RemoteMCPServerSpec{ Description: "Tools server", URL: "http://tools.example.com/mcp", - // No AllowedNamespaces = same namespace only }, }, agent: &v1alpha2.Agent{ @@ -419,51 +274,7 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { wantErr: false, }, { - name: "Cross-namespace reference - denied by default", - remoteMCPServer: &v1alpha2.RemoteMCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tools-server", - Namespace: "target-ns", - }, - Spec: v1alpha2.RemoteMCPServerSpec{ - Description: "Tools server", - URL: "http://tools.example.com/mcp", - // No AllowedNamespaces = same namespace only - }, - }, - agent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "agent", - Namespace: "source-ns", - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are an agent", - ModelConfig: "test-model", - Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_McpServer, - McpServer: &v1alpha2.McpServerTool{ - TypedLocalReference: v1alpha2.TypedLocalReference{ - Kind: "RemoteMCPServer", - ApiGroup: "kagent.dev", - Name: "tools-server", - Namespace: "target-ns", - }, - ToolNames: []string{"tool1"}, - }, - }, - }, - }, - }, - }, - wantErr: true, - errContains: "cross-namespace reference to RemoteMCPServer", - }, - { - name: "Cross-namespace reference - allowed with From=All", + name: "Cross-namespace reference - translates successfully (validation is in reconciler)", remoteMCPServer: &v1alpha2.RemoteMCPServer{ ObjectMeta: metav1.ObjectMeta{ Name: "tools-server", @@ -472,9 +283,6 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { Spec: v1alpha2.RemoteMCPServerSpec{ Description: "Tools server", URL: "http://tools.example.com/mcp", - AllowedNamespaces: &v1alpha2.AllowedNamespaces{ - From: v1alpha2.NamespacesFromAll, - }, }, }, agent: &v1alpha2.Agent{ @@ -507,56 +315,6 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) { }, wantErr: false, }, - { - name: "Cross-namespace reference - allowed with matching selector", - remoteMCPServer: &v1alpha2.RemoteMCPServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tools-server", - Namespace: "target-ns", - }, - Spec: v1alpha2.RemoteMCPServerSpec{ - Description: "Tools server", - URL: "http://tools.example.com/mcp", - AllowedNamespaces: &v1alpha2.AllowedNamespaces{ - From: v1alpha2.NamespacesFromSelector, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "shared-tools-access": "true", - }, - }, - }, - }, - }, - agent: &v1alpha2.Agent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "agent", - Namespace: "source-ns", // Has label "shared-tools-access": "true" - }, - Spec: v1alpha2.AgentSpec{ - Type: v1alpha2.AgentType_Declarative, - Description: "Agent", - Declarative: &v1alpha2.DeclarativeAgentSpec{ - SystemMessage: "You are an agent", - ModelConfig: "test-model", - Tools: []*v1alpha2.Tool{ - { - Type: v1alpha2.ToolProviderType_McpServer, - McpServer: &v1alpha2.McpServerTool{ - TypedLocalReference: v1alpha2.TypedLocalReference{ - Kind: "RemoteMCPServer", - ApiGroup: "kagent.dev", - Name: "tools-server", - Namespace: "target-ns", - }, - ToolNames: []string{"tool1"}, - }, - }, - }, - }, - }, - }, - wantErr: false, - }, } for _, tt := range tests {