From bb26f81a71e99913bb0c33a2bffbc61cb9c41dc0 Mon Sep 17 00:00:00 2001
From: wklken <wklken@gmail.com>
Date: Wed, 25 Oct 2023 12:00:28 +0800
Subject: [PATCH 1/3] fix(bk-rate-limit): update the key to use new limits if
 the config changed

---
 src/apisix/plugins/bk-rate-limit/init.lua     | 10 ++++++++++
 src/apisix/plugins/bk-resource-rate-limit.lua |  2 ++
 src/apisix/plugins/bk-stage-rate-limit.lua    |  1 -
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/apisix/plugins/bk-rate-limit/init.lua b/src/apisix/plugins/bk-rate-limit/init.lua
index 7b0f8be..4d5cdb7 100644
--- a/src/apisix/plugins/bk-rate-limit/init.lua
+++ b/src/apisix/plugins/bk-rate-limit/init.lua
@@ -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(
@@ -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
@@ -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)
diff --git a/src/apisix/plugins/bk-resource-rate-limit.lua b/src/apisix/plugins/bk-resource-rate-limit.lua
index 142a8d7..4775ee4 100644
--- a/src/apisix/plugins/bk-resource-rate-limit.lua
+++ b/src/apisix/plugins/bk-resource-rate-limit.lua
@@ -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,
diff --git a/src/apisix/plugins/bk-stage-rate-limit.lua b/src/apisix/plugins/bk-stage-rate-limit.lua
index 4851fc7..a233322 100644
--- a/src/apisix/plugins/bk-stage-rate-limit.lua
+++ b/src/apisix/plugins/bk-stage-rate-limit.lua
@@ -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

From 3bf9e087aad32f861c786f2fb80a5ef129fcd657 Mon Sep 17 00:00:00 2001
From: wklken <wklken@gmail.com>
Date: Wed, 25 Oct 2023 14:19:34 +0800
Subject: [PATCH 2/3] test(bk-rate-limit/init.lua): add unittest for
 gen_limit_key

---
 src/apisix/plugins/bk-rate-limit/init.lua    |  4 ++++
 src/apisix/tests/bk-rate-limit/test-init.lua | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/src/apisix/plugins/bk-rate-limit/init.lua b/src/apisix/plugins/bk-rate-limit/init.lua
index 4d5cdb7..128e74b 100644
--- a/src/apisix/plugins/bk-rate-limit/init.lua
+++ b/src/apisix/plugins/bk-rate-limit/init.lua
@@ -155,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
diff --git a/src/apisix/tests/bk-rate-limit/test-init.lua b/src/apisix/tests/bk-rate-limit/test-init.lua
index 75a1d26..df83a44 100644
--- a/src/apisix/tests/bk-rate-limit/test-init.lua
+++ b/src/apisix/tests/bk-rate-limit/test-init.lua
@@ -48,6 +48,19 @@ 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")
+                end
+            )
+            end
+        )
+
+
         context(
             "rate_limit", function()
                 local conf

From 9aec07e36cd509fb6ed9992519b8f5aaf7f2a504 Mon Sep 17 00:00:00 2001
From: wklken <wklken@gmail.com>
Date: Wed, 25 Oct 2023 14:26:48 +0800
Subject: [PATCH 3/3] test(bk-rate-limit/init.lua): add more test cases

---
 src/apisix/tests/bk-rate-limit/test-init.lua | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/apisix/tests/bk-rate-limit/test-init.lua b/src/apisix/tests/bk-rate-limit/test-init.lua
index df83a44..95d4cf5 100644
--- a/src/apisix/tests/bk-rate-limit/test-init.lua
+++ b/src/apisix/tests/bk-rate-limit/test-init.lua
@@ -55,6 +55,12 @@ describe(
                     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