Skip to content

Commit ea009cb

Browse files
braghettosclaude
andcommitted
fix(k8s): support merge/json patch types in k8s_patch_resource
k8s_patch_resource always invoked `kubectl patch -p <patch>` with no `--type`, so kubectl defaulted to a strategic merge patch. Strategic merge is only implemented for built-in Kubernetes types — every CustomResource (CRD) rejects it: error: application/strategic-merge-patch+json is not supported by <group>/<version>, Kind=<Kind>: the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml This made the tool unable to patch any custom resource. Add an optional `patch_type` parameter (strategic|merge|json) and pass it through as `--type`. Default stays "strategic" so built-in patching is unchanged; callers patching a CRD set "merge" (or "json"). This mirrors the sibling k8s_patch_status handler, which already uses --type=merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9d18d83 commit ea009cb

2 files changed

Lines changed: 63 additions & 4 deletions

File tree

pkg/k8s/k8s.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,20 @@ func (k *K8sTool) handlePatchResource(ctx context.Context, request mcp.CallToolR
132132
resourceName := mcp.ParseString(request, "resource_name", "")
133133
patch := mcp.ParseString(request, "patch", "")
134134
namespace := mcp.ParseString(request, "namespace", "default")
135+
patchType := mcp.ParseString(request, "patch_type", "strategic")
135136

136137
if resourceType == "" || resourceName == "" || patch == "" {
137138
return mcp.NewToolResultError("resource_type, resource_name, and patch parameters are required"), nil
138139
}
139140

141+
// Validate patch type. "strategic" is only implemented for built-in Kubernetes
142+
// types; CustomResources (CRDs) reject it and require "merge" or "json".
143+
switch patchType {
144+
case "strategic", "merge", "json":
145+
default:
146+
return mcp.NewToolResultError(fmt.Sprintf("Invalid patch_type %q: must be one of strategic, merge, json", patchType)), nil
147+
}
148+
140149
// Validate resource name for security
141150
if err := security.ValidateK8sResourceName(resourceName); err != nil {
142151
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
152161
return mcp.NewToolResultError(fmt.Sprintf("Invalid patch content: %v", err)), nil
153162
}
154163

155-
args := []string{"patch", resourceType, resourceName, "-p", patch, "-n", namespace}
164+
args := []string{"patch", resourceType, resourceName, "--type=" + patchType, "-p", patch, "-n", namespace}
156165

157166
return k.runKubectlCommandWithCacheInvalidation(ctx, request.Header, args...)
158167
}
@@ -717,10 +726,11 @@ func RegisterTools(s *server.MCPServer, llm llms.Model, kubeconfig string, readO
717726
), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_scale", k8sTool.handleScaleDeployment)))
718727

719728
s.AddTool(mcp.NewTool("k8s_patch_resource",
720-
mcp.WithDescription("Patch a Kubernetes resource using strategic merge patch"),
729+
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."),
721730
mcp.WithString("resource_type", mcp.Description("Type of resource (deployment, service, etc.)"), mcp.Required()),
722731
mcp.WithString("resource_name", mcp.Description("Name of the resource"), mcp.Required()),
723732
mcp.WithString("patch", mcp.Description("JSON patch to apply"), mcp.Required()),
733+
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).")),
724734
mcp.WithString("namespace", mcp.Description("Namespace of the resource (default: default)")),
725735
), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_patch_resource", k8sTool.handlePatchResource)))
726736

pkg/k8s/k8s_test.go

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func TestHandlePatchResource(t *testing.T) {
242242
t.Run("valid parameters", func(t *testing.T) {
243243
mock := cmd.NewMockShellExecutor()
244244
expectedOutput := `deployment.apps/test-deployment patched`
245-
mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "-p", `{"spec":{"replicas":5}}`, "-n", "default"}, expectedOutput, nil)
245+
mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default"}, expectedOutput, nil)
246246
ctx := cmd.WithShellExecutor(ctx, mock)
247247

248248
k8sTool := newTestK8sTool()
@@ -262,6 +262,55 @@ func TestHandlePatchResource(t *testing.T) {
262262
resultText := getResultText(result)
263263
assert.Contains(t, resultText, "patched")
264264
})
265+
266+
t.Run("merge patch type for CustomResource", func(t *testing.T) {
267+
mock := cmd.NewMockShellExecutor()
268+
expectedOutput := `installer.composition.krateo.io/installer patched`
269+
mock.AddCommandString("kubectl", []string{"patch", "installers.composition.krateo.io", "installer", "--type=merge", "-p", `{"spec":{"features":{"composableportal":true}}}`, "-n", "krateo-system"}, expectedOutput, nil)
270+
ctx := cmd.WithShellExecutor(ctx, mock)
271+
272+
k8sTool := newTestK8sTool()
273+
274+
req := mcp.CallToolRequest{}
275+
req.Params.Arguments = map[string]interface{}{
276+
"resource_type": "installers.composition.krateo.io",
277+
"resource_name": "installer",
278+
"patch": `{"spec":{"features":{"composableportal":true}}}`,
279+
"patch_type": "merge",
280+
"namespace": "krateo-system",
281+
}
282+
283+
result, err := k8sTool.handlePatchResource(ctx, req)
284+
assert.NoError(t, err)
285+
assert.NotNil(t, result)
286+
assert.False(t, result.IsError)
287+
288+
resultText := getResultText(result)
289+
assert.Contains(t, resultText, "patched")
290+
})
291+
292+
t.Run("invalid patch type", func(t *testing.T) {
293+
mock := cmd.NewMockShellExecutor()
294+
ctx := cmd.WithShellExecutor(context.Background(), mock)
295+
296+
k8sTool := newTestK8sTool()
297+
298+
req := mcp.CallToolRequest{}
299+
req.Params.Arguments = map[string]interface{}{
300+
"resource_type": "deployment",
301+
"resource_name": "test-deployment",
302+
"patch": `{"spec":{"replicas":5}}`,
303+
"patch_type": "bogus",
304+
}
305+
306+
result, err := k8sTool.handlePatchResource(ctx, req)
307+
assert.NoError(t, err)
308+
assert.NotNil(t, result)
309+
assert.True(t, result.IsError)
310+
311+
// No command should run for an invalid patch type.
312+
assert.Len(t, mock.GetCallLog(), 0)
313+
})
265314
}
266315

267316
func TestHandlePatchStatus(t *testing.T) {
@@ -1232,7 +1281,7 @@ log line 2`
12321281
t.Run("patch resource with bearer token", func(t *testing.T) {
12331282
mock := cmd.NewMockShellExecutor()
12341283
expectedOutput := `deployment.apps/test-deployment patched`
1235-
mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "-p", `{"spec":{"replicas":5}}`, "-n", "default", "--token", "patch-token"}, expectedOutput, nil)
1284+
mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default", "--token", "patch-token"}, expectedOutput, nil)
12361285
ctx := cmd.WithShellExecutor(ctx, mock)
12371286

12381287
k8sTool := newTestK8sToolWithPassthrough(true)

0 commit comments

Comments
 (0)