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


Reply via email to