kube-openapi
kube-openapi copied to clipboard
Allow marker comments to extend validations for subschemas
This extends current marker syntax to support extending the subschemas of a struct inside items, properties, or additionalProperties. This is useful in cases like declarative validation for native types where we have many shared types whose validations change at point-of-use, so it does not make sense to annotate the type. And using an alias is too disruptive.
Example:
package foo
// +k8s:openapi-gen=true
// +k8s:validation:properties:field:maxLength=10
// +k8s:validation:properties:aliasMap:additionalProperties:pattern>^foo$
type Blah struct {
// +k8s:validation:items:cel[0]:rule="self.length() % 2 == 0"
// +k8s:validation:items:cel[0]:message="length must be even"
Field MyAlias `json:"field"`
// +k8s:validation:additionalProperties:maxLength=10
AliasMap MyAliasMap `json:"aliasMap"`
}
type MyAliasMap map[string]MyAlias
type MyAlias []string
In the above example we can override subschemas at multiple different levels, with type checking supported.
This yields schema:
type: object
allOf:
properties:
field:
items: {maxLength: 10}
aliasMap: {pattern: ^foo$}
properties:
field:
type: array
items:
type: string
allOf:
items:
x-kubernetes-validations:
- rule: self.length() % 2 == 0
message: length must be even
aliasMap:
additionalProperties: {type: string}
allOf:
additionalProperties:
maxLength: 10
The value validations are placed in an allOf inside the object they were attached to and override subproperties. New structure is not allowed to be placed in these tags, only validations. We validate that any named properties are also in the structure of the Go type.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: alexzielenski
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [alexzielenski]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/cc @Jefftree @jpbetz
I feel like there are quite a few implicit changes here, would you be able to separate them out into another PR/commit to explain the relevance? The changes move the codebase in a good direction but I'm a bit confused about the interdependency between them and the PR description goal.
OTOH
- a set of fields being transformed into ptrs
- deterministic output
- validations such as minItems/etc should only be attached corresponding types
I split the deterministic output change into a separate commit (no test changes, since theres only 1 possible element so it is more of a future-proofing change)
I also removed the MinProperties/MaxProperties behavior change, so diff is much smaller.
ready for another pass
/hold
@jpbetz While trying to use this I've ran into the problem that CEL expressions inside allOf are not validated, since it is not part of the "structural" portion of the schema. Is this something that will ever change in the future? I'm also curious why this is the case, since CEL expressions are definitely value validations and don't impact schema structure unlike other X-extensions
Ive tested this extensively as part of https://github.com/kubernetes/kubernetes/pull/123163
To use CEL within these still requires https://github.com/kubernetes/kubernetes/pull/124381 but I think this is ready to merge
/cc @jpbetz /remove-hold
lgtm from me, I'll leave lgtm with @jpbetz if he wants another pass
One question about the removed test cases then this looks ready to merge.
/approve /lgtm Leaning on @Jefftree's review here, but it all looked reasonable to me.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: alexzielenski, jpbetz
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [alexzielenski,jpbetz]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment