From 8ad28701178b0e07644055f6427aefdb0ce3946f Mon Sep 17 00:00:00 2001 From: Chrono Date: Fri, 12 Jan 2024 15:42:56 +0800 Subject: [PATCH] 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 --- ...upport_net_src_dst_field_in_expression.yml | 4 + kong/db/schema/entities/routes.lua | 5 +- kong/router/atc.lua | 18 ++- kong/router/compat.lua | 4 +- kong/router/expressions.lua | 34 +++++- kong/router/fields.lua | 12 +- .../01-db/01-schema/06-routes_spec.lua | 51 ++++++++ spec/01-unit/08-router_spec.lua | 113 +++++++++++++++++- 8 files changed, 222 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/kong/support_net_src_dst_field_in_expression.yml diff --git a/changelog/unreleased/kong/support_net_src_dst_field_in_expression.yml b/changelog/unreleased/kong/support_net_src_dst_field_in_expression.yml new file mode 100644 index 00000000000..e60ea66c3ea --- /dev/null +++ b/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 diff --git a/kong/db/schema/entities/routes.lua b/kong/db/schema/entities/routes.lua index 0ff3943ddce..621b08cfe70 100644 --- a/kong/db/schema/entities/routes.lua +++ b/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 diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 5111da5b19e..a391544ddcc 100644 --- a/kong/router/atc.lua +++ b/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) diff --git a/kong/router/compat.lua b/kong/router/compat.lua index 86864dfce51..e09f84966de 100644 --- a/kong/router/compat.lua +++ b/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 diff --git a/kong/router/expressions.lua b/kong/router/expressions.lua index 6790939699f..129689f1313 100644 --- a/kong/router/expressions.lua +++ b/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*)([=> 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") diff --git a/kong/router/fields.lua b/kong/router/fields.lua index 59d4cee86ec..082bd6db9b0 100644 --- a/kong/router/fields.lua +++ b/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 diff --git a/spec/01-unit/01-db/01-schema/06-routes_spec.lua b/spec/01-unit/01-db/01-schema/06-routes_spec.lua index f4ef090ce0f..62e55db628a 100644 --- a/spec/01-unit/01-db/01-schema/06-routes_spec.lua +++ b/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) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 777f530e183..ede1a4a7bd5 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/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"