From 018c5bf3b195f9f2ce83e921aab52ecc9bf7adee Mon Sep 17 00:00:00 2001 From: Michael Martin Date: Wed, 4 Oct 2023 11:59:09 -0700 Subject: [PATCH] feat(wasm): combine config and json_config fields This allows JSON filter configuration to live in the same .config attribute as raw configuration. Filters must have an associated JSON schema in order to use this feature. --- .../kong/wasm-filter-json-config.yml | 3 + kong/db/schema/entities/filter_chains.lua | 18 +++--- kong/db/schema/init.lua | 2 +- kong/db/schema/metaschema.lua | 25 +++++++++ kong/runloop/wasm.lua | 16 ++++-- .../20-wasm/01-admin-api_spec.lua | 11 ++-- spec/02-integration/20-wasm/02-db_spec.lua | 56 ++++++++++++++----- .../20-wasm/09-filter-meta_spec.lua | 38 ++++++++++--- 8 files changed, 126 insertions(+), 43 deletions(-) create mode 100644 changelog/unreleased/kong/wasm-filter-json-config.yml diff --git a/changelog/unreleased/kong/wasm-filter-json-config.yml b/changelog/unreleased/kong/wasm-filter-json-config.yml new file mode 100644 index 000000000000..70216e7830cd --- /dev/null +++ b/changelog/unreleased/kong/wasm-filter-json-config.yml @@ -0,0 +1,3 @@ +message: Support JSON in Wasm filter configuration +type: feature +scope: Core diff --git a/kong/db/schema/entities/filter_chains.lua b/kong/db/schema/entities/filter_chains.lua index c83d59a82f60..b01fd91ff677 100644 --- a/kong/db/schema/entities/filter_chains.lua +++ b/kong/db/schema/entities/filter_chains.lua @@ -20,8 +20,7 @@ local constants = require "kong.constants" --- ---@field name string ---@field enabled boolean ----@field config string|nil ----@field json_config any|nil +---@field config any|nil local filter = { @@ -29,28 +28,25 @@ local filter = { fields = { { name = { type = "string", required = true, one_of = wasm.filter_names, err = "no such filter", }, }, - { config = { type = "string", required = false, }, }, { enabled = { type = "boolean", default = true, required = true, }, }, - { json_config = { + { config = { type = "json", required = false, json_schema = { parent_subschema_key = "name", namespace = constants.SCHEMA_NAMESPACES.PROXY_WASM_FILTERS, optional = true, + default = { + -- filters with no user-defined JSON schema may accept an optional + -- config, but only as a string + type = { "string", "null" }, + }, }, }, }, }, - entity_checks = { - { mutually_exclusive = { - "config", - "json_config", - }, - }, - }, } return { diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index ce29a41038d4..0a3db763ad6d 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -1135,7 +1135,7 @@ validate_fields = function(self, input) if json_subschema_key then local schema_name = json_schema.namespace .. "/" .. json_subschema_key - inline_schema = json.get_schema(schema_name) + inline_schema = json.get_schema(schema_name) or json_schema.default if inline_schema then _, errors[k] = json_validate(v, inline_schema) diff --git a/kong/db/schema/metaschema.lua b/kong/db/schema/metaschema.lua index 8d829daf9ec3..6483aaab5260 100644 --- a/kong/db/schema/metaschema.lua +++ b/kong/db/schema/metaschema.lua @@ -128,6 +128,30 @@ local validators = { -- means that input validation will fail. When `optional` is `true`, the input -- is always accepted. -- +-- Schemas which use this dynamic reference format can also optionally supply +-- a default inline schema, which will be evaluated when the dynamic schema +-- does not exist: +-- +-- ```lua +-- local record = { +-- type = "record", +-- fields = { +-- { name = { type = "string" } }, +-- { config = { +-- type = "json", +-- json_schema = { +-- namespace = "my-record-type", +-- parent_subschema_key = "name", +-- default = { +-- { type = { "string", "null" } }, +-- }, +-- }, +-- }, +-- }, +-- }, +-- } +-- ``` +-- local json_metaschema = { type = "record", fields = { @@ -135,6 +159,7 @@ local json_metaschema = { { parent_subschema_key = { type = "string" }, }, { optional = { type = "boolean", }, }, { inline = { type = "any", custom_validator = json_lib.validate_schema, }, }, + { default = { type = "any", custom_validator = json_lib.validate_schema, }, }, }, entity_checks = { { at_least_one_of = { "inline", "namespace", "parent_subschema_key" }, }, diff --git a/kong/runloop/wasm.lua b/kong/runloop/wasm.lua index 00a6054b4495..99622410a507 100644 --- a/kong/runloop/wasm.lua +++ b/kong/runloop/wasm.lua @@ -398,10 +398,18 @@ local function rebuild_state(db, version, old_state) for _, filter in ipairs(chain.filters) do if filter.enabled then - -- serialize all JSON configurations up front - if not filter.config and filter.json_config ~= nil then - filter.config = cjson_encode(filter.json_config) - filter.json_config = nil + -- Serialize all JSON configurations up front + -- + -- NOTE: there is a subtle difference between a raw, non-JSON filter + -- configuration which requires no encoding (e.g. `my config bytes`) + -- and a JSON filter configuration of type=string, which should be + -- JSON-encoded (e.g. `"my config string"`). + -- + -- Properly disambiguating between the two cases requires an + -- inspection of the filter metadata, which is not guaranteed to be + -- present on data-plane/proxy nodes. + if filter.config ~= nil and type(filter.config) ~= "string" then + filter.config = cjson_encode(filter.config) end end end diff --git a/spec/02-integration/20-wasm/01-admin-api_spec.lua b/spec/02-integration/20-wasm/01-admin-api_spec.lua index 51ce7d5f1069..fe2079f938ed 100644 --- a/spec/02-integration/20-wasm/01-admin-api_spec.lua +++ b/spec/02-integration/20-wasm/01-admin-api_spec.lua @@ -4,7 +4,6 @@ local utils = require "kong.tools.utils" local fmt = string.format local FILTER_PATH = assert(helpers.test_conf.wasm_filters_path) -local NULL = ngx.null local function json(body) return { @@ -207,9 +206,9 @@ describe("wasm admin API [#" .. strategy .. "]", function() assert.same({ "foo", "bar" }, patched.tags) assert.is_false(patched.enabled) assert.equals(2, #patched.filters) - assert.same({ name = "tests", config = "123", enabled = true, json_config = NULL }, + assert.same({ name = "tests", config = "123", enabled = true }, patched.filters[1]) - assert.same({ name = "tests", config = "456", enabled = false, json_config = NULL }, + assert.same({ name = "tests", config = "456", enabled = false }, patched.filters[2]) end) end) @@ -366,9 +365,9 @@ describe("wasm admin API [#" .. strategy .. "]", function() assert.same({ "foo", "bar" }, patched.tags) assert.is_false(patched.enabled) assert.equals(2, #patched.filters) - assert.same({ name = "tests", config = "123", enabled = true, json_config = NULL }, + assert.same({ name = "tests", config = "123", enabled = true }, patched.filters[1]) - assert.same({ name = "tests", config = "456", enabled = false, json_config = NULL }, + assert.same({ name = "tests", config = "456", enabled = false }, patched.filters[2]) end) end) @@ -428,7 +427,7 @@ describe("wasm admin API [#" .. strategy .. "]", function() fcs = { assert(bp.filter_chains:insert({ filters = { - { name = "tests", config = ngx.null, enabled = true }, + { name = "tests", config = nil, enabled = true }, { name = "response_transformer", config = "{}", enabled = false }, }, service = { id = service.id }, diff --git a/spec/02-integration/20-wasm/02-db_spec.lua b/spec/02-integration/20-wasm/02-db_spec.lua index 1969f4b238e6..b19b252ac6cb 100644 --- a/spec/02-integration/20-wasm/02-db_spec.lua +++ b/spec/02-integration/20-wasm/02-db_spec.lua @@ -371,7 +371,13 @@ describe("wasm DB entities [#" .. strategy .. "]", function() end) describe(".config", function() - it("is an optional string", function() + local schema_name = "proxy-wasm-filters/test" + + lazy_teardown(function() + schema_lib.remove_schema(schema_name) + end) + + it("is an optional string when no json schema exists", function() local service = assert(db.services:insert({ url = "http://example.test", })) @@ -381,7 +387,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - config = "foo", + config = nil, } } })) @@ -395,18 +401,40 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - config = nil, + config = "my config", } } })) - end) - end) - describe(".json_config", function() - local schema_name = "proxy-wasm-filters/test" + assert.falsy(dao:insert({ + service = { id = service.id }, + filters = { + { + name = "test", + config = 123, + } + } + })) - lazy_teardown(function() - schema_lib.remove_schema(schema_name) + assert.falsy(dao:insert({ + service = { id = service.id }, + filters = { + { + name = "test", + config = true, + } + } + })) + + assert.falsy(dao:insert({ + service = { id = service.id }, + filters = { + { + name = "test", + config = { a = 1, b = 2 }, + } + } + })) end) it("is validated against user schema", function() @@ -414,7 +442,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() url = "http://example.test", })) - schema_lib.add_schema("proxy-wasm-filters/test", { + schema_lib.add_schema(schema_name, { type = "object", properties = { foo = { type = "string" }, @@ -429,7 +457,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - json_config = { + config = { foo = "foo string", bar = { a = 1, b = 2 }, }, @@ -446,7 +474,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - json_config = { + config = { foo = 123, bar = { a = 1, b = 2 }, }, @@ -465,7 +493,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - json_config = ngx.null, + config = ngx.null, } } }) @@ -481,7 +509,7 @@ describe("wasm DB entities [#" .. strategy .. "]", function() filters = { { name = "test", - json_config = nil, + config = nil, } } }) diff --git a/spec/02-integration/20-wasm/09-filter-meta_spec.lua b/spec/02-integration/20-wasm/09-filter-meta_spec.lua index 84a94eaf498d..f458c01b27e0 100644 --- a/spec/02-integration/20-wasm/09-filter-meta_spec.lua +++ b/spec/02-integration/20-wasm/09-filter-meta_spec.lua @@ -158,7 +158,7 @@ describe("filter metadata [#" .. strategy .. "]", function() filters = { { name = "rt_with_validation", - json_config = {}, -- empty + config = {}, -- empty }, }, }) @@ -185,7 +185,7 @@ describe("filter metadata [#" .. strategy .. "]", function() assert.same({ filters = { { - json_config = "property add is required" + config = "property add is required" } } }, body.fields) @@ -197,7 +197,7 @@ describe("filter metadata [#" .. strategy .. "]", function() filters = { { name = "rt_with_validation", - json_config = { + config = { add = { headers = { "x-foo:123", @@ -217,7 +217,7 @@ describe("filter metadata [#" .. strategy .. "]", function() end).has_no_error() end) - it("filters without config schemas are not validated", function() + it("filters without config schemas can only accept a string", function() local host = random_name() .. ".test" local res = create_filter_chain(host, { @@ -225,11 +225,36 @@ describe("filter metadata [#" .. strategy .. "]", function() filters = { { name = "rt_no_validation", - json_config = { + config = { add = { headers = 1234, }, - }, + } + }, + }, + }) + + assert.response(res).has.status(400) + local body = assert.response(res).has.jsonbody() + assert.same({ + filters = { + { config = "wrong type: expected one of string, null, got object" } + } + }, body.fields) + end) + + it("filters without config schemas are not validated", function() + local host = random_name() .. ".test" + local res = create_filter_chain(host, { + name = random_name(), + filters = { + { + name = "rt_no_validation", + config = cjson.encode({ + add = { + headers = 123, + }, + }), }, }, }) @@ -242,7 +267,6 @@ describe("filter metadata [#" .. strategy .. "]", function() assert.logfile().has.line("failed parsing filter config", true, 0) end).has_no_error() end) - end) end)