Skip to content

Commit

Permalink
fix(bk-rate-limit): update the key to use new limits if the config ch…
Browse files Browse the repository at this point in the history
…anged (#62)
  • Loading branch information
wklken authored Nov 1, 2023
1 parent 8b80aec commit 9c84097
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 1 deletion.
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

0 comments on commit 9c84097

Please sign in to comment.