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(bk-rate-limit): update the key to use new limits if the config changed #62

Merged
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 14 additions & 0 deletions src/apisix/plugins/bk-rate-limit/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
-- redis_database: 0
-- redis_timeout: 1001
--
local apisix_plugin = require("apisix.plugin")
local core = require("apisix.core")
local rate_limit_redis = require("apisix.plugins.bk-rate-limit.rate-limit-redis")
local lrucache = core.lrucache.new(
Expand Down Expand Up @@ -77,6 +78,14 @@ local _M = {
},
}

local function gen_limit_key(conf, key)
-- Here we use plugin-level conf version to prevent the counter from being resetting
-- because of the change elsewhere.
-- e.g. conf_version = 1969078430
local new_key = key .. ':' .. apisix_plugin.conf_version(conf)
return new_key
end

---Create rate-limit-redis object
---@param plugin_name string @apisix plugin name
---@return table @rate-limit-redis object
Expand Down Expand Up @@ -108,6 +117,7 @@ function _M.rate_limit(conf, ctx, plugin_name, key, count, time_window)
return 500
end

key = gen_limit_key(conf, key)
core.log.info("limit key: ", key)

local delay, remaining, reset = lim:incoming(key, count, time_window)
Expand Down Expand Up @@ -145,4 +155,8 @@ function _M.rate_limit(conf, ctx, plugin_name, key, count, time_window)
end
end

if _TEST then
_M.gen_limit_key = gen_limit_key
end

return _M
2 changes: 2 additions & 0 deletions src/apisix/plugins/bk-resource-rate-limit.lua
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ function _M.access(conf, ctx)
end

-- TODO: make it lazy, share the key with other plugins
-- FIXME: should change the bk_reosurce_name to bk_resource_id if you need more accurate rate limit
-- while the developer may change the bk_resource_name from the frontend page.
local key = table_concat(
{
bk_app_code,
Expand Down
1 change: 0 additions & 1 deletion src/apisix/plugins/bk-stage-rate-limit.lua
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ function _M.access(conf, ctx)
else
for i, rate in ipairs(rates) do
-- here we should add the rate index into key, otherwise the rate limit will be shared(will be wrong)
-- FIXME: if the rate changes, will wait for the period to effect
local limit_key = key .. ":" .. tostring(i)
local code = ratelimit.rate_limit(conf, ctx, plugin_name, limit_key, rate.tokens, rate.period)
if code then
Expand Down
19 changes: 19 additions & 0 deletions src/apisix/tests/bk-rate-limit/test-init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,25 @@ describe(
end
)

context(
"gen_limit_key", function()
it("ok", function()
local conf = {}
local key = ratelimit.gen_limit_key(conf, "key")
assert.equal(key, "key:223132457")
assert.equal(conf._version, "223132457")

-- mock the conf change
conf._version = nil
conf["hello"] = "world"
key = ratelimit.gen_limit_key(conf, "key")
assert.equal(key, "key:3624485329")
end
)
end
)


context(
"rate_limit", function()
local conf
Expand Down