diff --git a/CHANGELOG.md b/CHANGELOG.md index 57ac0ec0f10..2926e9466cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,12 @@ - The PDK is no longer versioned [#8585](https://github.com/Kong/kong/pull/8585) +#### Plugins + +- The HTTP-log plugin `headers` field now only takes a single string per header name, + where it previously took an array of values + [#6992](https://github.com/Kong/kong/pull/6992) + ### Deprecations - The `go_pluginserver_exe` and `go_plugins_dir` directives are no longer supported. diff --git a/kong-2.8.0-0.rockspec b/kong-2.8.0-0.rockspec index a521d8738a1..ab3e687430f 100644 --- a/kong-2.8.0-0.rockspec +++ b/kong-2.8.0-0.rockspec @@ -235,6 +235,7 @@ build = { ["kong.db.migrations.operations.200_to_210"] = "kong/db/migrations/operations/200_to_210.lua", ["kong.db.migrations.operations.210_to_211"] = "kong/db/migrations/operations/210_to_211.lua", ["kong.db.migrations.operations.212_to_213"] = "kong/db/migrations/operations/212_to_213.lua", + ["kong.db.migrations.operations.280_to_300"] = "kong/db/migrations/operations/280_to_300.lua", ["kong.pdk"] = "kong/pdk/init.lua", ["kong.pdk.private.checks"] = "kong/pdk/private/checks.lua", @@ -299,6 +300,8 @@ build = { ["kong.plugins.http-log.handler"] = "kong/plugins/http-log/handler.lua", ["kong.plugins.http-log.schema"] = "kong/plugins/http-log/schema.lua", + ["kong.plugins.http-log.migrations"] = "kong/plugins/http-log/migrations/init.lua", + ["kong.plugins.http-log.migrations.001_280_to_300"] = "kong/plugins/http-log/migrations/001_280_to_300.lua", ["kong.plugins.file-log.handler"] = "kong/plugins/file-log/handler.lua", ["kong.plugins.file-log.schema"] = "kong/plugins/file-log/schema.lua", diff --git a/kong/db/migrations/operations/280_to_300.lua b/kong/db/migrations/operations/280_to_300.lua new file mode 100644 index 00000000000..016e4bf2caa --- /dev/null +++ b/kong/db/migrations/operations/280_to_300.lua @@ -0,0 +1,119 @@ +-- Helper module for 280_to_300 migration operations. +-- +-- Operations are versioned and specific to a migration so they remain +-- fixed in time and are not modified for use in future migrations. +-- +-- If you want to reuse these operations in a future migration, +-- copy the functions over to a new versioned module. + + +local function render(template, keys) + return (template:gsub("$%(([A-Z_]+)%)", keys)) +end + + +-------------------------------------------------------------------------------- +-- Postgres operations for Workspace migration +-------------------------------------------------------------------------------- + + +local postgres = { + + up = {}, + + teardown = { + + ------------------------------------------------------------------------------ + -- General function to fixup a plugin configuration + fixup_plugin_config = function(_, connector, plugin_name, fixup_fn) + local pgmoon_json = require("pgmoon.json") + for plugin, err in connector:iterate("SELECT id, name, config FROM plugins") do + if err then + return nil, err + end + + if plugin.name == plugin_name then + local fix = fixup_fn(plugin.config) + + if fix then + local sql = render( + "UPDATE plugins SET config = $(NEW_CONFIG)::jsonb WHERE id = '$(ID)'", { + NEW_CONFIG = pgmoon_json.encode_json(plugin.config), + ID = plugin.id, + }) + + local _, err = connector:query(sql) + if err then + return nil, err + end + end + end + end + + return true + end, + }, + +} + + +-------------------------------------------------------------------------------- +-- Cassandra operations for Workspace migration +-------------------------------------------------------------------------------- + + +local cassandra = { + + up = {}, + + teardown = { + + ------------------------------------------------------------------------------ + -- General function to fixup a plugin configuration + fixup_plugin_config = function(_, connector, plugin_name, fixup_fn) + local coordinator = assert(connector:get_stored_connection()) + local cassandra = require("cassandra") + local cjson = require("cjson") + + for rows, err in coordinator:iterate("SELECT id, name, config FROM plugins") do + if err then + return nil, err + end + + for i = 1, #rows do + local plugin = rows[i] + if plugin.name == plugin_name then + if type(plugin.config) ~= "string" then + return nil, "plugin config is not a string" + end + local config = cjson.decode(plugin.config) + local fix = fixup_fn(config) + + if fix then + local _, err = coordinator:execute("UPDATE plugins SET config = ? WHERE id = ?", { + cassandra.text(cjson.encode(config)), + cassandra.uuid(plugin.id) + }) + if err then + return nil, err + end + end + end + end + end + + return true + end, + + } + +} + + +-------------------------------------------------------------------------------- + + +return { + postgres = postgres, + cassandra = cassandra, +} diff --git a/kong/plugins/http-log/migrations/001_280_to_300.lua b/kong/plugins/http-log/migrations/001_280_to_300.lua new file mode 100644 index 00000000000..aeaf1f5d263 --- /dev/null +++ b/kong/plugins/http-log/migrations/001_280_to_300.lua @@ -0,0 +1,36 @@ +local operations = require "kong.db.migrations.operations.280_to_300" + + +local function ws_migration_teardown(ops) + return function(connector) + return ops:fixup_plugin_config(connector, "http-log", function(config) + local updated = false + if type(config) == "table" then -- not required, but let's be defensive here + local headers = config.headers + if type(headers) == "table" then + for header_name, value_array in pairs(headers) do + if type(value_array) == "table" then + -- only update if it's still a table, so it is reentrant + headers[header_name] = value_array[1] or "empty header value" + updated = true + end + end + end + end + return updated + end) + end +end + + +return { + postgres = { + up = "", + teardown = ws_migration_teardown(operations.postgres.teardown), + }, + + cassandra = { + up = "", + teardown = ws_migration_teardown(operations.cassandra.teardown), + }, +} diff --git a/kong/plugins/http-log/migrations/init.lua b/kong/plugins/http-log/migrations/init.lua new file mode 100644 index 00000000000..8a19b72d32f --- /dev/null +++ b/kong/plugins/http-log/migrations/init.lua @@ -0,0 +1,3 @@ +return { + "001_280_to_300", +} diff --git a/kong/plugins/http-log/schema.lua b/kong/plugins/http-log/schema.lua index c01f6cbe226..3af661c21cb 100644 --- a/kong/plugins/http-log/schema.lua +++ b/kong/plugins/http-log/schema.lua @@ -17,7 +17,8 @@ return { { retry_count = { type = "integer", default = 10 }, }, { queue_size = { type = "integer", default = 1 }, }, { flush_timeout = { type = "number", default = 2 }, }, - { headers = typedefs.headers { + { headers = { + type = "map", keys = typedefs.header_name { match_none = { { @@ -34,7 +35,10 @@ return { }, }, }, - } }, + values = { + type = "string", + }, + }}, { custom_fields_by_lua = typedefs.lua_code }, }, custom_validator = function(config) diff --git a/spec/03-plugins/03-http-log/01-log_spec.lua b/spec/03-plugins/03-http-log/01-log_spec.lua index b389bae6abd..68bfcd105fa 100644 --- a/spec/03-plugins/03-http-log/01-log_spec.lua +++ b/spec/03-plugins/03-http-log/01-log_spec.lua @@ -101,7 +101,7 @@ for _, strategy in helpers.each_strategy() do .. helpers.mock_upstream_port .. "/post_auth_log/basic_auth" .. "/testuser/testpassword", - headers = { ["Hello-World"] = { "hi!", "there" } }, + headers = { ["Hello-World"] = "hi there" }, } } @@ -449,7 +449,7 @@ for _, strategy in helpers.each_strategy() do ok = ok + 1 end if name == "hello-world" then - assert.same({ "hi!", "there" }, value) + assert.equal("hi there", value) ok = ok + 1 end end diff --git a/spec/03-plugins/03-http-log/02-schema_spec.lua b/spec/03-plugins/03-http-log/02-schema_spec.lua index 71f7e6d21a8..40670d61f76 100644 --- a/spec/03-plugins/03-http-log/02-schema_spec.lua +++ b/spec/03-plugins/03-http-log/02-schema_spec.lua @@ -38,19 +38,34 @@ describe(PLUGIN_NAME .. ": (schema)", function() local ok, err = validate({ http_endpoint = "http://myservice.com/path", headers = { - ["X-My-Header"] = { "123" } + ["X-My-Header"] = "123", + ["X-Your-Header"] = "abc", } }) assert.is_nil(err) assert.is_truthy(ok) end) + it("does not accept empty header values", function() + local ok, err = validate({ + http_endpoint = "http://myservice.com/path", + headers = { + ["X-My-Header"] = "", + } + }) + assert.same({ + config = { + headers = "length must be at least 1" + } }, err) + assert.is_falsy(ok) + end) it("does not accept Host header", function() local ok, err = validate({ http_endpoint = "http://myservice.com/path", headers = { - Host = { "MyHost" } + ["X-My-Header"] = "123", + Host = "MyHost", } }) assert.same({ @@ -65,7 +80,7 @@ describe(PLUGIN_NAME .. ": (schema)", function() local ok, err = validate({ http_endpoint = "http://myservice.com/path", headers = { - ["coNTEnt-Length"] = { "123" } -- also validate casing + ["coNTEnt-Length"] = "123", -- also validate casing } }) assert.same({ @@ -80,27 +95,28 @@ describe(PLUGIN_NAME .. ": (schema)", function() local ok, err = validate({ http_endpoint = "http://myservice.com/path", headers = { - ["coNTEnt-Type"] = { "bad" } -- also validate casing + ["coNTEnt-Type"] = "bad" -- also validate casing } }) - assert.same({ - config = { - headers = "cannot contain 'Content-Type' header" - } }, err) - assert.is_falsy(ok) - end) + assert.same({ + config = { + headers = "cannot contain 'Content-Type' header" + } }, err) + assert.is_falsy(ok) + end) - it("does not accept userinfo in URL and 'Authorization' header", function() - local ok, err = validate({ - http_endpoint = "http://hi:there@myservice.com/path", - headers = { - ["AuthoRIZATion"] = { "bad" } -- also validate casing - } - }) - assert.same({ - config = "specifying both an 'Authorization' header and user info in 'http_endpoint' is not allowed" - }, err) - assert.is_falsy(ok) - end) + it("does not accept userinfo in URL and 'Authorization' header", function() + local ok, err = validate({ + http_endpoint = "http://hi:there@myservice.com/path", + headers = { + ["AuthoRIZATion"] = "bad" -- also validate casing + } + }) + assert.same({ + config = "specifying both an 'Authorization' header and user info in 'http_endpoint' is not allowed" + }, err) + assert.is_falsy(ok) end) + +end)