fix(http-log) set headers with single value, not array (#6992)

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.

kong-2.8.0-0.rockspec

@@ -234,6 +234,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",
@@ -296,6 +297,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",

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,
+}

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

kong/plugins/http-log/migrations/init.lua

@@ -0,0 +1,3 @@
+return {
+  "001_280_to_300",
+}

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)

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

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)