You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "nic-6443 (via GitHub)" <gi...@apache.org> on 2026/04/29 06:35:05 UTC

[PR] test(ai-rate-limiting): add test for instance name containing dots [apisix]

nic-6443 opened a new pull request, #13316:
URL: https://github.com/apache/apisix/pull/13316

   Instance names with dots (e.g. model version + IP like `Qwen3.5-397B-10.249.238.157`) should be treated as constant keys by the rate limiting framework, not as `ctx.var` expressions.
   
   The existing `key_type = "constant"` in `transform_limit_conf()` already handles this correctly. This test adds coverage for the scenario to prevent future regressions.


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(ai-rate-limiting): add test for instance name containing dots [apisix]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13316:
URL: https://github.com/apache/apisix/pull/13316#discussion_r3159049326


##########
t/plugin/ai-rate-limiting.t:
##########
@@ -1470,3 +1470,71 @@ X-AI-Fixture: openai/chat-model-echo.json
 X-AI-Project-RateLimit-Limit: 30
 X-AI-Project-RateLimit-Remaining: 30
 X-AI-Project-RateLimit-Reset: 10
+
+
+
+=== TEST 33: setup route with instance name containing dots
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                    "uri": "/ai",
+                    "plugins": {
+                        "ai-proxy-multi": {
+                            "fallback_strategy": "instance_health_and_rate_limiting",
+                            "instances": [
+                                {
+                                    "name": "Qwen3.5-397B-10.249.238.157",
+                                    "provider": "openai",
+                                    "weight": 1,
+                                    "priority": 1,
+                                    "auth": {"header": {"Authorization": "Bearer token"}},
+                                    "options": {"model": "gpt-4"},
+                                    "override": {"endpoint": "http://127.0.0.1:1980"}
+                                }
+                            ],
+                            "ssl_verify": false
+                        },
+                        "ai-rate-limiting": {
+                            "instances": [
+                                {
+                                    "name": "Qwen3.5-397B-10.249.238.157",
+                                    "limit": 100,
+                                    "time_window": 60
+                                }
+                            ]
+                        }
+                    },
+                    "upstream": {
+                        "type": "roundrobin",
+                        "nodes": {
+                            "canbeanything.com": 1
+                        }
+                    }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 34: instance name with dots should not trigger ctx.var resolution
+--- request
+POST /ai
+{"messages":[{"role":"system","content":"You are a mathematician"},{"role":"user","content":"What is 1+1?"}]}
+--- more_headers
+Authorization: Bearer token
+X-AI-Fixture: openai/chat-model-echo.json
+--- error_code: 200
+--- no_error_log
+[error]

Review Comment:
   TEST 34 only asserts HTTP 200 and absence of `[error]` logs. A regression where the instance name is treated as a `ctx.var[...]` key (instead of a constant) wouldn’t necessarily log an error: `limit-count` would resolve `ctx.var["Qwen3.5-..."]` to nil and silently fall back to `remote_addr`, changing semantics to per-client rate limiting. To actually cover the reported bug, add an assertion that the rate-limit key is independent of client IP (e.g., make two requests with different `X-Real-IP` values after setting a low enough `limit` so the first request exhausts the quota; the second request should still be rejected if the key is truly constant).



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(ai-rate-limiting): add test for instance name containing dots [apisix]

Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 merged PR #13316:
URL: https://github.com/apache/apisix/pull/13316


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org