Copilot commented on code in PR #13670:
URL: https://github.com/apache/apisix/pull/13670#discussion_r3541138862
##########
apisix/plugins/ai-rate-limiting.lua:
##########
@@ -135,7 +142,31 @@ local schema = {
{
required = {"rules"},
}
- }
+ },
+ ["if"] = {
+ properties = {
+ policy = {
+ enum = {"redis"},
+ },
+ },
+ },
+ ["then"] = policy_to_additional_properties.redis,
+ ["else"] = {
+ ["if"] = {
+ properties = {
+ policy = {
+ enum = {"redis-cluster"},
+ },
+ },
+ },
+ ["then"] = policy_to_additional_properties["redis-cluster"],
+ ["else"] = {
+ ["if"] = {
+ properties = { policy = { enum = { "redis-sentinel" } } },
+ },
+ ["then"] = policy_to_additional_properties["redis-sentinel"],
+ },
+ },
}
Review Comment:
The schema now accepts Redis credentials (redis_password /
sentinel_password) via the reused limit-count Redis branches, but this plugin
schema does not mark them as `encrypt_fields`. Without this, those secrets may
be stored/returned in plaintext (unlike other Redis-backed limit plugins).
##########
apisix/plugins/ai-rate-limiting.lua:
##########
@@ -191,43 +222,69 @@ end
local function transform_limit_conf(plugin_conf, instance_conf, instance_name)
local limit_conf = {
+ _meta = plugin_conf._meta,
rejected_code = plugin_conf.rejected_code,
rejected_msg = plugin_conf.rejected_msg,
show_limit_quota_header = plugin_conf.show_limit_quota_header,
- -- we may expose those fields to ai-rate-limiting later
- policy = "local",
+ -- counters can be shared across nodes via redis policies
+ policy = plugin_conf.policy or "local",
key_type = "constant",
- allow_degradation = false,
+ allow_degradation = plugin_conf.allow_degradation,
sync_interval = -1,
limit_header = "X-AI-RateLimit-Limit",
remaining_header = "X-AI-RateLimit-Remaining",
reset_header = "X-AI-RateLimit-Reset",
}
+ local name = instance_name or ""
if plugin_conf.rules and #plugin_conf.rules > 0 then
limit_conf.rules = plugin_conf.rules
- limit_conf._meta = plugin_conf._meta
- return limit_conf
+ else
+ local key = plugin_name .. "#global"
+ local limit = plugin_conf.limit
+ local time_window = plugin_conf.time_window
+ if instance_conf then
+ name = instance_conf.name
+ key = instance_conf.name
+ limit = instance_conf.limit
+ time_window = instance_conf.time_window
+ end
+ limit_conf._vid = key
+ limit_conf.key = key
+ limit_conf.count = limit
+ limit_conf.time_window = time_window
+ limit_conf.limit_header = "X-AI-RateLimit-Limit-" .. name
+ limit_conf.remaining_header = "X-AI-RateLimit-Remaining-" .. name
+ limit_conf.reset_header = "X-AI-RateLimit-Reset-" .. name
end
- local key = plugin_name .. "#global"
- local limit = plugin_conf.limit
- local time_window = plugin_conf.time_window
- local name = instance_name or ""
- if instance_conf then
- name = instance_conf.name
- key = instance_conf.name
- limit = instance_conf.limit
- time_window = instance_conf.time_window
+ if plugin_conf.policy == "redis" then
+ limit_conf.redis_host = plugin_conf.redis_host
+ limit_conf.redis_port = plugin_conf.redis_port
+ limit_conf.redis_username = plugin_conf.redis_username
+ limit_conf.redis_password = plugin_conf.redis_password
+ limit_conf.redis_database = plugin_conf.redis_database
+ limit_conf.redis_timeout = plugin_conf.redis_timeout
+ limit_conf.redis_ssl = plugin_conf.redis_ssl
+ limit_conf.redis_ssl_verify = plugin_conf.redis_ssl_verify
+ elseif plugin_conf.policy == "redis-cluster" then
+ limit_conf.redis_cluster_nodes = plugin_conf.redis_cluster_nodes
+ limit_conf.redis_cluster_name = plugin_conf.redis_cluster_name
+ limit_conf.redis_password = plugin_conf.redis_password
+ limit_conf.redis_timeout = plugin_conf.redis_timeout
+ limit_conf.redis_cluster_ssl = plugin_conf.redis_cluster_ssl
+ limit_conf.redis_cluster_ssl_verify =
plugin_conf.redis_cluster_ssl_verify
+ elseif plugin_conf.policy == "redis-sentinel" then
+ limit_conf.redis_sentinels = plugin_conf.redis_sentinels
+ limit_conf.redis_master_name = plugin_conf.redis_master_name
+ limit_conf.sentinel_username = plugin_conf.sentinel_username
+ limit_conf.sentinel_password = plugin_conf.sentinel_password
+ limit_conf.redis_role = plugin_conf.redis_role
+ limit_conf.redis_connect_timeout = plugin_conf.redis_connect_timeout
+ limit_conf.redis_read_timeout = plugin_conf.redis_read_timeout
+ limit_conf.redis_keepalive_timeout =
plugin_conf.redis_keepalive_timeout
+ limit_conf.redis_database = plugin_conf.redis_database
end
Review Comment:
For `policy = "redis-sentinel"`, the transformed limit-count config does not
pass through `redis_username` / `redis_password`. The limit-count Sentinel
client supports authenticating to both Sentinel and the Redis master, so
omitting these fields will break setups where the Redis master requires
ACL/requirepass (and contradicts the documented support for redis_password on
redis-sentinel).
##########
t/plugin/ai-rate-limiting.t:
##########
@@ -1538,3 +1538,133 @@ X-AI-Fixture: openai/chat-model-echo.json
--- error_code: 200
--- no_error_log
[error]
+
+
+
+=== TEST 35: schema check, redis policy requires redis_host
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.ai-rate-limiting")
+ local ok, err = plugin.check_schema({
+ limit = 30,
+ time_window = 60,
+ policy = "redis",
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("passed")
+ end
+ }
+ }
+--- response_body
+then clause did not match
+
+
+
+=== TEST 36: flush redis and set route with redis policy
+--- config
+ location /t {
+ content_by_lua_block {
+ local redis = require("resty.redis")
+ local red = redis:new()
+ red:set_timeout(1000)
+ local ok, err = red:connect("127.0.0.1", 6379)
+ if not ok then
+ ngx.say("failed to connect: ", err)
+ return
+ end
+ red:flushall()
Review Comment:
This test uses `FLUSHALL`, which clears *all* Redis databases and can make
the test suite flaky if other tests share the same Redis instance in parallel.
Since this route explicitly uses `redis_database: 1`, flushing only DB 1 is
enough and avoids cross-test interference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
