Skip to content

Feat | Add --ignore-array-order option - #511

Draft
quangkhainguyen wants to merge 3 commits into
dag-andersen:mainfrom
quangkhainguyen:main
Draft

quangkhainguyen wants to merge 3 commits into
dag-andersen:mainfrom
quangkhainguyen:main

Conversation

@quangkhainguyen

Copy link
Copy Markdown

Draft - waiting for feedback on the feature request

What

Adds an opt-in --ignore-array-order option (env IGNORE_ARRAY_ORDER). It takes a
list of JSONPaths. Arrays matched by a path are compared without regard to element
order. The default is empty, so the current behavior is unchanged.

Behavior

  • JSONPath subset: .field, ..field, [*], [n], ['a.b'] and a root filter
    $[?(@.kind == 'Deployment')]. Anything else is rejected with an error.
  • Matched arrays are sorted on copies before matching. Files from
    --output-app-manifests and --output-branch-manifests keep the original order.
  • Elements are sorted by name if all have one, otherwise by their JSON form.
  • When rendering the diff, paired elements keep the same position on both sides.
    A changed or renamed entry therefore does not make unchanged neighbors look
    removed and re-added.
  • Real changes (values, additions, removals, duplicates) are still shown.

Open questions

  • Flag name and the supported JSONPath subset.
  • Is the root kind filter wanted, or should it be dropped?

Tests

  • Unit tests for the parser, sorting, alignment and the end-to-end diff.
  • Integration test branch-22 (target-1 without the option, target-2 with it).
    Its data branches currently live in my fork. They would need to be created in
    this repository as integration-test/branch-22/base and target, and the
    GitHubRepo field of the two test cases removed.

Warning

Array order is not always irrelevant ($(VAR) expansion in env, command,
args, initContainers). This is documented in docs/filter-output.md.

Add `--ignore-array-order` (env `IGNORE_ARRAY_ORDER`) to ignore the element
order of selected arrays, such as env vars or volumes, when diffing rendered
manifests. The option is opt-in and empty by default.

- Add pkg/arrayorder with a JSONPath subset: .field, ..field, [*], [n],
  ['a.b'] and a root `[?(@.kind == 'Deployment')]` string equality filter.
- Sort matched arrays on deep copies before matching, so output manifests
  keep their original order.
- Align paired elements by identity, then by `name`, when rendering the diff.
  Changed entries stay at their sorted position instead of showing up as
  removed and re-added.
- Wire the option through cmd, GeneratePreview and matching.GenerateAppDiffs.
- Add unit tests.
- Document the option in docs/options.md and docs/filter-output.md.
Add branch-22 test cases for `--ignore-array-order`. The target reorders `env`,
`volumes` and `args` of a Deployment and changes one env var value.

- target-1: without the option, all reorderings show up in the diff.
- target-2: with the option, only the env value change and the `args` reorder
  remain.

The test data branches live in a fork. Add `TestCase.GitHubRepo` to clone the
branches from, and pass as --repo, a repository other than the default.
Replace the hardcoded repository in cloneBranch, buildArgs and runWithDocker
with gitHubRepo(tc). Drop GitHubRepo from both test cases once the branches
exist in the main repository.

This branch has not been deployed

No deployments
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.

2 participants