Fixing the plugins loader

kong/core/plugins_iterator.lua

@@ -3,8 +3,6 @@ local constants = require "kong.constants"
 local responses = require "kong.tools.responses"
 
 local table_remove = table.remove
-local table_insert = table.insert
-local ipairs = ipairs
 
 --- Load the configuration for a plugin entry in the DB.
 -- Given an API, a Consumer and a plugin name, retrieve the plugin's configuration if it exists.
@@ -15,7 +13,6 @@ local ipairs = ipairs
 -- @treturn table Plugin retrieved from the cache or database.
 local function load_plugin_configuration(api_id, consumer_id, plugin_name)
   local cache_key = cache.plugin_key(plugin_name, api_id, consumer_id)
-
   local plugin = cache.get_or_set(cache_key, function()
     local rows, err = dao.plugins:find_by_keys {
       api_id = api_id,
@@ -30,7 +27,6 @@ local function load_plugin_configuration(api_id, consumer_id, plugin_name)
       return table_remove(rows, 1)
     else
       -- insert a cached value to not trigger too many DB queries.
-      -- for now, this will lock the cache for the expiraiton duration.
       return {null = true}
     end
   end)
@@ -40,78 +36,47 @@ local function load_plugin_configuration(api_id, consumer_id, plugin_name)
   end
 end
 
-local function load_plugins_for_req(loaded_plugins)
-  if ngx.ctx.plugins_for_request == nil then
-    local t = {}
-    -- Build an array of plugins that must be executed for this particular request.
-    -- A plugin is considered to be executed if there is a row in the DB which contains:
-    -- 1. the API id (contained in ngx.ctx.api.id, retrived by the core resolver)
-    -- 2. a Consumer id, in which case it overrides any previous plugin found in 1.
-    --    this use case will be treated once the authentication plugins have run (access phase).
-    -- Such a row will contain a `config` value, which is a table.
-    if ngx.ctx.api ~= nil then
-      for _, plugin in ipairs(loaded_plugins) do
-        local plugin_configuration = load_plugin_configuration(ngx.ctx.api.id, nil, plugin.name)
-        if plugin_configuration ~= nil then
-          table_insert(t, {plugin, plugin_configuration})
-        end
-      end
-    end
-
-    ngx.ctx.plugins_for_request = t
-  end
-end
-
 --- Plugins for request iterator.
 -- Iterate over the plugin loaded for a request, stored in `ngx.ctx.plugins_for_request`.
--- @param[type=string] context_name Name of the current nginx context. We don't use `ngx.get_phase()` simply because we can avoid it.
+-- @param[type=boolean] is_access_or_certificate_context Tells if the context is access_by_lua_block. We don't use `ngx.get_phase()` simply because we can avoid it.
 -- @treturn function iterator
-local function iter_plugins_for_req(loaded_plugins, context_name)
-  -- In case previous contexts did not run, we need to handle
-  -- the case when plugins have not been fetched for a given request.
-  -- This will simply make it so the look gets skipped if no API is set in the context
-  load_plugins_for_req(loaded_plugins)
+local function iter_plugins_for_req(loaded_plugins, is_access_or_certificate_context)
+  if not ngx.ctx.plugins_for_request then 
+    ngx.ctx.plugins_for_request = {} 
+  end
 
   local i = 0
-
-  -- Iterate on plugins to execute for this request until
-  -- a plugin with a handler for the given context is found.
-  local function get_next()
+  local function get_next_plugin()
     i = i + 1
-    local p = ngx.ctx.plugins_for_request[i]
-    if p == nil then
-      return
-    end
-
-    local plugin, plugin_configuration = p[1], p[2]
-    if plugin.handler[context_name] == nil then
-      ngx.log(ngx.DEBUG, "No handler for "..context_name.." phase on "..plugin.name.." plugin")
-      return get_next()
-    end
-
-    return plugin, plugin_configuration
+    return loaded_plugins[i]
   end
 
-  return function()
-    local plugin, plugin_configuration = get_next()
-
-    -- Check if any Consumer was authenticated during the access phase.
-    -- If so, retrieve the configuration for this Consumer which overrides
-    -- the API-wide configuration.
-    if plugin ~= nil and context_name == "access" then
-      local consumer_id = ngx.ctx.authenticated_credential and ngx.ctx.authenticated_credential.consumer_id or nil
-      if consumer_id ~= nil then
-        local consumer_plugin_configuration = load_plugin_configuration(ngx.ctx.api.id, consumer_id, plugin.name)
-        if consumer_plugin_configuration ~= nil then
-          -- This Consumer has a special configuration when this plugin gets executed.
-          -- Override this plugin's configuration for this request.
-          plugin_configuration = consumer_plugin_configuration
-          ngx.ctx.plugins_for_request[i][2] = consumer_plugin_configuration
+  local function get_next()
+    local plugin = get_next_plugin()
+    if plugin and ngx.ctx.api then
+      if is_access_or_certificate_context then
+        ngx.ctx.plugins_for_request[plugin.name] = load_plugin_configuration(ngx.ctx.api.id, nil, plugin.name)
+
+        local consumer_id = ngx.ctx.authenticated_credential and ngx.ctx.authenticated_credential.consumer_id or nil
+        if consumer_id and not plugin.schema.no_consumer then
+          local consumer_plugin_configuration = load_plugin_configuration(ngx.ctx.api.id, consumer_id, plugin.name)
+          if consumer_plugin_configuration then
+            ngx.ctx.plugins_for_request[plugin.name] = consumer_plugin_configuration
+          end
         end
       end
+
+      -- Return the configuration
+      if ngx.ctx.plugins_for_request[plugin.name] then
+        return plugin, ngx.ctx.plugins_for_request[plugin.name]
+      end
+
+      return get_next() -- Load next plugin
     end
+  end
 
-    return plugin, plugin_configuration
+  return function()
+    return get_next()
   end
 end
 

kong/kong.lua

@@ -73,10 +73,15 @@ local function load_node_plugins(configuration)
     if not loaded then
       error("The following plugin has been enabled in the configuration but it is not installed on the system: "..v)
     else
+      local loaded, plugin_schema_mod = utils.load_module_if_exists("kong.plugins."..v..".schema")
+      if not loaded then
+        error("Cannot find the schema for the following plugin: "..v)
+      end
       ngx.log(ngx.DEBUG, "Loading plugin: "..v)
       table_insert(sorted_plugins, {
         name = v,
-        handler = plugin_handler_mod()
+        handler = plugin_handler_mod(),
+        schema = plugin_schema_mod
       })
     end
 
@@ -154,15 +159,15 @@ end
 function Kong.ssl_certificate()
   core.certificate.before()
 
-  for plugin, plugin_conf in plugins_iterator(loaded_plugins, "certificate") do
+  for plugin, plugin_conf in plugins_iterator(loaded_plugins, true) do
     plugin.handler:certificate(plugin_conf)
   end
 end
 
 function Kong.access()
   core.access.before()
 
-  for plugin, plugin_conf in plugins_iterator(loaded_plugins, "access") do
+  for plugin, plugin_conf in plugins_iterator(loaded_plugins, true) do
     plugin.handler:access(plugin_conf)
   end
 
@@ -172,23 +177,23 @@ end
 function Kong.header_filter()
   core.header_filter.before()
 
-  for plugin, plugin_conf in plugins_iterator(loaded_plugins, "header_filter") do
+  for plugin, plugin_conf in plugins_iterator(loaded_plugins) do
     plugin.handler:header_filter(plugin_conf)
   end
 
   core.header_filter.after()
 end
 
 function Kong.body_filter()
-  for plugin, plugin_conf in plugins_iterator(loaded_plugins, "body_filter") do
+  for plugin, plugin_conf in plugins_iterator(loaded_plugins) do
     plugin.handler:body_filter(plugin_conf)
   end
 
   core.body_filter.after()
 end
 
 function Kong.log()
-  for plugin, plugin_conf in plugins_iterator(loaded_plugins, "log") do
+  for plugin, plugin_conf in plugins_iterator(loaded_plugins) do
     plugin.handler:log(plugin_conf)
   end
 

spec/plugins/rate-limiting/access_spec.lua

@@ -21,13 +21,14 @@ describe("RateLimiting Plugin", function()
     spec_helper.prepare_db()
     spec_helper.insert_fixtures {
       api = {
-        { name = "tests-rate-limiting1", request_host = "test3.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting2", request_host = "test4.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting3", request_host = "test5.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting4", request_host = "test6.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting5", request_host = "test7.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting6", request_host = "test8.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-rate-limiting7", request_host = "test9.com", upstream_url = "http://mockbin.com" }
+        { request_host = "test3.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test4.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test5.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test6.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test7.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test8.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test9.com", upstream_url = "http://mockbin.com" },
+        { request_host = "test10.com", upstream_url = "http://mockbin.com" }
       },
       consumer = {
         { custom_id = "provider_123" },
@@ -42,7 +43,9 @@ describe("RateLimiting Plugin", function()
         { name = "rate-limiting", config = { minute = 33 }, __api = 4 },
         { name = "rate-limiting", config = { minute = 6, async = true }, __api = 5 },
         { name = "rate-limiting", config = { minute = 6, continue_on_error = false }, __api = 6 },
-        { name = "rate-limiting", config = { minute = 6, continue_on_error = true }, __api = 7 }
+        { name = "rate-limiting", config = { minute = 6, continue_on_error = true }, __api = 7 },
+        { name = "key-auth", config = {}, __api = 8 },
+        { name = "rate-limiting", config = { minute = 6, continue_on_error = true }, __api = 8, __consumer = 1 }
       },
       keyauth_credential = {
         { key = "apikey122", __consumer = 1 },
@@ -108,9 +111,7 @@ describe("RateLimiting Plugin", function()
   end)
 
   describe("With authentication", function()
-
     describe("Default plugin", function()
-
       it("should get blocked if exceeding limit", function()
         -- Default rate-limiting plugin for this API says 6/minute
         local limit = 6
@@ -128,11 +129,9 @@ describe("RateLimiting Plugin", function()
         assert.are.equal(429, status)
         assert.are.equal("API rate limit exceeded", body.message)
       end)
-
     end)
 
     describe("Plugin customized for specific consumer", function()
-
       it("should get blocked if exceeding limit", function()
         -- This plugin says this consumer can make 4 requests/minute, not 6 like the default
         local limit = 8
@@ -149,8 +148,23 @@ describe("RateLimiting Plugin", function()
         assert.are.equal(429, status)
         assert.are.equal("API rate limit exceeded", body.message)
       end)
+      it("should get blocked if the only rate-limiting plugin existing is per consumer and not per API", function()
+        -- This plugin says this consumer can make 4 requests/minute, not 6 like the default
+        local limit = 6
 
-    end)
+        for i = 1, limit do
+          local _, status, headers = http_client.get(STUB_GET_URL, {apikey = "apikey122"}, {host = "test10.com"})
+          assert.are.equal(200, status)
+          assert.are.same(tostring(limit), headers["x-ratelimit-limit-minute"])
+          assert.are.same(tostring(limit - i), headers["x-ratelimit-remaining-minute"])
+        end
+
+        local response, status = http_client.get(STUB_GET_URL, {apikey = "apikey122"}, {host = "test10.com"})
+        local body = cjson.decode(response)
+        assert.are.equal(429, status)
+        assert.are.equal("API rate limit exceeded", body.message)
+      end)
+    end) 
   end)
 
   describe("Async increment", function()
@@ -231,4 +245,5 @@ describe("RateLimiting Plugin", function()
       assert.falsy(headers["x-ratelimit-remaining-minute"])
     end)
   end)
+
 end)