From 43c17628225487b2d11dc56ca94ec74bea02ee84 Mon Sep 17 00:00:00 2001 From: Neeraj Krishna Gopalakrishna Date: Mon, 27 Oct 2025 10:02:41 +0530 Subject: [PATCH 1/3] Fix the node log query Signed-off-by: Neeraj Krishna Gopalakrishna --- pkg/kubernetes/accesscontrol_clientset.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/kubernetes/accesscontrol_clientset.go b/pkg/kubernetes/accesscontrol_clientset.go index 0ce64c49..d4cddaaf 100644 --- a/pkg/kubernetes/accesscontrol_clientset.go +++ b/pkg/kubernetes/accesscontrol_clientset.go @@ -49,10 +49,11 @@ func (a *AccessControlClientset) NodesLogs(ctx context.Context, name, logPath st return nil, fmt.Errorf("failed to get node %s: %w", name, err) } - url := []string{"api", "v1", "nodes", name, "proxy", "logs", logPath} + url := []string{"api", "v1", "nodes", name, "proxy", "logs"} return a.delegate.CoreV1().RESTClient(). Get(). - AbsPath(url...), nil + AbsPath(url...). + Param("query", logPath), nil } func (a *AccessControlClientset) Pods(namespace string) (corev1.PodInterface, error) { From 14fa2b31618f370ba01bea9528e7c11261b55b6c Mon Sep 17 00:00:00 2001 From: Neeraj Krishna Gopalakrishna Date: Mon, 27 Oct 2025 15:03:26 +0530 Subject: [PATCH 2/3] Fix the node log query Signed-off-by: Neeraj Krishna Gopalakrishna --- pkg/mcp/nodes_test.go | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/pkg/mcp/nodes_test.go b/pkg/mcp/nodes_test.go index ce2cbc7e..f0d42bbf 100644 --- a/pkg/mcp/nodes_test.go +++ b/pkg/mcp/nodes_test.go @@ -43,23 +43,29 @@ func (s *NodesSuite) TestNodesLog() { }`)) return } - // Get Empty Log response - if req.URL.Path == "/api/v1/nodes/existing-node/proxy/logs/empty.log" { - w.Header().Set("Content-Type", "text/plain") - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(``)) - return - } - // Get Kubelet Log response - if req.URL.Path == "/api/v1/nodes/existing-node/proxy/logs/kubelet.log" { - w.Header().Set("Content-Type", "text/plain") - w.WriteHeader(http.StatusOK) - logContent := "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n" - if req.URL.Query().Get("tailLines") != "" { - logContent = "Line 4\nLine 5\n" + // Check for log proxy requests based on path and query parameters + if req.URL.Path == "/api/v1/nodes/existing-node/proxy/logs" { + logPath := req.URL.Query().Get("query") + + // Get Empty Log response + if logPath == "empty.log" { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(``)) + return + } + + // Get Kubelet Log response + if logPath == "kubelet.log" { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusOK) + logContent := "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n" + if req.URL.Query().Get("tailLines") != "" { + logContent = "Line 4\nLine 5\n" + } + _, _ = w.Write([]byte(logContent)) + return } - _, _ = w.Write([]byte(logContent)) - return } w.WriteHeader(http.StatusNotFound) })) From 5553a4e8ccb1cbb86f2ff1a6ef0dd018f3af0bf0 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Mon, 27 Oct 2025 18:32:56 +0100 Subject: [PATCH 3/3] fix(nodes): nodes_log query and tailLines arguments Signed-off-by: Marc Nuri --- README.md | 5 + pkg/kubernetes/accesscontrol_clientset.go | 5 +- pkg/kubernetes/nodes.go | 10 +- pkg/mcp/nodes_test.go | 94 +++++++++++-------- pkg/mcp/testdata/toolsets-core-tools.json | 14 +-- ...toolsets-full-tools-multicluster-enum.json | 14 +-- .../toolsets-full-tools-multicluster.json | 14 +-- .../toolsets-full-tools-openshift.json | 14 +-- pkg/mcp/testdata/toolsets-full-tools.json | 14 +-- pkg/toolsets/core/nodes.go | 25 +++-- 10 files changed, 115 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index 600f4981..522e0fab 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,11 @@ In case multi-cluster support is enabled (default) and you have access to multip - **projects_list** - List all the OpenShift projects in the current cluster +- **nodes_log** - Get logs from a Kubernetes node (kubelet, kube-proxy, or other system logs). This accesses node logs through the Kubernetes API proxy to the kubelet + - `name` (`string`) **(required)** - Name of the node to get logs from + - `query` (`string`) **(required)** - query specifies services(s) or files from which to return logs (required). Example: "kubelet" to fetch kubelet logs, "/" to fetch a specific log file from the node (e.g., "/var/log/kubelet.log" or "/var/log/kube-proxy.log") + - `tailLines` (`integer`) - Number of lines to retrieve from the end of the logs (Optional, 0 means all logs) + - **pods_list** - List all the Kubernetes pods in the current cluster from all namespaces - `labelSelector` (`string`) - Optional Kubernetes label selector (e.g. 'app=myapp,env=prod' or 'app in (myapp,yourapp)'), use this option when you want to filter the pods by label diff --git a/pkg/kubernetes/accesscontrol_clientset.go b/pkg/kubernetes/accesscontrol_clientset.go index d4cddaaf..b4ab315c 100644 --- a/pkg/kubernetes/accesscontrol_clientset.go +++ b/pkg/kubernetes/accesscontrol_clientset.go @@ -39,7 +39,7 @@ func (a *AccessControlClientset) DiscoveryClient() discovery.DiscoveryInterface return a.discoveryClient } -func (a *AccessControlClientset) NodesLogs(ctx context.Context, name, logPath string) (*rest.Request, error) { +func (a *AccessControlClientset) NodesLogs(ctx context.Context, name string) (*rest.Request, error) { gvk := &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"} if !isAllowed(a.staticConfig, gvk) { return nil, isNotAllowedError(gvk) @@ -52,8 +52,7 @@ func (a *AccessControlClientset) NodesLogs(ctx context.Context, name, logPath st url := []string{"api", "v1", "nodes", name, "proxy", "logs"} return a.delegate.CoreV1().RESTClient(). Get(). - AbsPath(url...). - Param("query", logPath), nil + AbsPath(url...), nil } func (a *AccessControlClientset) Pods(namespace string) (corev1.PodInterface, error) { diff --git a/pkg/kubernetes/nodes.go b/pkg/kubernetes/nodes.go index 76d9cc92..abdf16f2 100644 --- a/pkg/kubernetes/nodes.go +++ b/pkg/kubernetes/nodes.go @@ -5,21 +5,23 @@ import ( "fmt" ) -func (k *Kubernetes) NodesLog(ctx context.Context, name string, logPath string, tail int64) (string, error) { +func (k *Kubernetes) NodesLog(ctx context.Context, name string, query string, tailLines int64) (string, error) { // Use the node proxy API to access logs from the kubelet + // https://kubernetes.io/docs/concepts/cluster-administration/system-logs/#log-query // Common log paths: // - /var/log/kubelet.log - kubelet logs // - /var/log/kube-proxy.log - kube-proxy logs // - /var/log/containers/ - container logs - req, err := k.AccessControlClientset().NodesLogs(ctx, name, logPath) + req, err := k.AccessControlClientset().NodesLogs(ctx, name) if err != nil { return "", err } + req.Param("query", query) // Query parameters for tail - if tail > 0 { - req.Param("tailLines", fmt.Sprintf("%d", tail)) + if tailLines > 0 { + req.Param("tailLines", fmt.Sprintf("%d", tailLines)) } result := req.Do(ctx) diff --git a/pkg/mcp/nodes_test.go b/pkg/mcp/nodes_test.go index f0d42bbf..c0135de6 100644 --- a/pkg/mcp/nodes_test.go +++ b/pkg/mcp/nodes_test.go @@ -2,6 +2,7 @@ package mcp import ( "net/http" + "strconv" "testing" "github.com/BurntSushi/toml" @@ -43,29 +44,27 @@ func (s *NodesSuite) TestNodesLog() { }`)) return } - // Check for log proxy requests based on path and query parameters + // Get Proxy Logs if req.URL.Path == "/api/v1/nodes/existing-node/proxy/logs" { - logPath := req.URL.Query().Get("query") - - // Get Empty Log response - if logPath == "empty.log" { - w.Header().Set("Content-Type", "text/plain") - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(``)) + w.Header().Set("Content-Type", "text/plain") + query := req.URL.Query().Get("query") + var logContent string + switch query { + case "/empty.log": + logContent = "" + case "/kubelet.log": + logContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n" + default: + w.WriteHeader(http.StatusNotFound) return } - - // Get Kubelet Log response - if logPath == "kubelet.log" { - w.Header().Set("Content-Type", "text/plain") - w.WriteHeader(http.StatusOK) - logContent := "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n" - if req.URL.Query().Get("tailLines") != "" { - logContent = "Line 4\nLine 5\n" - } - _, _ = w.Write([]byte(logContent)) - return + _, err := strconv.Atoi(req.URL.Query().Get("tailLines")) + if err == nil { + logContent = "Line 4\nLine 5\n" } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(logContent)) + return } w.WriteHeader(http.StatusNotFound) })) @@ -83,9 +82,25 @@ func (s *NodesSuite) TestNodesLog() { "expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) }) }) - s.Run("nodes_log(name=inexistent-node)", func() { + s.Run("nodes_log(name=existing-node, query=nil)", func() { + toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ + "name": "existing-node", + }) + s.Require().NotNil(toolResult, "toolResult should not be nil") + s.Run("has error", func() { + s.Truef(toolResult.IsError, "call tool should fail") + s.Nilf(err, "call tool should not return error object") + }) + s.Run("describes missing name", func() { + expectedMessage := "failed to get node log, missing argument query" + s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text, + "expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) + }) + }) + s.Run("nodes_log(name=inexistent-node, query=/kubelet.log)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "inexistent-node", + "name": "inexistent-node", + "query": "/kubelet.log", }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("has error", func() { @@ -98,10 +113,10 @@ func (s *NodesSuite) TestNodesLog() { "expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) }) }) - s.Run("nodes_log(name=existing-node, log_path=missing.log)", func() { + s.Run("nodes_log(name=existing-node, query=/missing.log)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "existing-node", - "log_path": "missing.log", + "name": "existing-node", + "query": "/missing.log", }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("has error", func() { @@ -114,10 +129,10 @@ func (s *NodesSuite) TestNodesLog() { "expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) }) }) - s.Run("nodes_log(name=existing-node, log_path=empty.log)", func() { + s.Run("nodes_log(name=existing-node, query=/empty.log)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "existing-node", - "log_path": "empty.log", + "name": "existing-node", + "query": "/empty.log", }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("no error", func() { @@ -130,10 +145,10 @@ func (s *NodesSuite) TestNodesLog() { "expected descriptive message '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) }) }) - s.Run("nodes_log(name=existing-node, log_path=kubelet.log)", func() { + s.Run("nodes_log(name=existing-node, query=/kubelet.log)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "existing-node", - "log_path": "kubelet.log", + "name": "existing-node", + "query": "/kubelet.log", }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("no error", func() { @@ -147,11 +162,11 @@ func (s *NodesSuite) TestNodesLog() { }) }) for _, tailCase := range []interface{}{2, int64(2), float64(2)} { - s.Run("nodes_log(name=existing-node, log_path=kubelet.log, tail=2)", func() { + s.Run("nodes_log(name=existing-node, query=/kubelet.log, tailLines=2)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "existing-node", - "log_path": "kubelet.log", - "tail": tailCase, + "name": "existing-node", + "query": "/kubelet.log", + "tailLines": tailCase, }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("no error", func() { @@ -164,11 +179,11 @@ func (s *NodesSuite) TestNodesLog() { "expected log content '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text) }) }) - s.Run("nodes_log(name=existing-node, log_path=kubelet.log, tail=-1)", func() { + s.Run("nodes_log(name=existing-node, query=/kubelet.log, tailLines=-1)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "existing-node", - "log_path": "kubelet.log", - "tail": -1, + "name": "existing-node", + "query": "/kubelet.log", + "tail": -1, }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("no error", func() { @@ -191,7 +206,8 @@ func (s *NodesSuite) TestNodesLogDenied() { s.InitMcpClient() s.Run("nodes_log (denied)", func() { toolResult, err := s.CallTool("nodes_log", map[string]interface{}{ - "name": "does-not-matter", + "name": "does-not-matter", + "query": "/does-not-matter-either.log", }) s.Require().NotNil(toolResult, "toolResult should not be nil") s.Run("has error", func() { diff --git a/pkg/mcp/testdata/toolsets-core-tools.json b/pkg/mcp/testdata/toolsets-core-tools.json index 37345100..fc5e86b7 100644 --- a/pkg/mcp/testdata/toolsets-core-tools.json +++ b/pkg/mcp/testdata/toolsets-core-tools.json @@ -45,16 +45,15 @@ "inputSchema": { "type": "object", "properties": { - "log_path": { - "default": "kubelet.log", - "description": "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - "type": "string" - }, "name": { "description": "Name of the node to get logs from", "type": "string" }, - "tail": { + "query": { + "description": "query specifies services(s) or files from which to return logs (required). Example: \"kubelet\" to fetch kubelet logs, \"/\u003clog-file-name\u003e\" to fetch a specific log file from the node (e.g., \"/var/log/kubelet.log\" or \"/var/log/kube-proxy.log\")", + "type": "string" + }, + "tailLines": { "default": 100, "description": "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", "minimum": 0, @@ -62,7 +61,8 @@ } }, "required": [ - "name" + "name", + "query" ] }, "name": "nodes_log" diff --git a/pkg/mcp/testdata/toolsets-full-tools-multicluster-enum.json b/pkg/mcp/testdata/toolsets-full-tools-multicluster-enum.json index 7041bd3f..83b32139 100644 --- a/pkg/mcp/testdata/toolsets-full-tools-multicluster-enum.json +++ b/pkg/mcp/testdata/toolsets-full-tools-multicluster-enum.json @@ -215,16 +215,15 @@ ], "type": "string" }, - "log_path": { - "default": "kubelet.log", - "description": "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - "type": "string" - }, "name": { "description": "Name of the node to get logs from", "type": "string" }, - "tail": { + "query": { + "description": "query specifies services(s) or files from which to return logs (required). Example: \"kubelet\" to fetch kubelet logs, \"/\u003clog-file-name\u003e\" to fetch a specific log file from the node (e.g., \"/var/log/kubelet.log\" or \"/var/log/kube-proxy.log\")", + "type": "string" + }, + "tailLines": { "default": 100, "description": "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", "minimum": 0, @@ -232,7 +231,8 @@ } }, "required": [ - "name" + "name", + "query" ] }, "name": "nodes_log" diff --git a/pkg/mcp/testdata/toolsets-full-tools-multicluster.json b/pkg/mcp/testdata/toolsets-full-tools-multicluster.json index a454f1ef..a7cdeb47 100644 --- a/pkg/mcp/testdata/toolsets-full-tools-multicluster.json +++ b/pkg/mcp/testdata/toolsets-full-tools-multicluster.json @@ -191,16 +191,15 @@ "description": "Optional parameter selecting which context to run the tool in. Defaults to fake-context if not set", "type": "string" }, - "log_path": { - "default": "kubelet.log", - "description": "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - "type": "string" - }, "name": { "description": "Name of the node to get logs from", "type": "string" }, - "tail": { + "query": { + "description": "query specifies services(s) or files from which to return logs (required). Example: \"kubelet\" to fetch kubelet logs, \"/\u003clog-file-name\u003e\" to fetch a specific log file from the node (e.g., \"/var/log/kubelet.log\" or \"/var/log/kube-proxy.log\")", + "type": "string" + }, + "tailLines": { "default": 100, "description": "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", "minimum": 0, @@ -208,7 +207,8 @@ } }, "required": [ - "name" + "name", + "query" ] }, "name": "nodes_log" diff --git a/pkg/mcp/testdata/toolsets-full-tools-openshift.json b/pkg/mcp/testdata/toolsets-full-tools-openshift.json index 5e5fa4ea..3bba52d2 100644 --- a/pkg/mcp/testdata/toolsets-full-tools-openshift.json +++ b/pkg/mcp/testdata/toolsets-full-tools-openshift.json @@ -151,16 +151,15 @@ "inputSchema": { "type": "object", "properties": { - "log_path": { - "default": "kubelet.log", - "description": "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - "type": "string" - }, "name": { "description": "Name of the node to get logs from", "type": "string" }, - "tail": { + "query": { + "description": "query specifies services(s) or files from which to return logs (required). Example: \"kubelet\" to fetch kubelet logs, \"/\u003clog-file-name\u003e\" to fetch a specific log file from the node (e.g., \"/var/log/kubelet.log\" or \"/var/log/kube-proxy.log\")", + "type": "string" + }, + "tailLines": { "default": 100, "description": "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", "minimum": 0, @@ -168,7 +167,8 @@ } }, "required": [ - "name" + "name", + "query" ] }, "name": "nodes_log" diff --git a/pkg/mcp/testdata/toolsets-full-tools.json b/pkg/mcp/testdata/toolsets-full-tools.json index 56a160ed..d9d6b91e 100644 --- a/pkg/mcp/testdata/toolsets-full-tools.json +++ b/pkg/mcp/testdata/toolsets-full-tools.json @@ -151,16 +151,15 @@ "inputSchema": { "type": "object", "properties": { - "log_path": { - "default": "kubelet.log", - "description": "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - "type": "string" - }, "name": { "description": "Name of the node to get logs from", "type": "string" }, - "tail": { + "query": { + "description": "query specifies services(s) or files from which to return logs (required). Example: \"kubelet\" to fetch kubelet logs, \"/\u003clog-file-name\u003e\" to fetch a specific log file from the node (e.g., \"/var/log/kubelet.log\" or \"/var/log/kube-proxy.log\")", + "type": "string" + }, + "tailLines": { "default": 100, "description": "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", "minimum": 0, @@ -168,7 +167,8 @@ } }, "required": [ - "name" + "name", + "query" ] }, "name": "nodes_log" diff --git a/pkg/toolsets/core/nodes.go b/pkg/toolsets/core/nodes.go index 6c669398..c9e84032 100644 --- a/pkg/toolsets/core/nodes.go +++ b/pkg/toolsets/core/nodes.go @@ -22,19 +22,18 @@ func initNodes() []api.ServerTool { Type: "string", Description: "Name of the node to get logs from", }, - "log_path": { + "query": { Type: "string", - Description: "Path to the log file on the node (e.g. 'kubelet.log', 'kube-proxy.log'). Default is 'kubelet.log'", - Default: api.ToRawMessage("kubelet.log"), + Description: `query specifies services(s) or files from which to return logs (required). Example: "kubelet" to fetch kubelet logs, "/" to fetch a specific log file from the node (e.g., "/var/log/kubelet.log" or "/var/log/kube-proxy.log")`, }, - "tail": { + "tailLines": { Type: "integer", Description: "Number of lines to retrieve from the end of the logs (Optional, 0 means all logs)", Default: api.ToRawMessage(100), Minimum: ptr.To(float64(0)), }, }, - Required: []string{"name"}, + Required: []string{"name", "query"}, }, Annotations: api.ToolAnnotations{ Title: "Node: Log", @@ -52,25 +51,25 @@ func nodesLog(params api.ToolHandlerParams) (*api.ToolCallResult, error) { if !ok || name == "" { return api.NewToolCallResult("", errors.New("failed to get node log, missing argument name")), nil } - logPath, ok := params.GetArguments()["log_path"].(string) - if !ok || logPath == "" { - logPath = "kubelet.log" + query, ok := params.GetArguments()["query"].(string) + if !ok || query == "" { + return api.NewToolCallResult("", errors.New("failed to get node log, missing argument query")), nil } - tail := params.GetArguments()["tail"] + tailLines := params.GetArguments()["tailLines"] var tailInt int64 - if tail != nil { + if tailLines != nil { // Convert to int64 - safely handle both float64 (JSON number) and int types - switch v := tail.(type) { + switch v := tailLines.(type) { case float64: tailInt = int64(v) case int: case int64: tailInt = v default: - return api.NewToolCallResult("", fmt.Errorf("failed to parse tail parameter: expected integer, got %T", tail)), nil + return api.NewToolCallResult("", fmt.Errorf("failed to parse tail parameter: expected integer, got %T", tailLines)), nil } } - ret, err := params.NodesLog(params, name, logPath, tailInt) + ret, err := params.NodesLog(params, name, query, tailInt) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to get node log for %s: %v", name, err)), nil } else if ret == "" {