From e6f8a76648f761b461e9fe392aeeb7f613f70bb2 Mon Sep 17 00:00:00 2001 From: Andrew Kew Date: Tue, 27 Aug 2024 10:14:00 +0100 Subject: [PATCH 01/16] feat(plugin)(correlationId) increase priority order for plugin to match EE so it can be used in other/custom plugins --- kong/plugins/correlation-id/handler.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/plugins/correlation-id/handler.lua b/kong/plugins/correlation-id/handler.lua index 5f1f82b9fdef..c88364573c41 100644 --- a/kong/plugins/correlation-id/handler.lua +++ b/kong/plugins/correlation-id/handler.lua @@ -42,7 +42,7 @@ end local CorrelationIdHandler = {} -CorrelationIdHandler.PRIORITY = 1 +CorrelationIdHandler.PRIORITY = 100001 CorrelationIdHandler.VERSION = kong_meta.version From 4acf23fe2b5a4979fc9f390e4d55352915049a4a Mon Sep 17 00:00:00 2001 From: Andrew Kew Date: Tue, 27 Aug 2024 10:19:09 +0100 Subject: [PATCH 02/16] feat(plugin)(correlationId) adding changelog for update --- changelog/unreleased/kong/feat-correlation-id-order.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/kong/feat-correlation-id-order.yml diff --git a/changelog/unreleased/kong/feat-correlation-id-order.yml b/changelog/unreleased/kong/feat-correlation-id-order.yml new file mode 100644 index 000000000000..9e283ddc5731 --- /dev/null +++ b/changelog/unreleased/kong/feat-correlation-id-order.yml @@ -0,0 +1,5 @@ +message: | + Increased the priority order of the correlation id to 100001 from 1 so that the plugin can be used + with other plugins especially custom auth plugins. +type: feature +scope: Core From 1a99f3fc695da5d5ea7a5dcb9a8eaae6ea64be58 Mon Sep 17 00:00:00 2001 From: Andrew Kew Date: Tue, 27 Aug 2024 11:25:33 +0100 Subject: [PATCH 03/16] Adjusting tests due to plugin order --- spec/01-unit/12-plugins_order_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/12-plugins_order_spec.lua b/spec/01-unit/12-plugins_order_spec.lua index a051aeb98040..986b75122690 100644 --- a/spec/01-unit/12-plugins_order_spec.lua +++ b/spec/01-unit/12-plugins_order_spec.lua @@ -53,6 +53,7 @@ describe("Plugins", function() local order = { "pre-function", + "correlation-id", "zipkin", "bot-detection", "cors", @@ -94,7 +95,6 @@ describe("Plugins", function() "syslog", "grpc-web", "request-termination", - "correlation-id", "post-function", } From 9eb21a2e64abe3d84a7f64fa3b00fb30c4b79fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Nowak?= Date: Wed, 28 Aug 2024 15:22:55 +0200 Subject: [PATCH 04/16] fix(core): make deprecated shorthand fields take precedence when new value is null KAG-5287 --- .../kong/fix-for-null-aware-shorthand.yml | 5 ++++ kong/db/schema/init.lua | 12 +++++++--- kong/tools/table.lua | 23 +++++++++++++++++++ .../01-db/01-schema/01-schema_spec.lua | 10 ++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/kong/fix-for-null-aware-shorthand.yml diff --git a/changelog/unreleased/kong/fix-for-null-aware-shorthand.yml b/changelog/unreleased/kong/fix-for-null-aware-shorthand.yml new file mode 100644 index 000000000000..e8e5624bd621 --- /dev/null +++ b/changelog/unreleased/kong/fix-for-null-aware-shorthand.yml @@ -0,0 +1,5 @@ +message: | + Changed the way deprecated shorthand fields are used with new fields. + If the new field contains null it allows for deprecated field to overwrite it if both are present in the request. +type: bugfix +scope: Core diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index 32578af5efae..93fc6af258cf 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -14,6 +14,7 @@ local deepcompare = require "pl.tablex".deepcompare local cycle_aware_deep_copy = require "kong.tools.table".cycle_aware_deep_copy local table_merge = require "kong.tools.table".table_merge +local null_aware_table_merge = require "kong.tools.table".null_aware_table_merge local table_path = require "kong.tools.table".table_path local is_array = require "kong.tools.table".is_array @@ -1752,9 +1753,14 @@ function Schema:process_auto_fields(data, context, nulls, opts) local deprecation = sdata.deprecation for k, v in pairs(new_values) do if type(v) == "table" then - data[k] = deprecation and table_merge(v, data[k]) - or table_merge(data[k] or {}, v) - elseif not deprecation or data[k] == nil then + local source = {} + if data[k] and data[k] ~= null then + source = data[k] + end + data[k] = deprecation and null_aware_table_merge(v, source) + or table_merge(source, v) + + elseif not deprecation or (data[k] == nil or data[k] == null) then data[k] = v end end diff --git a/kong/tools/table.lua b/kong/tools/table.lua index 66fcb4feeb40..a86c831b18eb 100644 --- a/kong/tools/table.lua +++ b/kong/tools/table.lua @@ -45,6 +45,29 @@ function _M.table_merge(t1, t2) end +--- Merges two table together but does not replace values from `t1` if `t2` for a given key has `ngx.null` value +-- A new table is created with a non-recursive copy of the provided tables +-- @param t1 The first table +-- @param t2 The second table +-- @return The (new) merged table +function _M.null_aware_table_merge(t1, t2) + local res = {} + if t1 then + for k,v in pairs(t1) do + res[k] = v + end + end + if t2 then + for k,v in pairs(t2) do + if res[k] == nil or v ~= ngx.null then + res[k] = v + end + end + end + return res +end + + --- Checks if a value exists in a table. -- @param arr The table to use -- @param val The value to check diff --git a/spec/01-unit/01-db/01-schema/01-schema_spec.lua b/spec/01-unit/01-db/01-schema/01-schema_spec.lua index 183ed0bdeeda..a6c03d3d8455 100644 --- a/spec/01-unit/01-db/01-schema/01-schema_spec.lua +++ b/spec/01-unit/01-db/01-schema/01-schema_spec.lua @@ -4151,6 +4151,11 @@ describe("schema", function() local input = { username = "test1", name = "ignored", record = { x = "ignored" }, y = "test1" } local output, _ = TestSchema:process_auto_fields(input) assert.same({ name = "test1", record = { x = "test1" } }, output) + + -- deprecated fields does take precedence if the new fields are null + local input = { username = "overwritten-1", name = ngx.null, record = { x = ngx.null }, y = "overwritten-2" } + local output, _ = TestSchema:process_auto_fields(input) + assert.same({ name = "overwritten-1", record = { x = "overwritten-2" } }, output) end) it("does not take precedence if deprecated", function() @@ -4202,6 +4207,11 @@ describe("schema", function() local input = { username = "ignored", name = "test1", record = { x = "test1" }, y = "ignored" } local output, _ = TestSchema:process_auto_fields(input) assert.same({ name = "test1", record = { x = "test1" } }, output) + + -- deprecated fields does take precedence if the new fields are null + local input = { username = "overwritten-1", name = ngx.null, record = { x = ngx.null }, y = "overwritten-2" } + local output, _ = TestSchema:process_auto_fields(input) + assert.same({ name = "overwritten-1", record = { x = "overwritten-2" } }, output) end) it("can produce multiple fields", function() From 0663e3302b7d3f02046ef1bbf7fc696602315a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Nowak?= Date: Thu, 29 Aug 2024 16:56:59 +0200 Subject: [PATCH 05/16] fix(*): reject config if both deprecated and new field defined and their values mismatch (#13565) * fix(*): reject config if both deprecated and new field defined and their values mismatch This commit adds a validation to deprecated fields that checks if in case both new field and old field were defined - their values must match. Note that if one of the fields is null then the validation passes even if the other is not null. KAG-5262 * fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch PR fixes * fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch PR fixes 2 --- ...t-config-on-deprecated-fields-mismatch.yml | 5 + kong/db/schema/init.lua | 77 +++-- kong/db/schema/metaschema.lua | 9 + kong/plugins/acme/schema.lua | 4 + kong/plugins/rate-limiting/schema.lua | 9 + kong/plugins/response-ratelimiting/schema.lua | 9 + .../01-db/01-schema/01-schema_spec.lua | 320 ++++++++++++++++-- 7 files changed, 386 insertions(+), 47 deletions(-) create mode 100644 changelog/unreleased/kong/reject-config-on-deprecated-fields-mismatch.yml diff --git a/changelog/unreleased/kong/reject-config-on-deprecated-fields-mismatch.yml b/changelog/unreleased/kong/reject-config-on-deprecated-fields-mismatch.yml new file mode 100644 index 000000000000..c402a30f95e7 --- /dev/null +++ b/changelog/unreleased/kong/reject-config-on-deprecated-fields-mismatch.yml @@ -0,0 +1,5 @@ +message: | + Changed the behaviour of shorthand fields that are used to describe deprecated fields. If + both fields are sent in the request and their values mismatch - the request will be rejected. +type: bugfix +scope: Core diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index 93fc6af258cf..1d213a3a4ffe 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -17,6 +17,7 @@ local table_merge = require "kong.tools.table".table_merge local null_aware_table_merge = require "kong.tools.table".null_aware_table_merge local table_path = require "kong.tools.table".table_path local is_array = require "kong.tools.table".is_array +local join_string = require "kong.tools.string".join local setmetatable = setmetatable @@ -1704,6 +1705,35 @@ local function collect_field_reference(refs, key, reference) end +local function validate_deprecation_exclusiveness(data, shorthand_value, shorthand_name, shorthand_definition) + if shorthand_value == nil or + shorthand_value == ngx.null or + shorthand_definition.deprecation == nil or + shorthand_definition.deprecation.replaced_with == nil then + return true + end + + for _, replaced_with_element in ipairs(shorthand_definition.deprecation.replaced_with) do + local new_field_value = replaced_with_element.reverse_mapping_function and replaced_with_element.reverse_mapping_function(data) + or table_path(data, replaced_with_element.path) + + if new_field_value and + new_field_value ~= ngx.null and + not deepcompare(new_field_value, shorthand_value) then + local new_field_name = join_string(".", replaced_with_element.path) + + return nil, string.format( + "both deprecated and new field are used but their values mismatch: %s = %s vs %s = %s", + shorthand_name, tostring(shorthand_value), + new_field_name, tostring(new_field_value) + ) + end + end + + return true +end + + --- Given a table, update its fields whose schema -- definition declares them as `auto = true`, -- based on its CRUD operation context, and set @@ -1741,27 +1771,34 @@ function Schema:process_auto_fields(data, context, nulls, opts) errs[sname] = err has_errs = true else - data[sname] = nil - local new_values = sdata.func(value) - if new_values then - -- a shorthand field may have a deprecation property, that is used - -- to determine whether the shorthand's return value takes precedence - -- over the similarly named actual schema fields' value when both - -- are present. On deprecated shorthand fields the actual schema - -- field value takes the precedence, otherwise the shorthand's - -- return value takes the precedence. - local deprecation = sdata.deprecation - for k, v in pairs(new_values) do - if type(v) == "table" then - local source = {} - if data[k] and data[k] ~= null then - source = data[k] - end - data[k] = deprecation and null_aware_table_merge(v, source) - or table_merge(source, v) + local _, deprecation_error = validate_deprecation_exclusiveness(data, value, sname, sdata) - elseif not deprecation or (data[k] == nil or data[k] == null) then - data[k] = v + if deprecation_error then + errs[sname] = deprecation_error + has_errs = true + else + data[sname] = nil + local new_values = sdata.func(value) + if new_values then + -- a shorthand field may have a deprecation property, that is used + -- to determine whether the shorthand's return value takes precedence + -- over the similarly named actual schema fields' value when both + -- are present. On deprecated shorthand fields the actual schema + -- field value takes the precedence, otherwise the shorthand's + -- return value takes the precedence. + local deprecation = sdata.deprecation + for k, v in pairs(new_values) do + if type(v) == "table" then + local source = {} + if data[k] and data[k] ~= null then + source = data[k] + end + data[k] = deprecation and null_aware_table_merge(v, source) + or table_merge(source, v) + + elseif not deprecation or (data[k] == nil or data[k] == null) then + data[k] = v + end end end end diff --git a/kong/db/schema/metaschema.lua b/kong/db/schema/metaschema.lua index 504893d567e7..554e59eaddcd 100644 --- a/kong/db/schema/metaschema.lua +++ b/kong/db/schema/metaschema.lua @@ -206,6 +206,15 @@ local field_schema = { { message = { type = "string", required = true } }, { removal_in_version = { type = "string", required = true } }, { old_default = { type = "any", required = false } }, + { replaced_with = { type = "array", required = false, + elements = { type = "record", + required = false, + fields = { + { path = { type = "array", len_min = 1, required = true, elements = { type = "string"}} }, + { reverse_mapping_function = { type = "function", required = false }} + }, + } + } }, }, } }, } diff --git a/kong/plugins/acme/schema.lua b/kong/plugins/acme/schema.lua index 5ccc3ffdf4a2..5b349ac11dbd 100644 --- a/kong/plugins/acme/schema.lua +++ b/kong/plugins/acme/schema.lua @@ -43,6 +43,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = { len_min = 0, translate_backwards = {'password'}, deprecation = { + replaced_with = { { path = { 'password' } } }, message = "acme: config.storage_config.redis.auth is deprecated, please use config.storage_config.redis.password instead", removal_in_version = "4.0", }, func = function(value) @@ -53,6 +54,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = { type = "string", translate_backwards = {'server_name'}, deprecation = { + replaced_with = { { path = { 'server_name' } } }, message = "acme: config.storage_config.redis.ssl_server_name is deprecated, please use config.storage_config.redis.server_name instead", removal_in_version = "4.0", }, func = function(value) @@ -64,6 +66,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = { len_min = 0, translate_backwards = {'extra_options', 'namespace'}, deprecation = { + replaced_with = { { path = { 'extra_options', 'namespace' } } }, message = "acme: config.storage_config.redis.namespace is deprecated, please use config.storage_config.redis.extra_options.namespace instead", removal_in_version = "4.0", }, func = function(value) @@ -74,6 +77,7 @@ local LEGACY_SCHEMA_TRANSLATIONS = { type = "integer", translate_backwards = {'extra_options', 'scan_count'}, deprecation = { + replaced_with = { { path = { 'extra_options', 'scan_count' } } }, message = "acme: config.storage_config.redis.scan_count is deprecated, please use config.storage_config.redis.extra_options.scan_count instead", removal_in_version = "4.0", }, func = function(value) diff --git a/kong/plugins/rate-limiting/schema.lua b/kong/plugins/rate-limiting/schema.lua index 8928fb87fcd5..32a346c58ed7 100644 --- a/kong/plugins/rate-limiting/schema.lua +++ b/kong/plugins/rate-limiting/schema.lua @@ -104,6 +104,7 @@ return { type = "string", translate_backwards = {'redis', 'host'}, deprecation = { + replaced_with = { { path = { 'redis', 'host' } } }, message = "rate-limiting: config.redis_host is deprecated, please use config.redis.host instead", removal_in_version = "4.0", }, func = function(value) @@ -114,6 +115,7 @@ return { type = "integer", translate_backwards = {'redis', 'port'}, deprecation = { + replaced_with = { { path = { 'redis', 'port' } } }, message = "rate-limiting: config.redis_port is deprecated, please use config.redis.port instead", removal_in_version = "4.0", }, func = function(value) @@ -125,6 +127,7 @@ return { len_min = 0, translate_backwards = {'redis', 'password'}, deprecation = { + replaced_with = { { path = { 'redis', 'password' } } }, message = "rate-limiting: config.redis_password is deprecated, please use config.redis.password instead", removal_in_version = "4.0", }, func = function(value) @@ -135,6 +138,7 @@ return { type = "string", translate_backwards = {'redis', 'username'}, deprecation = { + replaced_with = { { path = { 'redis', 'username' } } }, message = "rate-limiting: config.redis_username is deprecated, please use config.redis.username instead", removal_in_version = "4.0", }, func = function(value) @@ -145,6 +149,7 @@ return { type = "boolean", translate_backwards = {'redis', 'ssl'}, deprecation = { + replaced_with = { { path = { 'redis', 'ssl' } } }, message = "rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead", removal_in_version = "4.0", }, func = function(value) @@ -155,6 +160,7 @@ return { type = "boolean", translate_backwards = {'redis', 'ssl_verify'}, deprecation = { + replaced_with = { { path = { 'redis', 'ssl_verify' } } }, message = "rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead", removal_in_version = "4.0", }, func = function(value) @@ -165,6 +171,7 @@ return { type = "string", translate_backwards = {'redis', 'server_name'}, deprecation = { + replaced_with = { { path = { 'redis', 'server_name' } } }, message = "rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead", removal_in_version = "4.0", }, func = function(value) @@ -175,6 +182,7 @@ return { type = "integer", translate_backwards = {'redis', 'timeout'}, deprecation = { + replaced_with = { { path = { 'redis', 'timeout' } } }, message = "rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead", removal_in_version = "4.0", }, func = function(value) @@ -185,6 +193,7 @@ return { type = "integer", translate_backwards = {'redis', 'database'}, deprecation = { + replaced_with = { { path = { 'redis', 'database' } } }, message = "rate-limiting: config.redis_database is deprecated, please use config.redis.database instead", removal_in_version = "4.0", }, func = function(value) diff --git a/kong/plugins/response-ratelimiting/schema.lua b/kong/plugins/response-ratelimiting/schema.lua index d919ced5a8ee..81b4a926474b 100644 --- a/kong/plugins/response-ratelimiting/schema.lua +++ b/kong/plugins/response-ratelimiting/schema.lua @@ -143,6 +143,7 @@ return { type = "string", translate_backwards = {'redis', 'host'}, deprecation = { + replaced_with = { { path = { 'redis', 'host' } } }, message = "response-ratelimiting: config.redis_host is deprecated, please use config.redis.host instead", removal_in_version = "4.0", }, func = function(value) @@ -153,6 +154,7 @@ return { type = "integer", translate_backwards = {'redis', 'port'}, deprecation = { + replaced_with = { { path = {'redis', 'port'} } }, message = "response-ratelimiting: config.redis_port is deprecated, please use config.redis.port instead", removal_in_version = "4.0", }, func = function(value) @@ -164,6 +166,7 @@ return { len_min = 0, translate_backwards = {'redis', 'password'}, deprecation = { + replaced_with = { { path = {'redis', 'password'} } }, message = "response-ratelimiting: config.redis_password is deprecated, please use config.redis.password instead", removal_in_version = "4.0", }, func = function(value) @@ -174,6 +177,7 @@ return { type = "string", translate_backwards = {'redis', 'username'}, deprecation = { + replaced_with = { { path = {'redis', 'username'} } }, message = "response-ratelimiting: config.redis_username is deprecated, please use config.redis.username instead", removal_in_version = "4.0", }, func = function(value) @@ -184,6 +188,7 @@ return { type = "boolean", translate_backwards = {'redis', 'ssl'}, deprecation = { + replaced_with = { { path = {'redis', 'ssl'} } }, message = "response-ratelimiting: config.redis_ssl is deprecated, please use config.redis.ssl instead", removal_in_version = "4.0", }, func = function(value) @@ -194,6 +199,7 @@ return { type = "boolean", translate_backwards = {'redis', 'ssl_verify'}, deprecation = { + replaced_with = { { path = {'redis', 'ssl_verify'} } }, message = "response-ratelimiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead", removal_in_version = "4.0", }, func = function(value) @@ -204,6 +210,7 @@ return { type = "string", translate_backwards = {'redis', 'server_name'}, deprecation = { + replaced_with = { { path = {'redis', 'server_name'} } }, message = "response-ratelimiting: config.redis_server_name is deprecated, please use config.redis.server_name instead", removal_in_version = "4.0", }, func = function(value) @@ -214,6 +221,7 @@ return { type = "integer", translate_backwards = {'redis', 'timeout'}, deprecation = { + replaced_with = { { path = {'redis', 'timeout'} } }, message = "response-ratelimiting: config.redis_timeout is deprecated, please use config.redis.timeout instead", removal_in_version = "4.0", }, func = function(value) @@ -224,6 +232,7 @@ return { type = "integer", translate_backwards = {'redis', 'database'}, deprecation = { + replaced_with = { { path = {'redis', 'database'} } }, message = "response-ratelimiting: config.redis_database is deprecated, please use config.redis.database instead", removal_in_version = "4.0", }, func = function(value) diff --git a/spec/01-unit/01-db/01-schema/01-schema_spec.lua b/spec/01-unit/01-db/01-schema/01-schema_spec.lua index a6c03d3d8455..c2581ec5a2c8 100644 --- a/spec/01-unit/01-db/01-schema/01-schema_spec.lua +++ b/spec/01-unit/01-db/01-schema/01-schema_spec.lua @@ -1,6 +1,7 @@ local Schema = require "kong.db.schema" local cjson = require "cjson" local helpers = require "spec.helpers" +local table_copy = require "kong.tools.table".deep_copy local SchemaKind = { @@ -4114,8 +4115,8 @@ describe("schema", function() local TestSchema = Schema.new({ name = "test", fields = { - { name = { type = "string" } }, - { record = { + { field_A = { type = "string" } }, + { field_B = { type = "record", fields = { { x = { type = "string" } } @@ -4124,21 +4125,21 @@ describe("schema", function() }, shorthand_fields = { { - username = { + shorthand_A = { type = "string", func = function(value) return { - name = value + field_A = value } end, }, }, { - y = { + shorthand_B = { type = "string", func = function(value) return { - record = { + field_B = { x = value, }, } @@ -4148,70 +4149,335 @@ describe("schema", function() }, }) - local input = { username = "test1", name = "ignored", record = { x = "ignored" }, y = "test1" } + local input = { shorthand_A = "test1", field_A = "ignored", + shorthand_B = "test2", field_B = { x = "ignored" } } local output, _ = TestSchema:process_auto_fields(input) - assert.same({ name = "test1", record = { x = "test1" } }, output) + assert.same({ field_A = "test1", field_B = { x = "test2" } }, output) - -- deprecated fields does take precedence if the new fields are null - local input = { username = "overwritten-1", name = ngx.null, record = { x = ngx.null }, y = "overwritten-2" } + -- shorthand value takes precedence if the destination field is null + local input = { shorthand_A = "overwritten-1", field_A = ngx.null, + shorthand_B = "overwritten-2", field_B = { x = ngx.null }} local output, _ = TestSchema:process_auto_fields(input) - assert.same({ name = "overwritten-1", record = { x = "overwritten-2" } }, output) + assert.same({ field_A = "overwritten-1", field_B = { x = "overwritten-2" } }, output) end) - it("does not take precedence if deprecated", function() + describe("with simple 'table_path' reverse mapping", function() local TestSchema = Schema.new({ name = "test", fields = { - { name = { type = "string" } }, - { record = { + { new_A = { type = "string" } }, + { new_B = { type = "record", fields = { { x = { type = "string" } } }, }}, + { new_C = { type = "string", default = "abc", required = true }}, + { new_D_1 = { type = "string" }}, + { new_D_2 = { type = "string" }}, }, shorthand_fields = { { - username = { + old_A = { type = "string", func = function(value) return { - name = value + new_A = value } end, deprecation = { - message = "username is deprecated, please use name instead", + replaced_with = { { path = { "new_A" } } }, + message = "old_A is deprecated, please use new_A instead", removal_in_version = "4.0", }, }, }, { - y = { + old_B = { type = "string", func = function(value) return { - record = { + new_B = { x = value, }, } end, deprecation = { - message = "y is deprecated, please use record.x instead", + replaced_with = { { path = { "new_B", "x" } } }, + message = "old_B is deprecated, please use new_B.x instead", removal_in_version = "4.0", }, }, }, + { + old_C = { + type = "string", + func = function(value) + return { + new_C = value + } + end, + deprecation = { + replaced_with = { { path = { "new_C" } } }, + message = "old_C is deprecated, please use new_C instead", + removal_in_version = "4.0", + } + } + }, + { + old_D = { + type = "string", + func = function(value) + return { new_D_1 = value, new_D_2 = value } + end, + deprecation = { + replaced_with = { { path = { "new_D_1" } }, { path = { "new_D_2" } } }, + message = "old_D is deprecated, please use new_D_1 and new_D_2 instead", + removal_in_version = "4.0", + } + } + } }, }) - local input = { username = "ignored", name = "test1", record = { x = "test1" }, y = "ignored" } - local output, _ = TestSchema:process_auto_fields(input) - assert.same({ name = "test1", record = { x = "test1" } }, output) + it("notifes of error if values mismatch with replaced field", function() + local input = { old_A = "not-test-1", new_A = "test-1", + old_B = "not-test-2", new_B = { x = "test-2" }, + old_C = "abc", new_C = "not-abc", -- "abc" is the default value + old_D = "test-4", new_D_1 = "test-4", new_D_2 = "not-test-4", } + local output, err = TestSchema:process_auto_fields(input) + assert.same({ + old_A = 'both deprecated and new field are used but their values mismatch: old_A = not-test-1 vs new_A = test-1', + old_B = 'both deprecated and new field are used but their values mismatch: old_B = not-test-2 vs new_B.x = test-2' , + old_C = 'both deprecated and new field are used but their values mismatch: old_C = abc vs new_C = not-abc', + old_D = 'both deprecated and new field are used but their values mismatch: old_D = test-4 vs new_D_2 = not-test-4' }, + err + ) + assert.falsy(output) + end) - -- deprecated fields does take precedence if the new fields are null - local input = { username = "overwritten-1", name = ngx.null, record = { x = ngx.null }, y = "overwritten-2" } - local output, _ = TestSchema:process_auto_fields(input) - assert.same({ name = "overwritten-1", record = { x = "overwritten-2" } }, output) + it("accepts config if both new field and deprecated field defined and their values match", function() + local input = { old_A = "test-1", new_A = "test-1", + old_B = "test-2", new_B = { x = "test-2" }, + -- "C" field is using default + old_D = "test-4", new_D_1 = "test-4", new_D_2 = "test-4", } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "test-1", new_B = { x = "test-2" }, new_C = "abc", new_D_1 = "test-4", new_D_2 = "test-4" }, output) + + + local input = { old_A = "test-1", new_A = "test-1", + old_B = "test-2", new_B = { x = "test-2" }, + old_C = "test-3", -- no new field C specified but it has a default which should be ignored + new_D_1 = "test-4-1", new_D_2 = "test-4-2", } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "test-1", new_B = { x = "test-2" }, new_C = "test-3", new_D_1 = "test-4-1", new_D_2 = "test-4-2" }, output) + + -- when new values are null it's still accepted + local input = { old_A = "test-1", new_A = ngx.null, + old_B = "test-2", new_B = { x = ngx.null }, + old_C = "test-3", new_C = ngx.null, + old_D = "test-4", new_D_1 = ngx.null, new_D_2 = ngx.null, } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({new_A = "test-1", new_B = { x = "test-2" }, new_C = "test-3", new_D_1 = "test-4", new_D_2 = "test-4" }, output) + + -- when old values are null it's still accepted + local input = { old_A = ngx.null, new_A = "test-1", + old_B = ngx.null, new_B = { x = "test-2" }, + old_C = ngx.null, new_C = "test-3", + old_D = ngx.null, new_D_1 = "test-4-1", new_D_2 = "test-4-2", } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "test-1", new_B = { x = "test-2" }, new_C = "test-3", new_D_1 = "test-4-1", new_D_2 = "test-4-2" }, output) + end) + + it("allows to set explicit nulls when only one set of fields was passed", function() + -- when new values are null it's still accepted + local input = { new_A = ngx.null, + new_B = { x = ngx.null }, + new_C = ngx.null, + new_D_1 = ngx.null, new_D_2 = ngx.null } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({new_A = ngx.null, new_B = { x = ngx.null }, new_C = ngx.null, new_D_1 = ngx.null, new_D_2 = ngx.null}, output) + + -- when old values are null it's still accepted + local input = { old_A = ngx.null, + old_B = ngx.null, + old_C = ngx.null, + old_D = ngx.null } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({new_A = ngx.null, new_B = { x = ngx.null }, new_C = ngx.null, new_D_1 = ngx.null, new_D_2 = ngx.null}, output) + end) + end) + + describe("with complex field reverse_mapping_function", function() + local TestSchema = Schema.new({ + name = "test", + fields = { + { new_A = { type = "string" } }, + { new_B = { + type = "record", + fields = { + { x = { type = "string" } } + }, + }}, + { new_C = { + type = "array", + elements = { + type = "number" + } + }} + }, + shorthand_fields = { + { + old_A = { + type = "string", + func = function(value) + if value == ngx.null then + return { new_A = ngx.null } + end + return { new_A = value:upper() } + end, + deprecation = { + replaced_with = { + { path = { "new_A" }, + reverse_mapping_function = function(data) + if data.new_A and data.new_A ~= ngx.null then + return data.new_A:lower() + end + + return data.new_A + end } + }, + message = "old_A is deprecated, please use new_A instead", + removal_in_version = "4.0", + }, + }, + }, + { + old_B = { + type = "string", + func = function(value) + if value == ngx.null then + return { + new_B = { + x = ngx.null, + }, + } + end + + return { + new_B = { + x = value:upper(), + }, + } + end, + deprecation = { + replaced_with = { + { path = { "new_B", "x" }, + reverse_mapping_function = function (data) + if data.new_B and data.new_B.x ~= ngx.null then + return data.new_B.x:lower() + end + return ngx.null + end + } }, + message = "old_B is deprecated, please use new_B.x instead", + removal_in_version = "4.0", + }, + }, + }, + { + old_C = { + type = "array", + elements = { + type = "number" + }, + func = function(value) + if value == ngx.null then + return { new_C = ngx.null } + end + local copy = table_copy(value) + table.sort(copy, function(a,b) return a > b end ) + return { new_C = copy } -- new field is reversed + end, + deprecation = { + replaced_with = { + { path = { "new_C" }, + reverse_mapping_function = function (data) + if data.new_C == ngx.null then + return ngx.null + end + + local copy = table_copy(data.new_C) + table.sort(copy, function(a,b) return a < b end) + return copy + end + }, + } + } + } + } + }, + }) + + it("notifes of error if values mismatch with replaced field", function() + local input = { old_A = "not-test-1", new_A = "TEST1", + old_B = "not-test-2", new_B = { x = "TEST2" }, + old_C = { 1, 2, 4 }, new_C = { 3, 2, 1 } } + local output, err = TestSchema:process_auto_fields(input) + assert.same('both deprecated and new field are used but their values mismatch: old_A = not-test-1 vs new_A = test1', err.old_A) + assert.same('both deprecated and new field are used but their values mismatch: old_B = not-test-2 vs new_B.x = test2', err.old_B) + assert.matches('both deprecated and new field are used but their values mismatch: old_C = .+ vs new_C = .+', err.old_C) + assert.falsy(output) + end) + + it("accepts config if both new field and deprecated field defined and their values match", function() + local input = { old_A = "test-1", new_A = "TEST-1", + old_B = "test-2", new_B = { x = "TEST-2" }, + old_C = { 1, 2, 3 }, new_C = { 3, 2, 1 } } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "TEST-1", new_B = { x = "TEST-2" }, new_C = { 3, 2, 1 }}, output) + + -- when new values are null it's still accepted + local input = { old_A = "test-1", new_A = ngx.null, + old_B = "test-2", new_B = { x = ngx.null }, + old_C = { 1, 2, 3 }, new_C = ngx.null } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "TEST-1", new_B = { x = "TEST-2" }, new_C = { 3, 2, 1 }}, output) + + -- when old values are null it's still accepted + local input = { old_A = ngx.null, new_A = "TEST-1", + old_B = ngx.null, new_B = { x = "TEST-2" }, + old_C = ngx.null, new_C = { 3, 2, 1 } } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({ new_A = "TEST-1", new_B = { x = "TEST-2" }, new_C = { 3, 2, 1 }}, output) + end) + + it("allows to set explicit nulls when only one set of fields was passed", function() + -- when new values are null it's still accepted + local input = { new_A = ngx.null, + new_B = { x = ngx.null }, + new_C = ngx.null } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({new_A = ngx.null, new_B = { x = ngx.null }, new_C = ngx.null}, output) + + -- when old values are null it's still accepted + local input = { old_A = ngx.null, + old_B = ngx.null, + old_C = ngx.null } + local output, err = TestSchema:process_auto_fields(input) + assert.is_nil(err) + assert.same({new_A = ngx.null, new_B = { x = ngx.null }, new_C = ngx.null}, output) + end) end) it("can produce multiple fields", function() From 7a22ba081069870d534b049c61f8788c1d38ccea Mon Sep 17 00:00:00 2001 From: samugi Date: Fri, 19 Jul 2024 13:47:26 +0200 Subject: [PATCH 06/16] feat(ci): pr diff add an action to enable executing the `/prdiff ` command, to get the diff between the current PR's changes and the other PR's. This gives a quick overview of the differences between two PRs and is meant to facilitate the process of reviewing cherry-picks. --- .github/workflows/pr-diff.yml | 102 ++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 .github/workflows/pr-diff.yml diff --git a/.github/workflows/pr-diff.yml b/.github/workflows/pr-diff.yml new file mode 100644 index 000000000000..2f6c05a9c50f --- /dev/null +++ b/.github/workflows/pr-diff.yml @@ -0,0 +1,102 @@ +name: PR Diff + +on: + issue_comment: + types: [created] + +permissions: + issues: write + pull-requests: write + contents: read + +jobs: + pr_diff: + runs-on: ubuntu-latest + + # Only run when a comment containing `/prdiff` is created + # and the author is a member, collaborator or owner + if: > + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(fromJSON('["MEMBER", "COLLABORATOR", "OWNER"]'), github.event.comment.author_association) && + startsWith(github.event.comment.body, '/prdiff') + ) + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Read comment + id: read_comment + env: + COMMENT_BODY: ${{ github.event.comment.body }} + run: | + if [[ "$COMMENT_BODY" =~ ^/prdiff[[:space:]]([^[:space:]]+) ]]; then + CMD_ARG=${BASH_REMATCH[1]} + echo "Using cmd argument: $CMD_ARG" + echo "other_pr=$CMD_ARG" >> $GITHUB_OUTPUT + else + echo "Comment does not match format: '/prdiff ': ignoring" + fi + + - name: Validate input + if: steps.read_comment.outputs.other_pr + id: validate_url + uses: actions/github-script@v7 + with: + script: | + const url = `${{ steps.read_comment.outputs.other_pr }}`; + + try { + const validUrl = new URL(url); + + // Check if URL is a GitHub PR URL + const regex = /^https:\/\/github\.com\/[^\/]+\/[^\/]+\/pull\/\d+$/; + if (!regex.test(validUrl.href)) { + core.setFailed('The provided URL is not a valid GitHub PR URL.'); + } + } catch (error) { + core.setFailed('The provided URL is not valid.'); + } + + - name: Get current PR URL + if: success() && steps.read_comment.outputs.other_pr + id: get_pr_url + run: | + PR_URL="https://github.com/${{ github.repository }}/pull/${{ github.event.issue.number }}" + echo "PR_URL=$PR_URL" >> $GITHUB_OUTPUT + + - name: Obtain diff with the PR provided + if: success() && steps.read_comment.outputs.other_pr + id: run_extension + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh extension install samugi/gh-compr --pin "3785a2d3270c52164fb1f7f63bd3c5df66bedead" + OTHER_PR=${{ steps.read_comment.outputs.other_pr }} + CURRENT_PR=${{ steps.get_pr_url.outputs.PR_URL }} + + set +e + OUTPUT=$(gh compr $OTHER_PR $CURRENT_PR) + EXIT_STATUS=$? + if [ $EXIT_STATUS -ne 0 ]; then + echo "MESSAGE<$OUTPUT\n"$'\n'EOF >> "$GITHUB_OUTPUT" + else + # escape to prepare for assignment to template literal + ESCAPED_OUTPUT=$(echo "$OUTPUT" | sed -e 's/`/\\`/g' -e 's/\$/\\\$/g') + echo "MESSAGE<\nClick to expand\n\n\\\`\\\`\\\`diff\n$ESCAPED_OUTPUT\n\\\`\\\`\\\`\n"$'\n'EOF >> "$GITHUB_OUTPUT" + fi + + - name: Post result as comment in the PR + uses: actions/github-script@v7 + if: steps.run_extension.outputs.MESSAGE + with: + script: | + const commentBody = `${{ steps.run_extension.outputs.MESSAGE }}`; + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: commentBody + }) From 25bc2c8dd9685f49a9f04f7ce11e76847367d4b2 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Tue, 3 Sep 2024 17:45:07 +0300 Subject: [PATCH 07/16] feat(deps): add ada - whatwg-compliant and fast url parser (#13120) ### Summary Adds libada and lua-resty-ada as a dependency. This is needed for: https://github.com/Kong/kong/pull/12758 But it may be great for many other uses too. The `lua-resty-ada` LuaJIT FFI bindings can be found here: https://github.com/bungle/lua-resty-ada Signed-off-by: Aapo Talvensaari --- .requirements | 2 ++ build/BUILD.bazel | 5 +-- build/openresty/ada/BUILD.bazel | 18 +++++++++++ build/openresty/ada/ada_repositories.bzl | 19 +++++++++++ build/openresty/repositories.bzl | 2 ++ changelog/unreleased/kong/feat-add-ada.yml | 4 +++ kong-3.9.0-0.rockspec | 1 + .../fixtures/amazonlinux-2-amd64.txt | 7 ++++ .../fixtures/amazonlinux-2023-amd64.txt | 7 ++++ .../fixtures/amazonlinux-2023-arm64.txt | 7 ++++ .../fixtures/debian-11-amd64.txt | 7 ++++ .../fixtures/debian-12-amd64.txt | 7 ++++ .../explain_manifest/fixtures/el8-amd64.txt | 7 ++++ .../explain_manifest/fixtures/el9-amd64.txt | 7 ++++ .../explain_manifest/fixtures/el9-arm64.txt | 7 ++++ .../fixtures/ubuntu-20.04-amd64.txt | 7 ++++ .../fixtures/ubuntu-22.04-amd64.txt | 7 ++++ .../fixtures/ubuntu-22.04-arm64.txt | 7 ++++ scripts/explain_manifest/suites.py | 7 +++- spec/01-unit/31-ada-url_spec.lua | 32 +++++++++++++++++++ 20 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 build/openresty/ada/BUILD.bazel create mode 100644 build/openresty/ada/ada_repositories.bzl create mode 100644 changelog/unreleased/kong/feat-add-ada.yml create mode 100644 spec/01-unit/31-ada-url_spec.lua diff --git a/.requirements b/.requirements index a68a68eca89f..48472d5efef4 100644 --- a/.requirements +++ b/.requirements @@ -8,6 +8,8 @@ OPENSSL=3.2.1 OPENSSL_SHA256=83c7329fe52c850677d75e5d0b0ca245309b97e8ecbcfdc1dfdc4ab9fac35b39 PCRE=10.44 PCRE_SHA256=86b9cb0aa3bcb7994faa88018292bc704cdbb708e785f7c74352ff6ea7d3175b +ADA=2.9.2 +ADA_SHA256=b2cce630590b490d79ea4f4460ba77efd5fb29c5a87a4e8cb7ebc4859bc4b564 LIBEXPAT=2.6.2 LIBEXPAT_SHA256=d4cf38d26e21a56654ffe4acd9cd5481164619626802328506a2869afab29ab3 diff --git a/build/BUILD.bazel b/build/BUILD.bazel index 008a71ffce57..05ea9aa880e0 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -13,14 +13,15 @@ clib_deps = [ "@openssl", "@libexpat", "@snappy", + "@ada", ] [ kong_install( name = "install-%s" % get_workspace_name(k), src = k, - prefix = "kong/lib" if k in ("@passwdqc", "@snappy") else "kong", - strip_path = "snappy" if k == "@snappy" else "", + prefix = "kong/lib" if k in ("@passwdqc", "@snappy", "@ada") else "kong", + strip_path = "snappy" if k == "@snappy" else "ada" if k == "@ada" else "", ) for k in clib_deps ] diff --git a/build/openresty/ada/BUILD.bazel b/build/openresty/ada/BUILD.bazel new file mode 100644 index 000000000000..9a157965e6b3 --- /dev/null +++ b/build/openresty/ada/BUILD.bazel @@ -0,0 +1,18 @@ +cc_library( + name = "ada-lib", + srcs = ["ada.cpp"], + hdrs = [ + "ada.h", + "ada_c.h", + ], + copts = [ + "-std=c++17", + ], + linkstatic = True, +) + +cc_shared_library( + name = "ada", + visibility = ["//visibility:public"], + deps = [":ada-lib"], +) diff --git a/build/openresty/ada/ada_repositories.bzl b/build/openresty/ada/ada_repositories.bzl new file mode 100644 index 000000000000..ae9777207a39 --- /dev/null +++ b/build/openresty/ada/ada_repositories.bzl @@ -0,0 +1,19 @@ +"""A module defining the third party dependency Ada""" + +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") +load("@kong_bindings//:variables.bzl", "KONG_VAR") + +def ada_repositories(): + """Defines the ada repository""" + + version = KONG_VAR["ADA"] + + maybe( + http_archive, + name = "ada", + sha256 = KONG_VAR["ADA_SHA256"], + url = "https://github.com/ada-url/ada/releases/download/v" + version + "/singleheader.zip", + type = "zip", + build_file = "//build/openresty/ada:BUILD.bazel", + ) diff --git a/build/openresty/repositories.bzl b/build/openresty/repositories.bzl index 3b01aa23901c..bb1e7389f58a 100644 --- a/build/openresty/repositories.bzl +++ b/build/openresty/repositories.bzl @@ -12,6 +12,7 @@ load("//build/openresty/wasmx:wasmx_repositories.bzl", "wasmx_repositories") load("//build/openresty/wasmx/filters:repositories.bzl", "wasm_filters_repositories") load("//build/openresty/brotli:brotli_repositories.bzl", "brotli_repositories") load("//build/openresty/snappy:snappy_repositories.bzl", "snappy_repositories") +load("//build/openresty/ada:ada_repositories.bzl", "ada_repositories") # This is a dummy file to export the module's repository. _NGINX_MODULE_DUMMY_FILE = """ @@ -37,6 +38,7 @@ def openresty_repositories(): wasm_filters_repositories() brotli_repositories() snappy_repositories() + ada_repositories() openresty_version = KONG_VAR["OPENRESTY"] diff --git a/changelog/unreleased/kong/feat-add-ada.yml b/changelog/unreleased/kong/feat-add-ada.yml new file mode 100644 index 000000000000..891d77fa3dab --- /dev/null +++ b/changelog/unreleased/kong/feat-add-ada.yml @@ -0,0 +1,4 @@ +message: | + **Core**: Added Ada dependency - WHATWG-compliant and fast URL parser. +type: feature +scope: Core diff --git a/kong-3.9.0-0.rockspec b/kong-3.9.0-0.rockspec index cc4a204fee05..6bf6989b3334 100644 --- a/kong-3.9.0-0.rockspec +++ b/kong-3.9.0-0.rockspec @@ -44,6 +44,7 @@ dependencies = { "lpeg == 1.1.0", "lua-resty-ljsonschema == 1.1.6-2", "lua-resty-snappy == 1.0-1", + "lua-resty-ada == 1.1.0", } build = { type = "builtin", diff --git a/scripts/explain_manifest/fixtures/amazonlinux-2-amd64.txt b/scripts/explain_manifest/fixtures/amazonlinux-2-amd64.txt index 8a457c581daa..f75994009046 100644 --- a/scripts/explain_manifest/fixtures/amazonlinux-2-amd64.txt +++ b/scripts/explain_manifest/fixtures/amazonlinux-2-amd64.txt @@ -47,6 +47,13 @@ - libc.so.6 Rpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libm.so.6 diff --git a/scripts/explain_manifest/fixtures/amazonlinux-2023-amd64.txt b/scripts/explain_manifest/fixtures/amazonlinux-2023-amd64.txt index 7b5c7a0bf8ec..1baf5d190001 100644 --- a/scripts/explain_manifest/fixtures/amazonlinux-2023-amd64.txt +++ b/scripts/explain_manifest/fixtures/amazonlinux-2023-amd64.txt @@ -43,6 +43,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/amazonlinux-2023-arm64.txt b/scripts/explain_manifest/fixtures/amazonlinux-2023-arm64.txt index dc4f126bab70..807cec769697 100644 --- a/scripts/explain_manifest/fixtures/amazonlinux-2023-arm64.txt +++ b/scripts/explain_manifest/fixtures/amazonlinux-2023-arm64.txt @@ -45,6 +45,13 @@ - libc.so.6 Rpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libm.so.6 diff --git a/scripts/explain_manifest/fixtures/debian-11-amd64.txt b/scripts/explain_manifest/fixtures/debian-11-amd64.txt index 7647cdfb4f13..768258ad6b1a 100644 --- a/scripts/explain_manifest/fixtures/debian-11-amd64.txt +++ b/scripts/explain_manifest/fixtures/debian-11-amd64.txt @@ -47,6 +47,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/debian-12-amd64.txt b/scripts/explain_manifest/fixtures/debian-12-amd64.txt index ebeb6014fe24..31cb3a4d6c7e 100644 --- a/scripts/explain_manifest/fixtures/debian-12-amd64.txt +++ b/scripts/explain_manifest/fixtures/debian-12-amd64.txt @@ -43,6 +43,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/el8-amd64.txt b/scripts/explain_manifest/fixtures/el8-amd64.txt index 629859f3ca62..ec2ba1998a3e 100644 --- a/scripts/explain_manifest/fixtures/el8-amd64.txt +++ b/scripts/explain_manifest/fixtures/el8-amd64.txt @@ -47,6 +47,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/el9-amd64.txt b/scripts/explain_manifest/fixtures/el9-amd64.txt index d1b29e97cb7f..fb837ac0c0ea 100644 --- a/scripts/explain_manifest/fixtures/el9-amd64.txt +++ b/scripts/explain_manifest/fixtures/el9-amd64.txt @@ -43,6 +43,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/el9-arm64.txt b/scripts/explain_manifest/fixtures/el9-arm64.txt index dc4f126bab70..807cec769697 100644 --- a/scripts/explain_manifest/fixtures/el9-arm64.txt +++ b/scripts/explain_manifest/fixtures/el9-arm64.txt @@ -45,6 +45,13 @@ - libc.so.6 Rpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libm.so.6 diff --git a/scripts/explain_manifest/fixtures/ubuntu-20.04-amd64.txt b/scripts/explain_manifest/fixtures/ubuntu-20.04-amd64.txt index dcaa0ef9271d..361c43bb7897 100644 --- a/scripts/explain_manifest/fixtures/ubuntu-20.04-amd64.txt +++ b/scripts/explain_manifest/fixtures/ubuntu-20.04-amd64.txt @@ -47,6 +47,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/ubuntu-22.04-amd64.txt b/scripts/explain_manifest/fixtures/ubuntu-22.04-amd64.txt index 60d62a4563c6..e0cdc94ca3b3 100644 --- a/scripts/explain_manifest/fixtures/ubuntu-22.04-amd64.txt +++ b/scripts/explain_manifest/fixtures/ubuntu-22.04-amd64.txt @@ -43,6 +43,13 @@ - libc.so.6 Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libstdc++.so.6 diff --git a/scripts/explain_manifest/fixtures/ubuntu-22.04-arm64.txt b/scripts/explain_manifest/fixtures/ubuntu-22.04-arm64.txt index 0d875dde28b0..cb06affdd985 100644 --- a/scripts/explain_manifest/fixtures/ubuntu-22.04-arm64.txt +++ b/scripts/explain_manifest/fixtures/ubuntu-22.04-arm64.txt @@ -31,6 +31,13 @@ - Path : /usr/local/kong/lib/engines-3/padlock.so Runpath : /usr/local/kong/lib +- Path : /usr/local/kong/lib/libada.so + Needed : + - libstdc++.so.6 + - libm.so.6 + - libgcc_s.so.1 + - libc.so.6 + - Path : /usr/local/kong/lib/libcrypto.so.3 Needed : - libc.so.6 diff --git a/scripts/explain_manifest/suites.py b/scripts/explain_manifest/suites.py index e17854c044f3..7c5987968ddf 100644 --- a/scripts/explain_manifest/suites.py +++ b/scripts/explain_manifest/suites.py @@ -1,3 +1,4 @@ +import re import os wasm_filters = [] @@ -102,7 +103,11 @@ def common_suites(expect, libxcrypt_no_obsolete_api: bool = False, skip_libsimdj expect("**/*.so", "dynamic libraries are compiled with OpenSSL 3.2.x") \ .version_requirement.key("libssl.so.3").less_than("OPENSSL_3.3.0") \ .version_requirement.key("libcrypto.so.3").less_than("OPENSSL_3.3.0") \ - + + ADA_VERSION = read_requirements()["ADA"] + expect("**/*.so", "ada version is less than %s" % ADA_VERSION) \ + .version_requirement.key("libada.so").is_not().greater_than("ADA_%s" % ADA_VERSION) \ + # wasm filters for f in wasm_filters: expect("/usr/local/kong/wasm/%s" % f, "wasm filter %s is installed under kong/wasm" % f).exists() diff --git a/spec/01-unit/31-ada-url_spec.lua b/spec/01-unit/31-ada-url_spec.lua new file mode 100644 index 000000000000..e75461fe95e2 --- /dev/null +++ b/spec/01-unit/31-ada-url_spec.lua @@ -0,0 +1,32 @@ +local ada = require("resty.ada") + + +local assert = assert +local describe = describe +local it = it + + +local equal = assert.equal +local is_nil = assert.is_nil +local is_table = assert.is_table + + +local function is_err(msg, ok, err) + is_nil(ok) + equal(msg, err) + return ok, err +end + + +describe("Ada", function() + describe("URL", function() + describe(".parse", function() + it("rejects invalid url", function() + is_err("invalid url", ada.parse("")) + end) + it("accepts valid url", function() + is_table(ada.parse("http://www.google.com/")) + end) + end) + end) +end) From 98b8b4a654f26385268da0ab2f7b945d4b410d5c Mon Sep 17 00:00:00 2001 From: Chrono Date: Wed, 4 Sep 2024 10:02:33 +0800 Subject: [PATCH 08/16] feat(tools): add `get_updated_now()` in time tools (#13575) https://konghq.atlassian.net/browse/KAG-5280 --- kong/cluster_events/init.lua | 8 +++----- kong/clustering/control_plane.lua | 5 ++--- kong/db/strategies/postgres/connector.lua | 8 +------- kong/db/strategies/postgres/init.lua | 9 +-------- kong/tools/time.lua | 5 +++++ 5 files changed, 12 insertions(+), 23 deletions(-) diff --git a/kong/cluster_events/init.lua b/kong/cluster_events/init.lua index c2179d6f865d..a8cfe5d700ce 100644 --- a/kong/cluster_events/init.lua +++ b/kong/cluster_events/init.lua @@ -10,7 +10,7 @@ local insert = table.insert local ngx_log = ngx.log local ngx_now = ngx.now local timer_at = ngx.timer.at -local ngx_update_time = ngx.update_time +local now_updated = require("kong.tools.time").get_updated_now local knode = kong and kong.node or require "kong.pdk.node".new() local concurrency = require "kong.concurrency" @@ -245,8 +245,7 @@ local function process_event(self, row, local_start_time) local delay if row.nbf and row.now then - ngx_update_time() - local now = row.now + max(ngx_now() - local_start_time, 0) + local now = row.now + max(now_updated() - local_start_time, 0) delay = max(row.nbf - now, 0) end @@ -308,8 +307,7 @@ local function poll(self) end end - ngx_update_time() - local local_start_time = ngx_now() + local local_start_time = now_updated() for i = 1, count do local ok, err = process_event(self, rows[i], local_start_time) if not ok then diff --git a/kong/clustering/control_plane.lua b/kong/clustering/control_plane.lua index 990eb5ec346b..080b7bd9bec9 100644 --- a/kong/clustering/control_plane.lua +++ b/kong/clustering/control_plane.lua @@ -27,13 +27,13 @@ local exiting = ngx.worker.exiting local worker_id = ngx.worker.id local ngx_time = ngx.time local ngx_now = ngx.now -local ngx_update_time = ngx.update_time local ngx_var = ngx.var local table_insert = table.insert local table_remove = table.remove local sub = string.sub local isempty = require("table.isempty") local sleep = ngx.sleep +local now_updated = require("kong.tools.time").get_updated_now local plugins_list_to_map = compat.plugins_list_to_map @@ -173,8 +173,7 @@ function _M:push_config() n = n + 1 end - ngx_update_time() - local duration = ngx_now() - start + local duration = now_updated() - start ngx_log(ngx_DEBUG, _log_prefix, "config pushed to ", n, " data-plane nodes in ", duration, " seconds") end diff --git a/kong/db/strategies/postgres/connector.lua b/kong/db/strategies/postgres/connector.lua index 94e0f9ee0215..4220ba01f2d1 100644 --- a/kong/db/strategies/postgres/connector.lua +++ b/kong/db/strategies/postgres/connector.lua @@ -19,7 +19,6 @@ local floor = math.floor local type = type local ngx = ngx local timer_every = ngx.timer.every -local update_time = ngx.update_time local get_phase = ngx.get_phase local null = ngx.null local now = ngx.now @@ -31,6 +30,7 @@ local utils_toposort = db_utils.topological_sort local insert = table.insert local table_merge = require("kong.tools.table").table_merge local strip = require("kong.tools.string").strip +local now_updated = require("kong.tools.time").get_updated_now local WARN = ngx.WARN @@ -54,12 +54,6 @@ local ADMIN_API_PHASE = kong_global.phases.admin_api local CORE_ENTITIES = constants.CORE_ENTITIES -local function now_updated() - update_time() - return now() -end - - local function iterator(rows) local i = 0 return function() diff --git a/kong/db/strategies/postgres/init.lua b/kong/db/strategies/postgres/init.lua index 1a33c5e54a27..c751b4b20727 100644 --- a/kong/db/strategies/postgres/init.lua +++ b/kong/db/strategies/postgres/init.lua @@ -12,7 +12,6 @@ local decode_base64 = ngx.decode_base64 local encode_array = arrays.encode_array local encode_json = json.encode_json local setmetatable = setmetatable -local update_time = ngx.update_time local get_phase = ngx.get_phase local tonumber = tonumber local concat = table.concat @@ -27,12 +26,12 @@ local null = ngx.null local type = type local load = load local find = string.find -local now = ngx.now local fmt = string.format local rep = string.rep local sub = string.sub local log = ngx.log local cycle_aware_deep_copy = require("kong.tools.table").cycle_aware_deep_copy +local now_updated = require("kong.tools.time").get_updated_now local NOTICE = ngx.NOTICE @@ -45,12 +44,6 @@ local function noop(...) end -local function now_updated() - update_time() - return now() -end - - -- @param name Query name, for debugging purposes -- @param query A string describing an array of single-quoted strings which -- contain parts of an SQL query including numeric placeholders like $0, $1, etc. diff --git a/kong/tools/time.lua b/kong/tools/time.lua index 5f52e5ff3cdd..06965bfb1851 100644 --- a/kong/tools/time.lua +++ b/kong/tools/time.lua @@ -78,6 +78,11 @@ do local start_time = ngx.req.start_time local monotonic_msec = require("resty.core.time").monotonic_msec + function _M.get_updated_now() + update_time() + return now() -- time is kept in seconds with millisecond resolution. + end + function _M.get_now_ms() return now() * 1000 -- time is kept in seconds with millisecond resolution. end From 044bb7f1bf26f74718bfe3804eb3151093294cfe Mon Sep 17 00:00:00 2001 From: Robin Xiang Date: Wed, 4 Sep 2024 15:16:02 +0900 Subject: [PATCH 09/16] fix(rate-limiting-plugin): fix a bug where the return values from `get_redis_connection()` are mistaken (#13613) * fix(rate-limiting-plugin): fix a bug where the return values from `get_redis_connnection()` are mistaken * fix changelog --- .../fix-return-values-mistaken-in-rate-limiting-plugin.yml | 3 +++ kong/plugins/rate-limiting/policies/init.lua | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/kong/fix-return-values-mistaken-in-rate-limiting-plugin.yml diff --git a/changelog/unreleased/kong/fix-return-values-mistaken-in-rate-limiting-plugin.yml b/changelog/unreleased/kong/fix-return-values-mistaken-in-rate-limiting-plugin.yml new file mode 100644 index 000000000000..1a88d50d739e --- /dev/null +++ b/changelog/unreleased/kong/fix-return-values-mistaken-in-rate-limiting-plugin.yml @@ -0,0 +1,3 @@ +message: "**Rate-limiting-Plugin**: Fix a bug where the return values from `get_redis_connection()` are mistaken." +scope: Plugin +type: bugfix diff --git a/kong/plugins/rate-limiting/policies/init.lua b/kong/plugins/rate-limiting/policies/init.lua index 0306d844339d..601dc3542b0c 100644 --- a/kong/plugins/rate-limiting/policies/init.lua +++ b/kong/plugins/rate-limiting/policies/init.lua @@ -396,7 +396,7 @@ return { return 0 end - local red, err = get_redis_connection(conf) + local red, db_key, err = get_redis_connection(conf) if not red then return nil, err end From 75d6c842bb64bf4da40091a5747134321db54e7c Mon Sep 17 00:00:00 2001 From: Chrono Date: Wed, 4 Sep 2024 16:07:01 +0800 Subject: [PATCH 10/16] tests(helpers): separate miscellaneous and shell functions (#13542) It is one of the serial refactors of helpers.lua KAG-5236 The function `kong_exec()` needs an upvalue `conf`, which must be reloaded each time. --- spec/details/misc.lua | 215 ++++++++++++++++++++++++++++ spec/details/shell.lua | 99 +++++++++++++ spec/helpers.lua | 317 ++++------------------------------------- 3 files changed, 345 insertions(+), 286 deletions(-) create mode 100644 spec/details/misc.lua create mode 100644 spec/details/shell.lua diff --git a/spec/details/misc.lua b/spec/details/misc.lua new file mode 100644 index 000000000000..1ec4bd9dc3ee --- /dev/null +++ b/spec/details/misc.lua @@ -0,0 +1,215 @@ +-- miscellaneous + + +local ffi = require("ffi") +local pl_path = require("pl.path") +local pkey = require("resty.openssl.pkey") +local nginx_signals = require("kong.cmd.utils.nginx_signals") +local shell = require("spec.details.shell") + + +local CONSTANTS = require("spec.details.constants") + + +ffi.cdef [[ + int setenv(const char *name, const char *value, int overwrite); + int unsetenv(const char *name); +]] + + +local pack = function(...) return { n = select("#", ...), ... } end +local unpack = function(t) return unpack(t, 1, t.n) end + + +--- Prints all returned parameters. +-- Simple debugging aid, it will pass all received parameters, hence will not +-- influence the flow of the code. See also `fail`. +-- @function intercept +-- @see fail +-- @usage -- modify +-- local a,b = some_func(c,d) +-- -- into +-- local a,b = intercept(some_func(c,d)) +local function intercept(...) + local args = pack(...) + print(require("pl.pretty").write(args)) + return unpack(args) +end + + +--- Returns the OpenResty version. +-- Extract the current OpenResty version in use and returns +-- a numerical representation of it. +-- Ex: `1.11.2.2` -> `11122` +-- @function openresty_ver_num +local function openresty_ver_num() + local nginx_bin = assert(nginx_signals.find_nginx_bin()) + local _, _, stderr = shell.run(string.format("%s -V", nginx_bin), nil, 0) + + local a, b, c, d = string.match(stderr or "", "openresty/(%d+)%.(%d+)%.(%d+)%.(%d+)") + if not a then + error("could not execute 'nginx -V': " .. stderr) + end + + return tonumber(a .. b .. c .. d) +end + + +--- Unindent a multi-line string for proper indenting in +-- square brackets. +-- @function unindent +-- @usage +-- local u = helpers.unindent +-- +-- u[[ +-- hello world +-- foo bar +-- ]] +-- +-- -- will return: "hello world\nfoo bar" +local function unindent(str, concat_newlines, spaced_newlines) + str = string.match(str, "(.-%S*)%s*$") + if not str then + return "" + end + + local level = math.huge + local prefix = "" + local len + + str = str:match("^%s") and "\n" .. str or str + for pref in str:gmatch("\n(%s+)") do + len = #prefix + + if len < level then + level = len + prefix = pref + end + end + + local repl = concat_newlines and "" or "\n" + repl = spaced_newlines and " " or repl + + return (str:gsub("^\n%s*", ""):gsub("\n" .. prefix, repl):gsub("\n$", ""):gsub("\\r", "\r")) +end + + +--- Write a yaml file. +-- @function make_yaml_file +-- @param content (string) the yaml string to write to the file, if omitted the +-- current database contents will be written using `kong config db_export`. +-- @param filename (optional) if not provided, a temp name will be created +-- @return filename of the file written +local function make_yaml_file(content, filename) + local filename = filename or pl_path.tmpname() .. ".yml" + if content then + local fd = assert(io.open(filename, "w")) + assert(fd:write(unindent(content))) + assert(fd:write("\n")) -- ensure last line ends in newline + assert(fd:close()) + else + assert(shell.kong_exec("config db_export --conf "..CONSTANTS.TEST_CONF_PATH.." "..filename)) + end + return filename +end + + +--- Set an environment variable +-- @function setenv +-- @param env (string) name of the environment variable +-- @param value the value to set +-- @return true on success, false otherwise +local function setenv(env, value) + assert(type(env) == "string", "env must be a string") + assert(type(value) == "string", "value must be a string") + return ffi.C.setenv(env, value, 1) == 0 +end + + +--- Unset an environment variable +-- @function unsetenv +-- @param env (string) name of the environment variable +-- @return true on success, false otherwise +local function unsetenv(env) + assert(type(env) == "string", "env must be a string") + return ffi.C.unsetenv(env) == 0 +end + + +local deep_sort +do + local function deep_compare(a, b) + if a == nil then + a = "" + end + + if b == nil then + b = "" + end + + deep_sort(a) + deep_sort(b) + + if type(a) ~= type(b) then + return type(a) < type(b) + end + + if type(a) == "table" then + return deep_compare(a[1], b[1]) + end + + -- compare cjson.null or ngx.null + if type(a) == "userdata" and type(b) == "userdata" then + return false + end + + return a < b + end + + deep_sort = function(t) + if type(t) == "table" then + for _, v in pairs(t) do + deep_sort(v) + end + table.sort(t, deep_compare) + end + + return t + end +end + + +--- Generate asymmetric keys +-- @function generate_keys +-- @param fmt format to receive the public and private pair +-- @return `pub, priv` key tuple or `nil + err` on failure +local function generate_keys(fmt) + fmt = string.upper(fmt) or "JWK" + local key, err = pkey.new({ + -- only support RSA for now + type = 'RSA', + bits = 2048, + exp = 65537 + }) + assert(key) + assert(err == nil, err) + local pub = key:tostring("public", fmt) + local priv = key:tostring("private", fmt) + return pub, priv +end + + +return { + pack = pack, + unpack = unpack, + + intercept = intercept, + openresty_ver_num = openresty_ver_num(), + unindent = unindent, + make_yaml_file = make_yaml_file, + setenv = setenv, + unsetenv = unsetenv, + deep_sort = deep_sort, + + generate_keys = generate_keys, +} diff --git a/spec/details/shell.lua b/spec/details/shell.lua new file mode 100644 index 000000000000..fffdfc49358d --- /dev/null +++ b/spec/details/shell.lua @@ -0,0 +1,99 @@ +local shell = require("resty.shell") +local conf_loader = require("kong.conf_loader") +local strip = require("kong.tools.string").strip + + +local CONSTANTS = require("spec.details.constants") + + +---------------- +-- Shell helpers +-- @section Shell-helpers + +--- Execute a command. +-- Modified version of `pl.utils.executeex()` so the output can directly be +-- used on an assertion. +-- @function execute +-- @param cmd command string to execute +-- @param returns (optional) boolean: if true, this function will +-- return the same values as Penlight's executeex. +-- @return if `returns` is true, returns four return values +-- (ok, code, stdout, stderr); if `returns` is false, +-- returns either (false, stderr) or (true, stderr, stdout). +local function exec(cmd, returns) + --100MB for retrieving stdout & stderr + local ok, stdout, stderr, _, code = shell.run(cmd, nil, 0, 1024*1024*100) + if returns then + return ok, code, stdout, stderr + end + if not ok then + stdout = nil -- don't return 3rd value if fail because of busted's `assert` + end + return ok, stderr, stdout +end + + +local conf = assert(conf_loader(CONSTANTS.TEST_CONF_PATH)) + + +--- Execute a Kong command. +-- @function kong_exec +-- @param cmd Kong command to execute, eg. `start`, `stop`, etc. +-- @param env (optional) table with kong parameters to set as environment +-- variables, overriding the test config (each key will automatically be +-- prefixed with `KONG_` and be converted to uppercase) +-- @param returns (optional) boolean: if true, this function will +-- return the same values as Penlight's `executeex`. +-- @param env_vars (optional) a string prepended to the command, so +-- that arbitrary environment variables may be passed +-- @return if `returns` is true, returns four return values +-- (ok, code, stdout, stderr); if `returns` is false, +-- returns either (false, stderr) or (true, stderr, stdout). +local function kong_exec(cmd, env, returns, env_vars) + cmd = cmd or "" + env = env or {} + + -- Insert the Lua path to the custom-plugin fixtures + do + local function cleanup(t) + if t then + t = strip(t) + if t:sub(-1,-1) == ";" then + t = t:sub(1, -2) + end + end + return t ~= "" and t or nil + end + local paths = {} + table.insert(paths, cleanup(CONSTANTS.CUSTOM_PLUGIN_PATH)) + table.insert(paths, cleanup(CONSTANTS.CUSTOM_VAULT_PATH)) + table.insert(paths, cleanup(env.lua_package_path)) + table.insert(paths, cleanup(conf.lua_package_path)) + env.lua_package_path = table.concat(paths, ";") + -- note; the nginx config template will add a final ";;", so no need to + -- include that here + end + + if not env.plugins then + env.plugins = "bundled,dummy,cache,rewriter,error-handler-log," .. + "error-generator,error-generator-last," .. + "short-circuit" + end + + -- build Kong environment variables + env_vars = env_vars or "" + for k, v in pairs(env) do + env_vars = string.format("%s KONG_%s='%s'", env_vars, k:upper(), v) + end + + return exec(env_vars .. " " .. CONSTANTS.BIN_PATH .. " " .. cmd, returns) +end + + +return { + run = shell.run, + + conf = conf, + exec = exec, + kong_exec = kong_exec, +} diff --git a/spec/helpers.lua b/spec/helpers.lua index 13fdcdc65e93..b36da513d757 100644 --- a/spec/helpers.lua +++ b/spec/helpers.lua @@ -5,10 +5,9 @@ -- @license [Apache 2.0](https://opensource.org/licenses/Apache-2.0) -- @module spec.helpers -local CONSTANTS = require("spec.details.constants") - local PLUGINS_LIST + local consumers_schema_def = require "kong.db.schema.entities.consumers" local services_schema_def = require "kong.db.schema.entities.services" local plugins_schema_def = require "kong.db.schema.entities.plugins" @@ -31,12 +30,8 @@ local Entity = require "kong.db.schema.entity" local cjson = require "cjson.safe" local kong_table = require "kong.tools.table" local http = require "resty.http" -local pkey = require "resty.openssl.pkey" -local nginx_signals = require "kong.cmd.utils.nginx_signals" local log = require "kong.cmd.utils.log" local DB = require "kong.db" -local shell = require "resty.shell" -local ffi = require "ffi" local ssl = require "ngx.ssl" local ws_client = require "resty.websocket.client" local table_clone = require "table.clone" @@ -50,12 +45,22 @@ local colors = require "ansicolors" local strip = require("kong.tools.string").strip local splitlines = require("pl.stringx").splitlines -ffi.cdef [[ - int setenv(const char *name, const char *value, int overwrite); - int unsetenv(const char *name); -]] -local kong_exec -- forward declaration +local function reload_module(name) + package.loaded[name] = nil + return require(name) +end + + +-- reload some modules when env or _G changes +local CONSTANTS = reload_module("spec.details.constants") +local shell = reload_module("spec.details.shell") +local misc = reload_module("spec.details.misc") + + +local conf = shell.conf +local exec = shell.exec +local kong_exec = shell.kong_exec log.set_lvl(log.levels.quiet) -- disable stdout logs in tests @@ -71,101 +76,6 @@ do package.path = table.concat(paths, ";") end ---- Returns the OpenResty version. --- Extract the current OpenResty version in use and returns --- a numerical representation of it. --- Ex: `1.11.2.2` -> `11122` --- @function openresty_ver_num -local function openresty_ver_num() - local nginx_bin = assert(nginx_signals.find_nginx_bin()) - local _, _, stderr = shell.run(string.format("%s -V", nginx_bin), nil, 0) - - local a, b, c, d = string.match(stderr or "", "openresty/(%d+)%.(%d+)%.(%d+)%.(%d+)") - if not a then - error("could not execute 'nginx -V': " .. stderr) - end - - return tonumber(a .. b .. c .. d) -end - ---- Unindent a multi-line string for proper indenting in --- square brackets. --- @function unindent --- @usage --- local u = helpers.unindent --- --- u[[ --- hello world --- foo bar --- ]] --- --- -- will return: "hello world\nfoo bar" -local function unindent(str, concat_newlines, spaced_newlines) - str = string.match(str, "(.-%S*)%s*$") - if not str then - return "" - end - - local level = math.huge - local prefix = "" - local len - - str = str:match("^%s") and "\n" .. str or str - for pref in str:gmatch("\n(%s+)") do - len = #prefix - - if len < level then - level = len - prefix = pref - end - end - - local repl = concat_newlines and "" or "\n" - repl = spaced_newlines and " " or repl - - return (str:gsub("^\n%s*", ""):gsub("\n" .. prefix, repl):gsub("\n$", ""):gsub("\\r", "\r")) -end - - ---- Set an environment variable --- @function setenv --- @param env (string) name of the environment variable --- @param value the value to set --- @return true on success, false otherwise -local function setenv(env, value) - return ffi.C.setenv(env, value, 1) == 0 -end - - ---- Unset an environment variable --- @function unsetenv --- @param env (string) name of the environment variable --- @return true on success, false otherwise -local function unsetenv(env) - return ffi.C.unsetenv(env) == 0 -end - - ---- Write a yaml file. --- @function make_yaml_file --- @param content (string) the yaml string to write to the file, if omitted the --- current database contents will be written using `kong config db_export`. --- @param filename (optional) if not provided, a temp name will be created --- @return filename of the file written -local function make_yaml_file(content, filename) - local filename = filename or pl_path.tmpname() .. ".yml" - if content then - local fd = assert(io.open(filename, "w")) - assert(fd:write(unindent(content))) - assert(fd:write("\n")) -- ensure last line ends in newline - assert(fd:close()) - else - assert(kong_exec("config db_export --conf "..CONSTANTS.TEST_CONF_PATH.." "..filename)) - end - return filename -end - - local get_available_port do local USED_PORTS = {} @@ -197,8 +107,6 @@ end --------------- -- Conf and DAO --------------- -local conf = assert(conf_loader(CONSTANTS.TEST_CONF_PATH)) - _G.kong = kong_global.new() kong_global.init_pdk(_G.kong, conf) ngx.ctx.KONG_PHASE = kong_global.phases.access @@ -529,25 +437,6 @@ end local resty_http_proxy_mt = setmetatable({}, { __index = http }) resty_http_proxy_mt.__index = resty_http_proxy_mt -local pack = function(...) return { n = select("#", ...), ... } end -local unpack = function(t) return unpack(t, 1, t.n) end - ---- Prints all returned parameters. --- Simple debugging aid, it will pass all received parameters, hence will not --- influence the flow of the code. See also `fail`. --- @function intercept --- @see fail --- @usage -- modify --- local a,b = some_func(c,d) --- -- into --- local a,b = intercept(some_func(c,d)) -local function intercept(...) - local args = pack(...) - print(require("pl.pretty").write(args)) - return unpack(args) -end - - -- Prepopulate Schema's cache Schema.new(consumers_schema_def) Schema.new(services_schema_def) @@ -1154,8 +1043,6 @@ local function proxy_client_h2() return http2_client(proxy_ip, proxy_port, true) end -local exec -- forward declaration - --- Creates a gRPC client, based on the grpcurl CLI. -- @function grpc_client -- @param host hostname to connect to @@ -2385,7 +2272,7 @@ luassert:register("assertion", "fail", fail, -- local i = assert.contains("two", arr) --> fails -- local i = assert.contains("ee$", arr, true) --> passes; i == 2 local function contains(state, args) - local expected, arr, pattern = unpack(args) + local expected, arr, pattern = misc.unpack(args) local found for i = 1, #arr do if (pattern and string.match(arr[i], expected)) or arr[i] == expected then @@ -2409,47 +2296,6 @@ luassert:register("assertion", "contains", contains, "assertion.contains.negative", "assertion.contains.positive") -local deep_sort do - local function deep_compare(a, b) - if a == nil then - a = "" - end - - if b == nil then - b = "" - end - - deep_sort(a) - deep_sort(b) - - if type(a) ~= type(b) then - return type(a) < type(b) - end - - if type(a) == "table" then - return deep_compare(a[1], b[1]) - end - - -- compare cjson.null or ngx.null - if type(a) == "userdata" and type(b) == "userdata" then - return false - end - - return a < b - end - - function deep_sort(t) - if type(t) == "table" then - for _, v in pairs(t) do - deep_sort(v) - end - table.sort(t, deep_compare) - end - - return t - end -end - local function copy_errlog(errlog_path) local file_path = "Unknown path" @@ -2989,13 +2835,13 @@ do return found end - say:set("assertion.match_line.negative", unindent [[ + say:set("assertion.match_line.negative", misc.unindent [[ Expected file at: %s To match: %s ]]) - say:set("assertion.match_line.positive", unindent [[ + say:set("assertion.match_line.positive", misc.unindent [[ Expected file at: %s To not match: @@ -3039,13 +2885,13 @@ local function match_re(_, args) end end -say:set("assertion.match_re.negative", unindent [[ +say:set("assertion.match_re.negative", misc.unindent [[ Expected log: %s To match: %s ]]) -say:set("assertion.match_re.positive", unindent [[ +say:set("assertion.match_re.positive", misc.unindent [[ Expected log: %s To not match: @@ -3320,87 +3166,6 @@ luassert:register("assertion", "partial_match", partial_match, "assertion.partial_match.negative") ----------------- --- Shell helpers --- @section Shell-helpers - ---- Execute a command. --- Modified version of `pl.utils.executeex()` so the output can directly be --- used on an assertion. --- @function execute --- @param cmd command string to execute --- @param returns (optional) boolean: if true, this function will --- return the same values as Penlight's executeex. --- @return if `returns` is true, returns four return values --- (ok, code, stdout, stderr); if `returns` is false, --- returns either (false, stderr) or (true, stderr, stdout). -function exec(cmd, returns) - --100MB for retrieving stdout & stderr - local ok, stdout, stderr, _, code = shell.run(cmd, nil, 0, 1024*1024*100) - if returns then - return ok, code, stdout, stderr - end - if not ok then - stdout = nil -- don't return 3rd value if fail because of busted's `assert` - end - return ok, stderr, stdout -end - - ---- Execute a Kong command. --- @function kong_exec --- @param cmd Kong command to execute, eg. `start`, `stop`, etc. --- @param env (optional) table with kong parameters to set as environment --- variables, overriding the test config (each key will automatically be --- prefixed with `KONG_` and be converted to uppercase) --- @param returns (optional) boolean: if true, this function will --- return the same values as Penlight's `executeex`. --- @param env_vars (optional) a string prepended to the command, so --- that arbitrary environment variables may be passed --- @return if `returns` is true, returns four return values --- (ok, code, stdout, stderr); if `returns` is false, --- returns either (false, stderr) or (true, stderr, stdout). -function kong_exec(cmd, env, returns, env_vars) - cmd = cmd or "" - env = env or {} - - -- Insert the Lua path to the custom-plugin fixtures - do - local function cleanup(t) - if t then - t = strip(t) - if t:sub(-1,-1) == ";" then - t = t:sub(1, -2) - end - end - return t ~= "" and t or nil - end - local paths = {} - table.insert(paths, cleanup(CONSTANTS.CUSTOM_PLUGIN_PATH)) - table.insert(paths, cleanup(CONSTANTS.CUSTOM_VAULT_PATH)) - table.insert(paths, cleanup(env.lua_package_path)) - table.insert(paths, cleanup(conf.lua_package_path)) - env.lua_package_path = table.concat(paths, ";") - -- note; the nginx config template will add a final ";;", so no need to - -- include that here - end - - if not env.plugins then - env.plugins = "bundled,dummy,cache,rewriter,error-handler-log," .. - "error-generator,error-generator-last," .. - "short-circuit" - end - - -- build Kong environment variables - env_vars = env_vars or "" - for k, v in pairs(env) do - env_vars = string.format("%s KONG_%s='%s'", env_vars, k:upper(), v) - end - - return exec(env_vars .. " " .. CONSTANTS.BIN_PATH .. " " .. cmd, returns) -end - - --- Prepares the Kong environment. -- Creates the working directory if it does not exist. -- @param prefix (optional) path to the working directory, if omitted the test @@ -4130,26 +3895,6 @@ local function clustering_client(opts) end ---- Generate asymmetric keys --- @function generate_keys --- @param fmt format to receive the public and private pair --- @return `pub, priv` key tuple or `nil + err` on failure -local function generate_keys(fmt) - fmt = string.upper(fmt) or "JWK" - local key, err = pkey.new({ - -- only support RSA for now - type = 'RSA', - bits = 2048, - exp = 65537 - }) - assert(key) - assert(err == nil, err) - local pub = key:tostring("public", fmt) - local priv = key:tostring("private", fmt) - return pub, priv -end - - local make_temp_dir do local seeded = false @@ -4212,10 +3957,10 @@ local function use_old_plugin(name) local origin_lua_path = os.getenv("LUA_PATH") -- put the old plugin path at first - assert(setenv("LUA_PATH", old_plugin_path .. "/?.lua;" .. old_plugin_path .. "/?/init.lua;" .. origin_lua_path), "failed to set LUA_PATH env") + assert(misc.setenv("LUA_PATH", old_plugin_path .. "/?.lua;" .. old_plugin_path .. "/?/init.lua;" .. origin_lua_path), "failed to set LUA_PATH env") return function () - setenv("LUA_PATH", origin_lua_path) + misc.setenv("LUA_PATH", origin_lua_path) if temp_dir then pl_dir.rmtree(temp_dir) end @@ -4377,13 +4122,14 @@ end stress_generator = stress_generator, -- miscellaneous - intercept = intercept, - openresty_ver_num = openresty_ver_num(), - unindent = unindent, - make_yaml_file = make_yaml_file, - setenv = setenv, - unsetenv = unsetenv, - deep_sort = deep_sort, + intercept = misc.intercept, + openresty_ver_num = misc.openresty_ver_num, + unindent = misc.unindent, + make_yaml_file = misc.make_yaml_file, + setenv = misc.setenv, + unsetenv = misc.unsetenv, + deep_sort = misc.deep_sort, + generate_keys = misc.generate_keys, -- launching Kong subprocesses start_kong = start_kong, @@ -4397,7 +4143,6 @@ end start_grpc_target = start_grpc_target, stop_grpc_target = stop_grpc_target, get_grpc_target_port = get_grpc_target_port, - generate_keys = generate_keys, -- plugin compatibility test use_old_plugin = use_old_plugin, From 78b5ebc28b718d1cfedcf48dadfa0fc40f51848d Mon Sep 17 00:00:00 2001 From: Chrono Date: Wed, 4 Sep 2024 19:01:47 +0800 Subject: [PATCH 11/16] style(tools): should not use `tools.utils` module (#13615) --- kong/llm/proxy/handler.lua | 2 +- kong/observability/logs.lua | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/llm/proxy/handler.lua b/kong/llm/proxy/handler.lua index 737b38c1bb4d..40ef85634b78 100644 --- a/kong/llm/proxy/handler.lua +++ b/kong/llm/proxy/handler.lua @@ -11,7 +11,7 @@ local llm_state = require("kong.llm.state") local cjson = require("cjson.safe") local kong_utils = require("kong.tools.gzip") local buffer = require "string.buffer" -local strip = require("kong.tools.utils").strip +local strip = require("kong.tools.string").strip local cycle_aware_deep_copy = require("kong.tools.table").cycle_aware_deep_copy diff --git a/kong/observability/logs.lua b/kong/observability/logs.lua index 0b7de49fb71f..4aeeb6b6188d 100644 --- a/kong/observability/logs.lua +++ b/kong/observability/logs.lua @@ -12,7 +12,7 @@ end local request_id_get = require "kong.observability.tracing.request_id".get local time_ns = require "kong.tools.time".time_ns local table_merge = require "kong.tools.table".table_merge -local deep_copy = require "kong.tools.utils".deep_copy +local deep_copy = require "kong.tools.table".deep_copy local get_log_level = require "resty.kong.log".get_log_level local constants_log_levels = require "kong.constants".LOG_LEVELS From c9a705818a58b8e01bba708bbd8498ce6876bde1 Mon Sep 17 00:00:00 2001 From: Chrono Date: Thu, 5 Sep 2024 10:47:14 +0800 Subject: [PATCH 12/16] tests(helper): separate module reload function (#13616) It is one of the serial refactors of helpers.lua This function (reload_module) may be used by test cases outside. --- spec/details/module.lua | 11 +++++++++++ spec/helpers.lua | 5 +---- 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 spec/details/module.lua diff --git a/spec/details/module.lua b/spec/details/module.lua new file mode 100644 index 000000000000..d9ef1839f2b5 --- /dev/null +++ b/spec/details/module.lua @@ -0,0 +1,11 @@ + +-- totally clean the module then load it +local function reload(name) + package.loaded[name] = nil + return require(name) +end + + +return { + reload = reload, +} diff --git a/spec/helpers.lua b/spec/helpers.lua index b36da513d757..c1de6b948a64 100644 --- a/spec/helpers.lua +++ b/spec/helpers.lua @@ -46,10 +46,7 @@ local strip = require("kong.tools.string").strip local splitlines = require("pl.stringx").splitlines -local function reload_module(name) - package.loaded[name] = nil - return require(name) -end +local reload_module = require("spec.details.module").reload -- reload some modules when env or _G changes From ad03a0b5c3453ccfc0d2765fa1ac4dbe31a10929 Mon Sep 17 00:00:00 2001 From: Chrono Date: Thu, 5 Sep 2024 10:47:40 +0800 Subject: [PATCH 13/16] tests(helpers): separate grpc functions (#13552) It is one of the serial refactors of helpers.lua KAG-5242 --- spec/details/grpc.lua | 90 +++++++++++++++++++++++++++++++++++++++++++ spec/helpers.lua | 79 +++---------------------------------- 2 files changed, 95 insertions(+), 74 deletions(-) create mode 100644 spec/details/grpc.lua diff --git a/spec/details/grpc.lua b/spec/details/grpc.lua new file mode 100644 index 000000000000..c22d85f01478 --- /dev/null +++ b/spec/details/grpc.lua @@ -0,0 +1,90 @@ +local pl_path = require("pl.path") +local shell = require("resty.shell") +local resty_signal = require("resty.signal") + + +local CONSTANTS = require("spec.details.constants") + + +local function isnewer(path_a, path_b) + if not pl_path.exists(path_a) then + return true + end + if not pl_path.exists(path_b) then + return false + end + return assert(pl_path.getmtime(path_b)) > assert(pl_path.getmtime(path_a)) +end + + +local function make(workdir, specs) + workdir = pl_path.normpath(workdir or pl_path.currentdir()) + + for _, spec in ipairs(specs) do + local targetpath = pl_path.join(workdir, spec.target) + for _, src in ipairs(spec.src) do + local srcpath = pl_path.join(workdir, src) + if isnewer(targetpath, srcpath) then + local ok, _, stderr = shell.run(string.format("cd %s; %s", workdir, spec.cmd), nil, 0) + assert(ok, stderr) + if isnewer(targetpath, srcpath) then + error(string.format("couldn't make %q newer than %q", targetpath, srcpath)) + end + break + end + end + end + + return true +end + + +local grpc_target_proc + + +local function start_grpc_target() + local ngx_pipe = require("ngx.pipe") + assert(make(CONSTANTS.GRPC_TARGET_SRC_PATH, { + { + target = "targetservice/targetservice.pb.go", + src = { "../targetservice.proto" }, + cmd = "protoc --go_out=. --go-grpc_out=. -I ../ ../targetservice.proto", + }, + { + target = "targetservice/targetservice_grpc.pb.go", + src = { "../targetservice.proto" }, + cmd = "protoc --go_out=. --go-grpc_out=. -I ../ ../targetservice.proto", + }, + { + target = "target", + src = { "grpc-target.go", "targetservice/targetservice.pb.go", "targetservice/targetservice_grpc.pb.go" }, + cmd = "go mod tidy && go mod download all && go build", + }, + })) + grpc_target_proc = assert(ngx_pipe.spawn({ CONSTANTS.GRPC_TARGET_SRC_PATH .. "/target" }, { + merge_stderr = true, + })) + + return true +end + + +local function stop_grpc_target() + if grpc_target_proc then + grpc_target_proc:kill(resty_signal.signum("QUIT")) + grpc_target_proc = nil + end +end + + +local function get_grpc_target_port() + return 15010 +end + + +return { + start_grpc_target = start_grpc_target, + stop_grpc_target = stop_grpc_target, + get_grpc_target_port = get_grpc_target_port, +} + diff --git a/spec/helpers.lua b/spec/helpers.lua index c1de6b948a64..b46baacaeec4 100644 --- a/spec/helpers.lua +++ b/spec/helpers.lua @@ -5,6 +5,7 @@ -- @license [Apache 2.0](https://opensource.org/licenses/Apache-2.0) -- @module spec.helpers + local PLUGINS_LIST @@ -37,7 +38,6 @@ local ws_client = require "resty.websocket.client" local table_clone = require "table.clone" local https_server = require "spec.fixtures.https_server" local stress_generator = require "spec.fixtures.stress_generator" -local resty_signal = require "resty.signal" local lfs = require "lfs" local luassert = require "luassert.assert" local uuid = require("kong.tools.uuid").uuid @@ -53,6 +53,7 @@ local reload_module = require("spec.details.module").reload local CONSTANTS = reload_module("spec.details.constants") local shell = reload_module("spec.details.shell") local misc = reload_module("spec.details.misc") +local grpc = reload_module("spec.details.grpc") local conf = shell.conf @@ -3389,76 +3390,6 @@ local function build_go_plugins(path) end end -local function isnewer(path_a, path_b) - if not pl_path.exists(path_a) then - return true - end - if not pl_path.exists(path_b) then - return false - end - return assert(pl_path.getmtime(path_b)) > assert(pl_path.getmtime(path_a)) -end - -local function make(workdir, specs) - workdir = pl_path.normpath(workdir or pl_path.currentdir()) - - for _, spec in ipairs(specs) do - local targetpath = pl_path.join(workdir, spec.target) - for _, src in ipairs(spec.src) do - local srcpath = pl_path.join(workdir, src) - if isnewer(targetpath, srcpath) then - local ok, _, stderr = shell.run(string.format("cd %s; %s", workdir, spec.cmd), nil, 0) - assert(ok, stderr) - if isnewer(targetpath, srcpath) then - error(string.format("couldn't make %q newer than %q", targetpath, srcpath)) - end - break - end - end - end - - return true -end - -local grpc_target_proc -local function start_grpc_target() - local ngx_pipe = require "ngx.pipe" - assert(make(CONSTANTS.GRPC_TARGET_SRC_PATH, { - { - target = "targetservice/targetservice.pb.go", - src = { "../targetservice.proto" }, - cmd = "protoc --go_out=. --go-grpc_out=. -I ../ ../targetservice.proto", - }, - { - target = "targetservice/targetservice_grpc.pb.go", - src = { "../targetservice.proto" }, - cmd = "protoc --go_out=. --go-grpc_out=. -I ../ ../targetservice.proto", - }, - { - target = "target", - src = { "grpc-target.go", "targetservice/targetservice.pb.go", "targetservice/targetservice_grpc.pb.go" }, - cmd = "go mod tidy && go mod download all && go build", - }, - })) - grpc_target_proc = assert(ngx_pipe.spawn({ CONSTANTS.GRPC_TARGET_SRC_PATH .. "/target" }, { - merge_stderr = true, - })) - - return true -end - -local function stop_grpc_target() - if grpc_target_proc then - grpc_target_proc:kill(resty_signal.signum("QUIT")) - grpc_target_proc = nil - end -end - -local function get_grpc_target_port() - return 15010 -end - - --- Start the Kong instance to test against. -- The fixtures passed to this function can be 3 types: -- @@ -4137,9 +4068,9 @@ end get_kong_workers = get_kong_workers, wait_until_no_common_workers = wait_until_no_common_workers, - start_grpc_target = start_grpc_target, - stop_grpc_target = stop_grpc_target, - get_grpc_target_port = get_grpc_target_port, + start_grpc_target = grpc.start_grpc_target, + stop_grpc_target = grpc.stop_grpc_target, + get_grpc_target_port = grpc.get_grpc_target_port, -- plugin compatibility test use_old_plugin = use_old_plugin, From 89669d515fe104fc3a745b961b1baf9866ec09c3 Mon Sep 17 00:00:00 2001 From: Keery Nie Date: Thu, 5 Sep 2024 10:49:29 +0800 Subject: [PATCH 14/16] fix(vault): let vault entity cache key not containing workspace id (#13610) This PR modifies the `cache_key` function of the vault entity to always generate a cache key without workspace id. Vault entity is workspace-able, but our secret rotation timer always run without workspace settings(thus the default workspace is being used), so during secret rotation, the code https://github.com/Kong/kong/blob/4e38b965b922f57febe8652fb96b7d74aeab591a/kong/pdk/vault.lua#L620-L621 will generate a duplicate vault cache with default workspace id for each non-default workspace vault entity, and those cache will never be refreshed. The result of this issue is that when you update a vault entity's configuration inside a non-default workspace, it will never take effect in the secret rotation. Since the prefix of vault entity is unique across workspaces, it should be safe to only use one cache key without workspace id, so that the correct cache is used during secret rotation. FTI-6152 --- .../unreleased/kong/fix-vault-cache-workspace-id.yml | 4 ++++ kong/db/dao/vaults.lua | 10 ++++++++++ spec/02-integration/13-vaults/01-vault_spec.lua | 5 +++++ 3 files changed, 19 insertions(+) create mode 100644 changelog/unreleased/kong/fix-vault-cache-workspace-id.yml diff --git a/changelog/unreleased/kong/fix-vault-cache-workspace-id.yml b/changelog/unreleased/kong/fix-vault-cache-workspace-id.yml new file mode 100644 index 000000000000..2ac640a2e16d --- /dev/null +++ b/changelog/unreleased/kong/fix-vault-cache-workspace-id.yml @@ -0,0 +1,4 @@ +message: | + **Vault**: Fixed an issue where updating a vault entity in a non-default workspace will not take effect. +type: bugfix +scope: Core diff --git a/kong/db/dao/vaults.lua b/kong/db/dao/vaults.lua index 1c7238b15b93..477f6960d7d5 100644 --- a/kong/db/dao/vaults.lua +++ b/kong/db/dao/vaults.lua @@ -84,4 +84,14 @@ function Vaults:load_vault_schemas(vault_set) end +function Vaults:cache_key(prefix) + if type(prefix) == "table" then + prefix = prefix.prefix + end + + -- Always return the cache_key without a workspace because prefix is unique across workspaces + return "vaults:" .. prefix .. ":::::" +end + + return Vaults diff --git a/spec/02-integration/13-vaults/01-vault_spec.lua b/spec/02-integration/13-vaults/01-vault_spec.lua index 0457923e7c64..dd5a58c6de42 100644 --- a/spec/02-integration/13-vaults/01-vault_spec.lua +++ b/spec/02-integration/13-vaults/01-vault_spec.lua @@ -175,5 +175,10 @@ for _, strategy in helpers.each_strategy() do assert.is_equal("{vault://unknown/missing-key}", certificate.key_alt) assert.is_nil(certificate["$refs"]) end) + + it("generate correct cache key", function () + local cache_key = db.vaults:cache_key("test") + assert.equal("vaults:test:::::", cache_key) + end) end) end From e7042d3e0bbc46748a75d967cf6d612958a607bd Mon Sep 17 00:00:00 2001 From: Keery Nie Date: Thu, 5 Sep 2024 15:28:55 +0800 Subject: [PATCH 15/16] fix(aws-lambda): add null handling for multiValueHeaders (#13533) A small PR for adding a null handling code for the multiValueHeaders field. Also contains a small refactor to put isBase64Encode check into the validate function to keep consistency with other fields FTI-6168 --- ...fix-aws-lambda-multi-value-header-null.yml | 3 ++ kong/plugins/aws-lambda/request-util.lua | 10 +++-- .../27-aws-lambda/99-access_spec.lua | 38 +++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/kong/fix-aws-lambda-multi-value-header-null.yml diff --git a/changelog/unreleased/kong/fix-aws-lambda-multi-value-header-null.yml b/changelog/unreleased/kong/fix-aws-lambda-multi-value-header-null.yml new file mode 100644 index 000000000000..253db7bbcfde --- /dev/null +++ b/changelog/unreleased/kong/fix-aws-lambda-multi-value-header-null.yml @@ -0,0 +1,3 @@ +message: "**AWS-Lambda**: Fixed an issue in proxy integration mode that caused internal server error when the `multiValueHeaders` is null." +type: bugfix +scope: Plugin diff --git a/kong/plugins/aws-lambda/request-util.lua b/kong/plugins/aws-lambda/request-util.lua index 478f8c619e89..9e761afc1959 100644 --- a/kong/plugins/aws-lambda/request-util.lua +++ b/kong/plugins/aws-lambda/request-util.lua @@ -1,6 +1,7 @@ local kong = kong local ngx_encode_base64 = ngx.encode_base64 local ngx_decode_base64 = ngx.decode_base64 +local null = ngx.null local cjson = require "cjson.safe" local date = require("date") @@ -89,6 +90,10 @@ local function validate_custom_response(response) return nil, "body must be a string" end + if response.isBase64Encoded ~= nil and type(response.isBase64Encoded) ~= "boolean" then + return nil, "isBase64Encoded must be a boolean" + end + return true end @@ -118,13 +123,10 @@ local function extract_proxy_response(content) local isBase64Encoded = serialized_content.isBase64Encoded if isBase64Encoded == true then body = ngx_decode_base64(body) - - elseif isBase64Encoded ~= false and isBase64Encoded ~= nil then - return nil, "isBase64Encoded must be a boolean" end local multiValueHeaders = serialized_content.multiValueHeaders - if multiValueHeaders then + if multiValueHeaders and multiValueHeaders ~= null then for header, values in pairs(multiValueHeaders) do headers[header] = values end diff --git a/spec/03-plugins/27-aws-lambda/99-access_spec.lua b/spec/03-plugins/27-aws-lambda/99-access_spec.lua index 76b79c64c665..f5a66b7933fb 100644 --- a/spec/03-plugins/27-aws-lambda/99-access_spec.lua +++ b/spec/03-plugins/27-aws-lambda/99-access_spec.lua @@ -194,6 +194,12 @@ for _, strategy in helpers.each_strategy() do service = null, } + local route29 = bp.routes:insert { + hosts = { "lambda29.test" }, + protocols = { "http", "https" }, + service = null, + } + bp.plugins:insert { name = "aws-lambda", route = { id = route1.id }, @@ -580,6 +586,19 @@ for _, strategy in helpers.each_strategy() do } } + bp.plugins:insert { + name = "aws-lambda", + route = { id = route29.id }, + config = { + port = 10001, + aws_key = "mock-key", + aws_secret = "mock-secret", + aws_region = "us-east-1", + function_name = "functionWithNullMultiValueHeaders", + is_proxy_integration = true, + } + } + fixtures.dns_mock:A({ name = "custom.lambda.endpoint", address = "127.0.0.1", @@ -1154,6 +1173,25 @@ for _, strategy in helpers.each_strategy() do assert.equal("Bad Gateway", b.message) end) + it("do not throw error when 'multiValueHeaders' is JSON null", function () + local res = assert(proxy_client:send { + method = "POST", + path = "/post", + headers = { + ["Host"] = "lambda11.test", + ["Content-Type"] = "application/json", + }, + body = { + statusCode = 201, + body = "test", + multiValueHeaders = cjson.null, + } + }) + + local body = assert.res_status(201, res) + assert.same(body, "test") + end) + it("returns HTTP 502 with when response from lambda is not valid JSON", function() local res = assert(proxy_client:send { method = "POST", From 4723a6e8bd53af7f8172f559f7dbca1b98c6cac9 Mon Sep 17 00:00:00 2001 From: "Zhefeng C." <38037704+catbro666@users.noreply.github.com> Date: Thu, 5 Sep 2024 15:57:39 +0800 Subject: [PATCH 16/16] feat(runloop): added the workspace id into the plugin config in the plugins iterator (#13602) the workspace id is needed in some cases, for example in the configure handler. https://konghq.atlassian.net/browse/FTI-6200 --- changelog/unreleased/kong/feat-plugin-conf-ws-id.yml | 3 +++ kong/runloop/plugins_iterator.lua | 1 + spec/02-integration/07-sdk/04-plugin-config_spec.lua | 1 + 3 files changed, 5 insertions(+) create mode 100644 changelog/unreleased/kong/feat-plugin-conf-ws-id.yml diff --git a/changelog/unreleased/kong/feat-plugin-conf-ws-id.yml b/changelog/unreleased/kong/feat-plugin-conf-ws-id.yml new file mode 100644 index 000000000000..06ab3da5da3d --- /dev/null +++ b/changelog/unreleased/kong/feat-plugin-conf-ws-id.yml @@ -0,0 +1,3 @@ +message: Added the workspace id into the plugin config in the plugins iterator. +type: feature +scope: Core diff --git a/kong/runloop/plugins_iterator.lua b/kong/runloop/plugins_iterator.lua index 78a6421011a6..ea0325b02d5b 100644 --- a/kong/runloop/plugins_iterator.lua +++ b/kong/runloop/plugins_iterator.lua @@ -161,6 +161,7 @@ local function get_plugin_config(plugin, name, ws_id) cfg.consumer_id = plugin.consumer and plugin.consumer.id cfg.plugin_instance_name = plugin.instance_name cfg.__plugin_id = plugin.id + cfg.__ws_id = ws_id local key = kong.db.plugins:cache_key(name, cfg.route_id, diff --git a/spec/02-integration/07-sdk/04-plugin-config_spec.lua b/spec/02-integration/07-sdk/04-plugin-config_spec.lua index b56e98e73112..2a02efec0daf 100644 --- a/spec/02-integration/07-sdk/04-plugin-config_spec.lua +++ b/spec/02-integration/07-sdk/04-plugin-config_spec.lua @@ -49,5 +49,6 @@ describe("Plugin configuration", function() local body = assert.status(200, res) local json = cjson.decode(body) assert.equal("test", json.plugin_instance_name) + assert.equal(kong.default_workspace, json.__ws_id) end) end)