Log an error when config data contains unknown fields #19106
Conversation
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/M
|
/assign @deads2k @smarterclayton @sdodson @jupierce Note that in the time that I started working on this and actually made the PR, #19070 merged, meaning that if this was a fatal check, the |
|
This requires Go 1.10, so it may fail to compile on CI. |
|
cc @soltysh I vaguely remember you complaining about a customer issue caused by a typo in a master config file... |
|
I don't want errors on this. At least not yet. A warning would be ok. Being able to have "extra" values allows us to modify a config first, then roll the images forward on a shared configmap. Errors would prevent that use-case. |
So leave the PR as-is since it only glogs? |
|
@deads2k where is the warning code you were referring to? |
blast from the past. I was thinking of this: https://github.com/openshift/origin/blob/master/pkg/cmd/server/start/start_master.go#L267-L275 |
|
The warning method called there is used by the diagnostics too. |
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
I'd love this be in for 3.11! |
This change logs an error in the following format: Encountered config error json: unknown field "foobar" in object *config.MasterConfig when config data contains unknown fields. It is written with the assumption that it will be changed to a fatal error at some future point (we will likely need to add some oc commands to help validate the various config objects). Signed-off-by: Monis Khan <[email protected]>
enj
force-pushed
the
enj/i/strict_decode_config
branch
from
eab1a87
to
6317463
Compare
|
@soltysh @stevekuznetsov who do we harass to get CI to be on Go 1.10? |
|
/refresh |
1 similar comment
|
/refresh |
@stevekuznetsov or @dobbymoodge is my wild guess 😉 |
|
But I thought we already are, since the rebase landed and it requires 1.10, no? |
|
The test failures are all compile errors due to CI not being on Go 1.10. |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, simo5 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
@enj: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest |
This change logs an error in the following format:
Encountered config error json: unknown field "foobar" in object *config.MasterConfigwhen config data contains unknown fields. It is written with the assumption that it will be changed to a fatal error at some future point (we will likely need to add some oc commands to help validate the various config objects).
Signed-off-by: Monis Khan [email protected]
Fixes #19914