kubernetes-sigs / kubernetes-sigs/controller-tools
Allow `AtMostOneOf`/`ExactlyOneOf`/`AtLeastOneOf` markers to emit OpenAPI `oneOf`/`anyOf` instead of CEL
- Dominant language
- Go
- Stars
- 868
- Forks
- 482
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 8
Description
This issue is a followup on https://github.com/kubernetes-sigs/controller-tools/pull/1212#issuecomment-3012321131:
> > So the OpenAPI based OneOf/AnyOf constraints would be a good alternative despite the unfriendly error message.
>
> Yeah so perhaps we take that route of a field on the marker useOpenAPIValidation=true or something like that. @\alvaroaleman any opinions?
>
> > With deeply nested types containing arrays, I have seen CEL break down. In most cases with newer k8s versions, MaxItems should suffice
>
> Yeah I guess when adding a compatible to change to an existing API, the argument for bounding all parents can be more difficult
>
PR #1212 added `AtMostOneOf`, `ExactlyOneOf`, and `AtLeastOneOf` markers, implemented with `x-kubernetes-validations` (CEL) for better error messages. An option to have those generate OpenAPI `oneOf`/`anyOf` would allow avoiding CEL budget limitations.
### Context
In the cilium project, we have the `CiliumNetworkPolicy` CRD that uses oneOf-style validation in several places. One of the most problematic ones for the CEL implementation is `CIDRRule`, which sits inside the ingress[]/egress[]/ingressDeny[]/egressDeny[] rule lists:
```go
// +kubebuilder:validation:ExactlyOneOf=cidr;cidrGroupRef;cidrGroupSelector
type CIDRRule struct {
Cidr CIDR `json:"cidr,omitempty"`
CIDRGroupRef CIDRGroupRef `json:"cidrGroupRef,omitempty"`
CIDRGroupSelector EndpointSelector `json:"cidrGroupSelector,omitzero"`
// ...
}
type IngressRule struct {
// ...
FromCIDRSet []CIDRRule `json:"fromCIDRSet,omitzero"`
}
type Rule struct {
Ingress []IngressRule `json:"ingress,omitempty"`
IngressDeny []IngressDenyRule `json:"ingressDeny,omitempty"`
Egress []EgressRule `json:"egress,omitempty"`
EgressDeny []EgressDenyRule `json:"egressDeny,omitempty"`
// ...
}
```
The generated CEL rule on `CIDRRule` is fine in isolation. But because `CIDRRule` is reachable through several layers of arrays, the rule is reapplied at every nesting level, so 8 separate sites in the schema each carry a copy of the same rule. The resulting CRD is rejected by the API server:
```
CustomResourceDefinition.apiextensions.k8s.io "ciliumnetworkpolicies.cilium.io" is invalid:
spec.validation.openAPIV3Schema.properties[spec].properties[ingress].items.properties[fromCIDRSet].items.x-kubernetes-validations[0].rule:
Forbidden: estimated rule cost exceeds budget by factor of 7.1x
...
spec.validation.openAPIV3Schema:
Forbidden: x-kubernetes-validations estimated rule cost total for entire OpenAPIv3 schema exceeds budget by factor of 12.7x
```
We did try optimizing the CEL expressions themselves (#1408) which managed to get each individual rule under budget, but the overall CRD cost still remained over budget.
We also looked at setting `MaxItems` to bound the array's costs, but in order for `MaxItems` to be effective, every single array in the chain needs to have it set and on a widely adopted stable (`v2`) CRD like the `CiliumNetworkPolicy`, we can't afford to add such constraints which would be a breaking change for users, just to get some validation.
I don't think cilium is the only project with this pattern of "polymorphic union under a list". Having the option to use openAPI validation instead of CEL for those cases would be beneficial. The cost would be a slightly worse error message for users but it's better than no validation at all.
### Proposal
We'd be highly interested in having a way to generate openAPI validation for oneOf markers as was initially suggested in #1212:
```go
// +kubebuilder:validation:ExactlyOneOf=cidr;cidrGroupRef;cidrGroupSelector,useOpenAPIValidation=true
```
When set, the marker would emit openAPI `oneOf`/`anyOf`/`allOf` instead of `x-kubernetes-validations` validation. The default behavior would remain CEL so existing users' error messages don't regress.
If `controller-tools` maintainers are open to this proposal, we'd be happy to contribute it (the actual implementation actually already exists as the first version of #1212 before it changed trajectory to use CEL instead of openAPI).
Contributor guide
Research direction
Start by reading PR #1212 and its marker implementation for AtMostOneOf, ExactlyOneOf, and AtLeastOneOf. Trace how those markers currently produce x-kubernetes-validations, then identify the existing OpenAPI-based implementation referenced in the issue. Done means an explicit marker option emits oneOf/anyOf/allOf while the default remains CEL, with coverage for the three marker types.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes, openapi
- Domain
- api, devtools
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100