From 69654d86f2e8a07f09f1e3f0bbbf8db0ddd0280e Mon Sep 17 00:00:00 2001 From: nic-chen Date: Sun, 22 Aug 2021 17:28:56 +0800 Subject: [PATCH 1/2] fix: the issue that plugins in global rule may be cached to route --- apisix/core.lua | 1 - apisix/init.lua | 2 +- apisix/plugin.lua | 2 +- t/node/global-rule.t | 119 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) diff --git a/apisix/core.lua b/apisix/core.lua index 4b5bc2b99b9d..726b13e78949 100644 --- a/apisix/core.lua +++ b/apisix/core.lua @@ -50,5 +50,4 @@ return { tablepool = require("tablepool"), resolver = require("apisix.core.resolver"), os = require("apisix.core.os"), - empty_tab = {}, } diff --git a/apisix/init.lua b/apisix/init.lua index dfd7044614e2..14e249a9af12 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -694,7 +694,7 @@ function _M.http_log_phase() end core.ctx.release_vars(api_ctx) - if api_ctx.plugins and api_ctx.plugins ~= core.empty_tab then + if api_ctx.plugins and core.table.nkeys(api_ctx.plugins) > 0 then core.tablepool.release("plugins", api_ctx.plugins) end diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 849b1e96bfc4..97f3f6eea9af 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -307,7 +307,7 @@ function _M.filter(conf, plugins, route_conf) trace_plugins_info_for_debug(nil) -- when 'plugins' is given, always return 'plugins' itself instead -- of another one - return plugins or core.empty_tab + return plugins or {} end local route_plugin_conf = route_conf and route_conf.value.plugins diff --git a/t/node/global-rule.t b/t/node/global-rule.t index b4fae8579a32..3b08fb7a3351 100644 --- a/t/node/global-rule.t +++ b/t/node/global-rule.t @@ -369,3 +369,122 @@ GET /hello changed --- no_error_log [error] + + + +=== TEST 17: delete global rules, ensure no stale data remain +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers', + ngx.HTTP_PUT, + [[{ + "username": "test", + "plugins": { + "basic-auth": { + "username": "test", + "password": "test" + } + }, + "desc": "test description" + }]] + ) + + if code >= 300 then + ngx.status = code + return + end + + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + } + }]] + ) + + if code >= 300 then + ngx.status = code + return + end + + local code, body = t('/apisix/admin/global_rules/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "basic-auth": {} + } + }]] + ) + + if code >= 300 then + ngx.status = code + return + end + + -- sleep for data sync + ngx.sleep(0.5) + + -- hit the route without authorization, should be 401 + local code, body = t('/hello', + ngx.HTTP_PUT + ) + + if code ~= 401 then + ngx.status = 400 + return + end + + -- hit the route with authorization + local code, body = t('/hello', + ngx.HTTP_PUT, + nil, + nil, + {Authorization = "Basic dGVzdDp0ZXN0"} + ) + + if code ~= 200 then + ngx.status = code + return + end + + local code, body = t('/apisix/admin/global_rules/1', + ngx.HTTP_DELETE, + [[{ + "plugins": { + "basic-auth": {} + } + }]] + ) + + if code >= 300 then + ngx.status = code + return + end + + ngx.sleep(0.5) + -- hit the route with authorization, should be 200 + local code, body = t('/hello', + ngx.HTTP_PUT + ) + + if code ~= 200 then + ngx.status = code + return + end + + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] From 11edba417a53c05b278aebae835a273a73f033b4 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Mon, 23 Aug 2021 16:04:47 +0800 Subject: [PATCH 2/2] fix review --- apisix/init.lua | 2 +- apisix/plugin.lua | 2 +- t/node/global-rule.t | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apisix/init.lua b/apisix/init.lua index 14e249a9af12..a6ad1a77c851 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -694,7 +694,7 @@ function _M.http_log_phase() end core.ctx.release_vars(api_ctx) - if api_ctx.plugins and core.table.nkeys(api_ctx.plugins) > 0 then + if api_ctx.plugins then core.tablepool.release("plugins", api_ctx.plugins) end diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 97f3f6eea9af..c158011d294d 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -307,7 +307,7 @@ function _M.filter(conf, plugins, route_conf) trace_plugins_info_for_debug(nil) -- when 'plugins' is given, always return 'plugins' itself instead -- of another one - return plugins or {} + return plugins or core.tablepool.fetch("plugins", 0, 0) end local route_plugin_conf = route_conf and route_conf.value.plugins diff --git a/t/node/global-rule.t b/t/node/global-rule.t index 3b08fb7a3351..9947e53a1ce0 100644 --- a/t/node/global-rule.t +++ b/t/node/global-rule.t @@ -372,7 +372,7 @@ changed -=== TEST 17: delete global rules, ensure no stale data remain +=== TEST 17: global rule works with the consumer, after deleting the global rule, ensure no stale plugins remaining --- config location /t { content_by_lua_block {