kubernetes-sigs / kubernetes-sigs/controller-tools

Allow `AtMostOneOf`/`ExactlyOneOf`/`AtLeastOneOf` markers to emit OpenAPI `oneOf`/`anyOf` instead of CEL

Open
#1,409 2 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.