Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (c *httpContext) beforeEach(t *testing.T) {
}
c.StaticConfig.Port = fmt.Sprintf("%d", ln.Addr().(*net.TCPAddr).Port)
mcpServer, err := mcp.NewServer(mcp.Configuration{
Profile: mcp.Profiles[0],
Toolset: mcp.Toolsets[0],
StaticConfig: c.StaticConfig,
})
if err != nil {
Expand Down
18 changes: 9 additions & 9 deletions pkg/kubernetes-mcp-server/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type MCPServerOptions struct {
HttpPort int
SSEBaseUrl string
Kubeconfig string
Profile string
Toolset string
ListOutput string
ReadOnly bool
DisableDestructive bool
Expand All @@ -77,7 +77,7 @@ type MCPServerOptions struct {
func NewMCPServerOptions(streams genericiooptions.IOStreams) *MCPServerOptions {
return &MCPServerOptions{
IOStreams: streams,
Profile: "full",
Toolset: "full",
ListOutput: "table",
StaticConfig: &config.StaticConfig{},
}
Expand Down Expand Up @@ -107,15 +107,15 @@ func NewMCPServer(streams genericiooptions.IOStreams) *cobra.Command {

cmd.Flags().BoolVar(&o.Version, "version", o.Version, "Print version information and quit")
cmd.Flags().IntVar(&o.LogLevel, "log-level", o.LogLevel, "Set the log level (from 0 to 9)")
cmd.Flags().StringVar(&o.ConfigPath, "config", o.ConfigPath, "Path of the config file. Each profile has its set of defaults.")
Copy link
Member

Choose a reason for hiding this comment

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

The removal of this statement is to fix the ambiguity or due to a behavior change in profiles/toolsets?

Copy link
Member Author

Choose a reason for hiding this comment

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

When I envisioned profiles, it was not only about tool and prompt groups but also about default settings for other MCP behaviors.
I never implemented that, and I don't think providing defaults is part of what we want from the toolsets feature.
I'm just removing it because it's setting inaccurate expectations and is also unrelated to config.

cmd.Flags().StringVar(&o.ConfigPath, "config", o.ConfigPath, "Path of the config file.")
cmd.Flags().IntVar(&o.SSEPort, "sse-port", o.SSEPort, "Start a SSE server on the specified port")
cmd.Flag("sse-port").Deprecated = "Use --port instead"
cmd.Flags().IntVar(&o.HttpPort, "http-port", o.HttpPort, "Start a streamable HTTP server on the specified port")
cmd.Flag("http-port").Deprecated = "Use --port instead"
cmd.Flags().StringVar(&o.Port, "port", o.Port, "Start a streamable HTTP and SSE HTTP server on the specified port (e.g. 8080)")
cmd.Flags().StringVar(&o.SSEBaseUrl, "sse-base-url", o.SSEBaseUrl, "SSE public base URL to use when sending the endpoint message (e.g. https://example.com)")
cmd.Flags().StringVar(&o.Kubeconfig, "kubeconfig", o.Kubeconfig, "Path to the kubeconfig file to use for authentication")
cmd.Flags().StringVar(&o.Profile, "profile", o.Profile, "MCP profile to use (one of: "+strings.Join(mcp.ProfileNames, ", ")+")")
Copy link
Member

Choose a reason for hiding this comment

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

If kubernetes-mcp-server was GA'ed, we need to first deprecate this flag to not cause any breakages. However, I think we are fine to update the flag name now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't deprecate it because as of now, the flag was useless.
It wasn't strictly no-op, but since there was just a profile, it had no effect.

Copy link
Member

Choose a reason for hiding this comment

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

That is fine. But from the user's perspective kubernetes-mcp-server --profile=invalid was working but now it starts failing.

Copy link
Member Author

Choose a reason for hiding this comment

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

that should not be working in v0.0.49.
Running the command should print the validation error and an exit 1.

Copy link
Member Author

Choose a reason for hiding this comment

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

$ npx -y kubernetes-mcp-server@latest --profile=invalid 
Error: invalid profile name: invalid, valid names are: full
Usage:
...

cmd.Flags().StringVar(&o.Toolset, "toolset", o.Toolset, "MCP toolset to use (one of: "+strings.Join(mcp.ToolsetNames, ", ")+")")
cmd.Flags().StringVar(&o.ListOutput, "list-output", o.ListOutput, "Output format for resource list operations (one of: "+strings.Join(output.Names, ", ")+"). Defaults to table.")
cmd.Flags().BoolVar(&o.ReadOnly, "read-only", o.ReadOnly, "If true, only tools annotated with readOnlyHint=true are exposed")
cmd.Flags().BoolVar(&o.DisableDestructive, "disable-destructive", o.DisableDestructive, "If true, tools annotated with destructiveHint=true are disabled")
Expand Down Expand Up @@ -237,17 +237,17 @@ func (m *MCPServerOptions) Validate() error {
}

func (m *MCPServerOptions) Run() error {
profile := mcp.ProfileFromString(m.Profile)
if profile == nil {
return fmt.Errorf("invalid profile name: %s, valid names are: %s", m.Profile, strings.Join(mcp.ProfileNames, ", "))
toolset := mcp.ToolsetFromString(m.Toolset)
if toolset == nil {
return fmt.Errorf("invalid toolset name: %s, valid names are: %s", m.Toolset, strings.Join(mcp.ToolsetNames, ", "))
}
listOutput := output.FromString(m.StaticConfig.ListOutput)
if listOutput == nil {
return fmt.Errorf("invalid output name: %s, valid names are: %s", m.StaticConfig.ListOutput, strings.Join(output.Names, ", "))
}
klog.V(1).Info("Starting kubernetes-mcp-server")
klog.V(1).Infof(" - Config: %s", m.ConfigPath)
klog.V(1).Infof(" - Profile: %s", profile.GetName())
klog.V(1).Infof(" - Toolset: %s", toolset.GetName())
klog.V(1).Infof(" - ListOutput: %s", listOutput.GetName())
klog.V(1).Infof(" - Read-only mode: %t", m.StaticConfig.ReadOnly)
klog.V(1).Infof(" - Disable destructive tools: %t", m.StaticConfig.DisableDestructive)
Expand Down Expand Up @@ -291,7 +291,7 @@ func (m *MCPServerOptions) Run() error {
}

mcpServer, err := mcp.NewServer(mcp.Configuration{
Profile: profile,
Toolset: toolset,
ListOutput: listOutput,
StaticConfig: m.StaticConfig,
})
Expand Down
18 changes: 9 additions & 9 deletions pkg/kubernetes-mcp-server/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,32 +129,32 @@ func TestConfig(t *testing.T) {
})
}

func TestProfile(t *testing.T) {
func TestToolset(t *testing.T) {
t.Run("available", func(t *testing.T) {
ioStreams, _ := testStream()
rootCmd := NewMCPServer(ioStreams)
rootCmd.SetArgs([]string{"--help"})
o, err := captureOutput(rootCmd.Execute) // --help doesn't use logger/klog, cobra prints directly to stdout
if !strings.Contains(o, "MCP profile to use (one of: full) ") {
t.Fatalf("Expected all available profiles, got %s %v", o, err)
if !strings.Contains(o, "MCP toolset to use (one of: full) ") {
t.Fatalf("Expected all available toolsets, got %s %v", o, err)
}
})
t.Run("default", func(t *testing.T) {
ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams)
rootCmd.SetArgs([]string{"--version", "--log-level=1"})
if err := rootCmd.Execute(); !strings.Contains(out.String(), "- Profile: full") {
t.Fatalf("Expected profile 'full', got %s %v", out, err)
if err := rootCmd.Execute(); !strings.Contains(out.String(), "- Toolset: full") {
t.Fatalf("Expected toolset 'full', got %s %v", out, err)
}
})
t.Run("set with --profile", func(t *testing.T) {
t.Run("set with --toolset", func(t *testing.T) {
ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams)
rootCmd.SetArgs([]string{"--version", "--log-level=1", "--profile", "full"}) // TODO: change by some non-default profile
rootCmd.SetArgs([]string{"--version", "--log-level=1", "--toolset", "full"}) // TODO: change by some non-default toolset
_ = rootCmd.Execute()
expected := `(?m)\" - Profile\: full\"`
expected := `(?m)\" - Toolset\: full\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil {
t.Fatalf("Expected profile to be %s, got %s %v", expected, out.String(), err)
t.Fatalf("Expected toolset to be %s, got %s %v", expected, out.String(), err)
}
})
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/mcp/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestMain(m *testing.M) {
}

type mcpContext struct {
profile Profile
toolset Toolset
listOutput output.Output
logLevel int

Expand All @@ -126,8 +126,8 @@ func (c *mcpContext) beforeEach(t *testing.T) {
c.ctx, c.cancel = context.WithCancel(t.Context())
c.tempDir = t.TempDir()
c.withKubeConfig(nil)
if c.profile == nil {
c.profile = &FullProfile{}
if c.toolset == nil {
c.toolset = &Full{}
}
if c.listOutput == nil {
c.listOutput = output.Yaml
Expand All @@ -149,7 +149,7 @@ func (c *mcpContext) beforeEach(t *testing.T) {
klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(c.logLevel), textlogger.Output(&c.logBuffer))))
// MCP Server
if c.mcpServer, err = NewServer(Configuration{
Profile: c.profile,
Toolset: c.toolset,
ListOutput: c.listOutput,
StaticConfig: c.staticConfig,
}); err != nil {
Expand Down Expand Up @@ -188,7 +188,7 @@ func (c *mcpContext) afterEach() {
}

func testCase(t *testing.T, test func(c *mcpContext)) {
testCaseWithContext(t, &mcpContext{profile: &FullProfile{}}, test)
testCaseWithContext(t, &mcpContext{toolset: &Full{}}, test)
}

func testCaseWithContext(t *testing.T, mcpCtx *mcpContext, test func(c *mcpContext)) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/mcp/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type ContextKey string
const TokenScopesContextKey = ContextKey("TokenScopesContextKey")

type Configuration struct {
Profile Profile
Toolset Toolset
ListOutput output.Output

StaticConfig *config.StaticConfig
Expand Down Expand Up @@ -89,7 +89,7 @@ func (s *Server) reloadKubernetesClient() error {
}
s.k = k
applicableTools := make([]server.ServerTool, 0)
for _, tool := range s.configuration.Profile.GetTools(s) {
for _, tool := range s.configuration.Toolset.GetTools(s) {
if !s.configuration.isToolApplicable(tool) {
continue
}
Expand Down
54 changes: 0 additions & 54 deletions pkg/mcp/profiles.go

This file was deleted.

54 changes: 54 additions & 0 deletions pkg/mcp/toolsets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package mcp

import (
"slices"

"github.com/mark3labs/mcp-go/server"
)

type Toolset interface {
GetName() string
GetDescription() string
GetTools(s *Server) []server.ServerTool
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how can we achieve go-sdk transition by using server.ServerTool but I trust your judgments. (This is independent from this PR, as there is no actual change)

Copy link
Member Author

Choose a reason for hiding this comment

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

You'll see in the follow up PRs.
The idea is to eventually make the toolset definition agnostic of the underlying MCP implementation.
This will actually be much easier with go-sdk, but I still want to do it with the m3labs implementation to be able to have a working snapshot in case there's something wrong with go-sdk and we need to revert.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Thanks.

}

var Toolsets = []Toolset{
&Full{},
}

var ToolsetNames []string

func ToolsetFromString(name string) Toolset {
for _, toolset := range Toolsets {
if toolset.GetName() == name {
return toolset
}
}
return nil
}

type Full struct{}

func (p *Full) GetName() string {
return "full"
}
func (p *Full) GetDescription() string {
return "Complete toolset with all tools and extended outputs"
}
func (p *Full) GetTools(s *Server) []server.ServerTool {
return slices.Concat(
s.initConfiguration(),
s.initEvents(),
s.initNamespaces(),
s.initPods(),
s.initResources(),
s.initHelm(),
)
}

func init() {
ToolsetNames = make([]string, 0)
for _, toolset := range Toolsets {
ToolsetNames = append(ToolsetNames, toolset.GetName())
}
}
8 changes: 4 additions & 4 deletions pkg/mcp/profiles_test.go → pkg/mcp/toolsets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/mark3labs/mcp-go/mcp"
)

func TestFullProfileTools(t *testing.T) {
func TestFullToolsetTools(t *testing.T) {
expectedNames := []string{
"configuration_view",
"events_list",
Expand All @@ -29,7 +29,7 @@ func TestFullProfileTools(t *testing.T) {
"resources_create_or_update",
"resources_delete",
}
mcpCtx := &mcpContext{profile: &FullProfile{}}
mcpCtx := &mcpContext{toolset: &Full{}}
testCaseWithContext(t, mcpCtx, func(c *mcpContext) {
tools, err := c.mcpClient.ListTools(c.ctx, mcp.ListToolsRequest{})
t.Run("ListTools returns tools", func(t *testing.T) {
Expand All @@ -53,9 +53,9 @@ func TestFullProfileTools(t *testing.T) {
})
}

func TestFullProfileToolsInOpenShift(t *testing.T) {
func TestFullToolsetToolsInOpenShift(t *testing.T) {
mcpCtx := &mcpContext{
profile: &FullProfile{},
toolset: &Full{},
before: inOpenShift,
after: inOpenShiftClear,
}
Expand Down
Loading