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 <marc@marcnuri.com>
This commit is contained in:
Marc Nuri
2025-09-17 17:03:40 +02:00
committed by GitHub
parent e16114dfc5
commit 94baad6570
3 changed files with 39 additions and 12 deletions

2
go.mod
View File

@@ -14,6 +14,7 @@ require (
github.com/spf13/cobra v1.10.1 github.com/spf13/cobra v1.10.1
github.com/spf13/pflag v1.0.10 github.com/spf13/pflag v1.0.10
github.com/stretchr/testify v1.11.1 github.com/stretchr/testify v1.11.1
golang.org/x/net v0.42.0
golang.org/x/oauth2 v0.31.0 golang.org/x/oauth2 v0.31.0
golang.org/x/sync v0.17.0 golang.org/x/sync v0.17.0
helm.sh/helm/v3 v3.18.6 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/v2 v2.4.2 // indirect
go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect
golang.org/x/crypto v0.40.0 // 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/sys v0.34.0 // indirect
golang.org/x/term v0.33.0 // indirect golang.org/x/term v0.33.0 // indirect
golang.org/x/text v0.28.0 // indirect golang.org/x/text v0.28.0 // indirect

View File

@@ -207,6 +207,12 @@ func (m *MCPServerOptions) loadFlags(cmd *cobra.Command) {
func (m *MCPServerOptions) initializeLogging() { func (m *MCPServerOptions) initializeLogging() {
flagSet := flag.NewFlagSet("klog", flag.ContinueOnError) flagSet := flag.NewFlagSet("klog", flag.ContinueOnError)
klog.InitFlags(flagSet) 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)} loggerOptions := []textlogger.ConfigOption{textlogger.Output(m.Out)}
if m.StaticConfig.LogLevel >= 0 { if m.StaticConfig.LogLevel >= 0 {
loggerOptions = append(loggerOptions, textlogger.Verbosity(m.StaticConfig.LogLevel)) loggerOptions = append(loggerOptions, textlogger.Verbosity(m.StaticConfig.LogLevel))

View File

@@ -10,6 +10,8 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/cli-runtime/pkg/genericiooptions"
) )
@@ -48,7 +50,7 @@ func TestConfig(t *testing.T) {
t.Run("defaults to none", func(t *testing.T) { t.Run("defaults to none", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) rootCmd := NewMCPServer(ioStreams)
rootCmd.SetArgs([]string{"--version", "--log-level=1"}) rootCmd.SetArgs([]string{"--version", "--port=1337", "--log-level=1"})
expectedConfig := `" - Config: "` expectedConfig := `" - Config: "`
if err := rootCmd.Execute(); !strings.Contains(out.String(), expectedConfig) { if err := rootCmd.Execute(); !strings.Contains(out.String(), expectedConfig) {
t.Fatalf("Expected config to be %s, got %s %v", expectedConfig, out.String(), err) 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) rootCmd := NewMCPServer(ioStreams)
_, file, _, _ := runtime.Caller(0) _, file, _, _ := runtime.Caller(0)
emptyConfigPath := filepath.Join(filepath.Dir(file), "testdata", "empty-config.toml") 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() _ = rootCmd.Execute()
expected := `(?m)\" - Config\:[^\"]+empty-config\.toml\"` expected := `(?m)\" - Config\:[^\"]+empty-config\.toml\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { 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) { t.Run("invalid path throws error", func(t *testing.T) {
ioStreams, _ := testStream() ioStreams, _ := testStream()
rootCmd := NewMCPServer(ioStreams) 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() err := rootCmd.Execute()
if err == nil { if err == nil {
t.Fatal("Expected error for invalid config path, got 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) { t.Run("default", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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") { if err := rootCmd.Execute(); !strings.Contains(out.String(), "- Toolsets: core, config, helm") {
t.Fatalf("Expected toolsets 'full', got %s %v", out, err) 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) { t.Run("set with --toolsets", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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() _ = rootCmd.Execute()
expected := `(?m)\" - Toolsets\: helm, config\"` expected := `(?m)\" - Toolsets\: helm, config\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { 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) { t.Run("defaults to table", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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") { if err := rootCmd.Execute(); !strings.Contains(out.String(), "- ListOutput: table") {
t.Fatalf("Expected list-output 'table', got %s %v", out, err) 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) { t.Run("set with --list-output", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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() _ = rootCmd.Execute()
expected := `(?m)\" - ListOutput\: yaml\"` expected := `(?m)\" - ListOutput\: yaml\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { 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) { t.Run("defaults to false", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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") { 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.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) { t.Run("set with --read-only", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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() _ = rootCmd.Execute()
expected := `(?m)\" - Read-only mode\: true\"` expected := `(?m)\" - Read-only mode\: true\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { 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) { t.Run("defaults to false", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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") { if err := rootCmd.Execute(); !strings.Contains(out.String(), " - Disable destructive tools: false") {
t.Fatalf("Expected disable destructive false, got %s %v", out, err) 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) { t.Run("set with --disable-destructive", func(t *testing.T) {
ioStreams, out := testStream() ioStreams, out := testStream()
rootCmd := NewMCPServer(ioStreams) 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() _ = rootCmd.Execute()
expected := `(?m)\" - Disable destructive tools\: true\"` expected := `(?m)\" - Disable destructive tools\: true\"`
if m, err := regexp.MatchString(expected, out.String()); !m || err != nil { 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())
})
}