Merge dc730c5e40 into e0446d7554

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.

internal/ingress/controller/config/config.go

@@ -867,6 +867,7 @@ func NewDefault() Configuration {
 			DenylistSourceRange:       []string{},
 			WhitelistSourceRange:      []string{},
 			SkipAccessLogURLs:         []string{},
+			SkipAccessLogHTTPStatuses: []string{},
 			LimitRate:                 0,
 			LimitRateAfter:            0,
 			ProxyBuffering:            "off",

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

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

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"`
 

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;
     }
 

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`)
+	})
+})