diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index f84a047..6def2f2 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -132,11 +132,20 @@ func (k *K8sTool) handlePatchResource(ctx context.Context, request mcp.CallToolR resourceName := mcp.ParseString(request, "resource_name", "") patch := mcp.ParseString(request, "patch", "") namespace := mcp.ParseString(request, "namespace", "default") + patchType := mcp.ParseString(request, "patch_type", "strategic") if resourceType == "" || resourceName == "" || patch == "" { return mcp.NewToolResultError("resource_type, resource_name, and patch parameters are required"), nil } + // Validate patch type. "strategic" is only implemented for built-in Kubernetes + // types; CustomResources (CRDs) reject it and require "merge" or "json". + switch patchType { + case "strategic", "merge", "json": + default: + return mcp.NewToolResultError(fmt.Sprintf("Invalid patch_type %q: must be one of strategic, merge, json", patchType)), nil + } + // Validate resource name for security if err := security.ValidateK8sResourceName(resourceName); err != nil { return mcp.NewToolResultError(fmt.Sprintf("Invalid resource name: %v", err)), nil @@ -152,7 +161,7 @@ func (k *K8sTool) handlePatchResource(ctx context.Context, request mcp.CallToolR return mcp.NewToolResultError(fmt.Sprintf("Invalid patch content: %v", err)), nil } - args := []string{"patch", resourceType, resourceName, "-p", patch, "-n", namespace} + args := []string{"patch", resourceType, resourceName, "--type=" + patchType, "-p", patch, "-n", namespace} return k.runKubectlCommandWithCacheInvalidation(ctx, request.Header, args...) } @@ -717,10 +726,11 @@ func RegisterTools(s *server.MCPServer, llm llms.Model, kubeconfig string, readO ), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_scale", k8sTool.handleScaleDeployment))) s.AddTool(mcp.NewTool("k8s_patch_resource", - mcp.WithDescription("Patch a Kubernetes resource using strategic merge patch"), + mcp.WithDescription("Patch a Kubernetes resource. Defaults to a strategic merge patch, which is only supported for built-in types; set patch_type to \"merge\" (or \"json\") to patch a CustomResource/CRD."), mcp.WithString("resource_type", mcp.Description("Type of resource (deployment, service, etc.)"), mcp.Required()), mcp.WithString("resource_name", mcp.Description("Name of the resource"), mcp.Required()), mcp.WithString("patch", mcp.Description("JSON patch to apply"), mcp.Required()), + mcp.WithString("patch_type", mcp.Description("Patch strategy: \"strategic\" (default; built-in Kubernetes types only), \"merge\" (RFC 7386 JSON merge patch; required for CustomResources/CRDs), or \"json\" (RFC 6902 JSON patch).")), mcp.WithString("namespace", mcp.Description("Namespace of the resource (default: default)")), ), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_patch_resource", k8sTool.handlePatchResource))) diff --git a/pkg/k8s/k8s_test.go b/pkg/k8s/k8s_test.go index 44df8a9..87ea388 100644 --- a/pkg/k8s/k8s_test.go +++ b/pkg/k8s/k8s_test.go @@ -242,7 +242,7 @@ func TestHandlePatchResource(t *testing.T) { t.Run("valid parameters", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment patched` - mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "-p", `{"spec":{"replicas":5}}`, "-n", "default"}, expectedOutput, nil) + mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default"}, expectedOutput, nil) ctx := cmd.WithShellExecutor(ctx, mock) k8sTool := newTestK8sTool() @@ -262,6 +262,55 @@ func TestHandlePatchResource(t *testing.T) { resultText := getResultText(result) assert.Contains(t, resultText, "patched") }) + + t.Run("merge patch type for CustomResource", func(t *testing.T) { + mock := cmd.NewMockShellExecutor() + expectedOutput := `installer.composition.krateo.io/installer patched` + mock.AddCommandString("kubectl", []string{"patch", "installers.composition.krateo.io", "installer", "--type=merge", "-p", `{"spec":{"features":{"composableportal":true}}}`, "-n", "krateo-system"}, expectedOutput, nil) + ctx := cmd.WithShellExecutor(ctx, mock) + + k8sTool := newTestK8sTool() + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "resource_type": "installers.composition.krateo.io", + "resource_name": "installer", + "patch": `{"spec":{"features":{"composableportal":true}}}`, + "patch_type": "merge", + "namespace": "krateo-system", + } + + result, err := k8sTool.handlePatchResource(ctx, req) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.False(t, result.IsError) + + resultText := getResultText(result) + assert.Contains(t, resultText, "patched") + }) + + t.Run("invalid patch type", func(t *testing.T) { + mock := cmd.NewMockShellExecutor() + ctx := cmd.WithShellExecutor(context.Background(), mock) + + k8sTool := newTestK8sTool() + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "resource_type": "deployment", + "resource_name": "test-deployment", + "patch": `{"spec":{"replicas":5}}`, + "patch_type": "bogus", + } + + result, err := k8sTool.handlePatchResource(ctx, req) + assert.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.IsError) + + // No command should run for an invalid patch type. + assert.Len(t, mock.GetCallLog(), 0) + }) } func TestHandlePatchStatus(t *testing.T) { @@ -1232,7 +1281,7 @@ log line 2` t.Run("patch resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment patched` - mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "-p", `{"spec":{"replicas":5}}`, "-n", "default", "--token", "patch-token"}, expectedOutput, nil) + mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default", "--token", "patch-token"}, expectedOutput, nil) ctx := cmd.WithShellExecutor(ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true)