Skip to content

fix(patch): bump jsonpatch to pick up recursive list-order-ignoring compare#424

Merged
JeroenSoeters merged 1 commit into
mainfrom
fix/patch-nested-list-ordering
Apr 21, 2026
Merged

fix(patch): bump jsonpatch to pick up recursive list-order-ignoring compare#424
JeroenSoeters merged 1 commit into
mainfrom
fix/patch-nested-list-ordering

Conversation

@JeroenSoeters
Copy link
Copy Markdown
Collaborator

Summary

Pulls in the recursive-set-compare fix from platform-engineering-labs/jsonpatch (merged as fb6f96b1) so that reapplying an unchanged forma no longer spuriously replaces resources whose stored state has nested lists in a different order than the PKL-evaluated desired state.

Root cause (recap)

jsonpatch.matchesValue's ignoreArrayOrder=true branch compared list elements by json.Marshal bytes. json.Marshal sorts map keys but preserves array order, so a reordered nested list inside a container-shaped element produced a different byte string at the outer level — even when the nested content was semantically identical. This propagated up as a false positive on the outer list's set-compare, which (under a createOnly field like ContainerDefinitions) tripped needsReplacement=true and destroyed-and-recreated the resource on every reapply.

The upstream fix replaces the byte-string multiset with a pairing algorithm that matches elements recursively via matchesValue itself. O(n*m) worst case, correct for arbitrary nesting depth.

Change here

  • go.mod / go.sum: bump github.com/platform-engineering-labs/jsonpatch to v0.0.0-20260421221004-fb6f96b174b5.
  • internal/metastructure/patch/patch_document_test.go: three new integration tests against generatePatch that cover the formae-side symptom end-to-end:
    • TestGeneratePatch_NestedListOrderingInsideArrayElement_NoReplace — the exact Grafana-style reproducer (Environment reordered inside a ContainerDefinition under a createOnly ContainerDefinitions list). Fails against the unpatched jsonpatch, passes after the bump.
    • TestGeneratePatch_NestedPortMappingsOrderInsideArrayElement_NoReplace — the tempo-container variant (PortMappings reordered).
    • TestGeneratePatch_NestedListContentChange_StillReplaces — negative guard: a real content change inside a nested list must still produce a replace.

All three were written failing-first against the current main dep, watched fail for the right reason, then passed after pointing at the patched jsonpatch.

Impact

Unblocks idempotent reapply for ECS Services referencing Task Definitions with multi-container shapes (and any similar pattern with nested canonicalised lists). Combined with the two earlier fixes on main (hasProviderDefault recursive stripping + PortMapping.hostPort annotation in the AWS plugin), reapplying an unchanged forma with an ECS Service should now be a true no-op.

…ompare

With the recursive-set-compare fix in jsonpatch, reapplying an unchanged
forma no longer spuriously replaces resources whose stored state has
nested lists in a different order than the PKL-evaluated desired state
(e.g. AWS returning an ECS task definition's Environment or PortMappings
lists in a canonicalised order that differs from the forma). Adds three
integration tests in the patch package covering the Environment reorder,
PortMappings reorder, and the negative case where a real content change
inside a nested list still correctly trips needsReplacement.
@JeroenSoeters JeroenSoeters merged commit a4aa447 into main Apr 21, 2026
25 checks passed
@JeroenSoeters JeroenSoeters deleted the fix/patch-nested-list-ordering branch April 21, 2026 22:43
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