From 6381b5cce0b5b4a1be2dba9037afef5881ddb005 Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Tue, 21 Nov 2023 16:05:43 +0800 Subject: [PATCH 1/6] fix(core): respect custom proxy_access_log Kong now has a fixed access log format `kong_log_format` that prevents customization and error on `kong start`. Related to #11663. If the `proxy_access_log` is not a valid pathname, then replace `kong_log_format` with the custom value. --- .../kong/respect-custom-proxy_access_log.yml | 3 +++ kong/cmd/utils/prefix_handler.lua | 17 +++++++++++++- kong/templates/nginx_kong.lua | 4 ++++ spec/01-unit/04-prefix_handler_spec.lua | 23 +++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/kong/respect-custom-proxy_access_log.yml diff --git a/changelog/unreleased/kong/respect-custom-proxy_access_log.yml b/changelog/unreleased/kong/respect-custom-proxy_access_log.yml new file mode 100644 index 000000000000..92b77e6d0680 --- /dev/null +++ b/changelog/unreleased/kong/respect-custom-proxy_access_log.yml @@ -0,0 +1,3 @@ +message: "respect custom `proxy_access_log`" +type: bugfix +scope: Configuration diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua index ea661fbf4ca0..38b9943e34cc 100644 --- a/kong/cmd/utils/prefix_handler.lua +++ b/kong/cmd/utils/prefix_handler.lua @@ -239,7 +239,7 @@ local function compile_conf(kong_config, conf_template, template_env_inject) -- computed config properties for templating local compile_env = { _escape = ">", - proxy_access_log_enabled = kong_config.proxy_access_log ~= "off", + proxy_access_log_custom = false, pairs = pairs, ipairs = ipairs, tostring = tostring, @@ -248,6 +248,21 @@ local function compile_conf(kong_config, conf_template, template_env_inject) } } + local kong_proxy_access_log = kong_config.proxy_access_log + if kong_proxy_access_log ~= "off" then + compile_env.proxy_access_log_enabled = true + end + if kong_proxy_access_log then + local _, custom_format = string.match(kong_proxy_access_log, "^(%S+)%s(%S+)") + if custom_format then + compile_env.proxy_access_log_custom = true + + if not string.match(kong_config.nginx_http_log_format or "", "^" .. custom_format) then + log.debug(table.concat({"a custom access_log format is set: '", custom_format, "', but not defined"})) + end + end + end + compile_env = pl_tablex.merge(compile_env, template_env_inject or {}, true) do diff --git a/kong/templates/nginx_kong.lua b/kong/templates/nginx_kong.lua index c12ba4b3f82e..2f0b1266df6b 100644 --- a/kong/templates/nginx_kong.lua +++ b/kong/templates/nginx_kong.lua @@ -89,7 +89,11 @@ server { lua_kong_error_log_request_id $kong_request_id; > if proxy_access_log_enabled then +> if proxy_access_log_custom then + access_log ${{PROXY_ACCESS_LOG}}; +> else access_log ${{PROXY_ACCESS_LOG}} kong_log_format; +> end > else access_log off; > end diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index 7cc4d9c56769..e48ae16f246a 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -507,10 +507,33 @@ describe("NGINX conf compiler", function() assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) local conf = assert(conf_loader(nil, { + proxy_access_log = "/dev/stdout apigw_json", + stream_listen = "0.0.0.0:9100", + nginx_stream_tcp_nodelay = "on", + })) + local nginx_conf = prefix_handler.compile_kong_conf(conf) + assert.matches("access_log%s/dev/stdout%sapigw_json;", nginx_conf) + local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) + + local conf = assert(conf_loader(nil, { + proxy_access_log = "/tmp/not_exist.log", + stream_listen = "0.0.0.0:9100", + nginx_stream_tcp_nodelay = "on", + })) + local nginx_conf = prefix_handler.compile_kong_conf(conf) + assert.matches("access_log%s/tmp/not_exist.log%skong_log_format;", nginx_conf) + local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) + + local conf = assert(conf_loader(nil, { + prefix = "servroot_tmp", + nginx_stream_log_format = "custom '$protocol $status'", proxy_stream_access_log = "/dev/stdout custom", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) + assert(prefix_handler.prepare_prefix(conf)) local nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%slogs/access.log%skong_log_format;", nginx_conf) local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) From ddcbf03de4864b50ebdf2b7e1d63bd95ebb0fc76 Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Thu, 23 Nov 2023 11:11:01 +0800 Subject: [PATCH 2/6] fix(config): cover log_format name with hyphen --- kong/cmd/utils/prefix_handler.lua | 5 +++-- spec/01-unit/04-prefix_handler_spec.lua | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua index 38b9943e34cc..f61ab6c2636e 100644 --- a/kong/cmd/utils/prefix_handler.lua +++ b/kong/cmd/utils/prefix_handler.lua @@ -257,8 +257,9 @@ local function compile_conf(kong_config, conf_template, template_env_inject) if custom_format then compile_env.proxy_access_log_custom = true - if not string.match(kong_config.nginx_http_log_format or "", "^" .. custom_format) then - log.debug(table.concat({"a custom access_log format is set: '", custom_format, "', but not defined"})) + local format_pattern = string.gsub(custom_format, "%-", "%%-") + if not string.match(kong_config.nginx_http_log_format or "", "^" .. format_pattern) then + log.debug(table.concat({"a custom access_log format is configured: '", custom_format, "', but not defined"})) end end end diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index e48ae16f246a..87fe51174fa2 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -507,22 +507,22 @@ describe("NGINX conf compiler", function() assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) local conf = assert(conf_loader(nil, { - proxy_access_log = "/dev/stdout apigw_json", + proxy_access_log = "/dev/stdout apigw-json", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) local nginx_conf = prefix_handler.compile_kong_conf(conf) - assert.matches("access_log%s/dev/stdout%sapigw_json;", nginx_conf) + assert.matches("access_log%s/dev/stdout%sapigw%-json;", nginx_conf) local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) local conf = assert(conf_loader(nil, { - proxy_access_log = "/tmp/not_exist.log", + proxy_access_log = "/tmp/not-exist.log", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) local nginx_conf = prefix_handler.compile_kong_conf(conf) - assert.matches("access_log%s/tmp/not_exist.log%skong_log_format;", nginx_conf) + assert.matches("access_log%s/tmp/not%-exist.log%skong_log_format;", nginx_conf) local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) From 4e51f2a715087ee778cb192dd282be58cafddfe2 Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Thu, 23 Nov 2023 21:56:53 +0800 Subject: [PATCH 3/6] fix(config): early error when access log format is not defined --- kong/cmd/utils/prefix_handler.lua | 15 +++++---- spec/01-unit/04-prefix_handler_spec.lua | 45 ++++++++++++++++--------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua index f61ab6c2636e..d00240ecc949 100644 --- a/kong/cmd/utils/prefix_handler.lua +++ b/kong/cmd/utils/prefix_handler.lua @@ -253,13 +253,16 @@ local function compile_conf(kong_config, conf_template, template_env_inject) compile_env.proxy_access_log_enabled = true end if kong_proxy_access_log then - local _, custom_format = string.match(kong_proxy_access_log, "^(%S+)%s(%S+)") - if custom_format then - compile_env.proxy_access_log_custom = true + -- example: 'logs/some-file.log apigw_json' + local _, custom_format_name = string.match(kong_proxy_access_log, "^(%S+)%s(%S+)") + if custom_format_name then + -- example: 'apigw_json ...' + local nginx_http_log_format_name = string.match(kong_config.nginx_http_log_format or "", "^%S+") + if custom_format_name == nginx_http_log_format_name then + compile_env.proxy_access_log_custom = true - local format_pattern = string.gsub(custom_format, "%-", "%%-") - if not string.match(kong_config.nginx_http_log_format or "", "^" .. format_pattern) then - log.debug(table.concat({"a custom access_log format is configured: '", custom_format, "', but not defined"})) + else + return nil, table.concat({"configured access_log format: ", custom_format_name, ", but got: ", tostring(nginx_http_log_format_name)}) end end end diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index 87fe51174fa2..42fe433fd733 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -486,47 +486,62 @@ describe("NGINX conf compiler", function() describe("injected NGINX directives", function() it("injects proxy_access_log directive", function() - local conf = assert(conf_loader(nil, { + local conf, nginx_conf, err + conf = assert(conf_loader(nil, { proxy_access_log = "/dev/stdout", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) - local nginx_conf = prefix_handler.compile_kong_conf(conf) + nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%s/dev/stdout%skong_log_format;", nginx_conf) - local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) - local conf = assert(conf_loader(nil, { + conf = assert(conf_loader(nil, { proxy_access_log = "off", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) - local nginx_conf = prefix_handler.compile_kong_conf(conf) + nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%soff;", nginx_conf) - local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) - local conf = assert(conf_loader(nil, { + conf = assert(conf_loader(nil, { proxy_access_log = "/dev/stdout apigw-json", + nginx_http_log_format = 'apigw-json "$kong_request_id"', stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) - local nginx_conf = prefix_handler.compile_kong_conf(conf) + nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%s/dev/stdout%sapigw%-json;", nginx_conf) - local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) - local conf = assert(conf_loader(nil, { + conf = assert(conf_loader(nil, { + proxy_access_log = "/dev/stdout apigw-json", + nginx_http_log_format = 'not-exist "$kong_request_id"', + stream_listen = "0.0.0.0:9100", + nginx_stream_tcp_nodelay = "on", + })) + nginx_conf, err = prefix_handler.compile_kong_conf(conf) + assert.is_nil(nginx_conf) + assert.are_equal(err, "configured access_log format: apigw-json, but got: not-exist") + nginx_conf, err = prefix_handler.compile_kong_stream_conf(conf) + assert.is_nil(nginx_conf) + assert.are_equal(err, "configured access_log format: apigw-json, but got: not-exist") + + conf = assert(conf_loader(nil, { proxy_access_log = "/tmp/not-exist.log", stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) - local nginx_conf = prefix_handler.compile_kong_conf(conf) + nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%s/tmp/not%-exist.log%skong_log_format;", nginx_conf) - local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) - local conf = assert(conf_loader(nil, { + conf = assert(conf_loader(nil, { prefix = "servroot_tmp", nginx_stream_log_format = "custom '$protocol $status'", proxy_stream_access_log = "/dev/stdout custom", @@ -534,9 +549,9 @@ describe("NGINX conf compiler", function() nginx_stream_tcp_nodelay = "on", })) assert(prefix_handler.prepare_prefix(conf)) - local nginx_conf = prefix_handler.compile_kong_conf(conf) + nginx_conf = prefix_handler.compile_kong_conf(conf) assert.matches("access_log%slogs/access.log%skong_log_format;", nginx_conf) - local nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%s/dev/stdout%scustom;", nginx_conf) end) From a6280b4b80f76144daa34370d57755dd35e15a17 Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Fri, 24 Nov 2023 11:06:39 +0800 Subject: [PATCH 4/6] fix(config): discard warning or return nil --- kong/cmd/utils/prefix_handler.lua | 9 +-------- spec/01-unit/04-prefix_handler_spec.lua | 14 ++++++-------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua index d00240ecc949..9264fe4431fc 100644 --- a/kong/cmd/utils/prefix_handler.lua +++ b/kong/cmd/utils/prefix_handler.lua @@ -256,14 +256,7 @@ local function compile_conf(kong_config, conf_template, template_env_inject) -- example: 'logs/some-file.log apigw_json' local _, custom_format_name = string.match(kong_proxy_access_log, "^(%S+)%s(%S+)") if custom_format_name then - -- example: 'apigw_json ...' - local nginx_http_log_format_name = string.match(kong_config.nginx_http_log_format or "", "^%S+") - if custom_format_name == nginx_http_log_format_name then - compile_env.proxy_access_log_custom = true - - else - return nil, table.concat({"configured access_log format: ", custom_format_name, ", but got: ", tostring(nginx_http_log_format_name)}) - end + compile_env.proxy_access_log_custom = true end end diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index 42fe433fd733..11f098b52b56 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -485,8 +485,8 @@ describe("NGINX conf compiler", function() end) describe("injected NGINX directives", function() - it("injects proxy_access_log directive", function() - local conf, nginx_conf, err + it("#test injects proxy_access_log directive", function() + local conf, nginx_conf conf = assert(conf_loader(nil, { proxy_access_log = "/dev/stdout", stream_listen = "0.0.0.0:9100", @@ -524,12 +524,10 @@ describe("NGINX conf compiler", function() stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) - nginx_conf, err = prefix_handler.compile_kong_conf(conf) - assert.is_nil(nginx_conf) - assert.are_equal(err, "configured access_log format: apigw-json, but got: not-exist") - nginx_conf, err = prefix_handler.compile_kong_stream_conf(conf) - assert.is_nil(nginx_conf) - assert.are_equal(err, "configured access_log format: apigw-json, but got: not-exist") + nginx_conf = prefix_handler.compile_kong_conf(conf) + assert.matches("access_log%s/dev/stdout%sapigw%-json;", nginx_conf) + nginx_conf = prefix_handler.compile_kong_stream_conf(conf) + assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) conf = assert(conf_loader(nil, { proxy_access_log = "/tmp/not-exist.log", From 3948f974cf7bab134c4736dd161d4302a932491a Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Fri, 24 Nov 2023 15:19:13 +0800 Subject: [PATCH 5/6] chore(config): style and comments --- kong/cmd/utils/prefix_handler.lua | 5 ++--- kong/templates/nginx_kong.lua | 2 +- spec/01-unit/04-prefix_handler_spec.lua | 4 +++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua index 9264fe4431fc..189c3a03981c 100644 --- a/kong/cmd/utils/prefix_handler.lua +++ b/kong/cmd/utils/prefix_handler.lua @@ -239,7 +239,6 @@ local function compile_conf(kong_config, conf_template, template_env_inject) -- computed config properties for templating local compile_env = { _escape = ">", - proxy_access_log_custom = false, pairs = pairs, ipairs = ipairs, tostring = tostring, @@ -253,10 +252,10 @@ local function compile_conf(kong_config, conf_template, template_env_inject) compile_env.proxy_access_log_enabled = true end if kong_proxy_access_log then - -- example: 'logs/some-file.log apigw_json' + -- example: proxy_access_log = 'logs/some-file.log apigw_json' local _, custom_format_name = string.match(kong_proxy_access_log, "^(%S+)%s(%S+)") if custom_format_name then - compile_env.proxy_access_log_custom = true + compile_env.custom_proxy_access_log = true end end diff --git a/kong/templates/nginx_kong.lua b/kong/templates/nginx_kong.lua index 2f0b1266df6b..3375dcf14572 100644 --- a/kong/templates/nginx_kong.lua +++ b/kong/templates/nginx_kong.lua @@ -89,7 +89,7 @@ server { lua_kong_error_log_request_id $kong_request_id; > if proxy_access_log_enabled then -> if proxy_access_log_custom then +> if custom_proxy_access_log then access_log ${{PROXY_ACCESS_LOG}}; > else access_log ${{PROXY_ACCESS_LOG}} kong_log_format; diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index 11f098b52b56..00ef47ee4c18 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -485,7 +485,7 @@ describe("NGINX conf compiler", function() end) describe("injected NGINX directives", function() - it("#test injects proxy_access_log directive", function() + it("injects proxy_access_log directive", function() local conf, nginx_conf conf = assert(conf_loader(nil, { proxy_access_log = "/dev/stdout", @@ -529,6 +529,8 @@ describe("NGINX conf compiler", function() nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) + -- configure an undefined log format will error + -- on kong start. This is expected conf = assert(conf_loader(nil, { proxy_access_log = "/tmp/not-exist.log", stream_listen = "0.0.0.0:9100", From 43760d0f28deacd7c4cc6ed955205afcd0fb4501 Mon Sep 17 00:00:00 2001 From: Zachary Hu Date: Fri, 24 Nov 2023 15:22:39 +0800 Subject: [PATCH 6/6] chore(*): comments --- spec/01-unit/04-prefix_handler_spec.lua | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/01-unit/04-prefix_handler_spec.lua b/spec/01-unit/04-prefix_handler_spec.lua index 00ef47ee4c18..35c1d703e767 100644 --- a/spec/01-unit/04-prefix_handler_spec.lua +++ b/spec/01-unit/04-prefix_handler_spec.lua @@ -518,19 +518,19 @@ describe("NGINX conf compiler", function() nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) + -- configure an undefined log format will error + -- on kong start. This is expected conf = assert(conf_loader(nil, { - proxy_access_log = "/dev/stdout apigw-json", - nginx_http_log_format = 'not-exist "$kong_request_id"', + proxy_access_log = "/dev/stdout not-exist", + nginx_http_log_format = 'apigw-json "$kong_request_id"', stream_listen = "0.0.0.0:9100", nginx_stream_tcp_nodelay = "on", })) nginx_conf = prefix_handler.compile_kong_conf(conf) - assert.matches("access_log%s/dev/stdout%sapigw%-json;", nginx_conf) + assert.matches("access_log%s/dev/stdout%snot%-exist;", nginx_conf) nginx_conf = prefix_handler.compile_kong_stream_conf(conf) assert.matches("access_log%slogs/access.log%sbasic;", nginx_conf) - -- configure an undefined log format will error - -- on kong start. This is expected conf = assert(conf_loader(nil, { proxy_access_log = "/tmp/not-exist.log", stream_listen = "0.0.0.0:9100",