-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
disableCompression: Expose configuration to toggle Envoy GZIP compression on the responses #6546
base: main
Are you sure you want to change the base?
Changes from 26 commits
b4d5b67
081a47c
818ccf0
7ddb6f0
8df07e2
100bf40
56e4ee7
548981e
934ca48
2efb693
b0cfe71
72e0e9c
73c4b67
235acc3
7e843b7
36dbadb
f7b261b
b68f0b1
5dcdc33
3d83394
cda9c76
ff84c37
5725a53
630541e
4610b80
45ea946
98225fc
661b552
75f8b7b
351a820
89060c0
95d38af
366f48d
1a6465f
43f6598
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
// Copyright Project Contour Authors | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package v1alpha1 | ||
|
||
import "fmt" | ||
|
||
// CompressionAlgorithm defines the type of compression algorithm applied in default HTTP listener filter chain. | ||
// Allowable values are defined as names of well known compression algorithms (plus "disabled"). | ||
type CompressionAlgorithm string | ||
|
||
// EnvoyCompression defines configuration related to compression in the default HTTP Listener filter chain. | ||
type EnvoyCompression struct { | ||
// Algorithm selects the compression type applied in the compression HTTP filter of the default Listener filters. | ||
// Values: `gzip` (default), `brotli`, `zstd`, `disabled`. | ||
// Setting this to `disabled` will make Envoy skip "Accept-Encoding: gzip,deflate" request header and always return uncompressed response | ||
// +kubebuilder:validation:Enum="gzip";"brotli";"zstd";"disabled" | ||
// +optional | ||
Algorithm CompressionAlgorithm `json:"algorithm,omitempty"` | ||
} | ||
|
||
func (a CompressionAlgorithm) Validate() error { | ||
switch a { | ||
case BrotliCompression, DisabledCompression, GzipCompression, ZstdCompression: | ||
return nil | ||
default: | ||
return fmt.Errorf("invalid compression type: %q", a) | ||
} | ||
} | ||
|
||
const ( | ||
// BrotliCompression specifies brotli as the default HTTP filter chain compression mechanism | ||
BrotliCompression CompressionAlgorithm = "brotli" | ||
|
||
// DisabledCompression specifies that there will be no compression in the default HTTP filter chain | ||
DisabledCompression CompressionAlgorithm = "disabled" | ||
|
||
// GzipCompression specifies gzip as the default HTTP filter chain compression mechanism | ||
GzipCompression CompressionAlgorithm = "gzip" | ||
|
||
// ZstdCompression specifies zstd as the default HTTP filter chain compression mechanism | ||
ZstdCompression CompressionAlgorithm = "zstd" | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Copyright Project Contour Authors | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package v1alpha1_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" | ||
) | ||
|
||
func TestValidateEnvoyCompressionAlgorithmType(t *testing.T) { | ||
require.Error(t, contour_v1alpha1.CompressionAlgorithm("").Validate()) | ||
require.Error(t, contour_v1alpha1.CompressionAlgorithm("foo").Validate()) | ||
|
||
require.NoError(t, contour_v1alpha1.BrotliCompression.Validate()) | ||
require.NoError(t, contour_v1alpha1.DisabledCompression.Validate()) | ||
require.NoError(t, contour_v1alpha1.GzipCompression.Validate()) | ||
require.NoError(t, contour_v1alpha1.ZstdCompression.Validate()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add "compression" object to define settings in default HTTP filters, initially just supporting changing/disabling compression algorithm. | ||
geomacy marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,8 +123,10 @@ | |
|
||
return nil | ||
} | ||
|
||
serve.Flag("accesslog-format", "Format for Envoy access logs.").PlaceHolder("<envoy|json>").StringVar((*string)(&ctx.Config.AccessLogFormat)) | ||
|
||
serve.Flag("compression", "Set or disable compression type in default Listener filters.").PlaceHolder("<gzip|brotli|zstd|disabled>").StringVar((*string)(&ctx.Config.Compression.Algorithm)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove the command line option and make configuration available only through config file and The command line options currently are a bit of a mess, so we previously agreed to avoid introducing new options there unless absolutely necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See 95d38af |
||
serve.Flag("config-path", "Path to base configuration.").Short('c').PlaceHolder("/path/to/file").Action(parseConfig).ExistingFileVar(&configFile) | ||
serve.Flag("contour-cafile", "CA bundle file name for serving gRPC with TLS.").Envar("CONTOUR_CAFILE").StringVar(&ctx.caFile) | ||
serve.Flag("contour-cert-file", "Contour certificate file name for serving gRPC over TLS.").PlaceHolder("/path/to/file").Envar("CONTOUR_CERT_FILE").StringVar(&ctx.contourCert) | ||
|
@@ -447,6 +449,7 @@ | |
} | ||
|
||
listenerConfig := xdscache_v3.ListenerConfig{ | ||
Compression: contourConfiguration.Envoy.Listener.Compression, | ||
UseProxyProto: *contourConfiguration.Envoy.Listener.UseProxyProto, | ||
HTTPAccessLog: contourConfiguration.Envoy.HTTPListener.AccessLog, | ||
HTTPSAccessLog: contourConfiguration.Envoy.HTTPSListener.AccessLog, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be cleaner to treat an empty string as a valid value here, rather than completely bypassing validation in
Parameters.Validate()
? While algorithm is the only field currently, that might change.Because
+optional
string in Go becomes an empty string when user did not set the field, it might be best to consider it as valid algorithm name in the Go side (CRD validation for the field can still reject it). It is already taken into account elsewhere in this change: inhttpConnectionManagerBuilder.DefaultFilters()
you've added switch-default branch which will usegzip
for empty algorithm string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 366f48d