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/12/03 06:41:53 UTC
[GitHub] [apisix] tzssangglass opened a new pull request #2943: feature: support enable/disable route
tzssangglass opened a new pull request #2943:
URL: https://github.com/apache/apisix/pull/2943
fix #2737
### 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. -->
### Pre-submission checklist:
* [ ] 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536806686
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,11 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
- push_host_router(route, host_routes, only_uri_routes)
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status == 1 then
Review comment:
I took a look at the apisix sync data from etcd,
https://github.com/apache/apisix/blob/c41aa8534a18f43ef0c3cd6780f106595f9f9b88/apisix/core/schema.lua#L53-L61
after `validator(json) `, the existing old data will has default `status` value.
but i agree your point, add `if not status` is more safer:-)
----------------------------------------------------------------
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] liuxiran commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536113049
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,226 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1)
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1)
Review comment:
it is better to change the title to
`default enable route (id: 1) with host`
or someone may mistoke TEST 5 as TEST 1, e. g:me😂
----------------------------------------------------------------
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] tzssangglass commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738170623
describe a strange phenomenon, I was using `/route_status` api and create new routes because I found that using route id 1 and `/hello` always failed when I ran my local test push, even through other test cases, like
```
$prove -I../test-nginx/lib -I./ -r -s t/router/radixtree-host-uri.t
t/router/radixtree-host-uri.t .. 10/?
# Failed test 'TEST 5: hit routes - status code ok'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
# got: '404'
# expected: '200'
# Failed test 'TEST 5: hit routes - response_body - response is expected (repeated req 0, req 0)'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
# got: '{"error_msg":"404 Route Not Found"}
# '
# expected: 'hello world
# '
# Failed test 'TEST 6: hit routes - status code ok'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
# got: '404'
# expected: '200'
# Failed test 'TEST 6: hit routes - response_body - response is expected (repeated req 0, req 0)'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
# got: '{"error_msg":"404 Route Not Found"}
# '
# expected: 'hello world
# '
```
until I debugged at https://github.com/apache/apisix/blob/9e3c34a0103b88d340285017e34021cbc1f0fde8/apisix/http/router/radixtree_uri.lua#L85
when run test, the `user_routes.values` is nil, but when start apisix, it work.
after find this exception, something change, run the test case is back to normal, and cannot be reproduced. >_<|||
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536530930
##########
File path: doc/zh-cn/admin-api.md
##########
@@ -76,6 +76,7 @@
|filter_func|可选|匹配规则|用户自定义的过滤函数。可以使用它来实现特殊场景的匹配要求实现。该函数默认接受一个名为 vars 的输入参数，可以用它来获取 Nginx 变量。|function(vars) return vars["arg_name"] == "json" end|
|labels |可选 |匹配规则|标识附加属性的键值对|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|可选 |辅助| 是否启用 `websocket`(boolean), 缺省 `false`.||
+|status |可选 |辅助| 是否启用此路由, 缺省 `1`。|`1` 表示启用，`0` 表示禁用|
|create_time|可选|辅助|单位为秒的 epoch 时间戳，如果不指定则自动创建|1602883670|
Review comment:
It should be a typo here. @spacewander please have a look.
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536518238
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -95,7 +95,8 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes or {}) do
local status = core.table.try_read_attr(route, "value", "status")
- if status and status ~= 1 then
+ -- check the status
+ if status and status == 0 then
Review comment:
You should add test cases for 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] membphis commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536496556
##########
File path: apisix/schema_def.lua
##########
@@ -496,6 +496,13 @@ _M.route = {
},
id = id_schema,
+
+ status = {
+ description = "route status, 1 to enable, 0 to disable",
+ type = "integer",
+ enum = {1, 0},
Review comment:
add new test case: specify an invalid status value, eg: `100`
----------------------------------------------------------------
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] liuxiran commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738798267
> describe a strange phenomenon, I was using `/route_status` api and create new routes because I found that using route id 1 and `/hello` always failed when I ran my local test cases, even through other test cases, like
>
> ```
> $prove -I../test-nginx/lib -I./ -r -s t/router/radixtree-host-uri.t
> t/router/radixtree-host-uri.t .. 10/?
> # Failed test 'TEST 5: hit routes - status code ok'
> # at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
> # got: '404'
> # expected: '200'
>
> # Failed test 'TEST 5: hit routes - response_body - response is expected (repeated req 0, req 0)'
> # at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
> # got: '{"error_msg":"404 Route Not Found"}
> # '
> # expected: 'hello world
> # '
>
> # Failed test 'TEST 6: hit routes - status code ok'
> # at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
> # got: '404'
> # expected: '200'
>
> # Failed test 'TEST 6: hit routes - response_body - response is expected (repeated req 0, req 0)'
> # at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
> # got: '{"error_msg":"404 Route Not Found"}
> # '
> # expected: 'hello world
> # '
> ```
>
> until I debugged at
>
> https://github.com/apache/apisix/blob/9e3c34a0103b88d340285017e34021cbc1f0fde8/apisix/http/router/radixtree_uri.lua#L85
>
> when run test, the `user_routes.values` is nil, but when start apisix, it work.
>
> after find this exception, something change, run the test case is back to normal, and cannot be reproduced. >_<|||
If this magical event happened next, you may cc
@membphis @spacewander @tokers for help😊
----------------------------------------------------------------
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] liuxiran commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-737829130
Thanks for you pr :), just wait for others preview.
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738045914
nice 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] spacewander commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536758557
##########
File path: doc/zh-cn/admin-api.md
##########
@@ -76,6 +76,7 @@
|filter_func|可选|匹配规则|用户自定义的过滤函数。可以使用它来实现特殊场景的匹配要求实现。该函数默认接受一个名为 vars 的输入参数，可以用它来获取 Nginx 变量。|function(vars) return vars["arg_name"] == "json" end|
|labels |可选 |匹配规则|标识附加属性的键值对|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|可选 |辅助| 是否启用 `websocket`(boolean), 缺省 `false`.||
+|status |可选 |辅助| 是否启用此路由, 缺省 `1`。|`1` 表示启用，`0` 表示禁用|
|create_time|可选|辅助|单位为秒的 epoch 时间戳，如果不指定则自动创建|1602883670|
Review comment:
@moonming @nic-chen
The last item `1602883670` is an example, not the 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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536498113
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,226 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1)
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1)
+--- 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",
+ "host": "foo.com",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: hit routes
Review comment:
solve
----------------------------------------------------------------
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] tokers commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r535752690
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,14 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ if status and status ~= 1 then
Review comment:
magic number is tricky, what about using literal string here?
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r535305034
##########
File path: t/lib/server.lua
##########
@@ -56,6 +56,11 @@ function _M.hello_()
end
+function _M.route_status()
Review comment:
we do not need this API
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,275 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
Review comment:
use `1` as route id is enough here
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,275 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/route_status"
Review comment:
use '/hello' should be enough
##########
File path: doc/zh-cn/admin-api.md
##########
@@ -76,6 +76,7 @@
|filter_func|可选|匹配规则|用户自定义的过滤函数。可以使用它来实现特殊场景的匹配要求实现。该函数默认接受一个名为 vars 的输入参数，可以用它来获取 Nginx 变量。|function(vars) return vars["arg_name"] == "json" end|
|labels |可选 |匹配规则|标识附加属性的键值对|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|可选 |辅助| 是否启用 `websocket`(boolean), 缺省 `false`.||
+|status |可选 |辅助| 是否启用此路由, 缺省 `1`.|`1` 表示启用，`0` 表示禁用|
Review comment:
Chinese "。"
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,275 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/route_status"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: hit route
+--- request
+GET /route_status
+--- response_body
+route status
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/uri_status_test"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /route_status
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: delete route(id: uri_status_test)
Review comment:
if the route id is `1`, then we can delete this test case
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,275 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/route_status"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: hit route
+--- request
+GET /route_status
+--- response_body
+route status
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/uri_status_test',
Review comment:
ditto
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,275 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/route_status"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: hit route
+--- request
+GET /route_status
+--- response_body
+route status
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/uri_status_test"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /route_status
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: delete route(id: uri_status_test)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/uri_status_test',
+ ngx.HTTP_DELETE
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: default enable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/host_uri_status_test',
Review comment:
user route id `1` too
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536518394
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,15 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status and status == 0 then
+ goto CONTINUE
Review comment:
Do we really need "goto"?
----------------------------------------------------------------
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] tzssangglass commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738154214
solve above
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536498096
##########
File path: apisix/schema_def.lua
##########
@@ -496,6 +496,13 @@ _M.route = {
},
id = id_schema,
+
+ status = {
+ description = "route status, 1 to enable, 0 to disable",
+ type = "integer",
+ enum = {1, 0},
Review comment:
add 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] spacewander merged pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #2943:
URL: https://github.com/apache/apisix/pull/2943
----------------------------------------------------------------
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] tzssangglass commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-741489372
thanks to the people at code review, and thanks for the guidance!
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536519752
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,15 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status and status == 0 then
+ goto CONTINUE
Review comment:
and we need to deal with old data which not has status
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536519281
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,15 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status and status == 0 then
+ goto CONTINUE
Review comment:
you mean like this?
```
if status = 1 then
push_host_router(route, host_routes, only_uri_routes)
end
```
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-739250972
got it
Thanks,
Ming Wen
Twitter: _WenMing
罗泽轩 <no...@github.com> 于2020年12月5日周六 下午9:25写道：
> *@spacewander* commented on this pull request.
> ------------------------------
>
> In doc/zh-cn/admin-api.md
> <https://github.com/apache/apisix/pull/2943#discussion_r536758557>:
>
> > @@ -76,6 +76,7 @@
>
> |filter_func|可选|匹配规则|用户自定义的过滤函数。可以使用它来实现特殊场景的匹配要求实现。该函数默认接受一个名为 vars 的输入参数，可以用它来获取 Nginx 变量。|function(vars) return vars["arg_name"] == "json" end|
>
> |labels |可选 |匹配规则|标识附加属性的键值对|{"version":"v2","build":"16","env":"production"}|
>
> |enable_websocket|可选 |辅助| 是否启用 `websocket`(boolean), 缺省 `false`.||
>
> +|status |可选 |辅助| 是否启用此路由, 缺省 `1`。|`1` 表示启用，`0` 表示禁用|
>
> |create_time|可选|辅助|单位为秒的 epoch 时间戳，如果不指定则自动创建|1602883670|
>
>
> @moonming <https://github.com/moonming> @nic-chen
> <https://github.com/nic-chen>
> The last item 1602883670 is an example, not the default value.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/apisix/pull/2943#discussion_r536758557>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK5QQEEOV5BXKKFHUA3STIYDBANCNFSM4ULQ4ZMA>
> .
>
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536991197
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,304 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1) with uri match
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1) with host_uri match
+--- 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",
+ "host": "foo.com",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: hit route
+--- request
+GET /hello
+--- yaml_config eval: $::yaml_config
+--- more_headers
+Host: foo.com
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: route not found, failed by disable
+--- request
+GET /hello
+--- yaml_config eval: $::yaml_config
+--- more_headers
+Host: foo.com
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: specify an invalid status value
+--- 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",
+ "status": 100,
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/\{"error_msg":"invalid configuration: property \\"status\\" validation failed: matches non of the enum values"\}/
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: compatible with old rpote data in etcd which not has status
Review comment:
`rpote` Is this a typo, it should be `route`, 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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536495162
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,226 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1)
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1)
Review comment:
good idea
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r535793418
##########
File path: apisix/schema_def.lua
##########
@@ -496,6 +496,13 @@ _M.route = {
},
id = id_schema,
+
+ status = {
+ description = "route status, 1 to enable, 0 to disable",
+ type = "integer",
+ enum = {1, 0},
Review comment:
@tokers the `status` is define here
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r535760947
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,14 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ if status and status ~= 1 then
Review comment:
just follow this
https://github.com/apache/apisix/blob/9e3c34a0103b88d340285017e34021cbc1f0fde8/apisix/schema_def.lua#L630-L635
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536518754
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -95,7 +95,8 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes or {}) do
local status = core.table.try_read_attr(route, "value", "status")
- if status and status ~= 1 then
+ -- check the status
+ if status and status == 0 then
Review comment:
yes, i will
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536519588
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,15 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status and status == 0 then
+ goto CONTINUE
Review comment:
yes
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536517191
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -95,7 +95,8 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes or {}) do
local status = core.table.try_read_attr(route, "value", "status")
- if status and status ~= 1 then
+ -- check the status
+ if status and status == 0 then
Review comment:
will there be incompatibilities?
such as an existing route in etcd does not have the `status` attribute?
----------------------------------------------------------------
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] tzssangglass edited a comment on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass edited a comment on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738170623
describe a strange phenomenon, I was using `/route_status` api and create new routes because I found that using route id 1 and `/hello` always failed when I ran my local test cases, even through other test cases, like
```
$prove -I../test-nginx/lib -I./ -r -s t/router/radixtree-host-uri.t
t/router/radixtree-host-uri.t .. 10/?
# Failed test 'TEST 5: hit routes - status code ok'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
# got: '404'
# expected: '200'
# Failed test 'TEST 5: hit routes - response_body - response is expected (repeated req 0, req 0)'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
# got: '{"error_msg":"404 Route Not Found"}
# '
# expected: 'hello world
# '
# Failed test 'TEST 6: hit routes - status code ok'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
# got: '404'
# expected: '200'
# Failed test 'TEST 6: hit routes - response_body - response is expected (repeated req 0, req 0)'
# at /usr/local/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
# got: '{"error_msg":"404 Route Not Found"}
# '
# expected: 'hello world
# '
```
until I debugged at https://github.com/apache/apisix/blob/9e3c34a0103b88d340285017e34021cbc1f0fde8/apisix/http/router/radixtree_uri.lua#L85
when run test, the `user_routes.values` is nil, but when start apisix, it work.
after find this exception, something change, run the test case is back to normal, and cannot be reproduced. >_<|||
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536524735
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,15 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status and status == 0 then
+ goto CONTINUE
Review comment:
get
----------------------------------------------------------------
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] tzssangglass commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-739117237
> is it necessary to add a case：
> create a route disabled -> hit route failed-> enable it -> hit route successfully?
test case 1 ~ 4 is testing for uri match, the order is
create a route enable(default) -> hit route successfully-> disable it -> hit route fail
test case 5 ~ 8 is testing for host + uri match, the order is same as above.
I think combining these four steps into one test case would have the same effect.
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536787433
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,11 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
- push_host_router(route, host_routes, only_uri_routes)
+ local status = core.table.try_read_attr(route, "value", "status")
+ -- check the status
+ if status == 1 then
Review comment:
The existing old data has no field "status".
we need to make sure it works fine for this case.
`if not status or status == 1 then`, this way is safer.
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536519362
##########
File path: doc/admin-api.md
##########
@@ -72,6 +72,7 @@
|service_protocol|False|Upstream protocol type|only `grpc` and `http` are supported|`http` is the default value; Must set `grpc` if using `gRPC proxy` or `gRPC transcode`|
|labels |False |Match Rules|Key/value pairs to specify attributes|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|False|Auxiliary| enable `websocket`(boolean), default `false`.||
+|status |False|Auxiliary| enable this route, default `1`.|`1` to enable, `0` to disable|
Review comment:
get
----------------------------------------------------------------
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] liuxiran commented on pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#issuecomment-738796523
is it necessary to add a case：
create a route disabled -> hit route failed-> enable it -> hit route successfully?
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536503511
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -95,7 +95,8 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes or {}) do
local status = core.table.try_read_attr(route, "value", "status")
- if status and status ~= 1 then
+ -- check the status
+ if status and status == 0 then
Review comment:
only `if status == 0 then`, this is enough
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r537017724
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,304 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1) with uri match
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1) with host_uri match
+--- 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",
+ "host": "foo.com",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: hit route
+--- request
+GET /hello
+--- yaml_config eval: $::yaml_config
+--- more_headers
+Host: foo.com
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: route not found, failed by disable
+--- request
+GET /hello
+--- yaml_config eval: $::yaml_config
+--- more_headers
+Host: foo.com
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: specify an invalid status value
+--- 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",
+ "status": 100,
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/\{"error_msg":"invalid configuration: property \\"status\\" validation failed: matches non of the enum values"\}/
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: compatible with old rpote data in etcd which not has status
Review comment:
> `rpote` Is this a typo, it should be `route`, right?
yep
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536519115
##########
File path: doc/zh-cn/admin-api.md
##########
@@ -76,6 +76,7 @@
|filter_func|可选|匹配规则|用户自定义的过滤函数。可以使用它来实现特殊场景的匹配要求实现。该函数默认接受一个名为 vars 的输入参数，可以用它来获取 Nginx 变量。|function(vars) return vars["arg_name"] == "json" end|
|labels |可选 |匹配规则|标识附加属性的键值对|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|可选 |辅助| 是否启用 `websocket`(boolean), 缺省 `false`.||
+|status |可选 |辅助| 是否启用此路由, 缺省 `1`。|`1` 表示启用，`0` 表示禁用|
|create_time|可选|辅助|单位为秒的 epoch 时间戳，如果不指定则自动创建|1602883670|
Review comment:
Why the default time is not "now"? @nic-chen @membphis
##########
File path: doc/admin-api.md
##########
@@ -72,6 +72,7 @@
|service_protocol|False|Upstream protocol type|only `grpc` and `http` are supported|`http` is the default value; Must set `grpc` if using `gRPC proxy` or `gRPC transcode`|
|labels |False |Match Rules|Key/value pairs to specify attributes|{"version":"v2","build":"16","env":"production"}|
|enable_websocket|False|Auxiliary| enable `websocket`(boolean), default `false`.||
+|status |False|Auxiliary| enable this route, default `1`.|`1` to enable, `0` to disable|
Review comment:
Need to add examples for how to use this.
----------------------------------------------------------------
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 #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536496336
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,226 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1)
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1)
+--- 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",
+ "host": "foo.com",
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- yaml_config eval: $::yaml_config
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: hit routes
Review comment:
Singular, `hit route`
##########
File path: apisix/http/router/radixtree_uri.lua
##########
@@ -38,6 +38,11 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes) do
if type(route) == "table" then
+ local status = core.table.try_read_attr(route, "value", "status")
+ if status and status ~= 1 then
Review comment:
we can use `if status == 0 then` here, it seems clearer
##########
File path: apisix/http/router/radixtree_host_uri.lua
##########
@@ -94,7 +94,14 @@ local function create_radixtree_router(routes)
host_router = nil
for _, route in ipairs(routes or {}) do
+ local status = core.table.try_read_attr(route, "value", "status")
+ if status and status ~= 1 then
Review comment:
add comments?
##########
File path: t/node/route-status.t
##########
@@ -0,0 +1,226 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+our $yaml_config = <<_EOC_;
+apisix:
+ node_listen: 1984
+ router:
+ http: 'radixtree_host_uri'
+ admin_key: null
+_EOC_
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: default enable route(id: 1)
+--- 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",
+ "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 2: hit route
+--- request
+GET /hello
+--- response_body
+hello world
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: disable route
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin").test
+
+ local data = {status = 0}
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ core.json.encode(data),
+ [[{
+ "node": {
+ "value": {
+ "status": 0
+ },
+ "key": "/apisix/routes/1"
+ },
+ "action": "compareAndSwap"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: route not found, failed by disable
+--- request
+GET /hello
+--- error_code: 404
+--- response_body
+{"error_msg":"404 Route Not Found"}
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: default enable route(id: 1)
Review comment:
not update?
----------------------------------------------------------------
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] tzssangglass commented on a change in pull request #2943: feat: support enable/disable route
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #2943:
URL: https://github.com/apache/apisix/pull/2943#discussion_r536498193
##########
File path: apisix/http/router/radixtree_uri.lua
##########
@@ -38,6 +38,11 @@ local function create_radixtree_router(routes)
for _, route in ipairs(routes) do
if type(route) == "table" then
+ local status = core.table.try_read_attr(route, "value", "status")
+ if status and status ~= 1 then
Review comment:
solve
----------------------------------------------------------------
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