search

kubernetes / kube-openapi

Kubernetes OpenAPI spec generation & serving
Apache License 2.0
319 stars 208 forks source link

spec: Fix missing field in v2 serialization #377

Closed apelisse closed 1 year ago

apelisse commented 1 year ago

cc @Jefftree @alexzielenski @liggitt

We should disallow certain fields if we want to in the schema fuzzer, not hard-code all the allowed fields, otherwise we might miss fields, or forget to update with new fields, but for now, this code and test properly show the failure and the fix. I've also tested this against k8s, and it fixes the problem properly.

liggitt commented 1 year ago

/lgtm

apelisse commented 1 year ago

/hold

I'm still looking into a few things right now, I'd rather clear these before we move forward :-(

apelisse commented 1 year ago

Found another bug, I'm working on a few other things.

apelisse commented 1 year ago

removed the seed and ran some of these tests a few thousands times, always passed. I'll try this PR against k8s (at least for the fuzzed test that used to fail)

apelisse commented 1 year ago

Ran again in k8s, passed a thousand times.

apelisse commented 1 year ago

/hold cancel

liggitt commented 1 year ago

/lgtm

liggitt commented 1 year ago

/approve /hold in case you wanted any other eyes on it

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, liggitt

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: - ~~[OWNERS](https://github.com/kubernetes/kube-openapi/blob/master/OWNERS)~~ [apelisse] - ~~[pkg/validation/OWNERS](https://github.com/kubernetes/kube-openapi/blob/master/pkg/validation/OWNERS)~~ [liggitt] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
apelisse commented 1 year ago

/hold cancel I've discussed it with Alex and Jeff already