From c4596c7c8d8532c653f443ff669f00226de5550e Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Sun, 26 Feb 2023 06:43:47 -0500 Subject: [PATCH] fix: check upstream reference in traffic-split plugin when delete upstream --- apisix/admin/upstreams.lua | 72 ++++++++-- t/admin/upstream5.t | 263 +++++++++++++++++++++++++++++++++++++ 2 files changed, 325 insertions(+), 10 deletions(-) diff --git a/apisix/admin/upstreams.lua b/apisix/admin/upstreams.lua index a85d24d8d9e16..00e813dd4d5d6 100644 --- a/apisix/admin/upstreams.lua +++ b/apisix/admin/upstreams.lua @@ -15,6 +15,8 @@ -- limitations under the License. -- local core = require("apisix.core") +local config_util = require("apisix.core.config_util") +local router = require("apisix.router") local get_routes = require("apisix.router").http_routes local get_services = require("apisix.http.service").services local apisix_upstream = require("apisix.upstream") @@ -33,17 +35,43 @@ local function check_conf(id, conf, need_id) return true end +local function up_id_in_plugins(plugins, up_id) + if plugins and plugins["traffic-split"] + and plugins["traffic-split"].rules then + + for _, rule in ipairs (plugins["traffic-split"].rules) do + local plugin_upstreams = rule.weighted_upstreams + + for _, plugin_upstream in ipairs(plugin_upstreams) do + if plugin_upstream and plugin_upstream.upstream_id + and tostring(plugin_upstream.upstream_id) == tostring(up_id) then + + return true + end + end + end + + return false + end +end + local function delete_checker(id) local routes, routes_ver = get_routes() if routes_ver and routes then for _, route in ipairs(routes) do - if type(route) == "table" and route.value - and route.value.upstream_id - and tostring(route.value.upstream_id) == id then - return 400, {error_msg = "can not delete this upstream," - .. " route [" .. route.value.id - .. "] is still using it now"} + if type(route) == "table" and route.value then + if up_id_in_plugins(route.value.plugins, id) == true then + return 400, {error_msg = "can not delete this upstream," + .. " traffic-split plugin in route [" .. route.value.id + .. "] is still using it now"} + end + + if route.value.upstream_id and tostring(route.value.upstream_id) == id then + return 400, {error_msg = "can not delete this upstream," + .. " route [" .. route.value.id + .. "] is still using it now"} + end end end end @@ -53,11 +81,35 @@ local function delete_checker(id) core.log.info("services_ver: ", services_ver) if services_ver and services then for _, service in ipairs(services) do - if type(service) == "table" and service.value - and service.value.upstream_id - and tostring(service.value.upstream_id) == id then + if type(service) == "table" and service.value then + if up_id_in_plugins(service.value.plugins, id) then + return 400, {error_msg = "can not delete this upstream," + .. " traffic-split plugin in service [" + .. service.value.id + .. "] is still using it now"} + end + + if service.value.upstream_id and tostring(service.value.upstream_id) == id then + return 400, {error_msg = "can not delete this upstream," + .. " service [" .. service.value.id + .. "] is still using it now"} + end + end + end + end + + local global_rules = router.global_rules + if global_rules and global_rules.values + and #global_rules.values > 0 then + + for _, global_rule in config_util.iterate_values(global_rules.values) do + if global_rule and global_rule.value + and global_rule.value.plugins + and up_id_in_plugins(global_rule.value.plugins, id) then + return 400, {error_msg = "can not delete this upstream," - .. " service [" .. service.value.id + .. " traffic-split plugin in global_rule [" + .. global_rule.value.id .. "] is still using it now"} end end diff --git a/t/admin/upstream5.t b/t/admin/upstream5.t index 9fd59bfe7e863..b013683dc064b 100644 --- a/t/admin/upstream5.t +++ b/t/admin/upstream5.t @@ -111,3 +111,266 @@ passed } --- response_body passed + + + +=== TEST4: prepare upstream +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/upstreams/1", ngx.HTTP_PUT, [[{ + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }]]) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + + } + } +--- response_body +passed + + + +=== TEST 4: prepare 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, [[{ + "plugins": { + "traffic-split": { + "rules": [ + { + "weighted_upstreams": [ + { + "upstream_id": 1, + "weight": 1 + }, + { + "weight": 1 + } + ] + } + ] + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]]) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + + } + } +--- response_body +passed + + + +=== TEST 5: delete upstream when traffic-split plugin in route still refer it +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/upstreams/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- error_code: 400 +--- response_body +{"error_msg":"can not delete this upstream, traffic-split plugin in route [1] is still using it now"} + + + +=== TEST 6: delete 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_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 7: prepare service +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/services/1", ngx.HTTP_PUT, [[{ + "plugins": { + "traffic-split": { + "rules": [ + { + "weighted_upstreams": [ + { + "upstream_id": 1, + "weight": 1 + }, + { + "weight": 1 + } + ] + } + ] + } + } + }]]) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 8: delete upstream when traffic-split plugin in service still refer it +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/upstreams/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- error_code: 400 +--- response_body +{"error_msg":"can not delete this upstream, traffic-split plugin in service [1] is still using it now"} + + + +=== TEST 9: delete service +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/services/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 10: prepare global_rule +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + + local code, body = t("/apisix/admin/global_rules/1", ngx.HTTP_PUT, [[{ + "plugins": { + "traffic-split": { + "rules": [ + { + "weighted_upstreams": [ + { + "upstream_id": 1, + "weight": 1 + }, + { + "weight": 1 + } + ] + } + ] + } + } + }]]) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 11: delete upstream when traffic-split plugin in global_rule still refer it +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/upstreams/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- error_code: 400 +--- response_body +{"error_msg":"can not delete this upstream, traffic-split plugin in global_rule [1] is still using it now"} + + + +=== TEST 12: delete global_rule +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/global_rules/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 13: delete upstream +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t("/apisix/admin/upstreams/1", ngx.HTTP_DELETE) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed