You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/21 08:57:47 UTC
[GitHub] [apisix] Firstsawyou opened a new pull request #2270: feature: The "limit-req" plugin supports flow control based on the "consumer" ID #2267
Firstsawyou opened a new pull request #2270:
URL: https://github.com/apache/apisix/pull/2270
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
#2267
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494087630
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493137344
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492793961
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
Is consumer_name or consumer_id?
I'm confused.
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
Why Chinese?
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
We need return error if consumer id not exist
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
Is consumer name in request header?
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Are the following test cases related to this pr?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
We need a real name!
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
Review comment:
Ditto
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
If they are the same, why use two different names? Is it id or name?
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
NO. Error handling is necessary
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
and test cases are also required
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Please remove the code not related to pr
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
I can't see how these test cases are related to pr
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Test case tests not coverage error conditions
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Test cases not coverage error conditions
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492813214
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
The values of "consumer_name" and "consumer_id" here are the same. This is to show that these two values are equivalent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492527688
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
fixed.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
fixed
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -98,6 +98,78 @@ Server: APISIX web server
这就表示 limit req 插件生效了。
+### 如何在`consumer`上启用插件
+
+consumer上开启`limit-req`插件，需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、将`limit-req`插件绑定到consumer上
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
fixed
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
I think `Test Plugin` is a part of "How to enable on the `consumer`".
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
fixed
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
The values of "consumer_name" and "consumer_id" here are the same. This is to show that these two values are equivalent.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
ok.
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Yes, this is to test whether it is normal when the "limit-req" plugin is bound to the "consumer" and the "key" is other values.
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
This is a mistake, I will fix it.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
ok
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
This is changed with other test cases, I will modify it to English.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
I think the "consumer_id" here does not need to be checked whether it does not exist, because when the "limit-req" is bound to the "consumer", the "consumer_id" can definitely be obtained. When bound to "route", it defaults to an empty string, so that it does not affect the use of the plug-in's "key" with other values. What do you think?
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
fixed.
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
fixed.
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
fixed.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
fixed.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
Review comment:
fixed
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-696672681
@membphis review required.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493177709
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
NO. Error handling is necessary
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494080517
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
There are no test cases. I will not modify it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492911698
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
fixed.
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492849734
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
This is a mistake, I will fix it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494061235
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
Sorry, this is a mistake. It should be `route`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492527688
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494267003
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
still not fix
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
take a look: https://github.com/apache/apisix/pull/2270#discussion_r494019356
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493178615
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Test cases not coverage error conditions
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493178364
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
I can't see how these test cases are related to pr
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492530987
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
@Firstsawyou
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-698111359
@liuhengloveyou please take a look at this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493178615
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Test case tests not coverage error conditions
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494055593
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494284498
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
sorry, it's fixed now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493176927
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
If they are the same, why use two different names? Is it id or name?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494070134
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
I think it is necessary, because the value of `consumer_name` needs to be determined according to the value of `key`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492852143
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
This is changed with other test cases, I will modify it to English.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-698111359
@liuhengloveyou please take a look at this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492850121
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493199090
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493199624
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
okay
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494265478
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
got it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494019356
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
Review comment:
`The username of consumer is nil.` -> `consumer not found`
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
keep the same error msg as error log
##########
File path: doc/plugins/limit-req.md
##########
@@ -76,7 +78,7 @@ Then add limit-req plugin:
-## Test Plugin
+**Test Plugin**
Review comment:
why change the doc style?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+}'
+```
+
+2. Create a `route` and enable the `key-auth` plugin
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "methods": ["GET"],
+ "uri": "/index.html",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+}'
+```
+
+**Test Plugin**
Review comment:
ditto
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
why change the type?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
When this plugin is binding to the consumer object, it can be handled automatically, the declaration here is redundant
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
what is `路线`?
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
What is the difference between these two? Do you add test cases to cover `number`?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
the `key` is always `consumer_name` when binding with consumer, right?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
> No, the 'key' enumeration type has no default value.
Does this answer have anything to do with my question?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
got it
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
still not fix
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
take a look: https://github.com/apache/apisix/pull/2270#discussion_r494019356
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494078429
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
No, the 'key' enumeration type has no default value.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492847838
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Yes, this is to test whether it is normal when the "limit-req" plugin is bound to the "consumer" and the "key" is other values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492912307
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-697099061
@Firstsawyou conflicted, please rebase your branch
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492556792
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494055159
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
Review comment:
ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494072296
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
the `key` is always `consumer_name` when binding with consumer, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494015437
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
This is to determine a version first, and later will be resolved by upgrading the version. This is a related to issue: #2275
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
Review comment:
ok.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
ok.
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
In Lua code, `rate` and `burst` are of type `number`, and `rejected_code` is of type `integer`.
##########
File path: doc/plugins/limit-req.md
##########
@@ -76,7 +78,7 @@ Then add limit-req plugin:
-## Test Plugin
+**Test Plugin**
Review comment:
Use `Test Plugin` as the title, and the title of "Test Plugin" in the following text, the repeated title, looks hard to understand. I think it is better after modification, do you think?
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
Sorry, this is a mistake. It should be `route`.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
I think it is necessary, because the value of `consumer_name` needs to be determined according to the value of `key`.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
No, the 'key' enumeration type has no default value.
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
There are no test cases. I will not modify it.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
fixed
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
fixed
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
Sorry, my response is not clear. When bound to the `consumer`, the `key` is optional, and when the `key` is other values (for example:`remote_addr`), it can be used normally.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("Missing consumer's username.")
Review comment:
sorry, it's fixed now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492802272
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
We need return error if consumer id not exist
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
Is consumer name in request header?
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Are the following test cases related to this pr?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
We need a real name!
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
Review comment:
Ditto
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494136610
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
> No, the 'key' enumeration type has no default value.
Does this answer have anything to do with my question?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492519914
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
bad indentation
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
[Test Plugin]()
[How to enable on the `consumer`]()
I think this sorting is better.
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -98,6 +98,78 @@ Server: APISIX web server
这就表示 limit req 插件生效了。
+### 如何在`consumer`上启用插件
+
+consumer上开启`limit-req`插件，需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、将`limit-req`插件绑定到consumer上
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
ditto
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
Important chapters should be first.
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
@Firstsawyou
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-696672681
@membphis review required.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-704011196
ping @moonming
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492889269
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
I think the "consumer_id" here does not need to be checked whether it does not exist, because when the "limit-req" is bound to the "consumer", the "consumer_id" can definitely be obtained. When bound to "route", it defaults to an empty string, so that it does not affect the use of the plug-in's "key" with other values. What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494181950
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
Sorry, my response is not clear. When bound to the `consumer`, the `key` is optional, and when the `key` is other values (for example:`remote_addr`), it can be used normally.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492912016
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
fixed.
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2270:
URL: https://github.com/apache/apisix/pull/2270
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492527777
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493229953
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
> @Firstsawyou conflicted, please rebase your branch
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492519914
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
bad indentation
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
[Test Plugin]()
[How to enable on the `consumer`]()
I think this sorting is better.
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -98,6 +98,78 @@ Server: APISIX web server
这就表示 limit req 插件生效了。
+### 如何在`consumer`上启用插件
+
+consumer上开启`limit-req`插件，需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、将`limit-req`插件绑定到consumer上
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
ditto
##########
File path: doc/plugins/limit-req.md
##########
@@ -21,11 +21,14 @@
# Summary
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
+- [Summary](#summary)
+ - [Name](#name)
+ - [Attributes](#attributes)
+ - [How To Enable](#how-to-enable)
+ - [Test Plugin](#test-plugin)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
Important chapters should be first.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493672640
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,16 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 401, { message = "Missing consumer's username."}
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493177821
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
and test cases are also required
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494088615
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492793961
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
Is consumer_name or consumer_id?
I'm confused.
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
Review comment:
Why Chinese?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-697116268
the commit msg ` feat: The "limit-req" plugin supports flow control based on the "consumer"` is unclear, what is ` flow control`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494060520
##########
File path: doc/plugins/limit-req.md
##########
@@ -76,7 +78,7 @@ Then add limit-req plugin:
-## Test Plugin
+**Test Plugin**
Review comment:
Use `Test Plugin` as the title, and the title of "Test Plugin" in the following text, the repeated title, looks hard to understand. I think it is better after modification, do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-697099061
@Firstsawyou conflicted, please rebase your branch
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493230618
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493539848
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,16 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 401, { message = "Missing consumer's username."}
Review comment:
the response code should be `500`.
This problem occurs without authentication plug-ins. We can write the reason to the error log, it can help the user to confirm where is the problem.
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,16 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 401, { message = "Missing consumer's username."}
+ end
+ key = ctx.consumer_id .. ctx.conf_type .. ctx.conf_version
+ else
Review comment:
add a blank line before `else`
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +107,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+## How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
Review comment:
we should use a real name like `Jack` or `Lee`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494019356
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
Review comment:
`The username of consumer is nil.` -> `consumer not found`
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 500, { message = "Missing consumer's username."}
Review comment:
keep the same error msg as error log
##########
File path: doc/plugins/limit-req.md
##########
@@ -76,7 +78,7 @@ Then add limit-req plugin:
-## Test Plugin
+**Test Plugin**
Review comment:
why change the doc style?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+}'
+```
+
+2. Create a `route` and enable the `key-auth` plugin
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "methods": ["GET"],
+ "uri": "/index.html",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+}'
+```
+
+**Test Plugin**
Review comment:
ditto
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
why change the type?
##########
File path: doc/plugins/limit-req.md
##########
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together with the authorization plugin. Here, the key-auth authorization plugin is taken as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "consumer_jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
Review comment:
When this plugin is binding to the consumer object, it can be handled automatically, the declaration here is redundant
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -19,26 +19,34 @@
- [English](../../plugins/limit-req.md)
-# limit-req
+# 目录
+ - [简介](#简介)
+ - [属性](#属性)
+ - [示例](#示例)
+ - [如何在 `route` 或 `service` 上使用](#如何在`route`或`service`上使用)
+ - [如何在 `consumer` 上使用](#如何在`consumer`上使用)
+ - [移除插件](#移除插件)
+
+## 简介
限制请求速度的插件，使用的是漏桶算法。
-## 参数
+## 属性
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------- | ------- | ------ | ------ | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
-| burst | integer | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
-| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"。 |
-| rejected_code | string | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码 |
+| rate | number | 必须 | | [0,...] | 指定的请求速率（以秒为单位），请求速率超过 `rate` 但没有超过 （`rate` + `brust`）的请求会被加上延时。 |
+| burst | number | 必须 | | [0,...] | t请求速率超过 （`rate` + `brust`）的请求会被直接拒绝。 |
+| key | string | 必须 | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用来做请求计数的依据，当前接受的 key 有："remote_addr"(客户端IP地址), "server_addr"(服务端 IP 地址), 请求头中的"X-Forwarded-For" 或 "X-Real-IP"，"consumer_name"(consumer 的 username)。 |
+| rejected_code | integer | 可选 | 503 | [200,...] | 当请求超过阈值被拒绝时，返回的 HTTP 状态码。 |
**key 是可以被用户自定义的，只需要修改插件的一行代码即可完成。并没有在插件中放开是处于安全的考虑。**
## 示例
-### 开启插件
+### 如何在`route`或`service`上使用
-下面是一个示例，在指定的 route 上开启了 limit req 插件:
+这里以`route`为例(`service`的使用是同样的方法)，在指定的路线上启用limit req插件。
Review comment:
what is `路线`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492556182
##########
File path: doc/plugins/limit-req.md
##########
@@ -26,8 +26,8 @@
- [Attributes](#attributes)
- [How To Enable](#how-to-enable)
- [Test Plugin](#test-plugin)
- - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
- - [Test Plugin](#test-plugin-1)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Test Plugin](#test-plugin-1)
Review comment:
I think `Test Plugin` is a part of "How to enable on the `consumer`".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494071689
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
What is the difference between these two? Do you add test cases to cover `number`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493669730
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,16 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ if not ctx.consumer_id then
+ core.log.error("The username of consumer is nil.")
+ return 401, { message = "Missing consumer's username."}
+ end
+ key = ctx.consumer_id .. ctx.conf_type .. ctx.conf_version
+ else
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492527870
##########
File path: doc/zh-cn/plugins/limit-req.md
##########
@@ -98,6 +98,78 @@ Server: APISIX web server
这就表示 limit req 插件生效了。
+### 如何在`consumer`上启用插件
+
+consumer上开启`limit-req`插件，需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、将`limit-req`插件绑定到consumer上
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "username": "limit_req_consumer_name",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 1,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494058041
##########
File path: doc/plugins/limit-req.md
##########
@@ -20,31 +20,33 @@
- [中文](../zh-cn/plugins/limit-req.md)
# Summary
+ - [Introduction](#introduction)
+ - [Attributes](#attributes)
+ - [Example](#example)
+ - [How to enable on the `route` or `serivce`](#how-to-enable-on-the-route-or-serivce)
+ - [How to enable on the `consumer`](#how-to-enable-on-the-consumer)
+ - [Disable Plugin](#disable-plugin)
-- [**Name**](#name)
-- [**Attributes**](#attributes)
-- [**How To Enable**](#how-to-enable)
-- [**Test Plugin**](#test-plugin)
-- [**Disable Plugin**](#disable-plugin)
-
-## Name
+## Introduction
limit request rate using the "leaky bucket" method.
## Attributes
| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| rate | integer | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
-| burst | integer | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
-| key | string | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for"] | the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header. |
-| rejected_code | string | optional | 503 | [200,...] | The HTTP status code returned when the request exceeds the threshold is rejected. The default is 503. |
+| rate | number | required | | [0,...] | the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate. |
+| burst | number | required | | [0,...] | the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately. |
Review comment:
In Lua code, `rate` and `burst` are of type `number`, and `rejected_code` is of type `integer`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-697116268
the commit msg ` feat: The "limit-req" plugin supports flow control based on the "consumer"` is unclear, what is ` flow control`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493177968
##########
File path: t/plugin/limit-req.t
##########
@@ -432,3 +432,367 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 12: consumer binds the limit-req plugin and `key` is `consumer_name`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 2,
+ "burst": 1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: route add "key-auth" plugin
+--- 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,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "plugins": {
+ "key-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "desc": "上游节点",
+ "uri": "/hello"
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "set"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: not exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[200, 200, 200]
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: update the limit-req plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "consumer_limit_req",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-jack"
+ },
+ "limit-req": {
+ "rate": 0.1,
+ "burst": 0.1,
+ "rejected_code": 403,
+ "key": "consumer_name"
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: exceeding the burst
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello", "GET /hello"]
+--- more_headers
+apikey: auth-jack
+--- error_code eval
+[403, 403, 403, 403]
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: consumer binds the limit-req plugin and `key` is `remote_addr`
Review comment:
Please remove the code not related to pr
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r492814426
##########
File path: apisix/plugins/limit-req.lua
##########
@@ -67,7 +67,12 @@ function _M.access(conf, ctx)
return 500
end
- local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+ local key
+ if conf.key == "consumer_name" then
+ key = (ctx.consumer_id or "") .. ctx.conf_type .. ctx.conf_version
Review comment:
ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r493989987
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
Here is my incomplete consideration, I have changed to the name method uniformly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on pull request #2270: feat: The "limit-req" plugin supports flow control based on the "consumer" ID
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#issuecomment-697138410
> the commit msg ` feat: The "limit-req" plugin supports flow control based on the "consumer"` is unclear, what is ` flow control`?
ok, I will re-edit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494015437
##########
File path: doc/plugins/limit-req.md
##########
@@ -37,7 +40,7 @@ limit request rate using the "leaky bucket" method.
|--------- |--------|-----------|
|rate |required|is the specified request rate (number per second) threshold. Requests exceeding this rate (and below `burst`) will get delayed to conform to the rate.|
|burst |required|is the number of excessive requests per second allowed to be delayed. Requests exceeding this hard limit will get rejected immediately.|
-| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header.|
+| key |required|is the user specified key to limit the rate, now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For", "X-Real-IP/consumer_name(consumer's ID)" in request header.|
Review comment:
This is to determine a version first, and later will be resolved by upgrading the version. This is a related to issue: #2275
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org