[GitHub] [apisix] spacewander commented on a diff in pull request #8219: feat: validate the subordinate and check reference when deleting and adding…

[GitBox <gi...@apache.org>]

spacewander commented on code in PR #8219:
URL: https://github.com/apache/apisix/pull/8219#discussion_r1023592625


##########
apisix/admin/stream_routes.lua:
##########
@@ -16,15 +16,38 @@
 --
 local core = require("apisix.core")
 local utils = require("apisix.admin.utils")
+local iterate_values = require("apisix.core.config_util").iterate_values
 local stream_route_checker = require("apisix.stream.router.ip_port").stream_route_checker
 local tostring = tostring
+local table = table
+local tonumber = tonumber
 
 
 local _M = {
     version = 0.1,
     need_v3_filter = true,
 }
 
+local function check_router_refer(items, id)
+    local warn_message = nil
+    local refer_list =  core.tablepool.fetch("refer_list", #items, 0)
+    for _, item in iterate_values(items) do
+        if item.value == nil then
+            goto CONTINUE
+        end
+        local route = item.value
+        if route.protocol and route.protocol.superior_id and route.protocol.superior_id == id then
+            table.insert(refer_list, item["key"])
+        end
+        ::CONTINUE::
+    end
+    if #refer_list > 0  then
+        warn_message = "/stream_routes/" .. id .. " is referred by "
+                        .. table.concat(refer_list,";;")

Review Comment:
   Is there a particular reason to use `;;` instead of `,` as the separator? 



##########
apisix/admin/stream_routes.lua:
##########
@@ -142,6 +165,15 @@ function _M.delete(id)
         return 400, {error_msg = "missing stream route id"}
     end
 
+    local status, body = _M.get()

Review Comment:
   Would be better to use synced data from https://github.com/apache/apisix/blob/2f4a4dba2c0cbc426ae99133c2546112ea71dba8/apisix/stream/router/ip_port.lua#L189.
   
   It may be too expensive to read the whole stream routes from etcd each time we want to delete one.
   
   It might be data race but I think it is acceptable. The current implementation still has a smaller data race since we don't use a transaction to deal with the deletion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org