fix(cmd): disable klog in STDIO mode (containers#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 <[email protected]>

Author: manusa

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

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))

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,15 +144,15 @@ 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)
 		}
 	})
 	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,15 +174,15 @@ 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)
 		}
 	})
 	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,15 +195,15 @@ 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)
 		}
 	})
 	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,15 +216,15 @@ 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)
 		}
 	})
 	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())
+	})
+}