Validate plugin PRIORITY and VERSION fields upon loading

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
 

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
 

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
 
 

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)

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

spec/fixtures/custom_plugins/kong/plugins/api-override/handler.lua

@@ -1 +1,4 @@
-return {}
+return {
+  PRIORITY = 1000,
+  VERSION = "1.0",
+}

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

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

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

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

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

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

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

spec/fixtures/custom_plugins/kong/plugins/foreign-entity/handler.lua

@@ -1,3 +1,4 @@
 return {
-  PRIORITY = 1
+  PRIORITY = 1,
+  VERSION = "1.0",
 }

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)

spec/fixtures/custom_plugins/kong/plugins/invalidations/handler.lua

@@ -6,7 +6,8 @@ local counts = {}
 
 
 local Invalidations = {
-  PRIORITY = 0
+  PRIORITY = 0,
+  VERSION = "1.0",
 }
 
 

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()

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

spec/fixtures/custom_plugins/kong/plugins/stream-api-echo/handler.lua

@@ -1,4 +1,5 @@
 
 return {
   PRIORITY = 1000,
+  VERSION = "1.0",
 }

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"