Skip to content

Commit a301b0f

Browse files
authored
test(mcp): refactor logging tests to use testify (#439)
All tests now use testify, removing unneeded testing infrastructure for legacy tests. Signed-off-by: Marc Nuri <marc@marcnuri.com>
1 parent 62bd582 commit a301b0f

File tree

2 files changed

+73
-193
lines changed

2 files changed

+73
-193
lines changed

pkg/mcp/common_test.go

Lines changed: 0 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,16 @@
11
package mcp
22

33
import (
4-
"bytes"
54
"context"
65
"encoding/json"
7-
"flag"
86
"fmt"
9-
"net/http/httptest"
107
"os"
118
"path/filepath"
129
"runtime"
13-
"strconv"
1410
"testing"
1511
"time"
1612

17-
"github.com/mark3labs/mcp-go/client"
1813
"github.com/mark3labs/mcp-go/client/transport"
19-
"github.com/mark3labs/mcp-go/mcp"
20-
"github.com/mark3labs/mcp-go/server"
2114
"github.com/pkg/errors"
2215
"github.com/spf13/afero"
2316
"github.com/stretchr/testify/suite"
@@ -30,11 +23,7 @@ import (
3023
"k8s.io/apimachinery/pkg/watch"
3124
"k8s.io/client-go/kubernetes"
3225
"k8s.io/client-go/rest"
33-
"k8s.io/client-go/tools/clientcmd"
34-
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
3526
toolswatch "k8s.io/client-go/tools/watch"
36-
"k8s.io/klog/v2"
37-
"k8s.io/klog/v2/textlogger"
3827
"k8s.io/utils/ptr"
3928
"sigs.k8s.io/controller-runtime/pkg/envtest"
4029
"sigs.k8s.io/controller-runtime/tools/setup-envtest/env"
@@ -45,7 +34,6 @@ import (
4534

4635
"github.com/containers/kubernetes-mcp-server/internal/test"
4736
"github.com/containers/kubernetes-mcp-server/pkg/config"
48-
"github.com/containers/kubernetes-mcp-server/pkg/output"
4937
)
5038

5139
// envTest has an expensive setup, so we only want to do it once per entire test run.
@@ -103,133 +91,6 @@ func TestMain(m *testing.M) {
10391
os.Exit(code)
10492
}
10593

106-
type mcpContext struct {
107-
toolsets []string
108-
listOutput output.Output
109-
logLevel int
110-
111-
staticConfig *config.StaticConfig
112-
clientOptions []transport.ClientOption
113-
before func(*mcpContext)
114-
after func(*mcpContext)
115-
ctx context.Context
116-
tempDir string
117-
cancel context.CancelFunc
118-
mcpServer *Server
119-
mcpHttpServer *httptest.Server
120-
mcpClient *client.Client
121-
klogState klog.State
122-
logBuffer bytes.Buffer
123-
}
124-
125-
func (c *mcpContext) beforeEach(t *testing.T) {
126-
var err error
127-
c.ctx, c.cancel = context.WithCancel(t.Context())
128-
c.tempDir = t.TempDir()
129-
c.withKubeConfig(nil)
130-
if c.staticConfig == nil {
131-
c.staticConfig = config.Default()
132-
// Default to use YAML output for lists (previously the default)
133-
c.staticConfig.ListOutput = "yaml"
134-
}
135-
if c.toolsets != nil {
136-
c.staticConfig.Toolsets = c.toolsets
137-
138-
}
139-
if c.listOutput != nil {
140-
c.staticConfig.ListOutput = c.listOutput.GetName()
141-
}
142-
if c.before != nil {
143-
c.before(c)
144-
}
145-
// Set up logging
146-
c.klogState = klog.CaptureState()
147-
flags := flag.NewFlagSet("test", flag.ContinueOnError)
148-
klog.InitFlags(flags)
149-
_ = flags.Set("v", strconv.Itoa(c.logLevel))
150-
klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(c.logLevel), textlogger.Output(&c.logBuffer))))
151-
// MCP Server
152-
if c.mcpServer, err = NewServer(Configuration{StaticConfig: c.staticConfig}); err != nil {
153-
t.Fatal(err)
154-
return
155-
}
156-
c.mcpHttpServer = server.NewTestServer(c.mcpServer.server, server.WithSSEContextFunc(contextFunc))
157-
if c.mcpClient, err = client.NewSSEMCPClient(c.mcpHttpServer.URL+"/sse", c.clientOptions...); err != nil {
158-
t.Fatal(err)
159-
return
160-
}
161-
// MCP Client
162-
if err = c.mcpClient.Start(c.ctx); err != nil {
163-
t.Fatal(err)
164-
return
165-
}
166-
initRequest := mcp.InitializeRequest{}
167-
initRequest.Params.ProtocolVersion = mcp.LATEST_PROTOCOL_VERSION
168-
initRequest.Params.ClientInfo = mcp.Implementation{Name: "test", Version: "1.33.7"}
169-
_, err = c.mcpClient.Initialize(c.ctx, initRequest)
170-
if err != nil {
171-
t.Fatal(err)
172-
return
173-
}
174-
}
175-
176-
func (c *mcpContext) afterEach() {
177-
if c.after != nil {
178-
c.after(c)
179-
}
180-
c.cancel()
181-
c.mcpServer.Close()
182-
_ = c.mcpClient.Close()
183-
c.mcpHttpServer.Close()
184-
c.klogState.Restore()
185-
}
186-
187-
func testCaseWithContext(t *testing.T, mcpCtx *mcpContext, test func(c *mcpContext)) {
188-
mcpCtx.beforeEach(t)
189-
defer mcpCtx.afterEach()
190-
test(mcpCtx)
191-
}
192-
193-
// withKubeConfig sets up a fake kubeconfig in the temp directory based on the provided rest.Config
194-
func (c *mcpContext) withKubeConfig(rc *rest.Config) *clientcmdapi.Config {
195-
fakeConfig := clientcmdapi.NewConfig()
196-
fakeConfig.Clusters["fake"] = clientcmdapi.NewCluster()
197-
fakeConfig.Clusters["fake"].Server = "https://127.0.0.1:6443"
198-
fakeConfig.Clusters["additional-cluster"] = clientcmdapi.NewCluster()
199-
fakeConfig.AuthInfos["fake"] = clientcmdapi.NewAuthInfo()
200-
fakeConfig.AuthInfos["additional-auth"] = clientcmdapi.NewAuthInfo()
201-
if rc != nil {
202-
fakeConfig.Clusters["fake"].Server = rc.Host
203-
fakeConfig.Clusters["fake"].CertificateAuthorityData = rc.CAData
204-
fakeConfig.AuthInfos["fake"].ClientKeyData = rc.KeyData
205-
fakeConfig.AuthInfos["fake"].ClientCertificateData = rc.CertData
206-
}
207-
fakeConfig.Contexts["fake-context"] = clientcmdapi.NewContext()
208-
fakeConfig.Contexts["fake-context"].Cluster = "fake"
209-
fakeConfig.Contexts["fake-context"].AuthInfo = "fake"
210-
fakeConfig.Contexts["additional-context"] = clientcmdapi.NewContext()
211-
fakeConfig.Contexts["additional-context"].Cluster = "additional-cluster"
212-
fakeConfig.Contexts["additional-context"].AuthInfo = "additional-auth"
213-
fakeConfig.CurrentContext = "fake-context"
214-
kubeConfig := filepath.Join(c.tempDir, "config")
215-
_ = clientcmd.WriteToFile(*fakeConfig, kubeConfig)
216-
_ = os.Setenv("KUBECONFIG", kubeConfig)
217-
if c.mcpServer != nil {
218-
if err := c.mcpServer.reloadKubernetesClusterProvider(); err != nil {
219-
panic(err)
220-
}
221-
}
222-
return fakeConfig
223-
}
224-
225-
// callTool helper function to call a tool by name with arguments
226-
func (c *mcpContext) callTool(name string, args map[string]interface{}) (*mcp.CallToolResult, error) {
227-
callToolRequest := mcp.CallToolRequest{}
228-
callToolRequest.Params.Name = name
229-
callToolRequest.Params.Arguments = args
230-
return c.mcpClient.CallTool(c.ctx, callToolRequest)
231-
}
232-
23394
func restoreAuth(ctx context.Context) {
23495
kubernetesAdmin := kubernetes.NewForConfigOrDie(envTest.Config)
23596
// Authorization

pkg/mcp/mcp_middleware_test.go

Lines changed: 73 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,87 @@
11
package mcp
22

33
import (
4+
"bytes"
5+
"flag"
46
"regexp"
5-
"strings"
7+
"strconv"
68
"testing"
79

810
"github.com/mark3labs/mcp-go/client/transport"
11+
"github.com/stretchr/testify/suite"
12+
"k8s.io/klog/v2"
13+
"k8s.io/klog/v2/textlogger"
914
)
1015

11-
func TestToolCallLogging(t *testing.T) {
12-
testCaseWithContext(t, &mcpContext{logLevel: 5}, func(c *mcpContext) {
13-
_, _ = c.callTool("configuration_view", map[string]interface{}{
14-
"minified": false,
15-
})
16-
t.Run("Logs tool name", func(t *testing.T) {
17-
expectedLog := "mcp tool call: configuration_view("
18-
if !strings.Contains(c.logBuffer.String(), expectedLog) {
19-
t.Errorf("Expected log to contain '%s', got: %s", expectedLog, c.logBuffer.String())
20-
}
21-
})
22-
t.Run("Logs tool call arguments", func(t *testing.T) {
23-
expected := `"mcp tool call: configuration_view\((.+)\)"`
24-
m := regexp.MustCompile(expected).FindStringSubmatch(c.logBuffer.String())
25-
if len(m) != 2 {
26-
t.Fatalf("Expected log entry to contain arguments, got %s", c.logBuffer.String())
27-
}
28-
if m[1] != "map[minified:false]" {
29-
t.Errorf("Expected log arguments to be 'map[minified:false]', got %s", m[1])
30-
}
31-
})
16+
type McpLoggingSuite struct {
17+
BaseMcpSuite
18+
klogState klog.State
19+
logBuffer bytes.Buffer
20+
}
21+
22+
func (s *McpLoggingSuite) SetupTest() {
23+
s.BaseMcpSuite.SetupTest()
24+
s.klogState = klog.CaptureState()
25+
}
26+
27+
func (s *McpLoggingSuite) TearDownTest() {
28+
s.BaseMcpSuite.TearDownTest()
29+
s.klogState.Restore()
30+
}
31+
32+
func (s *McpLoggingSuite) SetLogLevel(level int) {
33+
flags := flag.NewFlagSet("test", flag.ContinueOnError)
34+
klog.InitFlags(flags)
35+
_ = flags.Set("v", strconv.Itoa(level))
36+
klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(level), textlogger.Output(&s.logBuffer))))
37+
}
38+
39+
func (s *McpLoggingSuite) TestLogsToolCall() {
40+
s.SetLogLevel(5)
41+
s.InitMcpClient()
42+
_, err := s.CallTool("configuration_view", map[string]interface{}{"minified": false})
43+
s.Require().NoError(err, "call to tool configuration_view failed")
44+
45+
s.Run("Logs tool name", func() {
46+
s.Contains(s.logBuffer.String(), "mcp tool call: configuration_view(")
3247
})
33-
before := func(c *mcpContext) {
34-
c.clientOptions = append(c.clientOptions, transport.WithHeaders(map[string]string{
35-
"Accept-Encoding": "gzip",
36-
"Authorization": "Bearer should-not-be-logged",
37-
"authorization": "Bearer should-not-be-logged",
38-
"a-loggable-header": "should-be-logged",
39-
}))
48+
s.Run("Logs tool call arguments", func() {
49+
expected := `"mcp tool call: configuration_view\((.+)\)"`
50+
m := regexp.MustCompile(expected).FindStringSubmatch(s.logBuffer.String())
51+
s.Len(m, 2, "Expected log entry to contain arguments")
52+
s.Equal("map[minified:false]", m[1], "Expected log arguments to be 'map[minified:false]'")
53+
})
54+
}
55+
56+
func (s *McpLoggingSuite) TestLogsToolCallHeaders() {
57+
s.SetLogLevel(7)
58+
s.InitMcpClient(transport.WithHTTPHeaders(map[string]string{
59+
"Accept-Encoding": "gzip",
60+
"Authorization": "Bearer should-not-be-logged",
61+
"authorization": "Bearer should-not-be-logged",
62+
"a-loggable-header": "should-be-logged",
63+
}))
64+
_, err := s.CallTool("configuration_view", map[string]interface{}{"minified": false})
65+
s.Require().NoError(err, "call to tool configuration_view failed")
66+
67+
s.Run("Logs tool call headers", func() {
68+
expectedLog := "mcp tool call headers: A-Loggable-Header: should-be-logged"
69+
s.Contains(s.logBuffer.String(), expectedLog, "Expected log to contain loggable header")
70+
})
71+
sensitiveHeaders := []string{
72+
"Authorization:",
73+
// TODO: Add more sensitive headers as needed
4074
}
41-
testCaseWithContext(t, &mcpContext{logLevel: 7, before: before}, func(c *mcpContext) {
42-
_, _ = c.callTool("configuration_view", map[string]interface{}{
43-
"minified": false,
44-
})
45-
t.Run("Logs tool call headers", func(t *testing.T) {
46-
expectedLog := "mcp tool call headers: A-Loggable-Header: should-be-logged"
47-
if !strings.Contains(c.logBuffer.String(), expectedLog) {
48-
t.Errorf("Expected log to contain '%s', got: %s", expectedLog, c.logBuffer.String())
49-
}
50-
})
51-
sensitiveHeaders := []string{
52-
"Authorization:",
53-
// TODO: Add more sensitive headers as needed
75+
s.Run("Does not log sensitive headers", func() {
76+
for _, header := range sensitiveHeaders {
77+
s.NotContains(s.logBuffer.String(), header, "Log should not contain sensitive header")
5478
}
55-
t.Run("Does not log sensitive headers", func(t *testing.T) {
56-
for _, header := range sensitiveHeaders {
57-
if strings.Contains(c.logBuffer.String(), header) {
58-
t.Errorf("Log should not contain sensitive header '%s', got: %s", header, c.logBuffer.String())
59-
}
60-
}
61-
})
62-
t.Run("Does not log sensitive header values", func(t *testing.T) {
63-
if strings.Contains(c.logBuffer.String(), "should-not-be-logged") {
64-
t.Errorf("Log should not contain sensitive header value 'should-not-be-logged', got: %s", c.logBuffer.String())
65-
}
66-
})
6779
})
80+
s.Run("Does not log sensitive header values", func() {
81+
s.NotContains(s.logBuffer.String(), "should-not-be-logged", "Log should not contain sensitive header value")
82+
})
83+
}
84+
85+
func TestMcpLogging(t *testing.T) {
86+
suite.Run(t, new(McpLoggingSuite))
6887
}

0 commit comments

Comments
 (0)