Bug 1827600: Fix crd openAPI validation regression#1677
Bug 1827600: Fix crd openAPI validation regression#1677openshift-merge-robot merged 2 commits intoopenshift:masterfrom
Conversation
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
I'm not against using a nullable json parser. Does this mean we can drop all nullable fields in general?
Also I'd prefer if Update vendor and mco: Use "github.com/clarketm/json" for marshaling Ignition configs were just one commit, so reverting is easier if we needed to. I assume this is likely cherry picked from FCOS, and that's how it was originally laid out?
There was a problem hiding this comment.
Where did this change originate from?
There was a problem hiding this comment.
the marshaled data is much smaller now due to leaving out all the empty/nulled fields - it's a nice side benefit of using this parser. As the number (114 before, 15 now) is specified a few times, I introdued this constant.
I'm not entirely sure what you mean here, can you elaborate? We use this code in fcos: 563275c |
Replaces "encoding/json" with dropin replacement "github.com/clarketm/json" that supports zero values of structs with omittempty when marshaling Ignition configs. In effect, this will exclude empty pointer struct fields from the marshaled data instead of inserting nil values into them which do not pass openAPI validation on fields that are supposed to contain strings. See [1] for more context. Also updates the vendor directory (with `make go-deps`). [1] golang/go#11939
Reverts a regression on the openAPI validation scheme.
LorbusChris
force-pushed
the
fix-crd-validation-regression
branch
from
5fd0b83 to
1793580
Compare
|
@yuqi-zhang, I squashed the two commits. Wrt above, I think what it means is that we can drop all fields labeled |
LorbusChris
changed the title
openshift-ci-robot
added
bugzilla/unspecified
bugzilla/valid-bug
|
@LorbusChris: This pull request references Bugzilla bug 1827600, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
|
/retest |
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
Will let others also take a look in case someone has a concern with the vendoring
openshift-ci-robot
added
the
approved
|
/approve |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LorbusChris, runcom, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@LorbusChris: All pull requests linked via external trackers have merged: openshift/machine-config-operator#1677. Bugzilla bug 1827600 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
5 participants
#1474 introduced a small regression on the openAPI validation scheme, this fixes that.
- What I did
Replaces "encoding/json" with dropin replacement "github.com/clarketm/json"
that supports zero values of structs with omittempty when marshaling
Ignition configs.
In effect, this will exclude empty pointer struct fields from the
marshaled data instead of inserting nil values into them which do not
pass openAPI validation on fields that are supposed to contain strings.
See proposal: encoding/json, encoding/xml: support zero values of structs with omitempty golang/go#11939 for more context.
Reverts a regression on the openAPI validation scheme.
- How to verify it
CI
- Description for the changelog
Use "github.com/clarketm/json" for marshaling Ignition configs and revert a regression on the openAPI validation scheme.
@openshift/openshift-team-mco
Please let me know if you want this in 4.5 and therefore need a BZ.