envoyproxy / envoyproxy/gateway
Migrate EnvoyProxy Go validation to CRD CEL validation where possible
- Dominant language
- Go
- Stars
- 3k
- Forks
- 864
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 148
Description
*Description*:
Most of the runtime Go validation for the EnvoyProxy CRD in [api/v1alpha1/validation/envoyproxy_validate.go](https://github.com/envoyproxy/gateway/blob/68f8320b054702c60fb804c02aeda30807ec283e/api/v1alpha1/validation/envoyproxy_validate.go#L69) checks conditions that are scoped to a single object and can be expressed as CRD schema / CEL rules (+kubebuilder:validation:XValidation, Enum, Required, etc.). Moving these to the CRD means the API server rejects invalid configs at admission time (fast feedback via kubectl apply), removes duplicated checks from the translation hot path.
The code already anticipates this: two validators carry `// TODO: remove this function if CEL validation became stable` (validateProvider and validateService).
CRD Validation Rules (x-kubernetes-validations) reached GA in Kubernetes 1.29, and the CEL extension libraries (isIP(), isCIDR(), format.*) are GA in 1.31+. The compatibility matrix supports Kubernetes v1.30+ (latest: v1.33–v1.36), so the "if CEL validation became stable" precondition in those TODOs is now satisfied across all supported versions.
Candidates to move from Go validation to CRD CEL / kubebuilder markers
These checks in `api/v1alpha1/validation/envoyproxy_validate.go` are scoped to a single object (no cross-resource lookups), so they can be enforced by the API server via CRD schema markers instead of runtime Go validation.
| # | Check | Go location | Feasible | Proposed marker |
|---|---|---|---|---|
| 1 | `provider.type == Host` implies `host` is required | `validateProvider` | Yes | `XValidation` on `EnvoyProxyProvider` |
| 2 | Unsupported `provider.type` | `validateProvider` | Yes, already covered | `EnvoyProxyProviderType` already has `Enum=Kubernetes;Host`, so the Go check is redundant |
| 3 | `deployment` / `hpa` / `pdb` `patch.type` must be one of `StrategicMerge` or `JSONMerge` | `validateDeployment` / `validateHpa` / `validatePdb` | Yes | `Enum` on `MergeType` |
| 4 | `service.type` must be one of `LoadBalancer`, `ClusterIP`, or `NodePort` | `validateService` | Yes | `Enum` on `ServiceType` |
| 5 | `allocateLoadBalancerNodePorts`, `loadBalancerSourceRanges`, and `loadBalancerIP` are only valid when `type == LoadBalancer` | `validateService` | Yes | `XValidation` on `KubernetesServiceSpec` |
| 6 | `loadBalancerSourceRanges` must be valid CIDRs, and `loadBalancerIP` must be a valid IP address | `validateService` | Yes | `XValidation` with `isCIDR()` / `isIP()` (Kubernetes 1.31+, #4549) |
| 7 | `metrics.sinks[*].type == OpenTelemetry` implies `openTelemetry` is required | `validateProxyTelemetry` | Yes | `XValidation` on `ProxyMetricSink`; this also prevents a nil-deref panic |
| 8 | Access log sink / format type-specific fields: `Text` implies `text`, `File` implies `file`, `OpenTelemetry` implies `openTelemetry` | `validateProxyAccessLog` | Yes | `XValidation` on `ProxyAccessLogSink` / `ProxyAccessLogFormat` per item |
I’d like to work on the implementation
[optional *Relevant Links*:]
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.