Skip to content

Commit 9a56372

Browse files
ngopalak-redhatngopalak-redhat
committed
Rebase node_files and move the test to different file
Signed-off-by: Neeraj Krishna Gopalakrishna <[email protected]>
1 parent 8b38d37 commit 9a56372

File tree

8 files changed

+345
-301
lines changed

8 files changed

+345
-301
lines changed

‎pkg/kubernetes/nodes.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
v1 "k8s.io/api/core/v1"
1414
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/util/httpstream"
1516
"k8s.io/apimachinery/pkg/util/rand"
1617
"k8s.io/client-go/tools/remotecommand"
1718
"k8s.io/metrics/pkg/apis/metrics"
@@ -169,7 +170,7 @@ func (k *Kubernetes) NodesFiles(ctx context.Context, opts NodeFilesOptions) (str
169170
}
170171

171172
// Create the pod
172-
pods, err := k.manager.accessControlClientSet.Pods(opts.Namespace)
173+
pods, err := k.AccessControlClientset().Pods(opts.Namespace)
173174
if err != nil {
174175
return "", fmt.Errorf("failed to get pods client: %w", err)
175176
}
@@ -288,7 +289,26 @@ func (k *Kubernetes) execInPod(ctx context.Context, namespace, podName string, c
288289
Stderr: true,
289290
}
290291

291-
executor, err := k.manager.accessControlClientSet.PodsExec(namespace, podName, podExecOptions)
292+
// Compute URL
293+
execRequest := k.AccessControlClientset().CoreV1().RESTClient().
294+
Post().
295+
Resource("pods").
296+
Namespace(namespace).
297+
Name(podName).
298+
SubResource("exec")
299+
execRequest.VersionedParams(podExecOptions, ParameterCodec)
300+
301+
spdyExec, err := remotecommand.NewSPDYExecutor(k.AccessControlClientset().cfg, "POST", execRequest.URL())
302+
if err != nil {
303+
return "", err
304+
}
305+
webSocketExec, err := remotecommand.NewWebSocketExecutor(k.AccessControlClientset().cfg, "GET", execRequest.URL().String())
306+
if err != nil {
307+
return "", err
308+
}
309+
executor, err := remotecommand.NewFallbackExecutor(webSocketExec, spdyExec, func(err error) bool {
310+
return httpstream.IsUpgradeFailure(err) || httpstream.IsHTTPSProxyError(err)
311+
})
292312
if err != nil {
293313
return "", err
294314
}
@@ -316,7 +336,7 @@ func (k *Kubernetes) execInPod(ctx context.Context, namespace, podName string, c
316336

317337
// waitForPodReady waits for a pod to be ready
318338
func (k *Kubernetes) waitForPodReady(ctx context.Context, namespace, podName string, timeout time.Duration) error {
319-
pods, err := k.manager.accessControlClientSet.Pods(namespace)
339+
pods, err := k.AccessControlClientset().Pods(namespace)
320340
if err != nil {
321341
return err
322342
}

‎pkg/mcp/nodes_files_test.go‎

Lines changed: 314 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
package mcp
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/BurntSushi/toml"
8+
"github.com/containers/kubernetes-mcp-server/internal/test"
9+
"github.com/mark3labs/mcp-go/mcp"
10+
"github.com/stretchr/testify/suite"
11+
)
12+
13+
type NodeFilesSuite struct {
14+
BaseMcpSuite
15+
mockServer *test.MockServer
16+
}
17+
18+
func (s *NodeFilesSuite) SetupTest() {
19+
s.BaseMcpSuite.SetupTest()
20+
s.mockServer = test.NewMockServer()
21+
s.Cfg.KubeConfig = s.mockServer.KubeconfigFile(s.T())
22+
s.mockServer.Handle(&test.DiscoveryClientHandler{})
23+
}
24+
25+
func (s *NodeFilesSuite) TearDownTest() {
26+
s.BaseMcpSuite.TearDownTest()
27+
if s.mockServer != nil {
28+
s.mockServer.Close()
29+
}
30+
}
31+
32+
func (s *NodeFilesSuite) TestNodeFiles() {
33+
// Setup test files and directories
34+
s.T().Run("prepare test environment", func(t *testing.T) {
35+
// This ensures we have a node in the cluster for testing
36+
s.mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
37+
// Get Node response
38+
if req.URL.Path == "/api/v1/nodes/test-node" {
39+
w.Header().Set("Content-Type", "application/json")
40+
w.WriteHeader(http.StatusOK)
41+
_, _ = w.Write([]byte(`{
42+
"apiVersion": "v1",
43+
"kind": "Node",
44+
"metadata": {
45+
"name": "test-node"
46+
}
47+
}`))
48+
return
49+
}
50+
// Handle pod creation
51+
if req.URL.Path == "/api/v1/namespaces/default/pods" && req.Method == "POST" {
52+
w.Header().Set("Content-Type", "application/json")
53+
w.WriteHeader(http.StatusCreated)
54+
_, _ = w.Write([]byte(`{
55+
"apiVersion": "v1",
56+
"kind": "Pod",
57+
"metadata": {
58+
"name": "node-files-test",
59+
"namespace": "default"
60+
},
61+
"status": {
62+
"phase": "Running",
63+
"conditions": [{
64+
"type": "Ready",
65+
"status": "True"
66+
}]
67+
}
68+
}`))
69+
return
70+
}
71+
// Handle pod get (for wait)
72+
if req.URL.Path == "/api/v1/namespaces/default/pods/node-files-test" && req.Method == "GET" {
73+
w.Header().Set("Content-Type", "application/json")
74+
w.WriteHeader(http.StatusOK)
75+
_, _ = w.Write([]byte(`{
76+
"apiVersion": "v1",
77+
"kind": "Pod",
78+
"metadata": {
79+
"name": "node-files-test",
80+
"namespace": "default"
81+
},
82+
"status": {
83+
"phase": "Running",
84+
"conditions": [{
85+
"type": "Ready",
86+
"status": "True"
87+
}]
88+
}
89+
}`))
90+
return
91+
}
92+
// Not handled by this handler, let it fall through to discovery handler
93+
}))
94+
})
95+
96+
s.InitMcpClient()
97+
98+
// Test missing node_name parameter
99+
s.Run("node_files(node_name=nil)", func() {
100+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
101+
"operation": "list",
102+
"source_path": "/tmp",
103+
})
104+
s.Require().NotNil(toolResult, "toolResult should not be nil")
105+
s.Run("has error", func() {
106+
s.Truef(toolResult.IsError, "call tool should fail")
107+
s.Nilf(err, "call tool should not return error object")
108+
})
109+
s.Run("describes missing node_name", func() {
110+
expectedMessage := "missing required argument: node_name"
111+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
112+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
113+
})
114+
})
115+
116+
// Test missing operation parameter
117+
s.Run("node_files(operation=nil)", func() {
118+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
119+
"node_name": "test-node",
120+
"source_path": "/tmp",
121+
})
122+
s.Require().NotNil(toolResult, "toolResult should not be nil")
123+
s.Run("has error", func() {
124+
s.Truef(toolResult.IsError, "call tool should fail")
125+
s.Nilf(err, "call tool should not return error object")
126+
})
127+
s.Run("describes missing operation", func() {
128+
expectedMessage := "missing required argument: operation"
129+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
130+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
131+
})
132+
})
133+
134+
// Test missing source_path parameter
135+
s.Run("node_files(source_path=nil)", func() {
136+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
137+
"node_name": "test-node",
138+
"operation": "list",
139+
})
140+
s.Require().NotNil(toolResult, "toolResult should not be nil")
141+
s.Run("has error", func() {
142+
s.Truef(toolResult.IsError, "call tool should fail")
143+
s.Nilf(err, "call tool should not return error object")
144+
})
145+
s.Run("describes missing source_path", func() {
146+
expectedMessage := "missing required argument: source_path"
147+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
148+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
149+
})
150+
})
151+
152+
// Test invalid operation
153+
s.Run("node_files(operation=invalid)", func() {
154+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
155+
"node_name": "test-node",
156+
"operation": "invalid",
157+
"source_path": "/tmp",
158+
})
159+
s.Require().NotNil(toolResult, "toolResult should not be nil")
160+
s.Run("has error", func() {
161+
s.Truef(toolResult.IsError, "call tool should fail")
162+
s.Nilf(err, "call tool should not return error object")
163+
})
164+
s.Run("describes invalid operation", func() {
165+
content := toolResult.Content[0].(mcp.TextContent).Text
166+
s.Containsf(content, "failed to perform node file operation", "expected error to mention failed operation, got %v", content)
167+
})
168+
})
169+
170+
// Test with non-existent node
171+
s.Run("node_files(node_name=non-existent-node)", func() {
172+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
173+
"node_name": "non-existent-node",
174+
"operation": "list",
175+
"source_path": "/tmp",
176+
})
177+
s.Require().NotNil(toolResult, "toolResult should not be nil")
178+
s.Run("has error", func() {
179+
s.Truef(toolResult.IsError, "call tool should fail")
180+
s.Nilf(err, "call tool should not return error object")
181+
})
182+
s.Run("describes missing node", func() {
183+
content := toolResult.Content[0].(mcp.TextContent).Text
184+
s.Containsf(content, "failed to perform node file operation", "expected error to mention failed operation, got %v", content)
185+
})
186+
})
187+
188+
// Test with default namespace and image
189+
s.Run("node_files with defaults", func() {
190+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
191+
"node_name": "test-node",
192+
"operation": "list",
193+
"source_path": "/tmp",
194+
})
195+
s.Require().NotNil(toolResult, "toolResult should not be nil")
196+
// Note: This will fail in the mock environment, but we're testing parameter handling
197+
s.Run("attempts operation", func() {
198+
// The tool should attempt the operation even if it fails in mock environment
199+
s.NotNil(toolResult, "toolResult should not be nil")
200+
})
201+
})
202+
203+
// Test with custom namespace
204+
s.Run("node_files with custom namespace", func() {
205+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
206+
"node_name": "test-node",
207+
"operation": "list",
208+
"source_path": "/tmp",
209+
"namespace": "custom-ns",
210+
})
211+
s.Require().NotNil(toolResult, "toolResult should not be nil")
212+
// The operation will fail in mock environment, but we're verifying parameters are passed
213+
})
214+
215+
// Test with custom image
216+
s.Run("node_files with custom image", func() {
217+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
218+
"node_name": "test-node",
219+
"operation": "list",
220+
"source_path": "/tmp",
221+
"image": "alpine",
222+
})
223+
s.Require().NotNil(toolResult, "toolResult should not be nil")
224+
// The operation will fail in mock environment, but we're verifying parameters are passed
225+
s.NotNil(toolResult)
226+
})
227+
228+
// Test with privileged=false
229+
s.Run("node_files with privileged=false", func() {
230+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
231+
"node_name": "test-node",
232+
"operation": "list",
233+
"source_path": "/tmp",
234+
"privileged": false,
235+
})
236+
s.Require().NotNil(toolResult, "toolResult should not be nil")
237+
// The operation will fail in mock environment, but we're verifying parameters are passed
238+
s.NotNil(toolResult)
239+
})
240+
241+
// Test list operation
242+
s.Run("node_files operation=list", func() {
243+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
244+
"node_name": "test-node",
245+
"operation": "list",
246+
"source_path": "/proc",
247+
})
248+
s.Require().NotNil(toolResult, "toolResult should not be nil")
249+
// Will fail in mock environment but tests the operation type
250+
})
251+
252+
// Test get operation
253+
s.Run("node_files operation=get", func() {
254+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
255+
"node_name": "test-node",
256+
"operation": "get",
257+
"source_path": "/proc/cpuinfo",
258+
"dest_path": "/tmp/cpuinfo",
259+
})
260+
s.Require().NotNil(toolResult, "toolResult should not be nil")
261+
// Will fail in mock environment but tests the operation type
262+
})
263+
264+
// Test get operation without dest_path
265+
s.Run("node_files operation=get without dest_path", func() {
266+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
267+
"node_name": "test-node",
268+
"operation": "get",
269+
"source_path": "/proc/meminfo",
270+
})
271+
s.Require().NotNil(toolResult, "toolResult should not be nil")
272+
// Will fail in mock environment but tests the operation type
273+
})
274+
275+
// Test put operation
276+
s.Run("node_files operation=put", func() {
277+
toolResult, _ := s.CallTool("node_files", map[string]interface{}{
278+
"node_name": "test-node",
279+
"operation": "put",
280+
"source_path": "/tmp/local-file",
281+
"dest_path": "/tmp/node-file",
282+
})
283+
s.Require().NotNil(toolResult, "toolResult should not be nil")
284+
// Will fail in mock environment but tests the operation type
285+
})
286+
}
287+
288+
func (s *NodeFilesSuite) TestNodeFilesDenied() {
289+
s.Require().NoError(toml.Unmarshal([]byte(`
290+
denied_resources = [ { version = "v1", kind = "Pod" } ]
291+
`), s.Cfg), "Expected to parse denied resources config")
292+
s.InitMcpClient()
293+
s.Run("node_files (denied)", func() {
294+
toolResult, err := s.CallTool("node_files", map[string]interface{}{
295+
"node_name": "test-node",
296+
"operation": "list",
297+
"source_path": "/tmp",
298+
})
299+
s.Require().NotNil(toolResult, "toolResult should not be nil")
300+
s.Run("has error", func() {
301+
s.Truef(toolResult.IsError, "call tool should fail")
302+
s.Nilf(err, "call tool should not return error object")
303+
})
304+
s.Run("describes denial", func() {
305+
expectedMessage := "failed to perform node file operation: resource not allowed: /v1, Kind=Pod"
306+
s.Containsf(toolResult.Content[0].(mcp.TextContent).Text, "resource not allowed",
307+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
308+
})
309+
})
310+
}
311+
312+
func TestNodeFiles(t *testing.T) {
313+
suite.Run(t, new(NodeFilesSuite))
314+
}

0 commit comments

Comments
 (0)