fix(api): allow optional Cell TopoServer and adjust CEL validation limits #85
Conversation
…mits - Change `Cell.Spec.TopoServer` to a pointer (`*LocalTopoServerSpec`). This allows the field to be omitted (nil) when using the global default topology server, resolving validation errors where the empty struct failed the "oneOf" requirement (etcd vs external). - Adjusted validation limits due to CEL budget issues during integration tests. These limits may need to be adjusted as we go.
|
To provide more context on what caused the CEL validation budget errors and why I was conservative adjusting them all down: The Kubernetes API server rejected the CRD because the validation complexity (cost) is 100x higher than allowed. This happened because we had deep nesting ( The specific fields triggering the 100x budget explosion:
Summary: The deep nesting means a single validation rule on |
rytswd
left a comment
rytswd
left a comment
There was a problem hiding this comment.
LGTM, just one question left for clarification
| // Pools overrides. Keyed by pool name. | ||
| // +optional | ||
| // +kubebuilder:validation:MaxProperties=32 | ||
| // +kubebuilder:validation:MaxProperties=8 |
There was a problem hiding this comment.
Just curious, where is this number coming from?
There was a problem hiding this comment.
From my desperation and frugality to keep everything under CEL budget 😭
Don't take any of these limits as gospel, my guess is that we will need to adjust them further as we go along.
Change
Cell.Spec.TopoServerto a pointer (*LocalTopoServerSpec). This allows the field to be omitted (nil) when using the global default topology server, resolving validation errors where the empty struct failed the "oneOf" requirement (etcd vs external).Adjusted validation limits due to CEL budget issues during integration tests. These limits may need to be adjusted as we go.