-
Notifications
You must be signed in to change notification settings - Fork 680
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 11 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
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 @@ | ||||||
Add disableCompression boolean flag to disable GZIP compression HTTP filter from the default Listener filters. | ||||||
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. The changelog entry could mention where the flag can be found
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,6 +188,7 @@ type httpConnectionManagerBuilder struct { | |
maxRequestsPerConnection *uint32 | ||
http2MaxConcurrentStreams *uint32 | ||
enableWebsockets bool | ||
disableCompression bool | ||
} | ||
|
||
func (b *httpConnectionManagerBuilder) EnableWebsockets(enable bool) *httpConnectionManagerBuilder { | ||
|
@@ -271,6 +272,11 @@ func (b *httpConnectionManagerBuilder) MergeSlashes(enabled bool) *httpConnectio | |
return b | ||
} | ||
|
||
func (b *httpConnectionManagerBuilder) DisableCompression(disabled bool) *httpConnectionManagerBuilder { | ||
b.disableCompression = disabled | ||
return b | ||
} | ||
|
||
func (b *httpConnectionManagerBuilder) ServerHeaderTransformation(value contour_v1alpha1.ServerHeaderTransformationType) *httpConnectionManagerBuilder { | ||
switch value { | ||
case contour_v1alpha1.OverwriteServerHeader: | ||
|
@@ -308,34 +314,38 @@ func (b *httpConnectionManagerBuilder) DefaultFilters() *httpConnectionManagerBu | |
// Add a default set of ordered http filters. | ||
// The names are not required to match anything and are | ||
// identified by the TypeURL of each filter. | ||
b.filters = append(b.filters, | ||
&envoy_filter_network_http_connection_manager_v3.HttpFilter{ | ||
Name: CompressorFilterName, | ||
ConfigType: &envoy_filter_network_http_connection_manager_v3.HttpFilter_TypedConfig{ | ||
TypedConfig: protobuf.MustMarshalAny(&envoy_filter_http_compressor_v3.Compressor{ | ||
CompressorLibrary: &envoy_config_core_v3.TypedExtensionConfig{ | ||
Name: "gzip", | ||
TypedConfig: protobuf.MustMarshalAny( | ||
&envoy_compression_gzip_compressor_v3.Gzip{}, | ||
), | ||
}, | ||
ResponseDirectionConfig: &envoy_filter_http_compressor_v3.Compressor_ResponseDirectionConfig{ | ||
CommonConfig: &envoy_filter_http_compressor_v3.Compressor_CommonDirectionConfig{ | ||
ContentType: []string{ | ||
// Default content-types https://github.com/envoyproxy/envoy/blob/e74999dbdb12aa4d6b7a5d62d51731ea86bf72be/source/extensions/filters/http/compressor/compressor_filter.cc#L35-L38 | ||
"text/html", "text/plain", "text/css", "application/javascript", "application/x-javascript", | ||
"text/javascript", "text/x-javascript", "text/ecmascript", "text/js", "text/jscript", | ||
"text/x-js", "application/ecmascript", "application/x-json", "application/xml", | ||
"application/json", "image/svg+xml", "text/xml", "application/xhtml+xml", | ||
// Additional content-types for grpc-web https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md#protocol-differences-vs-grpc-over-http2 | ||
"application/grpc-web", "application/grpc-web+proto", "application/grpc-web+json", "application/grpc-web+thrift", | ||
"application/grpc-web-text", "application/grpc-web-text+proto", "application/grpc-web-text+thrift", | ||
if !b.disableCompression { | ||
// If compression is enabled add compressor filter | ||
b.filters = append(b.filters, | ||
&envoy_filter_network_http_connection_manager_v3.HttpFilter{ | ||
Name: CompressorFilterName, | ||
ConfigType: &envoy_filter_network_http_connection_manager_v3.HttpFilter_TypedConfig{ | ||
TypedConfig: protobuf.MustMarshalAny(&envoy_filter_http_compressor_v3.Compressor{ | ||
CompressorLibrary: &envoy_config_core_v3.TypedExtensionConfig{ | ||
Name: "gzip", | ||
TypedConfig: protobuf.MustMarshalAny( | ||
&envoy_compression_gzip_compressor_v3.Gzip{}, | ||
), | ||
}, | ||
ResponseDirectionConfig: &envoy_filter_http_compressor_v3.Compressor_ResponseDirectionConfig{ | ||
CommonConfig: &envoy_filter_http_compressor_v3.Compressor_CommonDirectionConfig{ | ||
ContentType: []string{ | ||
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. Possible alternative approach could be to leave the compressor filter in the chain but disable it by setting default to Enabled: &envoy_config_core_v3.RuntimeFeatureFlag{
// If compression is enabled add compressor filter
DefaultValue: wrapperspb.Bool(!b.disableCompression),
RuntimeKey: "contour.compression.response.enabled",
}, but since we do not have "per route" setting, which would override the default, it make sense to me that the whole filter is excluded from the chain when disabled, like suggested in this PR currently. 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. Yes, given
the thought in this PR is to work at the level of the whole filter. The functionality could be extended as described above if needed in future but that would best be left to another PR. |
||
// Default content-types https://github.com/envoyproxy/envoy/blob/e74999dbdb12aa4d6b7a5d62d51731ea86bf72be/source/extensions/filters/http/compressor/compressor_filter.cc#L35-L38 | ||
"text/html", "text/plain", "text/css", "application/javascript", "application/x-javascript", | ||
"text/javascript", "text/x-javascript", "text/ecmascript", "text/js", "text/jscript", | ||
"text/x-js", "application/ecmascript", "application/x-json", "application/xml", | ||
"application/json", "image/svg+xml", "text/xml", "application/xhtml+xml", | ||
// Additional content-types for grpc-web https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md#protocol-differences-vs-grpc-over-http2 | ||
"application/grpc-web", "application/grpc-web+proto", "application/grpc-web+json", "application/grpc-web+thrift", | ||
"application/grpc-web-text", "application/grpc-web-text+proto", "application/grpc-web-text+thrift", | ||
}, | ||
}, | ||
}, | ||
}, | ||
}), | ||
}, | ||
}, | ||
}), | ||
}, | ||
}) | ||
} | ||
b.filters = append(b.filters, | ||
&envoy_filter_network_http_connection_manager_v3.HttpFilter{ | ||
Name: GRPCWebFilterName, | ||
ConfigType: &envoy_filter_network_http_connection_manager_v3.HttpFilter_TypedConfig{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
// 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 v3 | ||
|
||
import ( | ||
"testing" | ||
|
||
envoy_service_discovery_v3 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" | ||
core_v1 "k8s.io/api/core/v1" | ||
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
||
contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" | ||
contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" | ||
envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3" | ||
"github.com/projectcontour/contour/internal/fixture" | ||
xdscache_v3 "github.com/projectcontour/contour/internal/xdscache/v3" | ||
) | ||
|
||
func TestDefaultCompression(t *testing.T) { | ||
rh, c, done := setup(t) | ||
defer done() | ||
|
||
s1 := fixture.NewService("backend"). | ||
WithPorts(core_v1.ServicePort{Name: "http", Port: 80}) | ||
rh.OnAdd(s1) | ||
|
||
hp1 := &contour_v1.HTTPProxy{ | ||
ObjectMeta: meta_v1.ObjectMeta{ | ||
Name: "simple", | ||
Namespace: s1.Namespace, | ||
}, | ||
Spec: contour_v1.HTTPProxySpec{ | ||
VirtualHost: &contour_v1.VirtualHost{ | ||
Fqdn: "example.com", | ||
}, | ||
Routes: []contour_v1.Route{{ | ||
Conditions: matchconditions(prefixMatchCondition("/")), | ||
Services: []contour_v1.Service{{ | ||
Name: s1.Name, | ||
Port: 80, | ||
}}, | ||
}}, | ||
}, | ||
} | ||
rh.OnAdd(hp1) | ||
|
||
httpListener := defaultHTTPListener() | ||
httpListener.FilterChains = envoy_v3.FilterChains(envoy_v3.HTTPConnectionManagerBuilder(). | ||
RouteConfigName(xdscache_v3.ENVOY_HTTP_LISTENER). | ||
MetricsPrefix(xdscache_v3.ENVOY_HTTP_LISTENER). | ||
AccessLoggers(envoy_v3.FileAccessLogEnvoy(xdscache_v3.DEFAULT_HTTP_ACCESS_LOG, "", nil, contour_v1alpha1.LogLevelInfo)). | ||
DefaultFilters(). | ||
Get(), | ||
) | ||
|
||
c.Request(listenerType, xdscache_v3.ENVOY_HTTP_LISTENER).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ | ||
TypeUrl: listenerType, | ||
Resources: resources(t, httpListener), | ||
}) | ||
} | ||
|
||
func TestDisableCompression(t *testing.T) { | ||
withDisableCompression := func(conf *xdscache_v3.ListenerConfig) { | ||
conf.DisableCompression = true | ||
} | ||
|
||
rh, c, done := setup(t, withDisableCompression) | ||
defer done() | ||
|
||
s1 := fixture.NewService("backend"). | ||
WithPorts(core_v1.ServicePort{Name: "http", Port: 80}) | ||
rh.OnAdd(s1) | ||
|
||
hp1 := &contour_v1.HTTPProxy{ | ||
ObjectMeta: meta_v1.ObjectMeta{ | ||
Name: "simple", | ||
Namespace: s1.Namespace, | ||
}, | ||
Spec: contour_v1.HTTPProxySpec{ | ||
VirtualHost: &contour_v1.VirtualHost{ | ||
Fqdn: "example.com", | ||
}, | ||
Routes: []contour_v1.Route{{ | ||
Conditions: matchconditions(prefixMatchCondition("/")), | ||
Services: []contour_v1.Service{{ | ||
Name: s1.Name, | ||
Port: 80, | ||
}}, | ||
}}, | ||
}, | ||
} | ||
rh.OnAdd(hp1) | ||
|
||
httpListener := defaultHTTPListener() | ||
httpListener.FilterChains = envoy_v3.FilterChains(envoy_v3.HTTPConnectionManagerBuilder(). | ||
DisableCompression(true). | ||
RouteConfigName(xdscache_v3.ENVOY_HTTP_LISTENER). | ||
MetricsPrefix(xdscache_v3.ENVOY_HTTP_LISTENER). | ||
AccessLoggers(envoy_v3.FileAccessLogEnvoy(xdscache_v3.DEFAULT_HTTP_ACCESS_LOG, "", nil, contour_v1alpha1.LogLevelInfo)). | ||
DefaultFilters(). | ||
Get(), | ||
) | ||
|
||
c.Request(listenerType, xdscache_v3.ENVOY_HTTP_LISTENER).Equals(&envoy_service_discovery_v3.DiscoveryResponse{ | ||
TypeUrl: listenerType, | ||
Resources: resources(t, httpListener), | ||
}) | ||
} |
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.