From 94baad657098c0f258b37bca299c4e774f02327d Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Wed, 17 Sep 2025 17:03:40 +0200 Subject: [PATCH] fix(cmd): disable klog in STDIO mode (#331) Prevents protocol issues with some clients. Kubernetes tooling uses klog to log messages. Some of these messages end up logged in the stderr or stdout which breaks some of the clients that expect jsonrpc messages. Signed-off-by: Marc Nuri --- go.mod | 2 +- pkg/kubernetes-mcp-server/cmd/root.go | 6 +++ pkg/kubernetes-mcp-server/cmd/root_test.go | 43 ++++++++++++++++------ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 7638648..39a35c7 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/spf13/cobra v1.10.1 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 + golang.org/x/net v0.42.0 golang.org/x/oauth2 v0.31.0 golang.org/x/sync v0.17.0 helm.sh/helm/v3 v3.18.6 @@ -122,7 +123,6 @@ require ( go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.40.0 // indirect - golang.org/x/net v0.42.0 // indirect golang.org/x/sys v0.34.0 // indirect golang.org/x/term v0.33.0 // indirect golang.org/x/text v0.28.0 // indirect diff --git a/pkg/kubernetes-mcp-server/cmd/root.go b/pkg/kubernetes-mcp-server/cmd/root.go index 4709d9f..0698815 100644 --- a/pkg/kubernetes-mcp-server/cmd/root.go +++ b/pkg/kubernetes-mcp-server/cmd/root.go @@ -207,6 +207,12 @@ func (m *MCPServerOptions) loadFlags(cmd *cobra.Command) { func (m *MCPServerOptions) initializeLogging() { flagSet := flag.NewFlagSet("klog", flag.ContinueOnError) klog.InitFlags(flagSet) + if m.StaticConfig.Port == "" { + // disable klog output for stdio mode + // this is needed to avoid klog writing to stderr and breaking the protocol + _ = flagSet.Parse([]string{"-logtostderr=false", "-alsologtostderr=false", "-stderrthreshold=FATAL"}) + return + } loggerOptions := []textlogger.ConfigOption{textlogger.Output(m.Out)} if m.StaticConfig.LogLevel >= 0 { loggerOptions = append(loggerOptions, textlogger.Verbosity(m.StaticConfig.LogLevel)) diff --git a/pkg/kubernetes-mcp-server/cmd/root_test.go b/pkg/kubernetes-mcp-server/cmd/root_test.go index c9ac8df..7b8c065 100644 --- a/pkg/kubernetes-mcp-server/cmd/root_test.go +++ b/pkg/kubernetes-mcp-server/cmd/root_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/cli-runtime/pkg/genericiooptions" ) @@ -48,7 +50,7 @@ func TestConfig(t *testing.T) { t.Run("defaults to none", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"}) expectedConfig := `" - Config: "` if err := rootCmd.Execute(); !strings.Contains(out.String(), expectedConfig) { t.Fatalf("Expected config to be %s, got %s %v", expectedConfig, out.String(), err) @@ -59,7 +61,7 @@ func TestConfig(t *testing.T) { rootCmd := NewMCPServer(ioStreams) _, file, _, _ := runtime.Caller(0) emptyConfigPath := filepath.Join(filepath.Dir(file), "testdata", "empty-config.toml") - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--config", emptyConfigPath}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--config", emptyConfigPath}) _ = rootCmd.Execute() expected := `(?m)\" - Config\:[^\"]+empty-config\.toml\"` if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { @@ -69,7 +71,7 @@ func TestConfig(t *testing.T) { t.Run("invalid path throws error", func(t *testing.T) { ioStreams, _ := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--config", "invalid-path-to-config.toml"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--config", "invalid-path-to-config.toml"}) err := rootCmd.Execute() if err == nil { t.Fatal("Expected error for invalid config path, got nil") @@ -142,7 +144,7 @@ func TestToolsets(t *testing.T) { t.Run("default", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"}) if err := rootCmd.Execute(); !strings.Contains(out.String(), "- Toolsets: core, config, helm") { t.Fatalf("Expected toolsets 'full', got %s %v", out, err) } @@ -150,7 +152,7 @@ func TestToolsets(t *testing.T) { t.Run("set with --toolsets", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--toolsets", "helm,config"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--toolsets", "helm,config"}) _ = rootCmd.Execute() expected := `(?m)\" - Toolsets\: helm, config\"` if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { @@ -172,7 +174,7 @@ func TestListOutput(t *testing.T) { t.Run("defaults to table", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"}) if err := rootCmd.Execute(); !strings.Contains(out.String(), "- ListOutput: table") { t.Fatalf("Expected list-output 'table', got %s %v", out, err) } @@ -180,7 +182,7 @@ func TestListOutput(t *testing.T) { t.Run("set with --list-output", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--list-output", "yaml"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--list-output", "yaml"}) _ = rootCmd.Execute() expected := `(?m)\" - ListOutput\: yaml\"` if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { @@ -193,7 +195,7 @@ func TestReadOnly(t *testing.T) { t.Run("defaults to false", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"}) if err := rootCmd.Execute(); !strings.Contains(out.String(), " - Read-only mode: false") { t.Fatalf("Expected read-only mode false, got %s %v", out, err) } @@ -201,7 +203,7 @@ func TestReadOnly(t *testing.T) { t.Run("set with --read-only", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--read-only"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--read-only"}) _ = rootCmd.Execute() expected := `(?m)\" - Read-only mode\: true\"` if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { @@ -214,7 +216,7 @@ func TestDisableDestructive(t *testing.T) { t.Run("defaults to false", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"}) if err := rootCmd.Execute(); !strings.Contains(out.String(), " - Disable destructive tools: false") { t.Fatalf("Expected disable destructive false, got %s %v", out, err) } @@ -222,7 +224,7 @@ func TestDisableDestructive(t *testing.T) { t.Run("set with --disable-destructive", func(t *testing.T) { ioStreams, out := testStream() rootCmd := NewMCPServer(ioStreams) - rootCmd.SetArgs([]string{"--version", "--log-level=1", "--disable-destructive"}) + rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1", "--disable-destructive"}) _ = rootCmd.Execute() expected := `(?m)\" - Disable destructive tools\: true\"` if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { @@ -255,3 +257,22 @@ func TestAuthorizationURL(t *testing.T) { } }) } + +func TestStdioLogging(t *testing.T) { + t.Run("stdio disables klog", func(t *testing.T) { + ioStreams, out := testStream() + rootCmd := NewMCPServer(ioStreams) + rootCmd.SetArgs([]string{"--version", "--log-level=1"}) + err := rootCmd.Execute() + require.NoErrorf(t, err, "Expected no error executing command, got %v", err) + assert.Equalf(t, "0.0.0\n", out.String(), "Expected only version output, got %s", out.String()) + }) + t.Run("http mode enables klog", func(t *testing.T) { + ioStreams, out := testStream() + rootCmd := NewMCPServer(ioStreams) + rootCmd.SetArgs([]string{"--version", "--log-level=1", "--port=1337"}) + err := rootCmd.Execute() + require.NoErrorf(t, err, "Expected no error executing command, got %v", err) + assert.Containsf(t, out.String(), "Starting kubernetes-mcp-server", "Expected klog output, got %s", out.String()) + }) +}