From 7161da2e3669d51a8fbfd52aaf433842ad383c58 Mon Sep 17 00:00:00 2001 From: Nate Campbell Date: Fri, 30 Sep 2022 11:00:29 -0400 Subject: [PATCH] Support none keyword in log-format escape (#8692) * Support none keyword in log-format escape ## What this PR does / why we need it: ingress-nginx does not support disabling escaping of special characters in the nginx log. This PR exposes the setting to support that functionality. ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes ## How Has This Been Tested? Followed the [getting-started](https://github.com/kubernetes/ingress-nginx/blob/2d2df959db131a89b86f946bea9ee28f42cdb331/docs/developer-guide/getting-started.md) guide. Used ppa:longsleep/golang-backports on WSL Ubuntu to establish a golang-1.18 environment with latest docker and recommended kind. Built the dev-env successfully; had issues with make test, but they are entirely unrelated to anything I touched. Ultimate test was ``` FOCUS=log-format make kind-e2e-test ... Ginkgo ran 1 suite in 6m29.7437865s Test Suite Passed ``` ## Checklist: - [x] My change requires a change to the documentation. - [x] I have updated the documentation accordingly. - [x] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [x] I have added tests to cover my changes. - [x] All new and existing tests passed. I did not update docs/e2e-tests.md. * gofmt -s ./internal/ingress/controller/config/config.go --- .../nginx-configuration/configmap.md | 5 ++ internal/ingress/controller/config/config.go | 4 ++ rootfs/etc/nginx/template/nginx.tmpl | 2 +- test/e2e/settings/log-format.go | 55 ++++++++++++++++--- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index cd25095b0..c985d7373 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -64,6 +64,7 @@ The following table shows a configuration option's name, type, and the default v |[keep-alive](#keep-alive)|int|75| |[keep-alive-requests](#keep-alive-requests)|int|100| |[large-client-header-buffers](#large-client-header-buffers)|string|"4 8k"| +|[log-format-escape-none](#log-format-escape-none)|bool|"false"| |[log-format-escape-json](#log-format-escape-json)|bool|"false"| |[log-format-upstream](#log-format-upstream)|string|`$remote_addr - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] [$proxy_alternative_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status $req_id`| |[log-format-stream](#log-format-stream)|string|`[$remote_addr] [$time_local] $protocol $status $bytes_sent $bytes_received $session_time`| @@ -468,6 +469,10 @@ Sets the maximum number and size of buffers used for reading large client reques _References:_ [https://nginx.org/en/docs/http/ngx_http_core_module.html#large_client_header_buffers](https://nginx.org/en/docs/http/ngx_http_core_module.html#large_client_header_buffers) +## log-format-escape-none + +Sets if the escape parameter is disabled entirely for character escaping in variables ("true") or controlled by log-format-escape-json ("false") Sets the nginx [log format](https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format). + ## log-format-escape-json Sets if the escape parameter allows JSON ("true") or default characters escaping in variables ("false") Sets the nginx [log format](https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format). diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 4ee2df965..8bf71b774 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -263,6 +263,10 @@ type Configuration struct { // Default: 4 8k LargeClientHeaderBuffers string `json:"large-client-header-buffers"` + // Disable all escaping + // http://nginx.org/en/docs/http/ngx_http_log_module.html#log_format + LogFormatEscapeNone bool `json:"log-format-escape-none,omitempty"` + // Enable json escaping // http://nginx.org/en/docs/http/ngx_http_log_module.html#log_format LogFormatEscapeJSON bool `json:"log-format-escape-json,omitempty"` diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index a04f71f0e..c315e4ee9 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -361,7 +361,7 @@ http { # $ingress_name # $service_name # $service_port - log_format upstreaminfo {{ if $cfg.LogFormatEscapeJSON }}escape=json {{ end }}'{{ $cfg.LogFormatUpstream }}'; + log_format upstreaminfo {{ if $cfg.LogFormatEscapeNone }}escape=none {{ else if $cfg.LogFormatEscapeJSON }}escape=json {{ end }}'{{ $cfg.LogFormatUpstream }}'; {{/* map urls that should not appear in access.log */}} {{/* http://nginx.org/en/docs/http/ngx_http_log_module.html#access_log */}} diff --git a/test/e2e/settings/log-format.go b/test/e2e/settings/log-format.go index b43fce874..24877818d 100644 --- a/test/e2e/settings/log-format.go +++ b/test/e2e/settings/log-format.go @@ -35,12 +35,12 @@ var _ = framework.DescribeSetting("log-format-*", func() { f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil)) }) - ginkgo.Context("Check log-format-escape-json", func() { + ginkgo.Context("Check log-format-escape-json and log-format-escape-none", func() { - ginkgo.It("should disable the log-format-escape-json by default", func() { + ginkgo.It("should not configure log-format escape by default", func() { f.WaitForNginxConfiguration( func(cfg string) bool { - return !strings.Contains(cfg, "log_format upstreaminfo escape=json") + return !strings.Contains(cfg, "log_format upstreaminfo escape") }) }) @@ -59,9 +59,25 @@ var _ = framework.DescribeSetting("log-format-*", func() { return !strings.Contains(cfg, "log_format upstreaminfo escape=json") }) }) + + ginkgo.It("should enable the log-format-escape-none", func() { + f.UpdateNginxConfigMapData("log-format-escape-none", "true") + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "log_format upstreaminfo escape=none") + }) + }) + + ginkgo.It("should disable the log-format-escape-none", func() { + f.UpdateNginxConfigMapData("log-format-escape-none", "false") + f.WaitForNginxConfiguration( + func(cfg string) bool { + return !strings.Contains(cfg, "log_format upstreaminfo escape=none") + }) + }) }) - ginkgo.Context("Check log-format-upstream with log-format-escape-json", func() { + ginkgo.Context("Check log-format-upstream with log-format-escape-json and log-format-escape-none", func() { ginkgo.It("log-format-escape-json enabled", func() { f.SetNginxConfigMapData(map[string]string{ @@ -86,7 +102,7 @@ var _ = framework.DescribeSetting("log-format-*", func() { assert.Contains(ginkgo.GinkgoT(), logs, `{"my_header1":"Here is \"header1\" with json escape", "my_header2":""}`) }) - ginkgo.It("log-format-escape-json disabled", func() { + ginkgo.It("log-format default escape", func() { f.SetNginxConfigMapData(map[string]string{ "log-format-escape-json": "false", "log-format-upstream": "\"{\"my_header3\":\"$http_header3\", \"my_header4\":\"$http_header4\"}\"", @@ -94,19 +110,42 @@ var _ = framework.DescribeSetting("log-format-*", func() { f.WaitForNginxConfiguration( func(cfg string) bool { - return !strings.Contains(cfg, "log_format upstreaminfo escape=json") + return !strings.Contains(cfg, "log_format upstreaminfo escape") }) f.HTTPTestClient(). GET("/"). WithHeader("Host", host). - WithHeader("header3", `Here is "header3" with json escape`). + WithHeader("header3", `Here is "header3" with default escape`). Expect(). Status(http.StatusOK) logs, err := f.NginxLogs() assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") - assert.Contains(ginkgo.GinkgoT(), logs, `{"my_header3":"Here is \x22header3\x22 with json escape", "my_header4":"-"}`) + assert.Contains(ginkgo.GinkgoT(), logs, `{"my_header3":"Here is \x22header3\x22 with default escape", "my_header4":"-"}`) + }) + + ginkgo.It("log-format-escape-none enabled", func() { + f.SetNginxConfigMapData(map[string]string{ + "log-format-escape-none": "true", + "log-format-upstream": "\"{\"my_header5\":\"$http_header5\", \"my_header6\":\"$http_header6\"}\"", + }) + + f.WaitForNginxConfiguration( + func(cfg string) bool { + return strings.Contains(cfg, "log_format upstreaminfo escape=none") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("header5", `Here is "header5" with none escape`). + Expect(). + Status(http.StatusOK) + + logs, err := f.NginxLogs() + assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs") + assert.Contains(ginkgo.GinkgoT(), logs, `{"my_header5":"Here is "header5" with none escape", "my_header6":""}`) }) }) })