carvel-dev / ytt

YAML templating tool that works on YAML structure instead of text
https://carvel.dev/ytt
Apache License 2.0
1.65k stars 135 forks source link

#@overlay/insert inserts out of order #308

Open StevenLocke opened 3 years ago

StevenLocke commented 3 years ago

ℹ️ This bug should be addressed by implementing #371. See that story for details.

Original Report 👇🏻

What steps did you take: Ran the following files through ytt:

#! template.yaml

non-empty: doc
---
array:
- seeded
#! overlay.yaml

#@ load("@ytt:overlay", "overlay")
#@overlay/match by=overlay.all, expects="1+"
---
#@overlay/match missing_ok=True
array:
#@overlay/append
- appended
#@overlay/match by=overlay.index(0)
#@overlay/insert before=True
- inserted before 0

What happened: The command executed successfully, and output this yaml:

non-empty: doc
array:
- appended
- inserted before 0
---
array:
- inserted before 0
- seeded
- appended

Note the order of the items in the array. The "inserted before 0" item is added after the "appended" item in the first document, but (properly) before in the second.

Anything else you would like to add: This doesn't occur if we insert an array key to the first doc

non-empty: doc
array: []

which gets properly templated as:

non-empty: doc
array:
- inserted before 0
- appended

Environment:

joaopapereira commented 3 years ago

I was able to reproduce the issue with

$ cat tmp/template.yml
non-empty: doc
---
array:
  - seeded
---
other: doc
array: []

$ cat tmp/overlay.yml
#@ load("@ytt:overlay", "overlay")
#@overlay/match by=overlay.all, expects="1+"
---
#@overlay/match missing_ok=True
array:
  #@overlay/append
  - appended
  #@overlay/match by=overlay.index(0)
  #@overlay/insert before=True
  - inserted before 0

$ ytt -f tmp/template.yml -f tmp/overlay.yml
non-empty: doc
array:
- appended
- inserted before 0
---
array:
- inserted before 0
- seeded
- appended
---
other: doc
array:
- inserted before 0
- appended

Also inverted the order that we do the overlay and looks like the ordering of the operations is correct.

$ cat tmp/overlay.yml
#@ load("@ytt:overlay", "overlay")
#@overlay/match by=overlay.all, expects="1+"
---
#@overlay/match missing_ok=True
array:
  #@overlay/match by=overlay.index(0), expects="0+"
  #@overlay/insert before=True
  - inserted before 0
  #@overlay/append
  - appended

$ ytt -f tmp/template.yml -f tmp/overlay.yml
non-empty: doc
array:
- inserted before 0
- appended
---
array:
- inserted before 0
- seeded
- appended
---
other: doc
array:
- appended

Not sure if this will be relevant but I also tried to add an expectation in the match overlay of 1+ and it didn't fail in any case:

$ cat tmp/overlay.yml
#@ load("@ytt:overlay", "overlay")
#@overlay/match by=overlay.all, expects="1+"
---
#@overlay/match missing_ok=True
array:
  #@overlay/append
  - appended
  #@overlay/match by=overlay.index(0), expects="1+"
  #@overlay/insert before=True
  - inserted before 0

$ ytt -f tmp/template.yml -f tmp/overlay.yml
non-empty: doc
array:
- appended
- inserted before 0
---
array:
- inserted before 0
- seeded
- appended
---
other: doc
array:
- inserted before 0
- appended

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this bug fixed as soon as possible" 👎 "There are more important bugs to focus on right now"

We are also happy to receive and review Pull Requests if you want to help fix this issue.

cppforlife commented 3 years ago

this related is task in https://github.com/vmware-tanzu/carvel-ytt/issues/112

traverse value for merge/replace/append/insert (since they may include overlay directives) when short circuiting

pivotaljohn commented 3 years ago

this ~related~ is task in #112

Which has been broken out into it's own issue: #371