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: us...@infra.apache.org