-
-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate doesn't correctly validate yaml files per the spec. #270
Comments
The method of validation dasel uses here is just to attempt to read the YAML and fail if it cannot be parsed, so largely the same approach kubernetes is using. We use |
It does indeed have a strict mode it claims will catch duplicates: https://pkg.go.dev/gopkg.in/yaml.v2#UnmarshalStrict. Happy to make a PR if that's a change you'll accept for |
Yes of course. It may be good to hide it behind a |
I spent some time looking at this over Christmas. Before I spend more time on it there's a bit of a refactor required and I wanted to run that by you before making the changes. In the So, there are 2 options to enable configurability on the parsers:
In this specific case, the addition of a "strict" mode feels like parser level configuration, not method level, i.e. a parser is configured once then can be used many times with that configuration. That means we're looking at options 1 or 2. Option 2 is probably slightly less work but it's essentially a half assed factory, i.e. Option 1. Additionally if somebody was using dasel as a library rather than a CLI tool, option 2 makes it easier to introduce bugs in sibling code paths (one changes the configuration that is then used by the other that doesn't expect the config). Enforcing this type of configuration at instantiation, option 1, makes it clear what the state of the parser is. Let me know what your thoughts are and I can take a crack at the changes. |
I agree that option 1 is probably the right way to go here.
Thanks for looking into this 🙂 |
Describe the bug
The yaml spec requires that all keys in a mapping are unique (spec for reference: https://yaml.org/spec/1.2.2/).
dasel validate
does not catch this issue in a yaml fileTo Reproduce
Steps to reproduce the behavior:
Expected behavior
Validation should fail because of the duplicate
key
key at the top level.Desktop (please complete the following information):
Additional context
This would be useful for validating kubernetes yaml files as every now and again somebody unintentionally redeclares a key in a map and kubernetes doesn't error, it just takes the last assignment.
The text was updated successfully, but these errors were encountered: