Skip to content

fix: coerce empty maps to objects in patch payloads - #31

Merged
abnegate merged 1 commit into
mainfrom
fix/patch-payload-empty-map-coercion
Aug 8, 2026
Merged

fix: coerce empty maps to objects in patch payloads#31
abnegate merged 1 commit into
mainfrom
fix/patch-payload-empty-map-coercion

Conversation

@abnegate

@abnegate abnegate commented Aug 8, 2026

Copy link
Copy Markdown
Member

The bug

jsonPatch() and jsonMergePatch() (and their status variants) encoded array payloads with a raw json_encode, while update() routes through toJsonPayload() which coerces empty PHP arrays back to JSON {} objects (#25).

Structures fetched from the apiserver decode {} to PHP [], so any patch payload built from fetched data re-encoded empty maps as [] — and the apiserver rejects that with 422. The canonical trigger is a StatefulSet/Deployment template's emptyDir: {} volumes: every stored template where medium is unset carries them (Go omitempty drops the medium key but keeps the {} struct), so a reconcile loop that fetches the template, modifies containers, and JSON-Patches it back fails on every tick.

$template = $statefulSet->getAttribute('spec.template');
// $template['spec']['volumes'][0]['emptyDir'] === [] after fetch

$statefulSet->jsonPatch([
    ['op' => 'replace', 'path' => '/spec/template', 'value' => $template],
]);
// Before: "emptyDir":[] → 422 Unprocessable Entity
// After:  "emptyDir":{} → accepted

The fix

Patch payloads now build through K8sResource::toJsonPatchPayload() / toJsonMergePatchPayload(), sharing the same structural coercion as full payloads:

  • JSON Patch (RFC 6902): each operation's value is coerced when it is a non-empty array. A value that is itself an empty array stays a list, so clearing list fields keeps working: ['op' => 'replace', 'path' => '/metadata/finalizers', 'value' => []] still encodes "value":[].
  • JSON Merge Patch (RFC 7396): the whole document is coerced. An empty patch encodes as {} (a merge patch document is always a JSON object; [] would replace the entire target).
  • $emptyArrayLists exemptions apply at any depth, exactly as in toJsonPayload(). The list gains finalizers and conditions — both are list-typed in every Kubernetes API, and the library's own finalizer clearing (jsonMergePatch(['metadata' => ['finalizers' => []]]), see FinalizerTest) and status patches (jsonMergePatchStatus(['status' => ['conditions' => []]]), see StatusSubresourceTest) rely on them staying [] once merge documents are coerced. This also fixes update() sending finalizers: {} when the last finalizer is removed.

Also fixed in passing: jsonPatchStatus() silently discarded array input — it wrapped the array in new JsonPatch($patch), but JsonPatch has no constructor, so an empty [] patch was sent instead of the given operations.

Tests

  • tests/PatchPayloadTest.php pins the encoded payload bytes for both patch flavors and both input types (array and patch object): 'emptyDir' => [] must encode as "emptyDir":{}, top-level empty op values stay [], exempted list fields stay [] at depth, strings containing ?: [] are never touched, empty JSON Patch stays [], empty merge patch becomes {}. The coercion assertions were written first and seen red against the raw-json_encode behavior (7 failures), then green with the fix.
  • tests/PatchIntegrationTest.php gains a live regression test that fetches a Deployment whose emptyDir: {} decoded to [] and JSON-Patches the template back — the exact fetch-modify-patch loop that previously 422'd.

Unit suites pass; Psalm reports the same 571 baseline issues before and after (no new); Pint clean; docs build clean.

Local verification

  • Full integration suite against minikube v1.34.0 (CI-mirror addons + VPA + Gateway API + SealedSecrets CRDs): 358 tests, 2870 assertions, 0 failures (8 skips are credential-gated EKS/token suites).
  • The new live regression test errors with 422 Unprocessable Entity when run against the pre-fix source and passes with the fix.

🤖 Generated with Claude Code

jsonPatch() and jsonMergePatch() (and their status variants) encoded
payloads with a raw json_encode, while update() routes through
toJsonPayload() which coerces empty PHP arrays back to JSON {} objects.
Structures fetched from the apiserver decode {} to PHP [], so any patch
built from fetched data re-encoded empty maps as [] and the apiserver
rejected it with 422 - e.g. a StatefulSet template's emptyDir: {}
volumes, which every stored template carries when medium is unset.

Patch payloads now build through toJsonPatchPayload() and
toJsonMergePatchPayload() on K8sResource, sharing the same structural
coercion as full payloads. A JSON Patch operation value that is itself
an empty array stays a list, so clearing list fields such as finalizers
keeps working. The exemption list gains finalizers and conditions: both
are list-typed in every Kubernetes API, and the library's own finalizer
clearing and status patches rely on them staying [] when the merge
document is coerced.

This also fixes jsonPatchStatus() silently dropping array input: it
wrapped the array in new JsonPatch($patch), but JsonPatch has no
constructor, so the operations were discarded and [] was sent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown

Greptile Summary

The PR routes JSON Patch and JSON Merge Patch requests through structural payload coercion, preserves known empty list fields, and fixes array input handling for status patches. It adds focused payload and integration coverage plus documentation for empty-map behavior.

Confidence Score: 4/5

The direct replacement of an empty map remains broken and should be corrected before merging.

The new serializer fixes nested empty maps but unconditionally preserves an exact empty operation value as [], so valid JSON Patch requests that clear map fields can still be rejected by Kubernetes.

Files Needing Attention: src/Kinds/K8sResource.php, tests/PatchPayloadTest.php

Important Files Changed

Filename Overview
src/Kinds/K8sResource.php Adds patch-specific serializers and list exemptions, but direct empty map operation values still serialize as JSON arrays.
src/Traits/RunsClusterOperations.php Routes all resource and status patch variants through the new payload serializers and fixes discarded array status-patch operations.
tests/PatchPayloadTest.php Thoroughly covers nested coercion and known list exemptions, but omits direct replacement of an empty map-valued field.
tests/PatchIntegrationTest.php Adds a live regression case for fetched templates containing emptyDir: {}.
docs/guide/usage/patching.md Documents nested empty-map coercion, top-level empty list handling, and the known list-field exemptions.

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
src/Kinds/K8sResource.php:176-178
**Empty map values remain arrays**

When a JSON Patch operation clears a map-valued field such as `/metadata/labels` using `value => []`, this guard skips coercion and serializes the value as `[]`, causing Kubernetes to reject the patch because the field requires `{}`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix: coerce empty maps to objects in pat..." | Re-trigger Greptile

Comment thread src/Kinds/K8sResource.php
Comment on lines +176 to +178
if (is_array($value) && $value !== []) {
$operations[$index]['value'] = $this->coerceEmptyArraysToObjects($value);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Empty map values remain arrays

When a JSON Patch operation clears a map-valued field such as /metadata/labels using value => [], this guard skips coercion and serializes the value as [], causing Kubernetes to reject the patch because the field requires {}.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Kinds/K8sResource.php
Line: 176-178

Comment:
**Empty map values remain arrays**

When a JSON Patch operation clears a map-valued field such as `/metadata/labels` using `value => []`, this guard skips coercion and serializes the value as `[]`, causing Kubernetes to reject the patch because the field requires `{}`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@abnegate
abnegate merged commit 4e48dbe into main Aug 8, 2026
4 checks passed
@abnegate
abnegate deleted the fix/patch-payload-empty-map-coercion branch August 8, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant