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 2021/08/12 04:47:35 UTC
[GitHub] [apisix] okaybase opened a new pull request #4808: feat(plugin): add custom reject-message for traffic control
okaybase opened a new pull request #4808:
URL: https://github.com/apache/apisix/pull/4808
### 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. -->
Add custom reject-message for traffic control, user can tell the caller what happens.
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687843023
##########
File path: docs/en/latest/plugins/limit-conn.md
##########
@@ -43,6 +43,7 @@ Limiting request concurrency plugin.
| only_use_default_delay | boolean | optional | false | [true,false] | enable the strict mode of the latency seconds. If you set this option to `true`, it will run strictly according to the latency seconds you set without additional calculation logic. |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | to limit the concurrency level. <br />For example, one can use the host name (or server zone) as the key so that we limit concurrency per host name. Otherwise, we can also use the client address as the key so that we can avoid a single client from flooding our service with too many parallel connections or requests. <br /> Now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header, "consumer_name"(consumer's username). |
| rejected_code | string | optional | 503 | [200,...,599] | returned when the request exceeds `conn` + `burst` will be rejected. |
+| rejected_msg | string | optional | | non-empty | The `rejected_msg` returned when the request exceeds `conn` + `burst` will be rejected. |
Review comment:
okay, thanks, done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687739353
##########
File path: t/plugin/limit-conn2.t
##########
@@ -228,3 +228,91 @@ GET /hello1
qr/request latency is nil/
--- grep_error_log_out
request latency is nil
+
+
+
+=== TEST 7: invalid route: wrong rejected_msg type
+--- 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": {
+ "limit-conn": {
+ "conn": 1,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "rejected_msg": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/limit_conn"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
Review comment:
okay, thanks, done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687739563
##########
File path: t/plugin/limit-count.t
##########
@@ -1371,3 +1371,63 @@ GET /t
qr/property \"time_window\" validation failed: expected 0 to be strictly greater than 0/
--- no_error_log
[error]
+
+
+
+=== TEST 43: set route, with rejected_msg
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": "Requests are too frequent, please try again later.",
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 44: rejected_msg, request normal
+--- request
+GET /hello
+--- response_body
+hello world
+
+
+
+=== TEST 45: rejected_msg, request frequent
+--- request
+GET /hello
+--- error_code eval
Review comment:
okay, thanks, done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r688187240
##########
File path: t/plugin/limit-count2.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: invalid route: wrong rejected_msg type
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": true,
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
Review comment:
Could you add the `add_block_preprocessor` like t/plugin/limit-conn2.t?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687402897
##########
File path: t/plugin/limit-conn.t
##########
@@ -1328,3 +1328,47 @@ GET /t
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"allow_degradation\" validation failed: wrong type: expected boolean, got string"}
--- no_error_log
[error]
+
+
+
+=== TEST 35: invalid route: wrong rejected_msg
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687738847
##########
File path: t/plugin/limit-count.t
##########
@@ -1371,3 +1371,63 @@ GET /t
qr/property \"time_window\" validation failed: expected 0 to be strictly greater than 0/
--- no_error_log
[error]
+
+
+
+=== TEST 43: set route, with rejected_msg
Review comment:
okay, thanks, done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687843265
##########
File path: docs/zh/latest/plugins/limit-conn.md
##########
@@ -33,6 +33,7 @@ title: limit-conn
| only_use_default_delay | boolean | optional | false | [true,false] | 延迟时间的严格模式。 如果设置为`true`的话，将会严格按照设置的时间来进行延迟 |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用户指定的限制并发级别的关键字，可以是客户端 IP 或服务端 IP。<br />例如，可以使用主机名（或服务器区域）作为关键字，以便限制每个主机名的并发性。 否则，我们也可以使用客户端地址作为关键字，这样我们就可以避免单个客户端用太多的并行连接或请求淹没我们的服务。 <br />当前接受的 key 有："remote_addr"（客户端 IP 地址）, "server_addr"（服务端 IP 地址）, 请求头中的"X-Forwarded-For" 或 "X-Real-IP", "consumer_name"（consumer 的 username）。 |
| rejected_code | string | optional | 503 | [200,...,599] | 当请求超过 `conn` + `burst` 这个阈值时，返回的 HTTP 状态码 |
+| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `rejected_msg` 值。 |
Review comment:
okay, thanks, done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687404189
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "key": "http_x_real_ip",
+ "rejected_msg": "Requests are too frequent, please try again later.",
+ "policy": "redis-cluster",
+ "redis_cluster_nodes": [
+ "127.0.0.1:8001",
+ "127.0.0.1:8002",
+ "127.0.0.1:8003",
+ "127.0.0.1:8004"
+ ],
+ "redis_cluster_name": "redis-cluster-1"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: rejected_msg for TEST 12, request normal
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r688188515
##########
File path: t/plugin/limit-count2.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: invalid route: wrong rejected_msg type
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": true,
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
Review comment:
yep, use `add_block_preprocessor` is better, thanks~ done~ @spacewander
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r688188515
##########
File path: t/plugin/limit-count2.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: invalid route: wrong rejected_msg type
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": true,
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
Review comment:
yep, use `add_block_preprocessor` is better, thanks~ @spacewander done~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687683718
##########
File path: t/plugin/limit-conn2.t
##########
@@ -228,3 +228,91 @@ GET /hello1
qr/request latency is nil/
--- grep_error_log_out
request latency is nil
+
+
+
+=== TEST 7: invalid route: wrong rejected_msg type
+--- 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": {
+ "limit-conn": {
+ "conn": 1,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "rejected_msg": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/limit_conn"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
Review comment:
We don't need to set request and error_log. They are set in https://github.com/apache/apisix/blob/05fc2306b5f005799e908273cfe11bd5f830459f/t/plugin/limit-conn2.t#L38
##########
File path: t/plugin/limit-count.t
##########
@@ -1371,3 +1371,63 @@ GET /t
qr/property \"time_window\" validation failed: expected 0 to be strictly greater than 0/
--- no_error_log
[error]
+
+
+
+=== TEST 43: set route, with rejected_msg
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": "Requests are too frequent, please try again later.",
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 44: rejected_msg, request normal
+--- request
+GET /hello
+--- response_body
+hello world
+
+
+
+=== TEST 45: rejected_msg, request frequent
+--- request
+GET /hello
+--- error_code eval
Review comment:
```suggestion
--- error_code: 503
```
##########
File path: t/plugin/limit-count.t
##########
@@ -1371,3 +1371,63 @@ GET /t
qr/property \"time_window\" validation failed: expected 0 to be strictly greater than 0/
--- no_error_log
[error]
+
+
+
+=== TEST 43: set route, with rejected_msg
Review comment:
Let's move the test to the new test file. See https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#check-code-style-and-test-case-style
> When the test file is too large, for example > 800 lines, you should split it to a new file. Please take a look at t/plugin/limit-conn.t and t/plugin/limit-conn2.t
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687404095
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
Review comment:
okay, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687403633
##########
File path: docs/zh/latest/plugins/limit-conn.md
##########
@@ -33,6 +33,7 @@ title: limit-conn
| only_use_default_delay | boolean | optional | false | [true,false] | 延迟时间的严格模式。 如果设置为`true`的话，将会严格按照设置的时间来进行延迟 |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用户指定的限制并发级别的关键字，可以是客户端 IP 或服务端 IP。<br />例如，可以使用主机名（或服务器区域）作为关键字，以便限制每个主机名的并发性。 否则，我们也可以使用客户端地址作为关键字，这样我们就可以避免单个客户端用太多的并行连接或请求淹没我们的服务。 <br />当前接受的 key 有："remote_addr"（客户端 IP 地址）, "server_addr"（服务端 IP 地址）, 请求头中的"X-Forwarded-For" 或 "X-Real-IP", "consumer_name"（consumer 的 username）。 |
| rejected_code | string | optional | 503 | [200,...,599] | 当请求超过 `conn` + `burst` 这个阈值时，返回的 HTTP 状态码 |
+| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `error_msg` 值。 |
Review comment:
okay, thanks~
##########
File path: docs/en/latest/plugins/limit-req.md
##########
@@ -42,6 +42,7 @@ limit request rate using the "leaky bucket" method.
| burst | integer | required | | burst >= 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", "consumer_name"] | 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, "consumer_name"(consumer's username). |
| rejected_code | integer | optional | 503 | [200,...,599] | The HTTP status code returned when the request exceeds the threshold is rejected. |
+| rejected_msg | string | optional | | non-empty | The `error_msg` returned when the request exceeds the threshold is rejected. |
Review comment:
okay, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687404189
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "key": "http_x_real_ip",
+ "rejected_msg": "Requests are too frequent, please try again later.",
+ "policy": "redis-cluster",
+ "redis_cluster_nodes": [
+ "127.0.0.1:8001",
+ "127.0.0.1:8002",
+ "127.0.0.1:8003",
+ "127.0.0.1:8004"
+ ],
+ "redis_cluster_name": "redis-cluster-1"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: rejected_msg for TEST 12, request normal
Review comment:
okay, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687404095
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#issuecomment-897722263
@spacewander I think we need to unify the `rejected_code` and `rejected_msg` attributes of the `limit-req`, `limit-conn` and `limit-count`.
At present they look somewhat incongruous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4808: feat(limit-*): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4808:
URL: https://github.com/apache/apisix/pull/4808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r688188515
##########
File path: t/plugin/limit-count2.t
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: invalid route: wrong rejected_msg type
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "rejected_code": 503,
+ "rejected_msg": true,
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687402897
##########
File path: t/plugin/limit-conn.t
##########
@@ -1328,3 +1328,47 @@ GET /t
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"allow_degradation\" validation failed: wrong type: expected boolean, got string"}
--- no_error_log
[error]
+
+
+
+=== TEST 35: invalid route: wrong rejected_msg
Review comment:
okay, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687808714
##########
File path: docs/en/latest/plugins/limit-conn.md
##########
@@ -43,6 +43,7 @@ Limiting request concurrency plugin.
| only_use_default_delay | boolean | optional | false | [true,false] | enable the strict mode of the latency seconds. If you set this option to `true`, it will run strictly according to the latency seconds you set without additional calculation logic. |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | to limit the concurrency level. <br />For example, one can use the host name (or server zone) as the key so that we limit concurrency per host name. Otherwise, we can also use the client address as the key so that we can avoid a single client from flooding our service with too many parallel connections or requests. <br /> Now accept those as key: "remote_addr"(client's IP), "server_addr"(server's IP), "X-Forwarded-For/X-Real-IP" in request header, "consumer_name"(consumer's username). |
| rejected_code | string | optional | 503 | [200,...,599] | returned when the request exceeds `conn` + `burst` will be rejected. |
+| rejected_msg | string | optional | | non-empty | The `rejected_msg` returned when the request exceeds `conn` + `burst` will be rejected. |
Review comment:
```suggestion
| rejected_msg | string | optional | | non-empty | the response body returned when the request exceeds `conn` + `burst` will be rejected. |
```
##########
File path: docs/zh/latest/plugins/limit-conn.md
##########
@@ -33,6 +33,7 @@ title: limit-conn
| only_use_default_delay | boolean | optional | false | [true,false] | 延迟时间的严格模式。 如果设置为`true`的话，将会严格按照设置的时间来进行延迟 |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用户指定的限制并发级别的关键字，可以是客户端 IP 或服务端 IP。<br />例如，可以使用主机名（或服务器区域）作为关键字，以便限制每个主机名的并发性。 否则，我们也可以使用客户端地址作为关键字，这样我们就可以避免单个客户端用太多的并行连接或请求淹没我们的服务。 <br />当前接受的 key 有："remote_addr"（客户端 IP 地址）, "server_addr"（服务端 IP 地址）, 请求头中的"X-Forwarded-For" 或 "X-Real-IP", "consumer_name"（consumer 的 username）。 |
| rejected_code | string | optional | 503 | [200,...,599] | 当请求超过 `conn` + `burst` 这个阈值时，返回的 HTTP 状态码 |
+| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `rejected_msg` 值。 |
Review comment:
```suggestion
| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的响应体内容。 |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687395069
##########
File path: t/plugin/limit-conn.t
##########
@@ -1328,3 +1328,47 @@ GET /t
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"allow_degradation\" validation failed: wrong type: expected boolean, got string"}
--- no_error_log
[error]
+
+
+
+=== TEST 35: invalid route: wrong rejected_msg
Review comment:
I just notice we have `t/plugin/limit-conn2.t`. Let's add test there.
##########
File path: docs/zh/latest/plugins/limit-conn.md
##########
@@ -33,6 +33,7 @@ title: limit-conn
| only_use_default_delay | boolean | optional | false | [true,false] | 延迟时间的严格模式。 如果设置为`true`的话，将会严格按照设置的时间来进行延迟 |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用户指定的限制并发级别的关键字，可以是客户端 IP 或服务端 IP。<br />例如，可以使用主机名（或服务器区域）作为关键字，以便限制每个主机名的并发性。 否则，我们也可以使用客户端地址作为关键字，这样我们就可以避免单个客户端用太多的并行连接或请求淹没我们的服务。 <br />当前接受的 key 有："remote_addr"（客户端 IP 地址）, "server_addr"（服务端 IP 地址）, 请求头中的"X-Forwarded-For" 或 "X-Real-IP", "consumer_name"（consumer 的 username）。 |
| rejected_code | string | optional | 503 | [200,...,599] | 当请求超过 `conn` + `burst` 这个阈值时，返回的 HTTP 状态码 |
+| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `error_msg` 值。 |
Review comment:
```suggestion
| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `rejected_msg` 值。 |
```
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 1,
+ "time_window": 600,
+ "key": "http_x_real_ip",
+ "rejected_msg": "Requests are too frequent, please try again later.",
+ "policy": "redis-cluster",
+ "redis_cluster_nodes": [
+ "127.0.0.1:8001",
+ "127.0.0.1:8002",
+ "127.0.0.1:8003",
+ "127.0.0.1:8004"
+ ],
+ "redis_cluster_name": "redis-cluster-1"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: rejected_msg for TEST 12, request normal
Review comment:
Normally, we don't repeat the test name in the other test.
##########
File path: t/plugin/limit-count-redis-cluster.t
##########
@@ -434,3 +434,70 @@ passed
GET /hello
--- response_body
hello world
+
+
+
+=== TEST 12: set route, with rejected_msg
Review comment:
Since this feature is not relative to the policy, we only need to test it in the limit-count.t instead of testing it in all three policies.
##########
File path: docs/en/latest/plugins/limit-req.md
##########
@@ -42,6 +42,7 @@ limit request rate using the "leaky bucket" method.
| burst | integer | required | | burst >= 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", "consumer_name"] | 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, "consumer_name"(consumer's username). |
| rejected_code | integer | optional | 503 | [200,...,599] | The HTTP status code returned when the request exceeds the threshold is rejected. |
+| rejected_msg | string | optional | | non-empty | The `error_msg` returned when the request exceeds the threshold is rejected. |
Review comment:
Ditto. Please fix other similar places.
##########
File path: apisix/plugins/limit-conn/init.lua
##########
@@ -53,6 +54,9 @@ function _M.increase(conf, ctx)
local delay, err = lim:incoming(key, true)
if not delay then
if err == "rejected" then
+ if conf.rejected_msg and str_len(conf.rejected_msg) > 0 then
Review comment:
Better to check the length with jsonschema.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4808: feat(plugin): add custom reject-message for traffic control
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4808:
URL: https://github.com/apache/apisix/pull/4808#discussion_r687403585
##########
File path: docs/en/latest/plugins/limit-req.md
##########
@@ -42,6 +42,7 @@ limit request rate using the "leaky bucket" method.
| burst | integer | required | | burst >= 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", "consumer_name"] | 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, "consumer_name"(consumer's username). |
| rejected_code | integer | optional | 503 | [200,...,599] | The HTTP status code returned when the request exceeds the threshold is rejected. |
+| rejected_msg | string | optional | | non-empty | The `error_msg` returned when the request exceeds the threshold is rejected. |
Review comment:
okay
##########
File path: docs/zh/latest/plugins/limit-conn.md
##########
@@ -33,6 +33,7 @@ title: limit-conn
| only_use_default_delay | boolean | optional | false | [true,false] | 延迟时间的严格模式。 如果设置为`true`的话，将会严格按照设置的时间来进行延迟 |
| key | object | required | | ["remote_addr", "server_addr", "http_x_real_ip", "http_x_forwarded_for", "consumer_name"] | 用户指定的限制并发级别的关键字，可以是客户端 IP 或服务端 IP。<br />例如，可以使用主机名（或服务器区域）作为关键字，以便限制每个主机名的并发性。 否则，我们也可以使用客户端地址作为关键字，这样我们就可以避免单个客户端用太多的并行连接或请求淹没我们的服务。 <br />当前接受的 key 有："remote_addr"（客户端 IP 地址）, "server_addr"（服务端 IP 地址）, 请求头中的"X-Forwarded-For" 或 "X-Real-IP", "consumer_name"（consumer 的 username）。 |
| rejected_code | string | optional | 503 | [200,...,599] | 当请求超过 `conn` + `burst` 这个阈值时，返回的 HTTP 状态码 |
+| rejected_msg | string | 可选 | | 非空 | 当请求超过 `conn` + `burst` 这个阈值时，返回的 `error_msg` 值。 |
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org