From e6dcd6845e3dc2e314a676cad7107fe75225def0 Mon Sep 17 00:00:00 2001 From: Samuel Vaillant Date: Tue, 17 Jan 2023 02:08:32 +0100 Subject: [PATCH] feat(configmap): expose gzip-disable (#9505) * docs(configmap): add link for gzip-min-length * feat(configmap): expose gzip-disable * test(e2e): cover gzip settings * docs(configmap): simplify description with NGINX link * refactor(configmap): simplify condition --- .../nginx-configuration/configmap.md | 6 ++ internal/ingress/controller/config/config.go | 6 +- .../controller/template/configmap_test.go | 2 + rootfs/etc/nginx/template/nginx.tmpl | 3 + test/e2e/settings/gzip.go | 99 +++++++++++++++++++ 5 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 test/e2e/settings/gzip.go diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 5c0b79510..2a3369044 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -103,7 +103,9 @@ The following table shows a configuration option's name, type, and the default v |[brotli-min-length](#brotli-min-length)|int|20| |[brotli-types](#brotli-types)|string|"application/xml+rss application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component"| |[use-http2](#use-http2)|bool|"true"| +|[gzip-disable](#gzip-disable)|string|""| |[gzip-level](#gzip-level)|int|1| +|[gzip-min-length](#gzip-min-length)|int|256| |[gzip-types](#gzip-types)|string|"application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/javascript text/plain text/x-component"| |[worker-processes](#worker-processes)|string|``| |[worker-cpu-affinity](#worker-cpu-affinity)|string|""| @@ -715,6 +717,10 @@ _**default:**_ `application/xml+rss application/atom+xml application/javascript Enables or disables [HTTP/2](https://nginx.org/en/docs/http/ngx_http_v2_module.html) support in secure connections. +## gzip-disable + +Disables [gzipping](http://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_disable) of responses for requests with "User-Agent" header fields matching any of the specified regular expressions. + ## gzip-level Sets the gzip Compression Level that will be used. _**default:**_ 1 diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index f064dad53..a4a4c5a0b 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -438,6 +438,11 @@ type Configuration struct { // Default: true UseHTTP2 bool `json:"use-http2,omitempty"` + // Disables gzipping of responses for requests with "User-Agent" header fields matching any of + // the specified regular expressions. + // http://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_disable + GzipDisable string `json:"gzip-disable,omitempty"` + // gzip Compression Level that will be used GzipLevel int `json:"gzip-level,omitempty"` @@ -794,7 +799,6 @@ func NewDefault() Configuration { defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}, false} cfg := Configuration{ - AllowSnippetAnnotations: true, AllowBackendServerHeader: false, AnnotationValueWordBlocklist: "", diff --git a/internal/ingress/controller/template/configmap_test.go b/internal/ingress/controller/template/configmap_test.go index c662e0bc7..ebe55d192 100644 --- a/internal/ingress/controller/template/configmap_test.go +++ b/internal/ingress/controller/template/configmap_test.go @@ -64,6 +64,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { "access-log-path": "/var/log/test/access.log", "error-log-path": "/var/log/test/error.log", "use-gzip": "false", + "gzip-disable": "msie6", "gzip-level": "9", "gzip-min-length": "1024", "gzip-types": "text/html", @@ -87,6 +88,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { def.ProxyReadTimeout = 1 def.ProxySendTimeout = 2 def.UseProxyProtocol = true + def.GzipDisable = "msie6" def.GzipLevel = 9 def.GzipMinLength = 1024 def.GzipTypes = "text/html" diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index fc48de912..911cf75ea 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -336,6 +336,9 @@ http { {{ if $cfg.UseGzip }} gzip on; gzip_comp_level {{ $cfg.GzipLevel }}; + {{- if $cfg.GzipDisable }} + gzip_disable "{{ $cfg.GzipDisable }}"; + {{- end }} gzip_http_version 1.1; gzip_min_length {{ $cfg.GzipMinLength}}; gzip_types {{ $cfg.GzipTypes }}; diff --git a/test/e2e/settings/gzip.go b/test/e2e/settings/gzip.go new file mode 100644 index 000000000..68e80d3a0 --- /dev/null +++ b/test/e2e/settings/gzip.go @@ -0,0 +1,99 @@ +/* +Copyright 2023 The Kubernetes 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 settings + +import ( + "fmt" + "strings" + + "github.com/onsi/ginkgo/v2" + + "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.DescribeSetting("gzip", func() { + f := framework.NewDefaultFramework("gzip") + + ginkgo.It("should be disabled by default", func() { + f.WaitForNginxConfiguration( + func(cfg string) bool { + return !strings.Contains(cfg, "gzip on;") + }) + }) + + ginkgo.It("should be enabled with default settings", func() { + f.UpdateNginxConfigMapData("use-gzip", "true") + + f.WaitForNginxConfiguration( + func(cfg string) bool { + defaultCfg := config.NewDefault() + return strings.Contains(cfg, "gzip on;") && + strings.Contains(cfg, fmt.Sprintf("gzip_comp_level %d;", defaultCfg.GzipLevel)) && + !strings.Contains(cfg, "gzip_disable") && + strings.Contains(cfg, "gzip_http_version 1.1;") && + strings.Contains(cfg, fmt.Sprintf("gzip_min_length %d;", defaultCfg.GzipMinLength)) && + strings.Contains(cfg, fmt.Sprintf("gzip_types %s;", defaultCfg.GzipTypes)) && + strings.Contains(cfg, "gzip_proxied any;") && + strings.Contains(cfg, "gzip_vary on;") + }) + }) + + ginkgo.It("should set gzip_comp_level to 4", func() { + f.UpdateNginxConfigMapData("use-gzip", "true") + f.UpdateNginxConfigMapData("gzip-level", "4") + + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "gzip on;") && + strings.Contains(cfg, "gzip_comp_level 4;") + }) + }) + + ginkgo.It("should set gzip_disable to msie6", func() { + f.UpdateNginxConfigMapData("use-gzip", "true") + f.UpdateNginxConfigMapData("gzip-disable", "msie6") + + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "gzip on;") && + strings.Contains(cfg, `gzip_disable "msie6";`) + }) + }) + + ginkgo.It("should set gzip_min_length to 100", func() { + f.UpdateNginxConfigMapData("use-gzip", "true") + f.UpdateNginxConfigMapData("gzip-min-length", "100") + + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "gzip on;") && + strings.Contains(cfg, "gzip_min_length 100;") + }) + }) + + ginkgo.It("should set gzip_types to application/javascript", func() { + f.UpdateNginxConfigMapData("use-gzip", "true") + f.UpdateNginxConfigMapData("gzip-types", "application/javascript") + + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "gzip on;") && + strings.Contains(cfg, "gzip_types application/javascript;") + }) + }) +})