diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 92ac39ee6..293e2598b 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -198,6 +198,7 @@ The following table shows a configuration option's name, type, and the default v |[denylist-source-range](#denylist-source-range)|[]string|[]string{}|| |[whitelist-source-range](#whitelist-source-range)|[]string|[]string{}|| |[skip-access-log-urls](#skip-access-log-urls)|[]string|[]string{}|| +|[skip-access-log-http-statuses](#skip-access-log-http-statuses)|[]string|[]string{}|| |[limit-rate](#limit-rate)|int|0|| |[limit-rate-after](#limit-rate-after)|int|0|| |[lua-shared-dicts](#lua-shared-dicts)|string|""|| @@ -1211,6 +1212,10 @@ See [ngx_http_access_module](https://nginx.org/en/docs/http/ngx_http_access_modu Sets a list of URLs that should not appear in the NGINX access log. This is useful with urls like `/health` or `health-check` that make "complex" reading the logs. _**default:**_ is empty +## skip-access-log-http-statuses + +Sets a comma separated list of HTTP statuses, each interpreted as a regular expression, that should not appear in the NGINX access log. This is useful for high volume ingress where turning access logging completely off is undesirable, but not logging things like 2xx and 3xx responses is desirable. _**default:**_ is empty + ## limit-rate Limits the rate of response transmission to a client. The rate is specified in bytes per second. The zero value disables rate limiting. The limit is set per a request, and so if a client simultaneously opens two connections, the overall rate will be twice as much as the specified limit. diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 67878f185..6011d475c 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -847,32 +847,33 @@ func NewDefault() Configuration { ProxyStreamNextUpstreamTimeout: "600s", ProxyStreamNextUpstreamTries: 3, Backend: defaults.Backend{ - ProxyBodySize: bodySize, - ProxyConnectTimeout: 5, - ProxyReadTimeout: 60, - ProxySendTimeout: 60, - ProxyBuffersNumber: 4, - ProxyBufferSize: "4k", - ProxyCookieDomain: "off", - ProxyCookiePath: "off", - ProxyNextUpstream: "error timeout", - ProxyNextUpstreamTimeout: 0, - ProxyNextUpstreamTries: 3, - ProxyRequestBuffering: "on", - ProxyRedirectFrom: "off", - ProxyRedirectTo: "off", - PreserveTrailingSlash: false, - SSLRedirect: true, - CustomHTTPErrors: []int{}, - DenylistSourceRange: []string{}, - WhitelistSourceRange: []string{}, - SkipAccessLogURLs: []string{}, - LimitRate: 0, - LimitRateAfter: 0, - ProxyBuffering: "off", - ProxyHTTPVersion: "1.1", - ProxyMaxTempFileSize: "1024m", - ServiceUpstream: false, + ProxyBodySize: bodySize, + ProxyConnectTimeout: 5, + ProxyReadTimeout: 60, + ProxySendTimeout: 60, + ProxyBuffersNumber: 4, + ProxyBufferSize: "4k", + ProxyCookieDomain: "off", + ProxyCookiePath: "off", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: 0, + ProxyNextUpstreamTries: 3, + ProxyRequestBuffering: "on", + ProxyRedirectFrom: "off", + ProxyRedirectTo: "off", + PreserveTrailingSlash: false, + SSLRedirect: true, + CustomHTTPErrors: []int{}, + DenylistSourceRange: []string{}, + WhitelistSourceRange: []string{}, + SkipAccessLogURLs: []string{}, + SkipAccessLogHTTPStatuses: []string{}, + LimitRate: 0, + LimitRateAfter: 0, + ProxyBuffering: "off", + ProxyHTTPVersion: "1.1", + ProxyMaxTempFileSize: "1024m", + ServiceUpstream: false, }, UpstreamKeepaliveConnections: 320, UpstreamKeepaliveTime: "1h", diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 9dc019bcc..918b88d62 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -40,6 +40,7 @@ import ( const ( customHTTPErrors = "custom-http-errors" skipAccessLogUrls = "skip-access-log-urls" + skipAccessLogHTTPStatuses = "skip-access-log-http-statuses" whitelistSourceRange = "whitelist-source-range" denylistSourceRange = "denylist-source-range" proxyRealIPCIDR = "proxy-real-ip-cidr" @@ -103,6 +104,7 @@ func ReadConfig(src map[string]string) config.Configuration { to := config.NewDefault() errors := make([]int, 0) skipUrls := make([]string, 0) + skipHTTPStatuses := make([]string, 0) denyList := make([]string, 0) whiteList := make([]string, 0) proxyList := make([]string, 0) @@ -173,6 +175,11 @@ func ReadConfig(src map[string]string) config.Configuration { skipUrls = splitAndTrimSpace(val, ",") } + if val, ok := conf[skipAccessLogHTTPStatuses]; ok { + delete(conf, skipAccessLogHTTPStatuses) + skipHTTPStatuses = strings.Split(val, ",") + } + if val, ok := conf[denylistSourceRange]; ok { delete(conf, denylistSourceRange) denyList = append(denyList, splitAndTrimSpace(val, ",")...) @@ -410,6 +417,7 @@ func ReadConfig(src map[string]string) config.Configuration { to.CustomHTTPErrors = filterErrors(errors) to.SkipAccessLogURLs = skipUrls + to.SkipAccessLogHTTPStatuses = skipHTTPStatuses to.DenylistSourceRange = denyList to.WhitelistSourceRange = whiteList to.ProxyRealIPCIDR = proxyList diff --git a/internal/ingress/controller/template/configmap_test.go b/internal/ingress/controller/template/configmap_test.go index dad841694..3184a956a 100644 --- a/internal/ingress/controller/template/configmap_test.go +++ b/internal/ingress/controller/template/configmap_test.go @@ -58,6 +58,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { "proxy-read-timeout": "1", "proxy-send-timeout": "2", "skip-access-log-urls": "/log,/demo,/test", + "skip-access-log-http-statuses": "^[23],204,302,^201", "use-proxy-protocol": "true", "disable-access-log": "true", "access-log-params": "buffer=4k gzip", @@ -85,6 +86,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { def.AccessLogPath = "/var/log/test/access.log" def.ErrorLogPath = "/var/log/test/error.log" def.SkipAccessLogURLs = []string{"/log", "/demo", "/test"} + def.SkipAccessLogHTTPStatuses = []string{"^[23]", "204", "302", "^201"} def.ProxyReadTimeout = 1 def.ProxySendTimeout = 2 def.UseProxyProtocol = true diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 0526d443e..02bffb4f5 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -107,6 +107,12 @@ type Backend struct { // By default this list is empty SkipAccessLogURLs []string `json:"skip-access-log-urls"` + // SkipAccessLogHTTPStatuses sets a list of HTTP statuses that should not appear in the NGINX access log + // The status strings provided are interpreted by an NGINX map as regex + // This is useful with statuses like 2xx and 3xx that make "complex" reading the logs + // By default this list is empty + SkipAccessLogHTTPStatuses []string `json:"skip-access-log-http-statuses"` + // Enables or disables the redirect (301) to the HTTPS port SSLRedirect bool `json:"ssl-redirect"` diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4454a4710..dd43664b5 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -382,9 +382,20 @@ http { {{/* map urls that should not appear in access.log */}} {{/* http://nginx.org/en/docs/http/ngx_http_log_module.html#access_log */}} - map $request_uri $loggable { + map $request_uri $loggable_request_url { {{ range $reqUri := $cfg.SkipAccessLogURLs }} {{ $reqUri }} 0;{{ end }} + } + + {{/* map HTTP statuses that should not appear in access.log */}} + {{/* http://nginx.org/en/docs/http/ngx_http_log_module.html#access_log */}} + map $status $loggable_http_status { + {{ range $reqHTTPStatus := $cfg.SkipAccessLogHTTPStatuses }} + ~{{ $reqHTTPStatus }} 0;{{ end }} + } + + map "${loggable_request_url}${loggable_http_status}" $loggable { + ~0 0; default 1; } diff --git a/test/e2e/annotations/skipaccessloghttpstatuses.go b/test/e2e/annotations/skipaccessloghttpstatuses.go new file mode 100644 index 000000000..76eb7eb72 --- /dev/null +++ b/test/e2e/annotations/skipaccessloghttpstatuses.go @@ -0,0 +1,101 @@ +/* +Copyright 2020 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 annotations + +import ( + "net/http" + "strings" + + "github.com/onsi/ginkgo/v2" + + "github.com/stretchr/testify/assert" + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.DescribeAnnotation("skip-access-log-http-statuses", func() { + f := framework.NewDefaultFramework("skipaccessloghttpstatuses") + + ginkgo.BeforeEach(func() { + f.NewEchoDeployment() + }) + + ginkgo.It("skip-access-log-http-statuses 200 literal, 200 OK", func() { + host := "skipaccessloghttpstatuses.go.foo.com" + + f.UpdateNginxConfigMapData("skip-access-log-http-statuses", "200") + ing := framework.NewSingleIngress(host, "/prefixOne", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(ngx string) bool { + return strings.Contains(ngx, `~200 0;`) + }) + + f.HTTPTestClient(). + GET("/prefixOne"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.NotContains(ginkgo.GinkgoT(), logs, `GET /prefixOne HTTP/1.1" 200`) + }) + + ginkgo.It("skip-access-log-http-statuses ^2.. regex, 200 OK", func() { + host := "skipaccessloghttpstatuses.go.foo.com" + + f.UpdateNginxConfigMapData("skip-access-log-http-statuses", "^2..") + ing := framework.NewSingleIngress(host, "/prefixOne", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(ngx string) bool { + return strings.Contains(ngx, `~^2.. 0;`) + }) + + f.HTTPTestClient(). + GET("/prefixOne"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.NotContains(ginkgo.GinkgoT(), logs, `GET /prefixOne HTTP/1.1" 200`) + }) + + ginkgo.It("skip-access-log-http-statuses ^2.. regex, 404 Not Found", func() { + host := "skipaccessloghttpstatuses.go.foo.com" + + f.UpdateNginxConfigMapData("skip-access-log-http-statuses", "^2..") + ing := framework.NewSingleIngress(host, "/prefixOne", host, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(ngx string) bool { + return strings.Contains(ngx, `~^2.. 0;`) + }) + + f.HTTPTestClient(). + GET("/404"). + WithHeader("Host", host). + Expect(). + Status(http.StatusNotFound) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.Contains(ginkgo.GinkgoT(), logs, `GET /404 HTTP/1.1" 404`) + }) +})