Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: the issue that plugins in global rule may be cached to route #4867

Merged
merged 2 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apisix/core.lua
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,4 @@ return {
tablepool = require("tablepool"),
resolver = require("apisix.core.resolver"),
os = require("apisix.core.os"),
empty_tab = {},
}
2 changes: 1 addition & 1 deletion apisix/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty plugins also need to be released.

core.tablepool.release("plugins", api_ctx.plugins)
end

Expand Down
2 changes: 1 addition & 1 deletion apisix/plugin.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return plugins or {}
return plugins or core.tablepool.fetch("plugins", 0, 0)

end

local route_plugin_conf = route_conf and route_conf.value.plugins
Expand Down
119 changes: 119 additions & 0 deletions t/node/global-rule.t
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,122 @@ GET /hello
changed
--- no_error_log
[error]



=== TEST 17: delete global rules, ensure no stale data remain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad title, need to more detail

--- 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]