Skip to content

Commit 779a7a8

Browse files
manusamanusa
authored
review: Multi Cluster support (#1)
* nit: rename contexts_list to configuration_contexts_list Besides the conventional naming, it helps LLMs understand the context of the tool by providing a certain level of hierarchy. Signed-off-by: Marc Nuri <[email protected]> * fix(mcp): ToolMutator doesn't rely on magic strings Signed-off-by: Marc Nuri <[email protected]> * refactor(api): don't expose ManagerProvider to toolsets Signed-off-by: Marc Nuri <[email protected]> * test(mcp): configuration_contexts_list basic tests Signed-off-by: Marc Nuri <[email protected]> * test(toolsets): revert edge-case test This test should not be touched. Signed-off-by: Marc Nuri <[email protected]> * test(toolsets): add specific metadata tests for multi-cluster Signed-off-by: Marc Nuri <[email protected]> * fix(mcp): ToolFilter doesn't rely on magic strings (partially) Signed-off-by: Marc Nuri <[email protected]> * test(api): IsClusterAware and IsTargetListProvider default values Signed-off-by: Marc Nuri <[email protected]> * test(mcp): revert unneeded changes in mcp_tools_test.go Signed-off-by: Marc Nuri <[email protected]> --------- Signed-off-by: Marc Nuri <[email protected]>
1 parent e9a4460 commit 779a7a8

21 files changed

+1723
-546
lines changed

‎internal/test/kubernetes.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,10 @@ func KubeConfigFake() *clientcmdapi.Config {
88
fakeConfig := clientcmdapi.NewConfig()
99
fakeConfig.Clusters["fake"] = clientcmdapi.NewCluster()
1010
fakeConfig.Clusters["fake"].Server = "https://127.0.0.1:6443"
11-
fakeConfig.Clusters["additional-cluster"] = clientcmdapi.NewCluster()
1211
fakeConfig.AuthInfos["fake"] = clientcmdapi.NewAuthInfo()
13-
fakeConfig.AuthInfos["additional-auth"] = clientcmdapi.NewAuthInfo()
1412
fakeConfig.Contexts["fake-context"] = clientcmdapi.NewContext()
1513
fakeConfig.Contexts["fake-context"].Cluster = "fake"
1614
fakeConfig.Contexts["fake-context"].AuthInfo = "fake"
17-
fakeConfig.Contexts["additional-context"] = clientcmdapi.NewContext()
18-
fakeConfig.Contexts["additional-context"].Cluster = "additional-cluster"
19-
fakeConfig.Contexts["additional-context"].AuthInfo = "additional-auth"
2015
fakeConfig.CurrentContext = "fake-context"
2116
return fakeConfig
2217
}

‎internal/test/mock_server.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,14 @@ func (m *MockServer) Kubeconfig() *api.Config {
7373
}
7474

7575
func (m *MockServer) KubeconfigFile(t *testing.T) string {
76-
kubeconfig := filepath.Join(t.TempDir(), "config")
77-
err := clientcmd.WriteToFile(*m.Kubeconfig(), kubeconfig)
76+
return KubeconfigFile(t, m.Kubeconfig())
77+
}
78+
79+
func KubeconfigFile(t *testing.T, kubeconfig *api.Config) string {
80+
kubeconfigFile := filepath.Join(t.TempDir(), "config")
81+
err := clientcmd.WriteToFile(*kubeconfig, kubeconfigFile)
7882
require.NoError(t, err, "Expected no error writing kubeconfig file")
79-
return kubeconfig
83+
return kubeconfigFile
8084
}
8185

8286
func WriteObject(w http.ResponseWriter, obj runtime.Object) {

‎pkg/api/toolsets.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,29 @@ import (
1010
)
1111

1212
type ServerTool struct {
13-
Tool Tool
14-
Handler ToolHandlerFunc
13+
Tool Tool
14+
Handler ToolHandlerFunc
15+
ClusterAware *bool
16+
TargetListProvider *bool
17+
}
18+
19+
// IsClusterAware indicates whether the tool can accept a "cluster" or "context" parameter
20+
// to operate on a specific Kubernetes cluster context.
21+
// Defaults to true if not explicitly set
22+
func (s *ServerTool) IsClusterAware() bool {
23+
if s.ClusterAware != nil {
24+
return *s.ClusterAware
25+
}
26+
return true
27+
}
28+
29+
// IsTargetListProvider indicates whether the tool is used to provide a list of targets (clusters/contexts)
30+
// Defaults to false if not explicitly set
31+
func (s *ServerTool) IsTargetListProvider() bool {
32+
if s.TargetListProvider != nil {
33+
return *s.TargetListProvider
34+
}
35+
return false
1536
}
1637

1738
type Toolset interface {
@@ -44,7 +65,6 @@ func NewToolCallResult(content string, err error) *ToolCallResult {
4465
type ToolHandlerParams struct {
4566
context.Context
4667
*internalk8s.Kubernetes
47-
internalk8s.ManagerProvider
4868
ToolCallRequest
4969
ListOutput output.Output
5070
}

‎pkg/api/toolsets_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package api
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/suite"
7+
"k8s.io/utils/ptr"
8+
)
9+
10+
type ToolsetsSuite struct {
11+
suite.Suite
12+
}
13+
14+
func (s *ToolsetsSuite) TestServerTool() {
15+
s.Run("IsClusterAware", func() {
16+
s.Run("defaults to true", func() {
17+
tool := &ServerTool{}
18+
s.True(tool.IsClusterAware(), "Expected IsClusterAware to be true by default")
19+
})
20+
s.Run("can be set to false", func() {
21+
tool := &ServerTool{ClusterAware: ptr.To(false)}
22+
s.False(tool.IsClusterAware(), "Expected IsClusterAware to be false when set to false")
23+
})
24+
s.Run("can be set to true", func() {
25+
tool := &ServerTool{ClusterAware: ptr.To(true)}
26+
s.True(tool.IsClusterAware(), "Expected IsClusterAware to be true when set to true")
27+
})
28+
})
29+
s.Run("IsTargetListProvider", func() {
30+
s.Run("defaults to false", func() {
31+
tool := &ServerTool{}
32+
s.False(tool.IsTargetListProvider(), "Expected IsTargetListProvider to be false by default")
33+
})
34+
s.Run("can be set to false", func() {
35+
tool := &ServerTool{TargetListProvider: ptr.To(false)}
36+
s.False(tool.IsTargetListProvider(), "Expected IsTargetListProvider to be false when set to false")
37+
})
38+
s.Run("can be set to true", func() {
39+
tool := &ServerTool{TargetListProvider: ptr.To(true)}
40+
s.True(tool.IsTargetListProvider(), "Expected IsTargetListProvider to be true when set to true")
41+
})
42+
})
43+
}
44+
45+
func TestToolsets(t *testing.T) {
46+
suite.Run(t, new(ToolsetsSuite))
47+
}

‎pkg/kubernetes/configuration.go‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"k8s.io/client-go/tools/clientcmd/api/latest"
99
)
1010

11+
const inClusterKubeConfigDefaultContext = "in-cluster"
12+
1113
// InClusterConfig is a variable that holds the function to get the in-cluster config
1214
// Exposed for testing
1315
var InClusterConfig = func() (*rest.Config, error) {
@@ -81,6 +83,40 @@ func (m *Manager) ToRawKubeConfigLoader() clientcmd.ClientConfig {
8183
return m.clientCmdConfig
8284
}
8385

86+
// ConfigurationContextsDefault returns the current context name
87+
// TODO: Should be moved to the Provider level ?
88+
func (k *Kubernetes) ConfigurationContextsDefault() (string, error) {
89+
if k.manager.IsInCluster() {
90+
return inClusterKubeConfigDefaultContext, nil
91+
}
92+
cfg, err := k.manager.clientCmdConfig.RawConfig()
93+
if err != nil {
94+
return "", err
95+
}
96+
return cfg.CurrentContext, nil
97+
}
98+
99+
// ConfigurationContextsList returns the list of available context names
100+
// TODO: Should be moved to the Provider level ?
101+
func (k *Kubernetes) ConfigurationContextsList() ([]string, error) {
102+
if k.manager.IsInCluster() {
103+
return []string{inClusterKubeConfigDefaultContext}, nil
104+
}
105+
cfg, err := k.manager.clientCmdConfig.RawConfig()
106+
if err != nil {
107+
return nil, err
108+
}
109+
contexts := make([]string, 0, len(cfg.Contexts))
110+
for context := range cfg.Contexts {
111+
contexts = append(contexts, context)
112+
}
113+
return contexts, nil
114+
}
115+
116+
// ConfigurationView returns the current kubeconfig content as a kubeconfig YAML
117+
// If minify is true, keeps only the current-context and the relevant pieces of the configuration for that context.
118+
// If minify is false, all contexts, clusters, auth-infos, and users are returned in the configuration.
119+
// TODO: Should be moved to the Provider level ?
84120
func (k *Kubernetes) ConfigurationView(minify bool) (runtime.Object, error) {
85121
var cfg clientcmdapi.Config
86122
var err error
@@ -93,11 +129,11 @@ func (k *Kubernetes) ConfigurationView(minify bool) (runtime.Object, error) {
93129
cfg.AuthInfos["user"] = &clientcmdapi.AuthInfo{
94130
Token: k.manager.cfg.BearerToken,
95131
}
96-
cfg.Contexts["context"] = &clientcmdapi.Context{
132+
cfg.Contexts[inClusterKubeConfigDefaultContext] = &clientcmdapi.Context{
97133
Cluster: "cluster",
98134
AuthInfo: "user",
99135
}
100-
cfg.CurrentContext = "context"
136+
cfg.CurrentContext = inClusterKubeConfigDefaultContext
101137
} else if cfg, err = k.manager.clientCmdConfig.RawConfig(); err != nil {
102138
return nil, err
103139
}

‎pkg/mcp/configuration_test.go‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package mcp
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/mark3labs/mcp-go/mcp"
78
"github.com/stretchr/testify/suite"
89
"k8s.io/client-go/rest"
10+
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
911
v1 "k8s.io/client-go/tools/clientcmd/api/v1"
1012
"sigs.k8s.io/yaml"
1113

@@ -22,7 +24,37 @@ func (s *ConfigurationSuite) SetupTest() {
2224
// Use mock server for predictable kubeconfig content
2325
mockServer := test.NewMockServer()
2426
s.T().Cleanup(mockServer.Close)
25-
s.Cfg.KubeConfig = mockServer.KubeconfigFile(s.T())
27+
kubeconfig := mockServer.Kubeconfig()
28+
for i := 0; i < 10; i++ {
29+
// Add multiple fake contexts to force configuration_contexts_list tool to appear
30+
// and test minification in configuration_view tool
31+
name := fmt.Sprintf("cluster-%d", i)
32+
kubeconfig.Contexts[name] = clientcmdapi.NewContext()
33+
kubeconfig.Clusters[name+"-cluster"] = clientcmdapi.NewCluster()
34+
kubeconfig.AuthInfos[name+"-auth"] = clientcmdapi.NewAuthInfo()
35+
kubeconfig.Contexts[name].Cluster = name + "-cluster"
36+
kubeconfig.Contexts[name].AuthInfo = name + "-auth"
37+
}
38+
s.Cfg.KubeConfig = test.KubeconfigFile(s.T(), kubeconfig)
39+
}
40+
41+
func (s *ConfigurationSuite) TestContextsList() {
42+
s.InitMcpClient()
43+
s.Run("configuration_contexts_list", func() {
44+
toolResult, err := s.CallTool("configuration_contexts_list", map[string]interface{}{})
45+
s.Run("returns contexts", func() {
46+
s.Nilf(err, "call tool failed %v", err)
47+
})
48+
s.Require().NotNil(toolResult, "Expected tool result from call")
49+
s.Lenf(toolResult.Content, 1, "invalid tool result content length %v", len(toolResult.Content))
50+
s.Run("contains context count", func() {
51+
s.Regexpf(`^Available Kubernetes contexts \(11 total`, toolResult.Content[0].(mcp.TextContent).Text, "invalid tool count result content %v", toolResult.Content[0].(mcp.TextContent).Text)
52+
})
53+
s.Run("contains default context name", func() {
54+
s.Regexpf(`^Available Kubernetes contexts \(\d+ total, default: fake-context\)`, toolResult.Content[0].(mcp.TextContent).Text, "invalid tool context default result content %v", toolResult.Content[0].(mcp.TextContent).Text)
55+
s.Regexpf(`(?m)^\*fake-context$`, toolResult.Content[0].(mcp.TextContent).Text, "invalid tool context default result content %v", toolResult.Content[0].(mcp.TextContent).Text)
56+
})
57+
})
2658
}
2759

2860
func (s *ConfigurationSuite) TestConfigurationView() {
@@ -70,19 +102,23 @@ func (s *ConfigurationSuite) TestConfigurationView() {
70102
s.Nilf(err, "invalid tool result content %v", err)
71103
})
72104
s.Run("returns additional context info", func() {
73-
s.Lenf(decoded.Contexts, 2, "invalid context count, expected 2, got %v", len(decoded.Contexts))
74-
s.Equalf("additional-context", decoded.Contexts[0].Name, "additional-context not found: %v", decoded.Contexts)
75-
s.Equalf("additional-cluster", decoded.Contexts[0].Context.Cluster, "additional-cluster not found: %v", decoded.Contexts)
76-
s.Equalf("additional-auth", decoded.Contexts[0].Context.AuthInfo, "additional-auth not found: %v", decoded.Contexts)
77-
s.Equalf("fake-context", decoded.Contexts[1].Name, "fake-context not found: %v", decoded.Contexts)
105+
s.Lenf(decoded.Contexts, 11, "invalid context count, expected 12, got %v", len(decoded.Contexts))
106+
s.Equalf("cluster-0", decoded.Contexts[0].Name, "cluster-0 not found: %v", decoded.Contexts)
107+
s.Equalf("cluster-0-cluster", decoded.Contexts[0].Context.Cluster, "cluster-0-cluster not found: %v", decoded.Contexts)
108+
s.Equalf("cluster-0-auth", decoded.Contexts[0].Context.AuthInfo, "cluster-0-auth not found: %v", decoded.Contexts)
109+
s.Equalf("fake", decoded.Contexts[10].Context.Cluster, "fake not found: %v", decoded.Contexts)
110+
s.Equalf("fake", decoded.Contexts[10].Context.AuthInfo, "fake not found: %v", decoded.Contexts)
111+
s.Equalf("fake-context", decoded.Contexts[10].Name, "fake-context not found: %v", decoded.Contexts)
78112
})
79113
s.Run("returns cluster info", func() {
80-
s.Lenf(decoded.Clusters, 2, "invalid cluster count, expected 2, got %v", len(decoded.Clusters))
81-
s.Equalf("additional-cluster", decoded.Clusters[0].Name, "additional-cluster not found: %v", decoded.Clusters)
114+
s.Lenf(decoded.Clusters, 11, "invalid cluster count, expected 2, got %v", len(decoded.Clusters))
115+
s.Equalf("cluster-0-cluster", decoded.Clusters[0].Name, "cluster-0-cluster not found: %v", decoded.Clusters)
116+
s.Equalf("fake", decoded.Clusters[10].Name, "fake not found: %v", decoded.Clusters)
82117
})
83118
s.Run("configuration_view with minified=false returns auth info", func() {
84-
s.Lenf(decoded.AuthInfos, 2, "invalid auth info count, expected 2, got %v", len(decoded.AuthInfos))
85-
s.Equalf("additional-auth", decoded.AuthInfos[0].Name, "additional-auth not found: %v", decoded.AuthInfos)
119+
s.Lenf(decoded.AuthInfos, 11, "invalid auth info count, expected 2, got %v", len(decoded.AuthInfos))
120+
s.Equalf("cluster-0-auth", decoded.AuthInfos[0].Name, "cluster-0-auth not found: %v", decoded.AuthInfos)
121+
s.Equalf("fake", decoded.AuthInfos[10].Name, "fake not found: %v", decoded.AuthInfos)
86122
})
87123
})
88124
}
@@ -109,11 +145,11 @@ func (s *ConfigurationSuite) TestConfigurationViewInCluster() {
109145
s.Nilf(err, "invalid tool result content %v", err)
110146
})
111147
s.Run("returns current-context", func() {
112-
s.Equalf("context", decoded.CurrentContext, "context not found: %v", decoded.CurrentContext)
148+
s.Equalf("in-cluster", decoded.CurrentContext, "context not found: %v", decoded.CurrentContext)
113149
})
114150
s.Run("returns context info", func() {
115151
s.Lenf(decoded.Contexts, 1, "invalid context count, expected 1, got %v", len(decoded.Contexts))
116-
s.Equalf("context", decoded.Contexts[0].Name, "context not found: %v", decoded.Contexts)
152+
s.Equalf("in-cluster", decoded.Contexts[0].Name, "context not found: %v", decoded.Contexts)
117153
s.Equalf("cluster", decoded.Contexts[0].Context.Cluster, "cluster not found: %v", decoded.Contexts)
118154
s.Equalf("user", decoded.Contexts[0].Context.AuthInfo, "user not found: %v", decoded.Contexts)
119155
})

‎pkg/mcp/m3labs.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ func ServerToolToM3LabsServerTool(s *Server, tools []api.ServerTool) ([]server.S
5555
result, err := tool.Handler(api.ToolHandlerParams{
5656
Context: ctx,
5757
Kubernetes: k,
58-
ManagerProvider: s.p,
5958
ToolCallRequest: request,
6059
ListOutput: s.configuration.ListOutput(),
6160
})

‎pkg/mcp/mcp.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ func (s *Server) reloadKubernetesClusterProvider() error {
132132
p.GetDefaultTarget(),
133133
p.GetTargetParameterName(),
134134
targets,
135-
[]string{"configuration_view", "contexts_list"}, // TODO: see which tools (if any) do not need the cluster parameter
136135
)
137136

138137
applicableTools := make([]api.ServerTool, 0)

‎pkg/mcp/mcp_tools_test.go‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,7 @@ func TestUnrestricted(t *testing.T) {
3434
}
3535

3636
func TestReadOnly(t *testing.T) {
37-
readOnlyServer := func(c *mcpContext) {
38-
c.staticConfig = &config.StaticConfig{
39-
ReadOnly: true,
40-
ClusterProviderStrategy: config.ClusterProviderKubeConfig,
41-
}
42-
}
37+
readOnlyServer := func(c *mcpContext) { c.staticConfig = &config.StaticConfig{ReadOnly: true} }
4338
testCaseWithContext(t, &mcpContext{before: readOnlyServer}, func(c *mcpContext) {
4439
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
4540
t.Run("ListTools returns tools", func(t *testing.T) {
@@ -61,12 +56,7 @@ func TestReadOnly(t *testing.T) {
6156
}
6257

6358
func TestDisableDestructive(t *testing.T) {
64-
disableDestructiveServer := func(c *mcpContext) {
65-
c.staticConfig = &config.StaticConfig{
66-
DisableDestructive: true,
67-
ClusterProviderStrategy: config.ClusterProviderKubeConfig,
68-
}
69-
}
59+
disableDestructiveServer := func(c *mcpContext) { c.staticConfig = &config.StaticConfig{DisableDestructive: true} }
7060
testCaseWithContext(t, &mcpContext{before: disableDestructiveServer}, func(c *mcpContext) {
7161
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
7262
t.Run("ListTools returns tools", func(t *testing.T) {
@@ -111,8 +101,7 @@ func TestEnabledTools(t *testing.T) {
111101
func TestDisabledTools(t *testing.T) {
112102
testCaseWithContext(t, &mcpContext{
113103
staticConfig: &config.StaticConfig{
114-
DisabledTools: []string{"namespaces_list", "events_list"},
115-
ClusterProviderStrategy: config.ClusterProviderKubeConfig,
104+
DisabledTools: []string{"namespaces_list", "events_list"},
116105
},
117106
}, func(c *mcpContext) {
118107
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})

0 commit comments

Comments
 (0)