carvel-dev / ytt

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

Schema validation does not work on first key in list dict #915

Closed henry-spanka closed 5 months ago

henry-spanka commented 5 months ago

What steps did you take:

A schema key is not validated when it's the first key in a list of dicts.

schema.yaml

#@data/values-schema
---
apps:
  - #@schema/validation min_len=1
    name: ""
    #@schema/validation min_len=1
    revision: master

values.yaml

#@data/values
---
apps:
  - name: argocd
    revision: dev
  - name: cert-manager
    revision: dev
  - name: ""
    revision: dev

What happened:

Templating with ytt -f schema.yaml -f values.yaml --data-values-inspect completes successfully.

What did you expect:

Templating to fail due to name being an empty string.

Anything else you would like to add:

Adding the comment after the value does trigger validation. However this is only necessary on the first key. Subsequent dict successfully validate when the comment is above the key.

#@data/values-schema
---
apps:
  - name: "" #@schema/validation min_len=1
    #@schema/validation min_len=1
    revision: master

Environment:

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 addressed as soon as possible" 👎 "There are other more important things to focus on right now"

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

prembhaskal commented 5 months ago

@henry-spanka I notice that if we move the schema validation comment to next line instead of putting in front of the name , the schema validation works fine see working example here https://carvel.dev/ytt/#gist:https://gist.github.com/prembhaskal/d5385d09183ed2f1959e03106674f485

#@data/values-schema
---
apps:
  - 
    #@schema/validation min_len=1
    name: ""
    #@schema/validation min_len=1
    revision: master

we will need to check this in detail a bit.

prembhaskal commented 5 months ago

Also according to this slack discussion here, https://kubernetes.slack.com/archives/CH8KCCKA5/p1618934385381800 the - #@schema/validation.... should have ideally caused an ytt error.

Also Dmitriy clarifies in same chat that

- #@x...

is same as

#@x...
-   
renuy commented 5 months ago

Thanks @prembhaskal !