Skip to content

Conversation

@onematchfox
Copy link
Contributor

Opening in draft for now so I can get some 👀 on this and hopefully start a discussion as to how we can get this feature in place. The code as it stands now, works, based on my local testing. However, I understand that there may be some security concerns here - particularly since namespaces are often used for tenant isolation. Do these need to be addressed now? And if so, does anyone got any thoughts on ways to tackle this? I would personally look towards Gateway API for inspiration if needed.

Closes #841

@EItanya
Copy link
Contributor

EItanya commented Dec 1, 2025

Hey @onematchfox, thanks for opening this. The reason we don't support this today is that tools, specifically RemoteMCPServer, have access to secrets which are loaded into the agent. Headers from secrets is the main one that I can think of right now.

I agree that we should look towards the Gateway API for solutions to namespace isolation. In particular we could use the ReferenceGrant or a similar mechanism.

I definitely don't want to create too much friction, but I want to make sure the API is secure by default.

@onematchfox
Copy link
Contributor Author

The reason we don't support this today is that tools, specifically RemoteMCPServer, have access to secrets which are loaded into the agent. Headers from secrets is the main one that I can think of right now.

I definitely don't want to create too much friction, but I want to make sure the API is secure by default.

👍 Got it - had missed that headersFrom on RemoteMCPServer ended up in the agent itself.

I agree that we should look towards the Gateway API for solutions to namespace isolation. In particular we could use the ReferenceGrant or a similar mechanism.

ReferenceGrant is quite onerous. How would you feel about using the mechanism behind restricting namespaces for route attachments instead? In essence, we could add a field, let's say allowedAgents on the RemoteMCPServer which could then be configured to allow use by agents outside of the MCP server namespace. This could default to not allowing cross-namespace attachments.

E.g.

apiVersion: kagent.dev/v1alpha2
kind: RemoteMCPServer
metadata:
  name: kagent-tool-server
  namespace: kagent
spec:
  description: Official KAgent tool server
  protocol: STREAMABLE_HTTP
  sseReadTimeout: 5m0s
  terminateOnClose: true
  timeout: 30s
  url: http://kagent-tools.kagent:8084/mcp
  allowedAgents:
    namespaces:
      from: All

@onematchfox onematchfox force-pushed the cross-namespace-tool-access branch 4 times, most recently from 62d6d80 to 3da82ac Compare December 3, 2025 12:56
@onematchfox
Copy link
Contributor Author

How would you feel about using the mechanism behind restricting namespaces for route attachments instead?

Went ahead and implemented this so you can see what I mean. Let me know what you think.

@onematchfox onematchfox marked this pull request as ready for review December 3, 2025 13:42
Copilot AI review requested due to automatic review settings December 3, 2025 13:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for cross-namespace tool references, allowing agents in one namespace to reference tools (RemoteMCPServers and Agents) in other namespaces. The implementation follows the Gateway API pattern with a bidirectional handshake mechanism using the AllowedNamespaces field, which provides security controls for multi-tenant environments.

Key Changes

  • Added namespace field to TypedLocalReference in both UI types and Go API, enabling explicit namespace specification for tool references
  • Implemented Gateway API-style AllowedNamespaces security policy on Agent and RemoteMCPServer CRDs with All, Same, and Selector modes
  • Updated UI to display tools with namespace prefix (e.g., namespace/toolname) and properly handle cross-namespace tool selection and matching
  • Modified Go translator and controller logic to validate cross-namespace references and resolve headers from the tool's namespace (not the agent's)

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ui/src/types/index.ts Added optional namespace field to TypedLocalReference interface
ui/src/lib/toolUtils.ts Updated display name generation to include namespace, modified toolResponseToAgentTool to parse and store namespace separately
ui/src/lib/__tests__/toolUtils.test.ts Added comprehensive test coverage for namespace handling in tool utilities
ui/src/components/sidebars/AgentDetailsSidebar.tsx Updated to display tools with namespace qualification
ui/src/components/create/ToolsSection.tsx Modified to pass and use agent namespace for tool display
ui/src/components/create/SelectToolsDialog.tsx Enhanced tool matching logic to compare both name and namespace for agents
ui/src/app/agents/new/page.tsx Passes agent namespace to ToolsSection component
ui/src/app/actions/agents.ts Converts tool references to use separate namespace field, defaults to agent namespace when not specified
go/api/v1alpha2/common_types.go Implements AllowedNamespaces type with validation logic for cross-namespace references
go/api/v1alpha2/common_types_test.go Comprehensive unit tests for AllowedNamespaces functionality
go/api/v1alpha2/agent_types.go Added AllowedNamespaces field to AgentSpec, added Namespace to TypedLocalReference, implemented NamespacedName() helper
go/api/v1alpha2/remotemcpserver_types.go Added AllowedNamespaces to spec, moved header resolution to RemoteMCPServer method (uses tool's namespace)
go/internal/controller/translator/agent/adk_api_translator.go Added cross-namespace validation checks, updated header resolution to use tool's namespace, ensures MCPServer and Service types reject cross-namespace refs
go/internal/controller/translator/agent/adk_api_translator_test.go New unit tests for cross-namespace agent and RemoteMCPServer tool references with various AllowedNamespaces configurations
go/internal/controller/agent_controller.go Updated tool matching logic to use NamespacedName() for proper cross-namespace comparison
go/internal/controller/reconciler/reconciler.go Updated to pass RemoteMCPServer object (not just spec) for proper namespace-aware header resolution
go/config/crd/bases/kagent.dev_agents.yaml Added allowedNamespaces field with validation rules, added namespace field to tool references
go/config/crd/bases/kagent.dev_remotemcpservers.yaml Added allowedNamespaces field with validation rules
helm/kagent-crds/templates/ Helm chart CRD templates updated to match base CRDs
go/api/v1alpha2/zz_generated.deepcopy.go Generated deep copy methods for new AllowedNamespaces type
go/internal/controller/translator/agent/testdata/ Test data for cross-namespace tool scenarios with expected outputs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@onematchfox onematchfox force-pushed the cross-namespace-tool-access branch from 3da82ac to 1090e26 Compare December 5, 2025 10:23
@onematchfox
Copy link
Contributor Author

Rebased this PR and now linting is failing on a change that was part of #1154.

cc @dongjiang1989 @EItanya

@onematchfox onematchfox force-pushed the cross-namespace-tool-access branch from 1090e26 to f77568a Compare December 8, 2025 07:36
@EItanya
Copy link
Contributor

EItanya commented Dec 8, 2025

Ok, I actually kind of love this approach! Maybe we should discuss it briefly in the community meeting tomorrow and then try to move ahead with it?

@onematchfox
Copy link
Contributor Author

Ok, I actually kind of love this approach! Maybe we should discuss it briefly in the community meeting tomorrow and then try to move ahead with it?

Sounds good. See you then.

@onematchfox
Copy link
Contributor Author

Sounds good. See you then.

Just realised you said community meeting (today) not contributor meeting (Thursday). Won't be able to attend today - but I should be able to attend on Thursday. Feel free to bring it up today as well though - would be great to get others' thoughts on the approach.

Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
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>
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
@onematchfox onematchfox force-pushed the cross-namespace-tool-access branch from f77568a to 9107df4 Compare December 9, 2025 10:19
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
@onematchfox
Copy link
Contributor Author

FYI @dimetron As discussed in the community meeting yesterday I have added validation to ensure that cross-namespaces references are not permitted outside of the namespaces that the controller is configured to watch.

@onematchfox
Copy link
Contributor Author

My latest refactors have given me a bit of room for thought. Whilst I personally like the route attachments pattern, it does have 2 shortcomings that are perhaps worth explicitly noting here:

  1. It only works for native CRDs - i.e. cross-namespace references will only work from Agent->Agent and Agent->RemoteMCPServer. As can be seen in the current validation, it cannot be used for MCP Servers that are created from watched resources - Service and McpServer.
  2. The namespace selector, whilst very convenient, can be seen as somewhat "insecure". This is best described here:

Although it's possible to use other labels with this selector, it is not quite as safe. While the kubernetes.io/metadata.name label is consistently set on namespaces to the name of the namespace, other labels do not have the same guarantee. If you used a custom label such as env, anyone that is able to label namespaces within your cluster would effectively be able to change the set of namespaces your Gateway supported.

On the other hand, the RefereceGrant pattern would support referencing secondary/watched resources whilst avoiding the potential insecure use of namespace selectors. The trade-off - at least in GatewayAPI's implementation of ReferenceGrant - is that there is no way to allow cross-namespace references from all (or even a set of) namespaces - every cross-namespace reference must be explicitly specified. This is why I feel that this is "quite onerous" and proposed the current implementation.

I wonder if:

  1. for now, this implementation is "good enough"? It is low friction and optional - it doesn't change the status-quo today which is that no cross-namespace references are allowed ("secure by default").
  2. going forward we may want to also introduce support for a ReferenceGrant-like pattern for those that have stricter security constraints and/or the requirement to support cross-namespace references to McpServer/Service resources. The two implementations could actually co-exist giving users the freedom to decide which pattern works best for them.

Feel free to share thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reference agents in other namespaces

2 participants