kubernetes / kube-openapi

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

validate: dont short circuit validator on MinProperties/MaxProperties error #412

Closed alexzielenski closed 1 year ago

alexzielenski commented 1 year ago

Currently object_validator does not explore any of the fields of an object if the number of fields violates any MinProperties/MaxProperties condition. This is inconsistent with slice_validator, which validates MinItems/MaxItems after visiting any of the items.

This prevents us from seeing all errors if MinProperties/MaxProperties is violated.

This is the only location in validation where a part of the input object is not explored/short circuited. Having this property would greatly simplify logic for map-type aware DeepEqual logic for CRD validation ratcheting. The map-type aware DeepEqual uses the tree traversed by SchemaValidator to inform which schema is associated with which subobject. Unfortunately if there is a MaxProperties/MinProperties error a portion of the tree is missing.

This change makes the validator keep going after seeing an error with number of properties and validate each of the subfields anyway, just as it does for arrays.

apelisse commented 1 year ago

That looks a lot better, a test would be good?

sttts commented 1 year ago

/approve

Leaving lgtm to @apelisse after a test is added.

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexzielenski, sttts

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

added test

/label tide/merge-method-squash