feat: add brotli and zstd compression support#14275
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends TrafficPolicy.compression.responseCompression from a gzip-only configuration to support multiple response compression codecs (Gzip, Brotli, Zstd) with Envoy-driven Accept-Encoding negotiation, while preserving backward compatibility (defaulting to gzip). It also fixes Equals handling for typed-nil IR interfaces and adds translator + e2e coverage for the new behaviors.
Changes:
- Add
responseCompression.librariesto the TrafficPolicy API/CRD (Gzip/Brotli/Zstd, preference-ordered) and update documentation. - Update the trafficpolicy plugin to install one compressor filter per selected codec and enable/disable them via per-route typed config (including multi-codec negotiation).
- Add unit tests, translator golden cases, and e2e tests for brotli/zstd and negotiation.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/e2e/features/compression/types.go |
Registers new e2e test cases/manifests for brotli, zstd, and negotiation. |
test/e2e/features/compression/suite.go |
Adds e2e assertions for brotli, zstd, and multi-codec negotiation behavior. |
test/e2e/features/compression/testdata/tp-route-brotli.yaml |
Adds TrafficPolicy manifest selecting Brotli for a route. |
test/e2e/features/compression/testdata/tp-route-zstd.yaml |
Adds TrafficPolicy manifest selecting Zstd for a route. |
test/e2e/features/compression/testdata/tp-route-negotiation.yaml |
Adds TrafficPolicy manifest offering Brotli then Gzip for negotiation. |
pkg/kgateway/translator/gateway/gateway_translator_test.go |
Adds translator test cases for brotli/zstd/negotiation/conflict scenarios. |
pkg/kgateway/translator/gateway/testutils/inputs/traffic-policy/brotli-compression-route.yaml |
Adds translator input for Brotli compression. |
pkg/kgateway/translator/gateway/testutils/inputs/traffic-policy/zstd-compression-route.yaml |
Adds translator input for Zstd compression. |
pkg/kgateway/translator/gateway/testutils/inputs/traffic-policy/compression-negotiation-route.yaml |
Adds translator input for multi-codec negotiation. |
pkg/kgateway/translator/gateway/testutils/inputs/traffic-policy/compression-codec-conflict.yaml |
Adds translator input for precedence resolution when policies disagree on codec. |
pkg/kgateway/translator/gateway/testutils/outputs/traffic-policy/brotli-compression-route.yaml |
Adds expected xDS output for Brotli compression. |
pkg/kgateway/translator/gateway/testutils/outputs/traffic-policy/zstd-compression-route.yaml |
Adds expected xDS output for Zstd compression. |
pkg/kgateway/translator/gateway/testutils/outputs/traffic-policy/compression-negotiation-route.yaml |
Adds expected xDS output for multi-codec negotiation (multiple filters + per-route configs). |
pkg/kgateway/translator/gateway/testutils/outputs/traffic-policy/compression-codec-conflict.yaml |
Adds expected xDS output for codec conflict precedence result. |
pkg/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go |
Adds optional per-route disable helper; updates plugin pass state to track multiple compressors per chain. |
pkg/kgateway/extensions2/plugins/trafficpolicy/compression.go |
Implements multi-library IR + filter emission logic; fixes typed-nil equality checks. |
pkg/kgateway/extensions2/plugins/trafficpolicy/compression_test.go |
Adds unit tests for compression IR equality and library defaulting. |
install/helm/kgateway-crds/templates/gateway.kgateway.dev_trafficpolicies.yaml |
Updates CRD schema/docs to include responseCompression.libraries. |
api/v1alpha1/kgateway/traffic_policy_types.go |
Adds CompressionLibrary enum + libraries field and updates API docs. |
api/v1alpha1/kgateway/zz_generated.deepcopy.go |
Updates deepcopy generation for the new Libraries slice field. |
Files not reviewed (1)
- api/v1alpha1/kgateway/zz_generated.deepcopy.go: Generated file
|
what if you had two routes, each with a TrafficPolicy, one specifying gzip, the other [brotli,gzip]. It appears that the "preference order" would be violated |
f592639 to
457f095
Compare
|
@puertomontt I hadn't thought of that case, but yes, it can't be resolved with my current approach, since the first one to be translated is the one that sets the preference. I'm checking to see if I can handle it the same way as I addressed the configuration hostalp requested, based on Content headers in #13659. |
|
@puertomontt Solved, it seems Envoy break ties through a Seems to work for 2-way [brotli,gzip], etc. and 3-way [brotli,gzip,zstd], etc. |
compressionIR.Equals and decompressionIR.Equals checked the interface operand (other == nil) instead of the concrete pointer when guarding the nil case. A typed-nil *compressionIR wrapped in the PolicySubIR interface is not == nil, so comparing a set IR against a nil one fell through to the field comparison and dereferenced the nil pointer. Guard on the asserted pointer (oc/od) instead. Signed-off-by: Pinguladora <[email protected]>
4210900 to
e5e88d0
Compare
|
the issue with the new approach is I believe it will break the "disable" because of the hash. Also it blows up the number of filters. |
e5e88d0 to
060c5ee
Compare
|
right on both. Disable breaks for hashed names, they never turns off. And the filters explode up to 33 filters at most with all possible ordering combinations, doesn't affect much performance wise but yeah it's a bit bloated. The hashing was the only way I could think of for the server side preference given how Envoy works. But I checked Envoy Gateway implementation and they don't seem to do server-side preference either, they let the client pick. Given it seems like an Envoy limitation and they're the official implementation, I think we should follow them, so I've reverted to the one shared compressor filter per codec (3 filters max). I also tested the case of route A offering [gzip] and route B offering [brotli,gzip], with a [brotli,gzip] request A returns gzip while B returns brotli. I think that solves the case you mentioned. |
|
can you add a translator test for the disable path also, the api docs state that Libraries is ordered but the e2e tests state that the client ordering wins. |
Add a CompressionLibrary enum (Gzip, Brotli, Zstd) and an ordered Libraries field on ResponseCompression. The list expresses the codecs to offer for response compression, in preference order, so Envoy can negotiate the codec against the downstream Accept-Encoding header. It defaults to [Gzip] to preserve existing behavior, allows at most three unique entries, and is validated as a non-empty, duplicate-free set. Regenerate the TrafficPolicy CRD, RBAC, and deepcopy. This only introduces the API surface; translation is wired up separately. Signed-off-by: Pinguladora <[email protected]>
Install one Envoy compressor filter per offered codec in the filter chain and let Envoy negotiate which one runs based on the request's Accept-Encoding header. Each codec uses a distinct filter name (gzip keeps the historical envoy.filters.http.compressor name so existing single-codec config stays byte-identical) and the route enables every offered codec via per-route config. Filters of differing types in the same stage are otherwise ordered by name, which would not reflect the configured preference, so each codec is placed at an increasing weight after the CORS stage. Chain order then follows the policy's preference order, which is the tie-break Envoy uses when the client accepts multiple codecs at equal quality. A lone gzip codec lands at the same position as before. On the disable path the codecs enabled by a higher-level policy are not known, so disable every codec's compressor filter with an optional per-route config that Envoy ignores for filters absent from the chain. Add unit tests for compressionIR.Equals and the codec-list defaulting. Signed-off-by: Pinguladora <[email protected]>
Add gateway translator golden cases for single-codec brotli and zstd (asserting the per-codec compressor filter name and typed_config type URL), for multi-codec negotiation (two compressor filters emitted with brotli ordered ahead of gzip), and for a codec conflict where two policies target the same route and the higher-priority policy's codec wins. Signed-off-by: Pinguladora <[email protected]>
Extend the compression e2e suite with single-codec brotli and zstd cases and a negotiation case offering [Brotli, Gzip]: assert that a client accepting both gets brotli, a gzip-only client gets gzip, and a client accepting only an unsupported codec gets an uncompressed response. Signed-off-by: Pinguladora <[email protected]>
When compression is disabled all codecs are turned off regardless of the configured libraries, so two disabled IRs that differ only in libraries produce identical config. Treat them as equal to avoid unnecessary KRT recomputation, matching the field's documented semantics. Signed-off-by: Pinguladora <[email protected]>
060c5ee to
5346f7b
Compare
|
both done |
|
Thanks for the contribution! |
|
@davidjumani seems like the status check are failing randomly for the current merge queue like #14276, can you take a look? |
Signed-off-by: Pinguladora <[email protected]>
Description
TrafficPolicy.compression.responseCompression was gzip-only, the compressor filter hardcoded the library to
envoy.compression.gzip.compressor. This adds support for brotli and zstd, with real Accept-Encoding content negotiation.Backward compatibility is maintained as an unset/empty list resolves to Gzip, and the gzip filter keeps its historical name
envoy.filters.http.compressor.Also fixes a bug where compressionIR.Equals / decompressionIR.Equals guarded the nil case on the interface operand (other == nil) instead of the asserted concrete pointer. A typed-nil *compressionIR wrapped in the PolicySubIR interface is not == nil, so comparing a set IR against a nil one fell through and dereferenced the nil pointer.
Fixes #13659
Change Type
/kind feature
/kind fix
Changelog
Additional Notes
Equalsincluding the nil case and codec-list defaulting), translator cases (single brotli/zstd, multi-codec negotiation and codec conflict), and e2e cases (brotli, zstd, negotiation). Also verified different combinations on a local kind cluster, including a request accepting neither and returning uncompressed.