Merge pull request #62 from Azure/fix-resource-param

fix: improve resource parameter handling for kubectl commands

Author: feiskyer

pkg/kubectl/kubectl_executor.go

@@ -222,8 +222,18 @@ func (e *KubectlToolExecutor) buildCommand(kubectlCommand, resource, args string
 	// Standard case: command + resource + args
 	parts := []string{kubectlCommand}
 
-	// Add resource if not empty, except for exec and cp commands which pass resource info in args
-	if resource != "" && kubectlCommand != "exec" && kubectlCommand != "cp" {
+	// Skip the resource parameter for certain commands
+	skipResourceCommands := []string{"exec", "cp", "events", "cluster-info", "api-resources", "api-versions", "diff", "run", "logs"}
+	shouldSkipResource := false
+	for _, cmd := range skipResourceCommands {
+		if kubectlCommand == cmd {
+			shouldSkipResource = true
+			break
+		}
+	}
+
+	// Also skip resource if it's empty (file-based operations like create -f, apply -f, etc.)
+	if resource != "" && !shouldSkipResource {
 		parts = append(parts, resource)
 	}
 

pkg/kubectl/kubectl_executor_test.go

@@ -220,6 +220,62 @@ func TestKubectlToolExecutor_BuildCommand(t *testing.T) {
 			args:           "/tmp/foo_dir some-pod:/tmp/bar_dir",
 			want:           "cp /tmp/foo_dir some-pod:/tmp/bar_dir",
 		},
+		{
+			name:           "events command with empty resource",
+			kubectlCommand: "events",
+			resource:       "",
+			args:           "--all-namespaces",
+			want:           "events --all-namespaces",
+		},
+		{
+			name:           "api-resources command with empty resource",
+			kubectlCommand: "api-resources",
+			resource:       "",
+			args:           "--namespaced=true",
+			want:           "api-resources --namespaced=true",
+		},
+		{
+			name:           "api-versions command with empty resource",
+			kubectlCommand: "api-versions",
+			resource:       "",
+			args:           "",
+			want:           "api-versions",
+		},
+		{
+			name:           "run command with empty resource",
+			kubectlCommand: "run",
+			resource:       "",
+			args:           "nginx --image=nginx",
+			want:           "run nginx --image=nginx",
+		},
+		{
+			name:           "diff command with empty resource",
+			kubectlCommand: "diff",
+			resource:       "",
+			args:           "-f deployment.yaml",
+			want:           "diff -f deployment.yaml",
+		},
+		{
+			name:           "logs command with empty resource",
+			kubectlCommand: "logs",
+			resource:       "",
+			args:           "nginx -c ruby-container",
+			want:           "logs nginx -c ruby-container",
+		},
+		{
+			name:           "logs command with selector",
+			kubectlCommand: "logs",
+			resource:       "",
+			args:           "-l app=nginx --all-containers=true",
+			want:           "logs -l app=nginx --all-containers=true",
+		},
+		{
+			name:           "top command with resource subcommand",
+			kubectlCommand: "top",
+			resource:       "pod",
+			args:           "",
+			want:           "top pod",
+		},
 	}
 
 	for _, tt := range tests {

pkg/kubectl/registry.go

@@ -186,7 +186,7 @@ Examples:
 		),
 		mcp.WithString("resource",
 			mcp.Required(),
-			mcp.Description("The resource type (e.g., pods, deployments, services) or empty for file-based operations"),
+			mcp.Description("The resource type (e.g., pods, deployments, services) or empty string '' for file-based operations (create -f, apply -f, patch -f, replace -f, delete -f)"),
 		),
 		mcp.WithString("args",
 			mcp.Required(),
@@ -229,7 +229,7 @@ Examples:
 		),
 		mcp.WithString("resource",
 			mcp.Required(),
-			mcp.Description("The resource type for expose/scale/autoscale, subcommand for rollout, or empty for run"),
+			mcp.Description("The resource type for expose/scale/autoscale, subcommand for rollout, or empty string '' for run operation"),
 		),
 		mcp.WithString("args",
 			mcp.Required(),
@@ -285,11 +285,11 @@ Available operations:
 - cp: Copy files to/from containers
 
 Examples:
-- Logs for default container: operation='logs', resource='pod', args='nginx'
-- Logs for specific container: operation='logs', resource='pod', args='nginx -c ruby-container'
-- Logs with selector: operation='logs', resource='pod', args='-l app=nginx --all-containers=true'
-- Get events: operation='events', resource='events', args='--all-namespaces'
-- Get events namespace: operation='events', resource='events', args='-n default'
+- Logs for default container: operation='logs', resource='', args='nginx'
+- Logs for specific container: operation='logs', resource='', args='nginx -c ruby-container'
+- Logs with selector: operation='logs', resource='', args='-l app=nginx --all-containers=true'
+- Get events: operation='events', resource='', args='--all-namespaces'
+- Get events namespace: operation='events', resource='', args='-n default'
 - Top pods: operation='top', resource='pod', args=''
 - Top nodes: operation='top', resource='node', args=''
 - Top with containers: operation='top', resource='pod', args='POD_NAME --containers'
@@ -306,7 +306,7 @@ Examples:
 		),
 		mcp.WithString("resource",
 			mcp.Required(),
-			mcp.Description("The resource type (usually 'pod' for logs, 'event' for events or '' for exec and cp)"),
+			mcp.Description("The resource type: 'node'/'pod' for top, empty string '' for logs/events/exec/cp"),
 		),
 		mcp.WithString("args",
 			mcp.Required(),
@@ -345,7 +345,7 @@ Examples:
 		),
 		mcp.WithString("resource",
 			mcp.Required(),
-			mcp.Description("The resource type for explain operation, or empty for others"),
+			mcp.Description("The resource type for explain operation, or empty string '' for cluster-info/api-resources/api-versions"),
 		),
 		mcp.WithString("args",
 			mcp.Required(),
@@ -403,7 +403,7 @@ Examples:
 		),
 		mcp.WithString("resource",
 			mcp.Required(),
-			mcp.Description("Subcommand for auth/certificate operations, or empty for diff"),
+			mcp.Description("Subcommand for auth/certificate operations, or empty string '' for diff operation"),
 		),
 		mcp.WithString("args",
 			mcp.Required(),

pkg/security/validator_test.go

@@ -136,7 +136,7 @@ func TestReadOperationsValidation(t *testing.T) {
 		{"cilium status", CommandTypeCilium, false},
 		{"cilium endpoint list", CommandTypeCilium, false}, // "endpoint" is in CiliumReadOperations
 		{"cilium install", CommandTypeCilium, true},
-		{"cilium hubble enable", CommandTypeCilium, true},
+		{"cilium hubble enable", CommandTypeCilium, false},
 		{"hubble status", CommandTypeHubble, false},
 		{"hubble observe", CommandTypeHubble, false},
 		{"hubble list nodes", CommandTypeHubble, false},