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

Improve the validation of unknown fields in configuration objects #1066

Closed
neolit123 opened this issue Aug 17, 2018 · 7 comments · Fixed by kubernetes/kubernetes#70901
Closed
Assignees
Labels
do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@neolit123
Copy link
Member

a separate bug was discovered here when we found a typo in an etcd config field:
#1045
explanation here:
kubernetes/website#9802 (comment)

if localEtcd is passed instead of local the parsing fails silently and we don't get an error from the api-machinery code.

improve the validation for unknown fields.
adding this as a reminder here even if it belongs in another tracker.

@timothysc @bradfitz

@neolit123 neolit123 added kind/bug Categorizes issue or PR as related to a bug. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Aug 17, 2018
@neolit123 neolit123 added this to the GA milestone Aug 17, 2018
@timothysc
Copy link
Member

/assign @liztio @timothysc

@timothysc timothysc removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Aug 27, 2018
@timothysc timothysc modified the milestones: GA, v1.12 Aug 27, 2018
@timothysc
Copy link
Member

I'm ok with just a simple error message that makes it easy to grok.

@timothysc timothysc added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Aug 30, 2018
@liztio liztio added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Aug 30, 2018
@liztio
Copy link

liztio commented Aug 31, 2018

sig-api-machinery is working on this:

kubernetes/kubernetes#67477
kubernetes/kubernetes#67594

unfortunately it looks like the PR to double-upstream (ghodss/yaml) got rejected, so they're not targeting v1.12 anymore.

I could write my own workaround, using reflect and a map, but we may just want to wait on k/k to merge the proper solution (especially since this is still alpha in v1.12)

@liztio
Copy link

liztio commented Aug 31, 2018

@dims is trying again with ghodss/yaml#38, gonna call this blocked on that.

@liztio liztio removed the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Aug 31, 2018
@timothysc timothysc modified the milestones: v1.12, v1.13 Aug 31, 2018
@timothysc timothysc added the do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. label Aug 31, 2018
@timothysc
Copy link
Member

We're going to have to punt this to 1.13 due to the dependency chain and 1.12 freeze.

@timothysc timothysc assigned dims and unassigned timothysc Aug 31, 2018
@timothysc timothysc assigned timothysc and unassigned liztio Oct 30, 2018
@timothysc timothysc added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Nov 14, 2018
@neolit123
Copy link
Member Author

@bradfitz
Copy link

@neolit123, great, thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants