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

Provide error on duplicate YAML keys in strict mode? #881

Open GUI opened 1 year ago

GUI commented 1 year ago

Describe the problem/challenge you have

It's easy to introduce bugs into YAML due to duplicative keys. For example:

key: value1
key: value2

Will evaluate to:

key: value2

I realize YAML allows duplicate keys and this evaluation makes sense, but in large YAML documents, it can be easy to overlook duplicate keys that are more spread out in the document, which can lead to bugs (since it may not be immediately obvious that some value will get overwritten later on in the document).

Describe the solution you'd like

I was wondering if YTT could perhaps help guard against this type of situation, since it seems like duplicative keys are usually a mistake within a single YAML document. In particular, I was wondering if this might be a good fit for ytt's "strict" mode, since its stated goal is: "tries to remove any kind of ambiguity in user’s intent when parsing YAML." Duplicative keys seem like they fit this bill, so I was wondering if you all think this would be a useful addition to strict mode.

Currently strict mode doesn't care about duplicate keys, and just uses the last value:

printf "key: value1\nkey: value2" | ytt -f- --strict
key: value2

But I might envision strict mode behaving something like this:

printf "key: value1\nkey: value2" | ytt -f- --strict
ytt: Error: Unmarshaling YAML template 'stdin.yml': yaml:
  Strict parsing:
    Found duplicate key 'key'

But non-strict mode could continue to work as-is.


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 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 12 months ago

@GUI Have you considered using a linting tool like yamllint?

I will check if this is something viable to include in lib, and how unmarshalling currently handles duplicate because go yaml lib seems to returns error as per this.

github-actions[bot] commented 10 months ago

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

prembhaskal commented 10 months ago

So i finally got around to check this. Ytt is removing the duplicate keys and retaining only the last one here. Plain yaml.v3 unmarshal rejects duplicate keys (https://go.dev/play/p/WEOlmjSJ0UQ)

So I think your suggestion is valid and it seems feasible that we can check in strict mode for unique keys. I will move the issue to backlog for now.