Validation: deserialization time vs creation time concerns

[lavalamp]

Proto (and, it seems, json-schema) have a "required" concept, by which they mean that you cannot decode/deserialize an object that lacks the field.

Kubernetes has a required concept, by which we mean you cannot create an object if it lacks the field.

Fields that are "required" in the Kubernetes sense are NOT required in the proto/json-schema sense: There are valid uses of kubernetes resources that involve deliberately partially specified objects.

This is just an example of an attribute for which it's very important when it is checked.

I'll categorize the aspects of our system in these two buckets here, as I think of them.

Deserialization-time enforcement:

• Correct types (e.g. no ints as strings, no arrays in place of scalars, etc)

Validation-time enforcement:

• required/optional
• immutability
• min/max
• regexp match / enum value

[lavalamp]

@roycaihw @apelisse @sttts @mbohlool fyi

Note, this might mean we can't use the available schema checkers, or have to modify our spec documents and use them in two passes.

[roycaihw]

JSON Schema considers an object instance valid if all required properties are present (value can be null as long as key exists)
https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.4.3

OpenAPI Spec v2.0 inherits json-schema definition on required
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#schema-object

My question is what's Kubernetes' concept for required field in apiserver response (e.g. some required field in resource status)? What client-side validation should be performed?

[lavalamp]

Clients should not perform the "validation-time enforcement" actions on the results they get from apiserver, IMO.

[roycaihw]

From client perspective (kubernetes-client/gen#52), apiserver isn't violating any rule by returning null for required arrays. We should remove the client-side validation generated by swagger-codegen @mbohlool?

[Arnavion]

From client perspective (kubernetes-client/gen#52), apiserver isn't violating any rule by returning null for required arrays.

As far as I can see, null is indeed a valid value for a "required" property, but the violation is that null is not a valid value for an array type. JSON schema defines the type of a null value to be null. So since APIGroup's serverAddressByClientCIDRs is defined as "type": "array", "items": { "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ServerAddressByClientCIDR" } then null is not a valid value for it.

OpenAPI 3 made it simpler by having a nullable boolean property, but in 2.0 it seems you would have to make it an anyOf of array or null.

[Arnavion]

Also, conversations about client-side validation should keep in mind that some languages are more strongly-typed than Python :) My Rust codegen trusts the spec and uses Vec<...> for APIGroup's serverAddressByClientCIDRs property. There is no client-side validation that can be "disabled" here that would allow null to be parsed as a Vec.

[roycaihw]

@Arnavion That makes a lot of sense. Thanks for clarifying!

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.