envoyproxy / envoyproxy/gateway
BackendTrafficPolicy: a `compressor` entry missing its codec object is silently dropped (Accepted=True, no filter, no fallback)
- Dominant language
- Go
- Stars
- 3k
- Forks
- 864
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 140
Description
### Description
A `BackendTrafficPolicy` entry such as
```yaml
compressor:
- type: Gzip
minContentLength: 1024
```
is accepted by the API server **and** by the controller (`Accepted=True`, `observedGeneration` current), produces **no** `envoy.filters.http.compressor` in the listener chain, no per-route `typed_per_filter_config`, and **no** log line or status condition anywhere. The only way to discover it is to diff the xDS config dump.
This is the same symptom as #9314, which was closed after a maintainer supplied the workaround (add `gzip: {}`) — with the note *"I can agree its not the best UX and could probably be improved."* I'm opening this to track that improvement specifically, rather than to re-litigate the behaviour itself.
### Why it fails closed
[`buildCompression()`](https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/backendtrafficpolicy.go#L2540) (line numbers from `main`):
```go
// Handle the Compressor field first (higher priority)
if len(compressor) > 0 {
result := make([]*ir.Compression, 0, len(compressor))
for i, c := range compressor {
// Only add compression if the corresponding compressor not null
if (c.Type == egv1a1.GzipCompressorType && c.Gzip != nil) ||
(c.Type == egv1a1.BrotliCompressorType && c.Brotli != nil) ||
(c.Type == egv1a1.ZstdCompressorType && c.Zstd != nil) {
result = append(result, &irCompression)
}
}
return result // <-- returns even when result is empty
}
// Fallback to the deprecated Compression field
```
Two things compound:
1. an entry whose codec object is nil is **skipped**, and
2. the `compressor` branch is taken whenever `len(compressor) > 0`, so an all-skipped list returns **empty** and never falls through to `compression`.
Net result is an empty IR compression list that is indistinguishable, from the user's side, from "no compression configured".
### Why the schema gives no hint
The task docs are correct — every example in `response-compression.md` includes `gzip: {}` / `brotli: {}` / `zstd: {}`. But the API says otherwise:
```go
// +required
Type CompressorType `json:"type"`
// +optional
Gzip *GzipCompressor `json:"gzip,omitempty"`
```
`GzipCompressor` is an empty struct, so `gzip: {}` reads as a no-op to anyone building config from `kubectl explain` or the CRD schema rather than from the docs. There is no CEL rule guarding it either, even though `BackendTrafficPolicySpec` already uses CEL for exactly this class of constraint:
```go
// +kubebuilder:validation:XValidation:rule="!has(self.compression) || !has(self.compressor)", message="either compression or compressor can be set, not both"
```
Notably, #6924 — which introduced the codec-object requirement so that `gzip: null` could express "disabled" — stated that *"Users should be required to provide both the `type` and the corresponding `brotli`/`gzip` attribute."* The translator half landed; the validation half did not.
### Suggested fix
I'd suggest **surfacing it** rather than rejecting it, because a CEL rule requiring the codec object may collide with the `gzip: null`-means-disabled semantics discussed in #6924 / #6775. In rough order of preference:
1. **Status condition or controller log** when a `compressor` entry is dropped for a missing codec object — turns a silent no-op into something greppable.
2. **Fall through to `compression`** when the `compressor` branch yields an empty result, so the deprecated field still works if someone sets both-ish configurations. (Behaviour change; less clearly correct.)
3. **CEL validation**, if `gzip: null` is *not* in fact load-bearing as a disable signal:
```
!has(self.compressor) || self.compressor.all(c,
(c.type == 'Gzip' && has(c.gzip)) ||
(c.type == 'Brotli' && has(c.brotli)) ||
(c.type == 'Zstd' && has(c.zstd)))
```
Happy to put up a PR for whichever direction maintainers prefer.
### Repro
```yaml
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
name: compress
spec:
targetRefs:
- group: gateway.networking.k8s.io
kind: Gateway
name: my-gateway
compressor:
- type: Gzip
minContentLength: 1024
```
Policy reports `Accepted=True`. `envoy.filters.http.compressor.gzip` is absent from the listener's `http_filters`. Adding `gzip: {}` makes it appear, with `min_content_length: 1024` and a `CompressorPerRoute` override on the routes.
⚠️ One gotcha worth recording for anyone verifying this: `grep compressor` over a full `config_dump` returns ~26 hits **even when it is not working** — Envoy lists every compiled-in extension under `BootstrapConfigDump`. Only the per-listener `filter_chains[].filters[].typed_config.http_filters` reflects the active chain.
### Environment
Envoy Gateway v1.8.3 (also present on `main` and v1.9.0 as of writing).
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at internal/gatewayapi/backendtrafficpolicy.go and inspect buildCompression(), then reproduce the BackendTrafficPolicy from the issue and verify the active listener filter chain rather than compiled-in extensions. Compare the possible validation, status/logging, and fallback directions with the existing gzip: null semantics; done requires an agreed behavior that no longer silently hides the invalid entry.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- api, backend-api-design
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100