feat(request-aware-table): enable checks when log_level is debug

CHANGELOG/unreleased/kong/11017.yaml

@@ -0,0 +1,5 @@
+message: Add a request-aware table able to detect accesses from different requests.
+type: feature
+scope: Core
+prs:
+  - 11017

kong/tools/request_aware_table.lua

@@ -1,5 +1,8 @@
 local table_clear = require "table.clear"
 
+local LOG_LEVELS = require "kong.constants".LOG_LEVELS
+local get_log_level = require("resty.kong.log").get_log_level
+
 local get_phase = ngx.get_phase
 local fmt = string.format
 local ngx_var = ngx.var
@@ -16,15 +19,18 @@ local NGX_VAR_PHASES = {
 }
 
 
+local function is_debug_mode()
+  local log_level = get_log_level(LOG_LEVELS[kong.configuration.log_level])
+  return log_level == LOG_LEVELS.debug
+end
+
+
 --- Request aware table constructor
 -- Wraps an existing table (or creates a new one) with request-aware access
 -- logic to protect the underlying data from race conditions.
 -- @param data_table (optional) The target table to use as underlying data
--- @param request_awareness_mode (optional) The mode for request awareness
---        - "off": No request awareness mode
---        - any other value, or nil: Control access for every r/w operation
 -- @return The newly created table with request-aware access
-local function new(data_table, request_awareness_mode)
+local function new(data_table)
   if data_table and type(data_table) ~= "table" then
     error("data_table must be a table", 2)
   end
@@ -65,15 +71,15 @@ local function new(data_table, request_awareness_mode)
 
   local _proxy_mt = {
     __index = function(_, k)
-      if request_awareness_mode ~= "off" then
+      if is_debug_mode() then
         enforce_sequential_access()
       end
 
       return data[k]
     end,
 
     __newindex = function(_, k, v)
-      if request_awareness_mode ~= "off" then
+      if is_debug_mode() then
         enforce_sequential_access()
       end
 

spec/01-unit/28-request-aware-table_spec.lua

@@ -3,6 +3,7 @@ local tablex = require "pl.tablex"
 local rat
 
 describe("Request aware table", function()
+  local orig_t = {}
   local old_ngx
   local tab
 
@@ -12,41 +13,41 @@ describe("Request aware table", function()
       get_phase = function() return "access" end,
       var = {},
     }
+    _G.kong = {
+      configuration = {
+        log_level = "debug",
+      },
+    }
     rat = require "kong.tools.request_aware_table"
   end)
 
   lazy_teardown(function()
     _G.ngx = old_ngx
   end)
 
-  describe("with concurrency check enabled", function()
-    local orig_t = {}
+  before_each(function()
+    tab = rat.new(orig_t)
+  end)
 
-    before_each(function()
-      tab = rat.new(orig_t, "on")
-    end)
+  it("allows defining a custom clear function", function()
+    orig_t.persist = "persistent_value"
+    orig_t.foo = "bar"
+    orig_t.baz = "qux"
 
-    it("allows defining a custom clear function", function()
-      orig_t.persist = "persistent_value"
-      orig_t.foo = "bar"
-      orig_t.baz = "qux"
-
-      -- custom clear function that keeps persistent_value
-      tab.clear(function(t)
-        for k in pairs(t) do
-          if k ~= "persist" then
-            t[k] = nil
-          end
+    -- custom clear function that keeps persistent_value
+    tab.clear(function(t)
+      for k in pairs(t) do
+        if k ~= "persist" then
+          t[k] = nil
         end
-      end)
-
-      -- confirm persistent_value is the only key left
-      assert.same(1, tablex.size(orig_t))
-      assert.equal("persistent_value", tab.persist)
-
-      -- clear the whole table and confirm it's empty
-      tab.clear()
-      assert.same(0, tablex.size(orig_t))
+      end
     end)
+
+    -- confirm persistent_value is the only key left
+    assert.same(1, tablex.size(orig_t))
+    assert.equal("persistent_value", tab.persist)
+    -- clear the whole table and confirm it's empty
+    tab.clear()
+    assert.same(0, tablex.size(orig_t))
   end)
 end)

spec/02-integration/05-proxy/31-request-aware-table_spec.lua

@@ -1,94 +1,89 @@
 local helpers = require "spec.helpers"
 
+local LOG_LEVELS = {
+  "debug",
+  "info",
+  -- any other log level behaves the same as "info"
+}
 
-local function clear_table(client, checks)
+
+local function clear_table(client)
   local res = client:get("/", {
     query = {
-      checks = checks,
       clear = true,
     }
   })
   assert.response(res).has.status(200)
   assert.logfile().has.no.line("[error]", true)
 end
 
-for _, checks in ipairs({ true, false }) do
-for _, strategy in helpers.each_strategy() do
-  describe("request aware table tests [#" .. strategy .. "] .. checks=" .. tostring(checks), function()
-    local client
-    lazy_setup(function()
-      local bp = helpers.get_db_utils(strategy, {
-        "plugins",
-        "routes",
-        "services",
-      }, {
-        "request-aware-table"
-      })
+for _, log_level in ipairs(LOG_LEVELS) do
+  local concurrency_checks = log_level == "debug"
 
-      local service = assert(bp.services:insert({
-        url = helpers.mock_upstream_url
-      }))
+  for _, strategy in helpers.each_strategy() do
+    describe("request aware table tests [#" .. strategy .. "] concurrency checks: " .. tostring(concurrency_checks), function()
+      local client
+      lazy_setup(function()
+        local bp = helpers.get_db_utils(strategy, {
+          "plugins",
+          "routes",
+          "services",
+        }, {
+          "request-aware-table"
+        })
 
-      local route = bp.routes:insert({
-        service = service,
-        paths = { "/" }
-      })
+        local service = assert(bp.services:insert({
+          url = helpers.mock_upstream_url
+        }))
 
-      bp.plugins:insert({
-        name = "request-aware-table",
-        route = { id = route.id },
-      })
+        local route = bp.routes:insert({
+          service = service,
+          paths = { "/" }
+        })
 
-      helpers.start_kong({
-        database = strategy,
-        plugins = "bundled, request-aware-table",
-        nginx_conf = "spec/fixtures/custom_nginx.template",
-      })
-    end)
+        bp.plugins:insert({
+          name = "request-aware-table",
+          route = { id = route.id },
+        })
 
-    lazy_teardown(function()
-      helpers.stop_kong()
-    end)
+        helpers.start_kong({
+          database = strategy,
+          plugins = "bundled, request-aware-table",
+          nginx_conf = "spec/fixtures/custom_nginx.template",
+          log_level = log_level,
+        })
+      end)
 
-    before_each(function()
-      helpers.clean_logfile()
-      client = helpers.proxy_client()
-      clear_table(client, checks)
-    end)
+      lazy_teardown(function()
+        helpers.stop_kong()
+      end)
 
-    after_each(function()
-      if client then
-        client:close()
-      end
-    end)
+      before_each(function()
+        helpers.clean_logfile()
+        client = helpers.proxy_client()
+        clear_table(client)
+      end)
+
+      after_each(function()
+        if client then
+          client:close()
+        end
+      end)
 
-    describe("with concurrency check enabled", function()
       it("allows access when there are no race conditions", function()
-        local res = client:get("/", {
-          query = {
-            checks = checks,
-          }
-        })
+        local res = client:get("/")
         assert.response(res).has.status(200)
         assert.logfile().has.no.line("[error]", true)
       end)
       it("denies access when there are race conditions and checks are enabled", function()
         -- access from request 1 (don't clear)
-        local ok, r = pcall(client.get, client, "/", {
-          query = {
-            checks = checks,
-          },
-        })
+        local ok, r = pcall(client.get, client, "/")
         assert(ok)
         assert.response(r).has.status(200)
 
         -- access from request 2
-        ok, r = pcall(client.get, client, "/", {
-          query = {
-            checks = checks,
-          },
-        })
-        if checks then
+        ok, r = pcall(client.get, client, "/")
+        if concurrency_checks then
           assert(not ok)
           assert.logfile().has.line("race condition detected", true)
         else
@@ -100,22 +95,16 @@ for _, strategy in helpers.each_strategy() do
         -- access from request 1 (clear)
         local r = client:get("/", {
           query = {
-            checks = checks,
             clear = true,
           },
         })
         assert.response(r).has.status(200)
 
         -- access from request 2
-        r = client:get("/", {
-          query = {
-            checks = checks,
-          },
-        })
+        r = client:get("/")
         assert.response(r).has.status(200)
         assert.logfile().has.no.line("[error]", true)
       end)
     end)
-  end)
-end
+  end
 end

spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua

@@ -10,29 +10,24 @@ local _M = {
   VERSION = "1.0",
 }
 
-local checks_tab = RAT.new({}, "on")
-local no_checks_tab = RAT.new({}, "off")
+local tab = RAT.new({})
 
 local function access_tables()
   local query = kong.request.get_query()
-  local tab
-
-  if query.checks ~= "false" then
-    tab = checks_tab
-  else
-    tab = no_checks_tab
-  end
 
   if query.clear == "true" and get_phase() == "access" then
+    -- clear before access
     tab.clear()
   end
 
   -- write access
   tab.foo = "bar"
-  -- read access
-  ngx.log(ngx.DEBUG, "accessing to tab.foo" .. tab.foo)
+  tab.bar = "baz"
+  -- read/write access
+  tab.baz = tab.foo .. tab.bar
 
   if query.clear == "true" and get_phase() == "body_filter" then
+    -- clear after access
     tab.clear()
   end
 end