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 2022/11/14 13:25:00 UTC

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

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


##########
apisix/stream/router/ip_port.lua:
##########
@@ -63,23 +63,46 @@ local create_router
 do
     local sni_to_items = {}
     local tls_routes = {}
+    local routeid_to_protocols = {}
 
     function create_router(items)
         local tls_routes_idx = 1
         local other_routes_idx = 1
         core.table.clear(tls_routes)
         core.table.clear(other_routes)
         core.table.clear(sni_to_items)
+        core.table.clear(routeid_to_protocols)
 
         for _, item in config_util.iterate_values(items) do
             if item.value == nil then
                 goto CONTINUE
             end
+            local route = item.value
+            if route.protocol  then
+               routeid_to_protocols[item.key]=route.protocol.name
+            else
+               routeid_to_protocols[item.key]="No-Protocol"
+            end
+            ::CONTINUE::
+        end
 
+        for _, item in config_util.iterate_values(items) do
+            if item.value == nil then
+                goto CONTINUE
+            end
             local route = item.value
             if route.protocol and route.protocol.superior_id then
                 -- subordinate route won't be matched in the entry
-                -- TODO: check the subordinate relationship in the Admin API
+                local key="/apisix/stream_routes/"..route.protocol.superior_id
+                if routeid_to_protocols[key] == nil then
+                    core.log.warn("There is not exist stream_route: "..key)
+                elseif routeid_to_protocols[key] == "No-Protocol" then
+                    core.log.warn("The stream_route: "..key.." may lacks procotol configuration")
+                elseif routeid_to_protocols[key] == route.protocol.name then
+                    goto CONTINUE
+                else
+                    core.log.warn("Different between stream_route:"..item.key.." and "..key)
+                end
                 goto CONTINUE

Review Comment:
   > Add a logic that checking this when adding the stream route , there are two situations to judge: 1 validate if the stream route with superior id exists The user needs to ensure the order when creating routes in batches --> "super is in front of sub " 2 validate if the stream route with superior id exists and its protocol matches the subordinate Validate if the protocol matches the subordinate only if the stream route with superior id exists。
   > 
   > So I would add a logic that Validate if the protocol matches the subordinate only if the stream route with superior id exists。Do you think it's ok?
   
   I have a suggestion that in this PR we only finish checking refer when deleting the stream route.
   
   You can open a new issue and PR to do what you said.



-- 
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