From fe52f2a1b8c77956d6912012e4ad6eb6e3de39c7 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 19 Dec 2024 15:27:06 +0530 Subject: [PATCH 01/14] fix: corrupt data in routes() due to user requeset --- apisix/http/router/radixtree_host_uri.lua | 2 +- apisix/http/router/radixtree_uri.lua | 2 +- apisix/http/router/radixtree_uri_with_parameter.lua | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index 680a04fbe815..71eea37450fa 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -143,7 +143,7 @@ local function create_radixtree_router(routes) end function _M.match(api_ctx) - local user_routes = _M.user_routes + local user_routes = core.table.deepcopy(_M.user_routes) local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version diff --git a/apisix/http/router/radixtree_uri.lua b/apisix/http/router/radixtree_uri.lua index 7c1b5c0c147a..f4862978677e 100644 --- a/apisix/http/router/radixtree_uri.lua +++ b/apisix/http/router/radixtree_uri.lua @@ -28,7 +28,7 @@ local _M = {version = 0.2} local uri_routes = {} local uri_router function _M.match(api_ctx) - local user_routes = _M.user_routes + local user_routes = core.table.deepcopy(_M.user_routes) local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version diff --git a/apisix/http/router/radixtree_uri_with_parameter.lua b/apisix/http/router/radixtree_uri_with_parameter.lua index 3f10f4fcac49..1ffeb6efbc42 100644 --- a/apisix/http/router/radixtree_uri_with_parameter.lua +++ b/apisix/http/router/radixtree_uri_with_parameter.lua @@ -28,7 +28,7 @@ local _M = {} local uri_routes = {} local uri_router function _M.match(api_ctx) - local user_routes = _M.user_routes + local user_routes = core.table.deepcopy(_M.user_routes) local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version From e7a1c329e21b913640ba5dfbd2a405c48dd44bbf Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Sat, 21 Dec 2024 04:32:00 +0530 Subject: [PATCH 02/14] fix tests --- apisix/http/router/radixtree_host_uri.lua | 2 +- apisix/http/router/radixtree_uri.lua | 2 +- apisix/http/router/radixtree_uri_with_parameter.lua | 2 +- apisix/init.lua | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index 71eea37450fa..680a04fbe815 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -143,7 +143,7 @@ local function create_radixtree_router(routes) end function _M.match(api_ctx) - local user_routes = core.table.deepcopy(_M.user_routes) + local user_routes = _M.user_routes local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version diff --git a/apisix/http/router/radixtree_uri.lua b/apisix/http/router/radixtree_uri.lua index f4862978677e..7c1b5c0c147a 100644 --- a/apisix/http/router/radixtree_uri.lua +++ b/apisix/http/router/radixtree_uri.lua @@ -28,7 +28,7 @@ local _M = {version = 0.2} local uri_routes = {} local uri_router function _M.match(api_ctx) - local user_routes = core.table.deepcopy(_M.user_routes) + local user_routes = _M.user_routes local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version diff --git a/apisix/http/router/radixtree_uri_with_parameter.lua b/apisix/http/router/radixtree_uri_with_parameter.lua index 1ffeb6efbc42..3f10f4fcac49 100644 --- a/apisix/http/router/radixtree_uri_with_parameter.lua +++ b/apisix/http/router/radixtree_uri_with_parameter.lua @@ -28,7 +28,7 @@ local _M = {} local uri_routes = {} local uri_router function _M.match(api_ctx) - local user_routes = core.table.deepcopy(_M.user_routes) + local user_routes = _M.user_routes local _, service_version = get_services() if not cached_router_version or cached_router_version ~= user_routes.conf_version or not cached_service_version or cached_service_version ~= service_version diff --git a/apisix/init.lua b/apisix/init.lua index 103a8c1d7584..c61ee5ce2aac 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -493,7 +493,7 @@ function _M.handle_upstream(api_ctx, route, enable_websocket) api_ctx.matched_route = route end - local route_val = route.value + local route_val = core.table.deepcopy(route.value) api_ctx.matched_upstream = (route.dns_value and route.dns_value.upstream) From 24826aab9f261c99a6657ac80e1fb57f1897d6a3 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Sun, 22 Dec 2024 22:41:17 +0530 Subject: [PATCH 03/14] remove unnnecessary fields --- apisix/control/router.lua | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index e6e5ff9b3b0e..2e7d26be1c43 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -81,7 +81,12 @@ do if type(body) == "table" and ngx.header["Content-Type"] == nil then core.response.set_header("Content-Type", "application/json") end - + if type(body) then + body.checker = nil + body.checker_idx = nil + body.checker_upstream = nil + body.clean_handlers = {} + end core.response.exit(code, body) end end From e24b9e6d870bb82249d6f6acfe97caa35760ed21 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Sun, 22 Dec 2024 22:43:56 +0530 Subject: [PATCH 04/14] revert init.lua --- apisix/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/init.lua b/apisix/init.lua index c61ee5ce2aac..103a8c1d7584 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -493,7 +493,7 @@ function _M.handle_upstream(api_ctx, route, enable_websocket) api_ctx.matched_route = route end - local route_val = core.table.deepcopy(route.value) + local route_val = route.value api_ctx.matched_upstream = (route.dns_value and route.dns_value.upstream) From 172292b7711805b2df31308efeba7dc5344ce923 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 23 Dec 2024 01:31:40 +0530 Subject: [PATCH 05/14] fix tests --- apisix/control/router.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index 2e7d26be1c43..793fae6e0de5 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -85,7 +85,9 @@ do body.checker = nil body.checker_idx = nil body.checker_upstream = nil - body.clean_handlers = {} + if body.clean_handlers then + body.clean_handlers = {} + end end core.response.exit(code, body) end From 21387f2822789ebdcbc7f197ae4696b99e0fb64a Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 23 Dec 2024 02:11:58 +0530 Subject: [PATCH 06/14] fix tests --- apisix/control/router.lua | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index 793fae6e0de5..a8b1e5b3d906 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -85,9 +85,7 @@ do body.checker = nil body.checker_idx = nil body.checker_upstream = nil - if body.clean_handlers then - body.clean_handlers = {} - end + body.clean_handlers = nil end core.response.exit(code, body) end From 8900fa1cd31a9cdcf2eaec376fcce9b53f331ef4 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 23 Dec 2024 02:12:55 +0530 Subject: [PATCH 07/14] fix test --- apisix/control/router.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index a8b1e5b3d906..2146ca14bb2c 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -81,11 +81,13 @@ do if type(body) == "table" and ngx.header["Content-Type"] == nil then core.response.set_header("Content-Type", "application/json") end - if type(body) then + if type(body) == "table" then body.checker = nil body.checker_idx = nil body.checker_upstream = nil - body.clean_handlers = nil + if body.clean_handlers then + body.clean_handlers = {} + end end core.response.exit(code, body) end From 0ca4ee384f459c53a9faf4b40d7406a456ef750d Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 23 Dec 2024 13:20:55 +0530 Subject: [PATCH 08/14] move logic to deepcopy --- apisix/control/router.lua | 8 -------- apisix/control/v1.lua | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index 2146ca14bb2c..bf4978d1db5c 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -81,14 +81,6 @@ do if type(body) == "table" and ngx.header["Content-Type"] == nil then core.response.set_header("Content-Type", "application/json") end - if type(body) == "table" then - body.checker = nil - body.checker_idx = nil - body.checker_upstream = nil - if body.clean_handlers then - body.clean_handlers = {} - end - end core.response.exit(code, body) end end diff --git a/apisix/control/v1.lua b/apisix/control/v1.lua index 213ed0ac46b8..1e3652fc59cd 100644 --- a/apisix/control/v1.lua +++ b/apisix/control/v1.lua @@ -269,6 +269,14 @@ local function iter_add_get_routes_info(values, route_id) if new_route.value.upstream and new_route.value.upstream.parent then new_route.value.upstream.parent = nil end + -- remove healthcheck info + new_route.checker = nil + new_route.checker_idx = nil + new_route.checker_upstream = nil + + if new_route.clean_handlers then + new_route.clean_handlers = {} + end core.table.insert(infos, new_route) -- check the route id if route_id and route.value.id == route_id then From 0086c69a4b1d7f7fded38b6a0e2e05b3e0ea7a92 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 23 Dec 2024 13:23:14 +0530 Subject: [PATCH 09/14] remove unnecessary line --- apisix/control/router.lua | 1 + apisix/control/v1.lua | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/control/router.lua b/apisix/control/router.lua index bf4978d1db5c..e6e5ff9b3b0e 100644 --- a/apisix/control/router.lua +++ b/apisix/control/router.lua @@ -81,6 +81,7 @@ do if type(body) == "table" and ngx.header["Content-Type"] == nil then core.response.set_header("Content-Type", "application/json") end + core.response.exit(code, body) end end diff --git a/apisix/control/v1.lua b/apisix/control/v1.lua index 1e3652fc59cd..ae94fb8c424d 100644 --- a/apisix/control/v1.lua +++ b/apisix/control/v1.lua @@ -273,7 +273,6 @@ local function iter_add_get_routes_info(values, route_id) new_route.checker = nil new_route.checker_idx = nil new_route.checker_upstream = nil - if new_route.clean_handlers then new_route.clean_handlers = {} end From 1620a48840eb1479ecb055efc892b20770c29cc8 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 26 Dec 2024 00:58:18 +0530 Subject: [PATCH 10/14] apply suggestions --- apisix/control/v1.lua | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apisix/control/v1.lua b/apisix/control/v1.lua index ae94fb8c424d..4d35018b89de 100644 --- a/apisix/control/v1.lua +++ b/apisix/control/v1.lua @@ -273,9 +273,7 @@ local function iter_add_get_routes_info(values, route_id) new_route.checker = nil new_route.checker_idx = nil new_route.checker_upstream = nil - if new_route.clean_handlers then - new_route.clean_handlers = {} - end + new_route.clean_handlers = nil core.table.insert(infos, new_route) -- check the route id if route_id and route.value.id == route_id then @@ -359,6 +357,11 @@ local function iter_add_get_services_info(values, svc_id) if new_svc.value.upstream and new_svc.value.upstream.parent then new_svc.value.upstream.parent = nil end + -- remove healthcheck info + new_svc.checker = nil + new_svc.checker_idx = nil + new_svc.checker_upstream = nil + new_svc.clean_handlers = nil core.table.insert(infos, new_svc) -- check the service id if svc_id and svc.value.id == svc_id then From 0c1e5441b20fe3e3c414e10d12ecd685ece8207c Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 26 Dec 2024 13:44:34 +0530 Subject: [PATCH 11/14] add test --- t/control/control-healthcheck-bug-fix.t | 139 ++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 t/control/control-healthcheck-bug-fix.t diff --git a/t/control/control-healthcheck-bug-fix.t b/t/control/control-healthcheck-bug-fix.t new file mode 100644 index 000000000000..81fa7a2b6b39 --- /dev/null +++ b/t/control/control-healthcheck-bug-fix.t @@ -0,0 +1,139 @@ +# +# 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(); + +run_tests(); + +__DATA__ + +=== TEST 1: setup route +--- 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, + [[{ + "methods": ["GET"], + "upstream": { + "nodes": { + "httpbin.org:80": 1, + "mockbin.org:80": 1 + }, + "type": "roundrobin" + }, + "uri": "/*" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- timeout: 10 + + + +=== TEST 2: hit the route +--- request +GET /status/403 +--- error_code: 403 +--- timeout: 10 + + + +=== TEST 3: hit control api +--- config + location /t { + content_by_lua_block { + local json = require("toolkit.json") + local t = require("lib.test_admin") + + local passed = true + + for i = 1, 40 do + local code, body, res = t.test('/v1/routes/1', ngx.HTTP_GET) + if code ~= ngx.HTTP_OK then + passed = code + break + end + end + + if passed then + ngx.say("passed") + else + ngx.say("failed. got status code: ", passed) + end + } + } +--- request +GET /t +--- response_body +passed +--- timeout: 10 + + + +=== TEST 4: hit the route again +--- request +GET /status/403 +--- error_code: 403 +--- timeout: 10 + + + +=== TEST 5: hit control api +--- config + location /t { + content_by_lua_block { + local json = require("toolkit.json") + local t = require("lib.test_admin") + + local passed = true + + for i = 1, 40 do + local code, body, res = t.test('/v1/routes/1', ngx.HTTP_GET) + if code ~= ngx.HTTP_OK then + passed = code + break + end + end + + if passed then + ngx.say("passed") + else + ngx.say("failed. got status code: ", passed) + end + } + } +--- request +GET /t +--- response_body +passed +--- timeout: 10 From 305c216710071e371bfc35c47e0c2cb5b50b3e4c Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 26 Dec 2024 13:45:45 +0530 Subject: [PATCH 12/14] Update control-healthcheck-bug-fix.t --- t/control/control-healthcheck-bug-fix.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/control/control-healthcheck-bug-fix.t b/t/control/control-healthcheck-bug-fix.t index 81fa7a2b6b39..95707fd719b2 100644 --- a/t/control/control-healthcheck-bug-fix.t +++ b/t/control/control-healthcheck-bug-fix.t @@ -38,7 +38,7 @@ __DATA__ "upstream": { "nodes": { "httpbin.org:80": 1, - "mockbin.org:80": 1 + "mockbin.org:80": 1 }, "type": "roundrobin" }, From 40b70aa8f0ba1cfb47241e2b889e0841dd374043 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 26 Dec 2024 13:46:43 +0530 Subject: [PATCH 13/14] Update control-healthcheck-bug-fix.t --- t/control/control-healthcheck-bug-fix.t | 5 ----- 1 file changed, 5 deletions(-) diff --git a/t/control/control-healthcheck-bug-fix.t b/t/control/control-healthcheck-bug-fix.t index 95707fd719b2..f30de83a70a7 100644 --- a/t/control/control-healthcheck-bug-fix.t +++ b/t/control/control-healthcheck-bug-fix.t @@ -56,7 +56,6 @@ __DATA__ GET /t --- response_body passed ---- timeout: 10 @@ -64,7 +63,6 @@ passed --- request GET /status/403 --- error_code: 403 ---- timeout: 10 @@ -96,7 +94,6 @@ GET /status/403 GET /t --- response_body passed ---- timeout: 10 @@ -104,7 +101,6 @@ passed --- request GET /status/403 --- error_code: 403 ---- timeout: 10 @@ -136,4 +132,3 @@ GET /status/403 GET /t --- response_body passed ---- timeout: 10 From 6217e11116b9395738a75e6c4eb7516c6ecbe740 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Thu, 26 Dec 2024 13:49:33 +0530 Subject: [PATCH 14/14] lint fix --- t/control/control-healthcheck-bug-fix.t | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/t/control/control-healthcheck-bug-fix.t b/t/control/control-healthcheck-bug-fix.t index f30de83a70a7..ee2e516dbea5 100644 --- a/t/control/control-healthcheck-bug-fix.t +++ b/t/control/control-healthcheck-bug-fix.t @@ -38,7 +38,7 @@ __DATA__ "upstream": { "nodes": { "httpbin.org:80": 1, - "mockbin.org:80": 1 + "mockbin.org:80": 1 }, "type": "roundrobin" }, @@ -66,15 +66,15 @@ GET /status/403 -=== TEST 3: hit control api +=== TEST 3: hit control api --- config location /t { content_by_lua_block { local json = require("toolkit.json") local t = require("lib.test_admin") - + local passed = true - + for i = 1, 40 do local code, body, res = t.test('/v1/routes/1', ngx.HTTP_GET) if code ~= ngx.HTTP_OK then @@ -82,7 +82,7 @@ GET /status/403 break end end - + if passed then ngx.say("passed") else @@ -104,15 +104,15 @@ GET /status/403 -=== TEST 5: hit control api +=== TEST 5: hit control api --- config location /t { content_by_lua_block { local json = require("toolkit.json") local t = require("lib.test_admin") - + local passed = true - + for i = 1, 40 do local code, body, res = t.test('/v1/routes/1', ngx.HTTP_GET) if code ~= ngx.HTTP_OK then @@ -120,7 +120,7 @@ GET /status/403 break end end - + if passed then ngx.say("passed") else