From 89a339b421373385bd5d3bd01b78c439c1cc9ea0 Mon Sep 17 00:00:00 2001 From: Thijs Schreijer Date: Fri, 20 May 2022 14:55:42 +0200 Subject: [PATCH] feat(plugins) validate PRIORITY and VERSION metadata --- CHANGELOG.md | 5 + kong/api/routes/kong.lua | 8 +- kong/db/dao/plugins.lua | 83 ++++++++++++---- kong/plugins/pre-function/handler.lua | 2 +- spec/02-integration/03-db/03-plugins_spec.lua | 97 ++++++++++++++++++- .../kong/plugins/api-override/handler.lua | 5 +- .../plugins/ctx-tests-response/handler.lua | 3 +- .../kong/plugins/ctx-tests/handler.lua | 3 +- .../enable-buffering-response/handler.lua | 3 +- .../kong/plugins/enable-buffering/handler.lua | 3 +- .../plugins/error-generator-last/handler.lua | 4 +- .../kong/plugins/error-generator/handler.lua | 3 +- .../plugins/error-handler-log/handler.lua | 2 +- .../kong/plugins/foreign-entity/handler.lua | 3 +- .../plugins/init-worker-lua-error/handler.lua | 1 + .../kong/plugins/invalidations/handler.lua | 3 +- .../kong/plugins/reports-api/handler.lua | 3 +- .../kong/plugins/short-circuit/handler.lua | 3 +- .../kong/plugins/stream-api-echo/handler.lua | 1 + .../plugins/tcp-trace-exporter/handler.lua | 1 + .../kong/plugins/transformations/handler.lua | 3 +- .../kong/plugins/unique-foreign/handler.lua | 3 +- 22 files changed, 198 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f59cd8ea79a..7a2bf215081 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,9 @@ [#8698](https://github.com/Kong/kong/pull/8698) - The PDK is no longer versioned [#8585](https://github.com/Kong/kong/pull/8585) +- Plugins MUST now have a valid `PRIORITY` (integer) and `VERSION` ("x.y.z" format) + field in their `handler.lua` file, otherwise the plugin will fail to load. + [#8836](https://github.com/Kong/kong/pull/8836) #### Plugins @@ -137,6 +140,8 @@ - **AWS Lambda**: `aws_region` field must be set through either plugin config or environment variables, allow both `host` and `aws_region` fields, and always apply SigV4 signature. [#8082](https://github.com/Kong/kong/pull/8082) +- The pre-functions plugin changed priority from `+inf` to `1000000`. + [#8836](https://github.com/Kong/kong/pull/8836) ### Deprecations diff --git a/kong/api/routes/kong.lua b/kong/api/routes/kong.lua index ed8382d82d2..e3cf728a1a1 100644 --- a/kong/api/routes/kong.lua +++ b/kong/api/routes/kong.lua @@ -91,15 +91,9 @@ return { local available_plugins = {} for name in pairs(kong.configuration.loaded_plugins) do - local pr = kong.db.plugins.handlers[name].PRIORITY - if pr ~= nil then - if type(pr) ~= "number" or math.abs(pr) == math.huge then - pr = tostring(pr) - end - end available_plugins[name] = { version = kong.db.plugins.handlers[name].VERSION, - priority = pr, + priority = kong.db.plugins.handlers[name].PRIORITY, } end diff --git a/kong/db/dao/plugins.lua b/kong/db/dao/plugins.lua index 70d0b47bb38..65af6a58ebb 100644 --- a/kong/db/dao/plugins.lua +++ b/kong/db/dao/plugins.lua @@ -4,7 +4,7 @@ local DAO = require "kong.db.dao" local plugin_loader = require "kong.db.schema.plugin_loader" local reports = require "kong.reports" local plugin_servers = require "kong.runloop.plugin_servers" - +local version = require "version" local Plugins = {} @@ -126,31 +126,76 @@ local function implements(plugin, method) end -local function load_plugin_handler(plugin) - -- NOTE: no version _G.kong (nor PDK) in plugins main chunk +local load_plugin_handler do - local plugin_handler = "kong.plugins." .. plugin .. ".handler" - local ok, handler = utils.load_module_if_exists(plugin_handler) - if not ok then - ok, handler = plugin_servers.load_plugin(plugin) - if type(handler) == "table" then - handler._go = true + local function valid_priority(prio) + if type(prio) ~= "number" or + prio ~= prio or -- NaN + math.abs(prio) == math.huge or + math.floor(prio) ~= prio then + return false end + return true end - if not ok then - return nil, plugin .. " plugin is enabled but not installed;\n" .. handler - end + -- Returns the cleaned version string, only x.y.z part + local function valid_version(v) + if type(v) ~= "string" then + return false + end + local vparsed = version(v) + if not vparsed or vparsed[4] ~= nil then + return false + end - if implements(handler, "response") and - (implements(handler, "header_filter") or implements(handler, "body_filter")) - then - return nil, fmt( - "Plugin %q can't be loaded because it implements both `response` " .. - "and `header_filter` or `body_filter` methods.\n", plugin) + return tostring(vparsed) end - return handler + + function load_plugin_handler(plugin) + -- NOTE: no version _G.kong (nor PDK) in plugins main chunk + + local plugin_handler = "kong.plugins." .. plugin .. ".handler" + local ok, handler = utils.load_module_if_exists(plugin_handler) + if not ok then + ok, handler = plugin_servers.load_plugin(plugin) + if type(handler) == "table" then + handler._go = true + end + end + + if not ok then + return nil, plugin .. " plugin is enabled but not installed;\n" .. handler + end + + if type(handler) == "table" then + + if not valid_priority(handler.PRIORITY) then + return nil, fmt( + "Plugin %q cannot be loaded because its PRIORITY field is not " .. + "a valid integer number, got: %q.\n", plugin, tostring(handler.PRIORITY)) + end + + local v = valid_version(handler.VERSION) + if v then + handler.VERSION = v -- update to cleaned version string + else + return nil, fmt( + "Plugin %q cannot be loaded because its VERSION field does not " .. + "follow the \"x.y.z\" format, got: %q.\n", plugin, tostring(handler.VERSION)) + end + end + + if implements(handler, "response") and + (implements(handler, "header_filter") or implements(handler, "body_filter")) + then + return nil, fmt( + "Plugin %q can't be loaded because it implements both `response` " .. + "and `header_filter` or `body_filter` methods.\n", plugin) + end + + return handler + end end diff --git a/kong/plugins/pre-function/handler.lua b/kong/plugins/pre-function/handler.lua index b4dfc798ace..2f906d56e5c 100644 --- a/kong/plugins/pre-function/handler.lua +++ b/kong/plugins/pre-function/handler.lua @@ -1 +1 @@ -return require("kong.plugins.pre-function._handler")(math.huge) +return require("kong.plugins.pre-function._handler")(1000000) diff --git a/spec/02-integration/03-db/03-plugins_spec.lua b/spec/02-integration/03-db/03-plugins_spec.lua index 6501f0d0eb5..1f98f4e07ca 100644 --- a/spec/02-integration/03-db/03-plugins_spec.lua +++ b/spec/02-integration/03-db/03-plugins_spec.lua @@ -193,8 +193,8 @@ for _, strategy in helpers.each_strategy() do local ok, err = db.plugins:load_plugin_schemas({ ["plugin-with-custom-dao"] = true, }) - assert.truthy(ok) assert.is_nil(err) + assert.truthy(ok) assert.same("I was implemented for " .. strategy, db.custom_dao:custom_method()) end) @@ -206,6 +206,101 @@ for _, strategy in helpers.each_strategy() do assert.falsy(ok) assert.match("missing plugin is enabled but not installed", err, 1, true) end) + + describe("with bad PRIORITY fails; ", function() + setup(function() + local schema = {} + package.loaded["kong.plugins.NaN_priority.schema"] = schema + package.loaded["kong.plugins.NaN_priority.handler"] = { PRIORITY = 0/0, VERSION = "1.0" } + package.loaded["kong.plugins.huge_negative.schema"] = schema + package.loaded["kong.plugins.huge_negative.handler"] = { PRIORITY = -math.huge, VERSION = "1.0" } + package.loaded["kong.plugins.string_priority.schema"] = schema + package.loaded["kong.plugins.string_priority.handler"] = { PRIORITY = "abc", VERSION = "1.0" } + end) + + teardown(function() + package.loaded["kong.plugins.NaN_priority.schema"] = nil + package.loaded["kong.plugins.NaN_priority.handler"] = nil + package.loaded["kong.plugins.huge_negative.schema"] = nil + package.loaded["kong.plugins.huge_negative.handler"] = nil + package.loaded["kong.plugins.string_priority.schema"] = nil + package.loaded["kong.plugins.string_priority.handler"] = nil + end) + + it("NaN", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["NaN_priority"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "NaN_priority" cannot be loaded because its PRIORITY field is not a valid integer number, got: "nan"', err, 1, true) + end) + + it("-math.huge", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["huge_negative"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "huge_negative" cannot be loaded because its PRIORITY field is not a valid integer number, got: "-inf"', err, 1, true) + end) + + it("string", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["string_priority"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "string_priority" cannot be loaded because its PRIORITY field is not a valid integer number, got: "abc"', err, 1, true) + end) + + end) + + describe("with bad VERSION fails; ", function() + setup(function() + local schema = {} + package.loaded["kong.plugins.no_version.schema"] = schema + package.loaded["kong.plugins.no_version.handler"] = { PRIORITY = 1000, VERSION = nil } + package.loaded["kong.plugins.too_many.schema"] = schema + package.loaded["kong.plugins.too_many.handler"] = { PRIORITY = 1000, VERSION = "1.0.0.0" } + package.loaded["kong.plugins.number.schema"] = schema + package.loaded["kong.plugins.number.handler"] = { PRIORITY = 1000, VERSION = 123 } + end) + + teardown(function() + package.loaded["kong.plugins.no_version.schema"] = nil + package.loaded["kong.plugins.no_version.handler"] = nil + package.loaded["kong.plugins.too_many.schema"] = nil + package.loaded["kong.plugins.too_many.handler"] = nil + package.loaded["kong.plugins.number.schema"] = nil + package.loaded["kong.plugins.number.handler"] = nil + end) + + it("without version", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["no_version"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "no_version" cannot be loaded because its VERSION field does not follow the "x.y.z" format, got: "nil"', err, 1, true) + end) + + it("too many components", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["too_many"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "too_many" cannot be loaded because its VERSION field does not follow the "x.y.z" format, got: "1.0.0.0"', err, 1, true) + end) + + it("number", function() + local ok, err = db.plugins:load_plugin_schemas({ + ["number"] = true, + }) + assert.falsy(ok) + assert.match('Plugin "number" cannot be loaded because its VERSION field does not follow the "x.y.z" format, got: "123"', err, 1, true) + end) + + end) + end) + end) -- kong.db [strategy] + end diff --git a/spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua index a564707544f..8d6ab00cdf2 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua @@ -1 +1,4 @@ -return {} +return { + PRIORITY = 1000, + VERSION = "1.0", +} diff --git a/spec/fixtures/custom_plugins/kong/plugins/ctx-tests-response/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/ctx-tests-response/handler.lua index 85dbd948d77..74128c72dab 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/ctx-tests-response/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/ctx-tests-response/handler.lua @@ -191,7 +191,8 @@ end local CtxTests = { - PRIORITY = -math.huge + PRIORITY = -1000000, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/ctx-tests/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/ctx-tests/handler.lua index 825d2cfff16..d9b6f0e14fb 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/ctx-tests/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/ctx-tests/handler.lua @@ -199,7 +199,8 @@ end local CtxTests = { - PRIORITY = -math.huge + PRIORITY = -1000000, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/enable-buffering-response/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/enable-buffering-response/handler.lua index 83c10e87310..2d205fc49a0 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/enable-buffering-response/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/enable-buffering-response/handler.lua @@ -3,7 +3,8 @@ local kong = kong local EnableBuffering = { - PRIORITY = math.huge + PRIORITY = 1000000, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/enable-buffering/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/enable-buffering/handler.lua index f7f4fc6d2ea..83c598429dc 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/enable-buffering/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/enable-buffering/handler.lua @@ -3,7 +3,8 @@ local kong = kong local EnableBuffering = { - PRIORITY = math.huge + PRIORITY = 1000000, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/error-generator-last/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/error-generator-last/handler.lua index 4c6d040f4e4..5601b28e397 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/error-generator-last/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/error-generator-last/handler.lua @@ -4,8 +4,8 @@ local error = error local ErrorGeneratorLastHandler = {} -ErrorGeneratorLastHandler.PRIORITY = -math.huge - +ErrorGeneratorLastHandler.PRIORITY = -1000000 +ErrorGeneratorLastHandler.VERSION = "1.0" function ErrorGeneratorLastHandler:init_worker() end diff --git a/spec/fixtures/custom_plugins/kong/plugins/error-generator/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/error-generator/handler.lua index 79e3bb4ae50..4c67f22d1e0 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/error-generator/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/error-generator/handler.lua @@ -3,7 +3,7 @@ local error = error local ErrorGeneratorHandler = { VERSION = "0.1-t", - PRIORITY = math.huge, + PRIORITY = 1000000, } @@ -60,4 +60,5 @@ function ErrorGeneratorHandler:log(conf) end + return ErrorGeneratorHandler diff --git a/spec/fixtures/custom_plugins/kong/plugins/error-handler-log/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/error-handler-log/handler.lua index 9c8459f0f1a..37bb47e93e4 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/error-handler-log/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/error-handler-log/handler.lua @@ -6,7 +6,7 @@ local ErrorHandlerLog = {} ErrorHandlerLog.PRIORITY = 1000 - +ErrorHandlerLog.VERSION = "1.0" local function register(phase) local ws_id = ngx.ctx.workspace or kong.default_workspace diff --git a/spec/fixtures/custom_plugins/kong/plugins/foreign-entity/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/foreign-entity/handler.lua index 553fae644fb..bd97e4e821a 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/foreign-entity/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/foreign-entity/handler.lua @@ -1,3 +1,4 @@ return { - PRIORITY = 1 + PRIORITY = 1, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua index 4fc115deb22..8530edd2876 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua @@ -2,6 +2,7 @@ local InitWorkerLuaError = {} InitWorkerLuaError.PRIORITY = 1000 +InitWorkerLuaError.VERSION = "1.0" function InitWorkerLuaError:init_worker(conf) diff --git a/spec/fixtures/custom_plugins/kong/plugins/invalidations/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/invalidations/handler.lua index d704574e8de..91ccfd67e5a 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/invalidations/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/invalidations/handler.lua @@ -6,7 +6,8 @@ local counts = {} local Invalidations = { - PRIORITY = 0 + PRIORITY = 0, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/reports-api/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/reports-api/handler.lua index 0ad96860339..64d3b3a10e3 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/reports-api/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/reports-api/handler.lua @@ -1,5 +1,6 @@ local ReportsApiHandler = { - PRIORITY = 1000 + PRIORITY = 1000, + VERSION = "1.0", } function ReportsApiHandler:preread() diff --git a/spec/fixtures/custom_plugins/kong/plugins/short-circuit/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/short-circuit/handler.lua index d1990c21af8..7f465c4254e 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/short-circuit/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/short-circuit/handler.lua @@ -11,7 +11,7 @@ local init_worker_called = false local ShortCircuitHandler = { VERSION = "0.1-t", - PRIORITY = math.huge, + PRIORITY = 1000000, } @@ -45,5 +45,4 @@ function ShortCircuitHandler:preread(conf) return exit(conf.status) end - return ShortCircuitHandler diff --git a/spec/fixtures/custom_plugins/kong/plugins/stream-api-echo/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/stream-api-echo/handler.lua index 9bee1ba35c7..271257907fe 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/stream-api-echo/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/stream-api-echo/handler.lua @@ -1,4 +1,5 @@ return { PRIORITY = 1000, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/tcp-trace-exporter/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/tcp-trace-exporter/handler.lua index 22ccfb58a2d..5067a9d62f9 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/tcp-trace-exporter/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/tcp-trace-exporter/handler.lua @@ -10,6 +10,7 @@ local to_hex = str.to_hex local _M = { PRIORITY = 1001, + VERSION = "1.0", } local tracer_name = "tcp-trace-exporter" diff --git a/spec/fixtures/custom_plugins/kong/plugins/transformations/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/transformations/handler.lua index 553fae644fb..bd97e4e821a 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/transformations/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/transformations/handler.lua @@ -1,3 +1,4 @@ return { - PRIORITY = 1 + PRIORITY = 1, + VERSION = "1.0", } diff --git a/spec/fixtures/custom_plugins/kong/plugins/unique-foreign/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/unique-foreign/handler.lua index 553fae644fb..bd97e4e821a 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/unique-foreign/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/unique-foreign/handler.lua @@ -1,3 +1,4 @@ return { - PRIORITY = 1 + PRIORITY = 1, + VERSION = "1.0", }