feat: automatically set first compressor with choose_first (#7406)

* feat: automatically set first compressor with choose_first

Signed-off-by: Steven Kreitzer <[email protected]>

Author: buroa

api/v1alpha1/backendtrafficpolicy_types.go

@@ -84,6 +84,7 @@ type BackendTrafficPolicySpec struct {
 
 	// The compressor config for the http streams.
 	// This provides more granular control over compression configuration.
+	// Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal.
 	//
 	// +patchMergeKey=type
 	// +patchStrategy=merge

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml

@@ -148,6 +148,7 @@ spec:
                 description: |-
                   The compressor config for the http streams.
                   This provides more granular control over compression configuration.
+                  Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal.
                 items:
                   description: |-
                     Compression defines the config of enabling compression.

charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml

@@ -147,6 +147,7 @@ spec:
                 description: |-
                   The compressor config for the http streams.
                   This provides more granular control over compression configuration.
+                  Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal.
                 items:
                   description: |-
                     Compression defines the config of enabling compression.

internal/xds/translator/compressor.go

@@ -45,59 +45,22 @@ func (*compressor) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTT
 	}
 
 	var (
-		brotli bool
-		gzip   bool
-		zstd   bool
 		filter *hcmv3.HttpFilter
 		err    error
 	)
 
 	for _, route := range irListener.Routes {
 		if route.Traffic != nil && route.Traffic.Compression != nil {
-			for _, irComp := range route.Traffic.Compression {
-				if irComp.Type == egv1a1.BrotliCompressorType {
-					brotli = true
+			for i, irComp := range route.Traffic.Compression {
+				filterName := compressorFilterName(irComp.Type)
+				if !hcmContainsFilter(mgr, filterName) {
+					chooseFirst := i == 0
+					if filter, err = buildCompressorFilter(irComp, chooseFirst); err != nil {
+						return err
+					}
+					mgr.HttpFilters = append(mgr.HttpFilters, filter)
 				}
-				if irComp.Type == egv1a1.GzipCompressorType {
-					gzip = true
-				}
-				if irComp.Type == egv1a1.ZstdCompressorType {
-					zstd = true
-				}
-			}
-		}
-	}
-
-	// Add the compressor filters for all the compression types required by the routes.
-	// All the compressor filters are disabled at the HCM level.
-	// The per route filter config will enable the compressor filters for the routes that require them.
-	if brotli {
-		brotliFilterName := compressorFilterName(egv1a1.BrotliCompressorType)
-		if !hcmContainsFilter(mgr, brotliFilterName) {
-			if filter, err = buildCompressorFilter(egv1a1.BrotliCompressorType); err != nil {
-				return err
-			}
-			mgr.HttpFilters = append(mgr.HttpFilters, filter)
-		}
-	}
-
-	if gzip {
-		gzipFilterName := compressorFilterName(egv1a1.GzipCompressorType)
-		if !hcmContainsFilter(mgr, gzipFilterName) {
-			if filter, err = buildCompressorFilter(egv1a1.GzipCompressorType); err != nil {
-				return err
 			}
-			mgr.HttpFilters = append(mgr.HttpFilters, filter)
-		}
-	}
-
-	if zstd {
-		zstdFilterName := compressorFilterName(egv1a1.ZstdCompressorType)
-		if !hcmContainsFilter(mgr, zstdFilterName) {
-			if filter, err = buildCompressorFilter(egv1a1.ZstdCompressorType); err != nil {
-				return err
-			}
-			mgr.HttpFilters = append(mgr.HttpFilters, filter)
 		}
 	}
 
@@ -109,7 +72,7 @@ func compressorFilterName(compressorType egv1a1.CompressorType) string {
 }
 
 // buildCompressorFilter builds a compressor filter with the provided compressionType.
-func buildCompressorFilter(compressionType egv1a1.CompressorType) (*hcmv3.HttpFilter, error) {
+func buildCompressorFilter(compression *ir.Compression, chooseFirst bool) (*hcmv3.HttpFilter, error) {
 	var (
 		compressorProto *compressorv3.Compressor
 		extensionName   string
@@ -119,7 +82,7 @@ func buildCompressorFilter(compressionType egv1a1.CompressorType) (*hcmv3.HttpFi
 		err             error
 	)
 
-	switch compressionType {
+	switch compression.Type {
 	case egv1a1.BrotliCompressorType:
 		extensionName = "envoy.compression.brotli.compressor"
 		extensionMsg = &brotliv3.Brotli{}
@@ -142,12 +105,16 @@ func buildCompressorFilter(compressionType egv1a1.CompressorType) (*hcmv3.HttpFi
 		},
 	}
 
+	if chooseFirst {
+		compressorProto.ChooseFirst = true
+	}
+
 	if compressorAny, err = proto.ToAnyWithValidation(compressorProto); err != nil {
 		return nil, err
 	}
 
 	return &hcmv3.HttpFilter{
-		Name: compressorFilterName(compressionType),
+		Name: compressorFilterName(compression.Type),
 		ConfigType: &hcmv3.HttpFilter_TypedConfig{
 			TypedConfig: compressorAny,
 		},
@@ -173,77 +140,39 @@ func (*compressor) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir
 	}
 
 	var (
-		brotli        bool
-		gzip          bool
-		zstd          bool
 		perFilterCfg  map[string]*anypb.Any
 		compressorAny *anypb.Any
 		err           error
 	)
 
-	for _, irComp := range irRoute.Traffic.Compression {
-		if irComp.Type == egv1a1.BrotliCompressorType {
-			brotli = true
-		}
-		if irComp.Type == egv1a1.GzipCompressorType {
-			gzip = true
-		}
-		if irComp.Type == egv1a1.ZstdCompressorType {
-			zstd = true
-		}
-	}
-
-	if !brotli && !gzip && !zstd {
-		return nil
-	}
-
 	// Overwrite the HCM level filter config with the per route filter config.
 	perFilterCfg = route.GetTypedPerFilterConfig()
 	if perFilterCfg == nil {
 		route.TypedPerFilterConfig = make(map[string]*anypb.Any)
 	}
 
-	compressorProto := compressorPerRouteConfig()
-	if compressorAny, err = proto.ToAnyWithValidation(compressorProto); err != nil {
-		return err
-	}
-
-	if brotli {
-		brotliFilterName := compressorFilterName(egv1a1.BrotliCompressorType)
-		if _, ok := perFilterCfg[brotliFilterName]; ok {
-			// This should not happen since this is the only place where the filter
-			// config is added in a route.
-			return fmt.Errorf("route already contains filter config: %s, %+v",
-				brotliFilterName, route)
-		}
-		route.TypedPerFilterConfig[brotliFilterName] = compressorAny
-	}
-	if gzip {
-		gzipFilterName := compressorFilterName(egv1a1.GzipCompressorType)
-		if _, ok := perFilterCfg[gzipFilterName]; ok {
+	for _, irComp := range irRoute.Traffic.Compression {
+		filterName := compressorFilterName(irComp.Type)
+		if _, ok := perFilterCfg[filterName]; ok {
 			// This should not happen since this is the only place where the filter
 			// config is added in a route.
 			return fmt.Errorf("route already contains filter config: %s, %+v",
-				gzipFilterName, route)
+				filterName, route)
 		}
-		route.TypedPerFilterConfig[gzipFilterName] = compressorAny
-	}
-	if zstd {
-		zstdFilterName := compressorFilterName(egv1a1.ZstdCompressorType)
-		if _, ok := perFilterCfg[zstdFilterName]; ok {
-			// This should not happen since this is the only place where the filter
-			// config is added in a route.
-			return fmt.Errorf("route already contains filter config: %s, %+v",
-				zstdFilterName, route)
+
+		compressorProto := compressorPerRouteConfig()
+		if compressorAny, err = proto.ToAnyWithValidation(compressorProto); err != nil {
+			return err
 		}
-		route.TypedPerFilterConfig[zstdFilterName] = compressorAny
+
+		route.TypedPerFilterConfig[filterName] = compressorAny
 	}
 
 	return nil
 }
 
+// Enable compression on this route if compression is configured.
 func compressorPerRouteConfig() *compressorv3.CompressorPerRoute {
-	// Enable compression on this route if compression is configured.
 	return &compressorv3.CompressorPerRoute{
 		Override: &compressorv3.CompressorPerRoute_Overrides{
 			Overrides: &compressorv3.CompressorOverrides{

internal/xds/translator/testdata/out/xds-ir/compression.listeners.yaml

@@ -18,6 +18,7 @@
           name: envoy.filters.http.compressor.brotli
           typedConfig:
             '@type': type.googleapis.com/envoy.extensions.filters.http.compressor.v3.Compressor
+            chooseFirst: true
             compressorLibrary:
               name: envoy.compression.brotli.compressor
               typedConfig:

site/content/en/latest/api/extension_types.md

@@ -542,7 +542,7 @@ _Appears in:_
 | `faultInjection` | _[FaultInjection](#faultinjection)_ |  false  |  | FaultInjection defines the fault injection policy to be applied. This configuration can be used to<br />inject delays and abort requests to mimic failure scenarios such as service failures and overloads |
 | `useClientProtocol` | _boolean_ |  false  |  | UseClientProtocol configures Envoy to prefer sending requests to backends using<br />the same HTTP protocol that the incoming request used. Defaults to false, which means<br />that Envoy will use the protocol indicated by the attached BackendRef. |
 | `compression` | _[Compression](#compression) array_ |  false  |  | The compression config for the http streams.<br />Deprecated: Use Compressor instead. |
-| `compressor` | _[Compression](#compression) array_ |  false  |  | The compressor config for the http streams.<br />This provides more granular control over compression configuration. |
+| `compressor` | _[Compression](#compression) array_ |  false  |  | The compressor config for the http streams.<br />This provides more granular control over compression configuration.<br />Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal. |
 | `responseOverride` | _[ResponseOverride](#responseoverride) array_ |  false  |  | ResponseOverride defines the configuration to override specific responses with a custom one.<br />If multiple configurations are specified, the first one to match wins. |
 | `httpUpgrade` | _[ProtocolUpgradeConfig](#protocolupgradeconfig) array_ |  false  |  | HTTPUpgrade defines the configuration for HTTP protocol upgrades.<br />If not specified, the default upgrade configuration(websocket) will be used. |
 | `requestBuffer` | _[RequestBuffer](#requestbuffer)_ |  false  |  | RequestBuffer allows the gateway to buffer and fully receive each request from a client before continuing to send the request<br />upstream to the backends. This can be helpful to shield your backend servers from slow clients, and also to enforce a maximum size per request<br />as any requests larger than the buffer size will be rejected.<br />This can have a negative performance impact so should only be enabled when necessary.<br />When enabling this option, you should also configure your connection buffer size to account for these request buffers. There will also be an<br />increase in memory usage for Envoy that should be accounted for in your deployment settings. |

test/e2e/tests/compression.go

@@ -55,6 +55,10 @@ var CompressionTest = suite.ConformanceTest{
 			testCompression(t, suite, egv1a1.ZstdCompressorType)
 		})
 
+		t.Run("HTTPRoute with brotli compression chooseFirst", func(t *testing.T) {
+			testCompressionChooseFirst(t, suite, egv1a1.BrotliCompressorType)
+		})
+
 		t.Run("HTTPRoute without compression", func(t *testing.T) {
 			ns := "gateway-conformance-infra"
 			routeNN := types.NamespacedName{Name: "no-compression", Namespace: ns}
@@ -122,6 +126,40 @@ func testCompression(t *testing.T, suite *suite.ConformanceTestSuite, compressio
 	http.MakeRequestAndExpectEventuallyConsistentResponse(t, roundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
 }
 
+func testCompressionChooseFirst(t *testing.T, suite *suite.ConformanceTestSuite, compressionType egv1a1.CompressorType) {
+	ns := "gateway-conformance-infra"
+	routeNN := types.NamespacedName{Name: "compression", Namespace: ns}
+	gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns}
+	gwAddr := kubernetes.GatewayAndRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), &gwapiv1.HTTPRoute{}, false, routeNN)
+
+	ancestorRef := gwapiv1.ParentReference{
+		Group:     gatewayapi.GroupPtr(gwapiv1.GroupName),
+		Kind:      gatewayapi.KindPtr(resource.KindGateway),
+		Namespace: gatewayapi.NamespacePtr(gwNN.Namespace),
+		Name:      gwapiv1.ObjectName(gwNN.Name),
+	}
+	BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "compression", Namespace: ns}, suite.ControllerName, ancestorRef)
+
+	encoding := ContentEncoding(compressionType)
+	expectedResponse := http.ExpectedResponse{
+		Request: http.Request{
+			Path: "/compression",
+			Headers: map[string]string{
+				"Accept-encoding": "gzip, br, zstd",
+			},
+		},
+		Response: http.Response{
+			StatusCodes: []int{200},
+			Headers: map[string]string{
+				"content-encoding": encoding,
+			},
+		},
+		Namespace: ns,
+	}
+	roundTripper := &CompressionRoundTripper{Debug: suite.Debug, TimeoutConfig: suite.TimeoutConfig}
+	http.MakeRequestAndExpectEventuallyConsistentResponse(t, roundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
+}
+
 // CompressionRoundTripper implements roundtripper.RoundTripper and adds support for compression.
 type CompressionRoundTripper struct {
 	Debug             bool

test/helm/gateway-crds-helm/all.out.yaml

@@ -21281,6 +21281,7 @@ spec:
                 description: |-
                   The compressor config for the http streams.
                   This provides more granular control over compression configuration.
+                  Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal.
                 items:
                   description: |-
                     Compression defines the config of enabling compression.

test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml

@@ -625,6 +625,7 @@ spec:
                 description: |-
                   The compressor config for the http streams.
                   This provides more granular control over compression configuration.
+                  Order matters: The first compressor in the list is preferred when q-values in Accept-Encoding are equal.
                 items:
                   description: |-
                     Compression defines the config of enabling compression.