We talked about this today in the API SIG - there was general agreement
that validation should be divided into "schema driven" and "turing
complete", where the former is the list of items you describe above (and
possibly some others, like which values are allowed) that could be easily
specified declaratively, and should be in swagger. The remainder would be
a much smaller set, and could be duplicated on different API versions more
easily than today.

On Wed, May 11, 2016 at 3:52 AM, Brian Grant notifications@github.com
wrote:

There are frequent errors in API field specification, such as:

• default vs inferred (e.g., fsType:
  https://github.com/kubernetes/kubernetes/blob/master/pkg/api/v1/types.go#L557);
  note that implicitly inferred is discouraged
• default vs required
• required/optional vs omitempty
• omitempty vs. pointers
• documentation vs actual defaulting and validation code

For examples of multiple errors in one type, see:
#18885 (comment)
#18885 (comment)

One way to make defaulting and validation less error prone would to be
allow them to be specified declaratively, on API fields using tags or
comments. Protocol buffers allow literal default values to be specified
(though, unlike in Kubernetes, they aren't conveyed on the wire;
non-literals, such as fields defaulted from other fields, can't be
specified this way). Declarative validation, where possible (i.e., simple
rules), was previously requested by #8116
#8116.

Since omitempty and pointer rules are apparently non-obvious, I think we
should require specification of required vs optional and then either verify
that the other properties are consistent or autogenerate them.

Auto-fixing field names in documentation is covered by #19319
#19319.

cc @lavalamp https://github.com/lavalamp @thockin
https://github.com/thockin @kubernetes/sig-api-machinery
https://github.com/orgs/kubernetes/teams/sig-api-machinery

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly or view it on GitHub
#25460

I'd like to move this along in 1.5 slightly, probably by focusing on the minimal thing that would allow required / optional to be correct (not just omitempty) and allow us to validate everything around that. Now that @mbohlool has generated openapi we can rely on struct tags more.

I have a proof of concept for a validation framework #32824. If you think we can add something to that please comment there.

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.

Prevent issues from auto-closing with an /lifecycle frozen comment.

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

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
/remove-lifecycle stale

/remove-lifecycle rotten
/lifecycle frozen

I still think we need to move to declarative specifications as much as possible.

cc @enisoc

Currently it is a big pain to work with YAML and the API in most strongly typed languages, because when deserialising a YAML file into swagger-generated Model classes, missing optional scalar fields like ContainerV1.failureThreshold (int) will get the runtime default value (0), but the server default value is 3, and this is only mentioned in the documentation, not in the schema. So when diffing the server state with the local instance, the patch will always include countless changes like this that reset values to their runtime default. But if the patch always excluded default values, then it would be impossible to generate a patch that resets a boolean from true to false.

What we would need is to expose the server default value in the schema through the JSON schema default field, so that the client can apply the right default values when deserialising, so that they won't make it into the patch if the values are the same.

The inconsistency is also problematic - for example, int Container1.failureThreshold is a non-nullable with a default value of 3 (i.e. to set it the default, it has to be set explicitly to 3, or to check if it is the default it has to be checked if it's 3), but int PodSpecV1.TerminationGracePeriodSeconds is a nullable with default value of 30 (where the documentation says it should be set to null to use the default value).

Should all scalars with a default value be nullable or should they all be non-nullable? It's a bit cumbersome to work with a nullable type when querying the API when in reality the value will always be set because the server applies a default value.

@apelisse I feel like we should probably close this old issue now, yes/no ?

I don't have a strong opinion on this one. We technically don't have validation markers yet (or at least, we could have more, CEL partially covers that too, @jpbetz).