[GitHub] [apisix] spacewander commented on a change in pull request #4980: fix(stream_route): replace ip match tools by using ipmatcher

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

spacewander commented on a change in pull request #4980:
URL: https://github.com/apache/apisix/pull/4980#discussion_r702537405



##########
File path: apisix/stream/router/ip_port.lua
##########
@@ -33,14 +33,30 @@ local _M = {version = 0.1}
 
 local function match_addrs(route, vars)
     -- todo: use resty-ipmatcher to support multiple ip address
-    if route.value.remote_addr and
-       route.value.remote_addr ~= vars.remote_addr then
-        return false
+    if route.value.remote_addr then

Review comment:
       Better to put the init in the `create_router` instead of the match

##########
File path: rockspec/apisix-master-0.rockspec
##########
@@ -69,6 +69,7 @@ dependencies = {
     "ext-plugin-proto = 0.3.0",
     "casbin = 1.26.0",
     "api7-snowflake = 2.0-1",
+    "lua-resty-ipmatcher = 0.6.1"

Review comment:
       Don't add lua-resty-ipmatcher twice

##########
File path: t/stream-plugin/mqtt-proxy.t
##########
@@ -220,7 +220,56 @@ hello world
 
 
 
-=== TEST 9: set route with invalid host
+=== TEST 9: set route with IP CIDR

Review comment:
       New test should be added from the bottom of the file. And it should be in stream-node/sanity.t




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