Skip to content
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

master-config.yaml with invalid keys are not validated #19914

Closed
gnufied opened this issue Jun 5, 2018 · 4 comments
Closed

master-config.yaml with invalid keys are not validated #19914

gnufied opened this issue Jun 5, 2018 · 4 comments
Assignees

Comments

@gnufied
Copy link
Member

gnufied commented Jun 5, 2018

Sometimes our users put keys in YAML in subtly incorrect indentation. This does not cause any validation errors or anything like that and master controllers start just fine but intended purpose of putting the key in YAML does not work.

YAML being whitespace senstive makes it harder to notice. Can we just start validating the keys in YAML and print warning or errors or even refuse to start the process if something is wrong with YAML?

Examples:

  1. https://bugzilla.redhat.com/show_bug.cgi?id=1582231
  2. https://bugzilla.redhat.com/show_bug.cgi?id=1570085
@gnufied
Copy link
Member Author

gnufied commented Jun 5, 2018

cc @liggitt @smarterclayton

@liggitt
Copy link
Contributor

liggitt commented Jun 5, 2018

Can we just start validating the keys in YAML

there's no facility for negative schema validation in our current json/yaml machinery. the data decodes to go structs, and unknown fields get dropped in the process, with no programmatic indication that occurred (this is identical to the way our API objects work when decoded by the server)

and print warning or errors or even refuse to start the process if something is wrong with YAML?

an unexpected key is indistinguishable from a config field added in a future version of the server. we should think carefully about hard-failing on that

@enj
Copy link
Contributor

enj commented Jun 7, 2018

Just waiting on Go 1.10 for #19106

It warns on unexpected fields but does not fail.

@gnufied
Copy link
Member Author

gnufied commented Jun 7, 2018

It warns on unexpected fields but does not fail.

Awesome, yeah I think warning should be "good enough" compromise for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants