feat(router/atc): support net.src.* and net.dst.* fields in HTTP …

…expression routes (#11950)

These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes.

KAG-2963
KAG-3032

changelog/unreleased/kong/support_net_src_dst_field_in_expression.yml

@@ -0,0 +1,4 @@
+message: |
+  `net.src.*` and `net.dst.*` match fields are now accessible in HTTP routes defined using expressions.
+type: feature
+scope: Core

kong/db/schema/entities/routes.lua

@@ -4,13 +4,14 @@ local deprecation = require("kong.deprecation")
 
 local validate_route
 do
-  local get_schema     = require("kong.router.atc").schema
+  local get_schema = require("kong.router.atc").schema
   local get_expression = require("kong.router.compat").get_expression
+  local transform_expression = require("kong.router.expressions").transform_expression
 
   -- works with both `traditional_compatiable` and `expressions` routes`
   validate_route = function(entity)
     local schema = get_schema(entity.protocols)
-    local exp = entity.expression or get_expression(entity)
+    local exp = transform_expression(entity) or get_expression(entity)
 
     local ok, err = router.validate(schema, exp)
     if not ok then

kong/router/atc.lua

@@ -66,7 +66,10 @@ do
                   "http.queries.*",
                  },
 
-    ["Int"]    = {"net.port",
+    ["Int"]    = {"net.src.port", "net.dst.port",
+                 },
+
+    ["IpAddr"] = {"net.src.ip", "net.dst.ip",
                  },
   }
 
@@ -404,8 +407,8 @@ function _M:matching(params)
   local req_host = params.host
 
   check_select_params(params.method, req_uri, req_host, params.scheme,
-                      nil, nil,
-                      nil, nil,
+                      params.src_ip, params.src_port,
+                      params.dst_ip, params.dst_port,
                       params.sni, params.headers, params.queries)
 
   local host, port = split_host_port(req_host)
@@ -464,8 +467,8 @@ end
 
 -- only for unit-testing
 function _M:select(req_method, req_uri, req_host, req_scheme,
-                   _, _,
-                   _, _,
+                   src_ip, src_port,
+                   dst_ip, dst_port,
                    sni, req_headers, req_queries)
 
   local params = {
@@ -476,6 +479,11 @@ function _M:select(req_method, req_uri, req_host, req_scheme,
     sni     = sni,
     headers = req_headers,
     queries = req_queries,
+
+    src_ip   = src_ip,
+    src_port = src_port,
+    dst_ip   = dst_ip,
+    dst_port = dst_port,
   }
 
   return self:matching(params)

kong/router/compat.lua

@@ -217,10 +217,10 @@ local function get_expression(route)
         host = host:sub(1, -2)
       end
 
-      local exp = "http.host ".. op .. " \"" .. host .. "\""
+      local exp = "http.host ".. op .. " r#\"" .. host .. "\"#"
       if port then
         exp = "(" .. exp .. LOGICAL_AND ..
-              "net.port ".. OP_EQUAL .. " " .. port .. ")"
+              "net.dst.port ".. OP_EQUAL .. " " .. port .. ")"
       end
       expression_append(hosts_buf, LOGICAL_OR, exp, i)
     end -- for route.hosts

kong/router/expressions.lua

@@ -1,11 +1,16 @@
 local _M = {}
 
 
+local re_gsub = ngx.re.gsub
+
+
 local atc = require("kong.router.atc")
 local gen_for_field = atc.gen_for_field
 
 
 local OP_EQUAL    = "=="
+local NET_PORT_REG = [[(net\.port)(\s*)([=><!])]]
+local NET_PORT_REPLACE = [[net.dst.port$2$3]]
 
 
 local LOGICAL_AND = atc.LOGICAL_AND
@@ -19,8 +24,35 @@ local PROTOCOLS_OVERRIDE = {
 }
 
 
-local function get_exp_and_priority(route)
+-- net.port => net.dst.port
+local function transform_expression(route)
   local exp = route.expression
+
+  if not exp then
+    return nil
+  end
+
+  if not exp:find("net.port", 1, true) then
+    return exp
+  end
+
+  -- there is "net.port" in expression
+
+  local new_exp = re_gsub(exp, NET_PORT_REG, NET_PORT_REPLACE, "jo")
+
+  if exp ~= new_exp then
+    ngx.log(ngx.WARN, "The field 'net.port' of expression is deprecated " ..
+                      "and will be removed in the upcoming major release, " ..
+                      "please use 'net.dst.port' instead.")
+  end
+
+  return new_exp
+end
+_M.transform_expression = transform_expression
+
+
+local function get_exp_and_priority(route)
+  local exp = transform_expression(route)
   if not exp then
     ngx.log(ngx.ERR, "expecting an expression route while it's not (probably a traditional route). ",
                      "Likely it's a misconfiguration. Please check the 'router_flavor' config in kong.conf")

kong/router/fields.lua

@@ -4,6 +4,7 @@ local buffer = require("string.buffer")
 local type = type
 local ipairs = ipairs
 local assert = assert
+local tonumber = tonumber
 local tb_sort = table.sort
 local tb_concat = table.concat
 local replace_dashes_lower = require("kong.tools.string").replace_dashes_lower
@@ -69,11 +70,6 @@ local FIELDS_FUNCS = {
     function(params)
       return params.scheme
     end,
-
-    ["net.port"] =
-    function(params)
-      return params.port
-    end,
 }
 
 
@@ -105,6 +101,10 @@ if is_http then
 
     FIELDS_FUNCS["net.dst.port"] =
     function(params, ctx)
+      if params.port then
+        return params.port
+      end
+
       if not params.dst_port then
         params.dst_port = tonumber((ctx or ngx.ctx).host_port, 10) or
                           tonumber(var.server_port, 10)
@@ -254,7 +254,7 @@ local function get_cache_key(fields, params, ctx)
   fields_visitor(fields, params, ctx, function(field, value)
 
     -- these fields were not in cache key
-    if field == "net.protocol" or field == "net.port" then
+    if field == "net.protocol" then
       return true
     end
 

spec/01-unit/01-db/01-schema/06-routes_spec.lua

@@ -1510,4 +1510,55 @@ describe("routes schema (flavor = expressions)", function()
     -- verified by `schema/typedefs.lua`
     assert.truthy(errs["@entity"])
   end)
+
+  it("http route still supports net.port but with warning", function()
+    local ngx_log = ngx.log
+    local log = spy.on(ngx, "log")
+
+    finally(function()
+      ngx.log = ngx_log  -- luacheck: ignore
+    end)
+
+    local route = {
+      id             = a_valid_uuid,
+      name           = "my_route",
+      protocols      = { "grpc" },
+      expression     = [[http.method == "GET" && net.port == 8000]],
+      priority       = 100,
+      service        = { id = another_uuid },
+    }
+    route = Routes:process_auto_fields(route, "insert")
+    assert.truthy(Routes:validate(route))
+
+    assert.spy(log).was.called_with(ngx.WARN,
+                                    "The field 'net.port' of expression is deprecated " ..
+                                    "and will be removed in the upcoming major release, " ..
+                                    "please use 'net.dst.port' instead.")
+  end)
+
+  it("http route supports net.src.* fields", function()
+    local route = {
+      id             = a_valid_uuid,
+      name           = "my_route",
+      protocols      = { "https" },
+      expression     = [[http.method == "GET" && net.src.ip == 1.2.3.4 && net.src.port == 80]],
+      priority       = 100,
+      service        = { id = another_uuid },
+    }
+    route = Routes:process_auto_fields(route, "insert")
+    assert.truthy(Routes:validate(route))
+  end)
+
+  it("http route supports net.dst.* fields", function()
+    local route = {
+      id             = a_valid_uuid,
+      name           = "my_route",
+      protocols      = { "grpcs" },
+      expression     = [[http.method == "GET" && net.dst.ip == 1.2.3.4 && net.dst.port == 80]],
+      priority       = 100,
+      service        = { id = another_uuid },
+    }
+    route = Routes:process_auto_fields(route, "insert")
+    assert.truthy(Routes:validate(route))
+  end)
 end)

spec/01-unit/08-router_spec.lua

@@ -2255,7 +2255,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
           end)
         end)
 
-        describe("match http.headers.*", function()
+        describe("generate http expression", function()
           local use_case
           local get_expression = atc_compat.get_expression
 
@@ -2265,19 +2265,27 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
                 service = service,
                 route = {
                   id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
-                  methods = { "GET" },
                 },
               },
             }
           end)
 
-          it("should always add lower()", function()
+          it("should always add lower() when matching http.headers.*", function()
+            use_case[1].route.methods = { "GET" }
             use_case[1].route.headers = { test = { "~*Quote" }, }
 
             assert.equal([[(http.method == r#"GET"#) && (any(lower(http.headers.test)) ~ r#"quote"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)
+
+          it("should use 'net.dst.port' when deprecating 'net.port'", function()
+            use_case[1].route.hosts = { "www.example.com:8000" }
+
+            assert.equal([[((http.host == r#"www.example.com"# && net.dst.port == 8000))]],
+                         get_expression(use_case[1].route))
+            assert(new_router(use_case))
+          end)
         end)
       end   -- if flavor ~= "traditional"
 
@@ -5248,5 +5256,104 @@ do
       assert.same(ctx.route_match_cached, "neg")
     end)
   end)
+
+  describe("Router (flavor = " .. flavor .. ") [http]", function()
+    reload_router(flavor)
+
+    local use_case, router
+
+    lazy_setup(function()
+      use_case = {
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
+            expression = [[http.host == "www.example.com" && net.port == 8000]],
+            priority = 100,
+          },
+        },
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8102",
+            expression = [[net.src.ip == 1.1.1.1 && net.dst.ip == 2.2.2.2 && http.method == "GET"]],
+            priority = 100,
+          },
+        },
+      }
+    end)
+
+    it("select() should convert 'net.port' to 'net.dst.port' and work well", function()
+      router = assert(new_router(use_case))
+
+      -- let atc-router happy
+      local _ngx = mock_ngx("GET", "/", { a = "1" })
+      router._set_ngx(_ngx)
+
+      local match_t = router:select("GET", "/", "www.example.com:80")
+      assert.falsy(match_t)
+
+      local match_t = router:select("GET", "/", "www.example.com:8000")
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+    end)
+
+    it("exec() should use var.server_port if host has no port", function()
+      router = assert(new_router(use_case))
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo", { host = "www.example.com" })
+      router._set_ngx(_ngx)
+
+      -- no port provided
+      local match_t = router:exec()
+      assert.falsy(match_t)
+
+      -- var.server_port
+      _ngx.var.server_port = 8000
+      router._set_ngx(_ngx)
+
+      -- first match
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.falsy(ctx.route_match_cached)
+
+      -- cache hit
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.same(ctx.route_match_cached, "pos")
+    end)
+
+    it("exec() should support net.src.* and net.dst.*", function()
+      router = assert(new_router(use_case))
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo", { host = "domain.org" })
+      router._set_ngx(_ngx)
+
+      -- no ip address provided
+      local match_t = router:exec()
+      assert.falsy(match_t)
+
+      -- ip address
+      _ngx.var.remote_addr = "1.1.1.1"
+      _ngx.var.server_addr = "2.2.2.2"
+      router._set_ngx(_ngx)
+
+      -- first match
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[2].route, match_t.route)
+      assert.falsy(ctx.route_match_cached)
+
+      -- cache hit
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[2].route, match_t.route)
+      assert.same(ctx.route_match_cached, "pos")
+    end)
+  end)
 end   -- local flavor = "expressions"