From 215db217d1165e9fdf8e0f01c682d2ff4de5f81e Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Fri, 29 Mar 2024 11:14:08 +0800 Subject: [PATCH 01/18] feat(router): extend `route.snis` to support wildcard - Change the element type of `route.snis` from sni to wildcard_host. - Change the router accordingly to support wildcard snis because the sni is involved in route matching. Additional Notes: - The expressions flavor doesn't support mTLS (mtls-auth plugin and tls-handshake-modifier plugin) theoretically. The reason is the current mTLS implementation is based on the `route.snis`, but at the ssl phase it is not yet known which route will match. As a workaround, it collects the sni set on routes that are associated with mtls plugins in advance. But for the expressions flavor, things are different. All the fields that were involved in route matching have been merged into the field `expression`. Without actually evaluating, we can't know in general if a sni will match a certain route. But again, you can't get all the parameters required for evaluation at the ssl phase. The correct solution is binding the mTLS logic to `snis` entity. We had some discussion about the mTLS issue on https://konghq.atlassian.net/browse/KAG-3757 - We don't distinguish the priority of plain-only-snis and contain-wildcard-snis for the traditional-compatible flavor because there are no reserved bits available. See https://github.com/Kong/kong/blob/master/kong/router/transform.lua#L515-L528 https://konghq.atlassian.net/browse/KAG-3832 --- .../unreleased/kong/route-wildcard-snis.yml | 3 + kong/db/schema/entities/routes.lua | 2 +- kong/router/traditional.lua | 134 ++++++++++--- kong/router/transform.lua | 51 +++-- .../01-db/01-schema/06-routes_spec.lua | 43 +++++ spec/01-unit/08-router_spec.lua | 61 ++++++ .../05-proxy/02-router_spec.lua | 176 ++++++++++++++++++ 7 files changed, 427 insertions(+), 43 deletions(-) create mode 100644 changelog/unreleased/kong/route-wildcard-snis.yml diff --git a/changelog/unreleased/kong/route-wildcard-snis.yml b/changelog/unreleased/kong/route-wildcard-snis.yml new file mode 100644 index 000000000000..4415c1da2645 --- /dev/null +++ b/changelog/unreleased/kong/route-wildcard-snis.yml @@ -0,0 +1,3 @@ +message: extend `route.snis` to support wildcard. +type: feature +scope: Core diff --git a/kong/db/schema/entities/routes.lua b/kong/db/schema/entities/routes.lua index 806b263cb240..0d5813dd7d53 100644 --- a/kong/db/schema/entities/routes.lua +++ b/kong/db/schema/entities/routes.lua @@ -159,7 +159,7 @@ local routes = { { snis = { type = "set", description = "A list of SNIs that match this Route.", - elements = typedefs.sni }, }, + elements = typedefs.wildcard_host }, }, { sources = typedefs.sources }, { destinations = typedefs.destinations }, diff --git a/kong/router/traditional.lua b/kong/router/traditional.lua index 70086db27f42..204e365c5387 100644 --- a/kong/router/traditional.lua +++ b/kong/router/traditional.lua @@ -204,6 +204,7 @@ local MATCH_SUBRULES = { HAS_REGEX_URI = 0x01, PLAIN_HOSTS_ONLY = 0x02, HAS_WILDCARD_HOST_PORT = 0x04, + PLAIN_SNIS_ONLY = 0x08, } @@ -297,7 +298,7 @@ local function marshall_route(r) local methods_t = {} local sources_t = { [0] = 0 } local destinations_t = { [0] = 0 } - local snis_t = {} + local snis_t = { [0] = 0 } -- hosts @@ -480,30 +481,55 @@ local function marshall_route(r) -- snis + if snis then if type(snis) ~= "table" then return nil, "snis field must be a table" end - local count = #snis - if count > 0 then - match_rules = bor(match_rules, MATCH_RULES.SNI) - match_weight = match_weight + 1 + local has_sni_wildcard + local has_sni_plain - for i = 1, count do - local sni = snis[i] - if type(sni) ~= "string" then - return nil, "sni elements must be strings" - end + for i = 1, #snis do + local sni = snis[i] + if type(sni) ~= "string" then + return nil, "snis values must be strings" + end - if #sni > 1 and byte(sni, -1) == DOT then - -- last dot in FQDNs must not be used for routing - sni = sub(sni, 1, -2) - end + if #sni > 1 and byte(sni, -1) == DOT then + -- last dot in FQDNs must not be used for routing + sni = sub(sni, 1, -2) + end + if find(sni, "*", nil, true) then + -- wildcard sni matching + has_sni_wildcard = true + + local wildcard_sni_regex = sni:gsub("%.", "\\.") + :gsub("%*", ".+") .. "$" + + append(snis_t, { + wildcard = true, + value = sni, + regex = wildcard_sni_regex, + }) + + else + -- plain sni matching + has_sni_plain = true + append(snis_t, { value = sni }) snis_t[sni] = sni end end + + if has_sni_plain or has_sni_wildcard then + match_rules = bor(match_rules, MATCH_RULES.SNI) + match_weight = match_weight + 1 + end + + if not has_sni_wildcard then + submatch_weight = bor(submatch_weight, MATCH_SUBRULES.PLAIN_SNIS_ONLY) + end end @@ -622,7 +648,7 @@ end local function index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, - wildcard_hosts, src_trust_funcs, dst_trust_funcs) + wildcard_hosts, wildcard_snis, src_trust_funcs, dst_trust_funcs) for i = 1, route_t.hosts[0] do local host_t = route_t.hosts[i] if host_t.wildcard then @@ -657,8 +683,14 @@ local function index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, plain_indexes.methods[method] = true end - for sni in pairs(route_t.snis) do - plain_indexes.snis[sni] = true + for i = 1, route_t.snis[0] do + local sni_t = route_t.snis[i] + if sni_t.wildcard then + append(wildcard_snis, sni_t) + + else + plain_indexes.snis[sni_t.value] = true + end end index_src_dst(route_t.sources, plain_indexes.sources, src_trust_funcs) @@ -776,7 +808,7 @@ local function sort_src_dst(source, func) end -local function categorize_hosts_headers_uris(route_t, source, category, key) +local function categorize_hosts_headers_uris_snis(route_t, source, category, key) for i = 1, source[0] do local value = source[i][key or "value"] if category[value] then @@ -789,7 +821,7 @@ local function categorize_hosts_headers_uris(route_t, source, category, key) end -local function categorize_methods_snis(route_t, source, category) +local function categorize_methods(route_t, source, category) for key in pairs(source) do if category[key] then append(category[key], route_t) @@ -847,7 +879,7 @@ local function categorize_route_t(route_t, bit_category, categories) routes_by_methods = {}, routes_by_sources = {}, routes_by_destinations = {}, - routes_by_sni = {}, + routes_by_snis = {}, all = { [0] = 0 }, } @@ -855,11 +887,11 @@ local function categorize_route_t(route_t, bit_category, categories) end append(category.all, route_t) - categorize_hosts_headers_uris(route_t, route_t.hosts, category.routes_by_hosts) - categorize_hosts_headers_uris(route_t, route_t.headers, category.routes_by_headers, "name") - categorize_hosts_headers_uris(route_t, route_t.uris, category.routes_by_uris) - categorize_methods_snis(route_t, route_t.methods, category.routes_by_methods) - categorize_methods_snis(route_t, route_t.snis, category.routes_by_sni) + categorize_hosts_headers_uris_snis(route_t, route_t.hosts, category.routes_by_hosts) + categorize_hosts_headers_uris_snis(route_t, route_t.headers, category.routes_by_headers, "name") + categorize_hosts_headers_uris_snis(route_t, route_t.uris, category.routes_by_uris) + categorize_hosts_headers_uris_snis(route_t, route_t.snis, category.routes_by_snis) + categorize_methods(route_t, route_t.methods, category.routes_by_methods) categorize_src_dst(route_t, route_t.sources, category.routes_by_sources) categorize_src_dst(route_t, route_t.destinations, category.routes_by_destinations) end @@ -1058,10 +1090,28 @@ do end, [MATCH_RULES.SNI] = function(route_t, ctx) - if ctx.req_scheme == "http" or route_t.snis[ctx.sni] then - ctx.matches.sni = ctx.sni + local snis = route_t.snis + local sni = ctx.sni + if ctx.req_scheme == "http" or snis[sni] then + ctx.matches.sni = sni return true end + + for i = 1, snis[0] do + local sni_t = snis[i] + if sni_t.wildcard then + local from, _, err = re_find(ctx.sni, sni_t.regex, "ajo") + if err then + log(ERR, "could not evaluate wildcard sni regex: ", err) + return + end + + if from then + ctx.matches.sni = sni_t.value + return true + end + end + end end, [MATCH_RULES.SRC] = function(route_t, ctx) @@ -1131,7 +1181,7 @@ do end, [MATCH_RULES.SNI] = function(category, ctx) - return category.routes_by_sni[ctx.sni] + return category.routes_by_snis[ctx.hits.sni or ctx.sni] end, [MATCH_RULES.SRC] = function(category, ctx) @@ -1368,6 +1418,7 @@ function _M.new(routes, cache, cache_neg) local prefix_uris = { [0] = 0 } -- will be sorted by length local regex_uris = { [0] = 0 } local wildcard_hosts = { [0] = 0 } + local wildcard_snis = { [0] = 0 } local src_trust_funcs = { [0] = 0 } local dst_trust_funcs = { [0] = 0 } @@ -1448,7 +1499,7 @@ function _M.new(routes, cache, cache_neg) local route_t = marshalled_routes[i] categorize_route_t(route_t, route_t.match_rules, categories) index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, - wildcard_hosts, src_trust_funcs, dst_trust_funcs) + wildcard_hosts, wildcard_snis, src_trust_funcs, dst_trust_funcs) end end @@ -1509,6 +1560,7 @@ function _M.new(routes, cache, cache_neg) local match_uris = not isempty(plain_indexes.uris) local match_methods = not isempty(plain_indexes.methods) local match_snis = not isempty(plain_indexes.snis) + local match_wildcard_snis = not isempty(wildcard_snis) local match_sources = not isempty(plain_indexes.sources) local match_destinations = not isempty(plain_indexes.destinations) @@ -1693,10 +1745,30 @@ function _M.new(routes, cache, cache_neg) -- sni match - if match_snis and plain_indexes.snis[sni] then - req_category = bor(req_category, MATCH_RULES.SNI) + if sni ~= "" then + if match_snis and plain_indexes.snis[sni] + then + req_category = bor(req_category, MATCH_RULES.SNI) + + elseif match_wildcard_snis then + for i = 1, wildcard_snis[0] do + local sni_t = wildcard_snis[i] + local from, _, err = re_find(sni, sni_t.regex, "ajo") + if err then + log(ERR, "could not match wildcard sni: ", err) + return + end + + if from then + hits.sni = sni_t.value + req_category = bor(req_category, MATCH_RULES.SNI) + break + end + end + end end + -- src match if match_sources and match_src_dst(plain_indexes.sources, src_ip, src_port, src_trust_funcs) then diff --git a/kong/router/transform.lua b/kong/router/transform.lua index 351eb480e68b..f92ee6e0dd2b 100644 --- a/kong/router/transform.lua +++ b/kong/router/transform.lua @@ -124,6 +124,7 @@ local values_buf = buffer.new(64) local nets_buf = buffer.new(64) local expr_buf = buffer.new(64) local hosts_buf = buffer.new(64) +local snis_buf = buffer.new(64) local headers_buf = buffer.new(64) local single_header_buf = buffer.new(64) @@ -264,6 +265,44 @@ local function gen_for_nets(ip_field, port_field, vals) end +local function gen_for_snis(snis) + if is_empty_field(snis) then + return nil + end + + snis_buf:reset():put("(") + + for i, sni in ipairs(snis) do + local op = OP_EQUAL + if #sni > 1 and byte(sni, -1) == DOT then + -- last dot in FQDNs must not be used for routing + sni = sni:sub(1, -2) + end + + if byte(sni) == ASTERISK then + -- postfix matching + op = OP_POSTFIX + sni = sni:sub(2) + + elseif byte(sni, -1) == ASTERISK then + -- prefix matching + op = OP_PREFIX + sni = sni:sub(1, -2) + end + + local exp = "tls.sni ".. op .. " r#\"" .. sni .. "\"#" + expression_append(snis_buf, LOGICAL_OR, exp, i) + end -- for route.snis + + -- consume the whole buffer + -- returns a local variable instead of using a tail call + -- to avoid NYI + local str = snis_buf:put(")"):get() + + return str +end + + local is_stream_route do local is_stream_protocol = { @@ -283,16 +322,6 @@ do end -local function sni_val_transform(_, p) - if #p > 1 and byte(p, -1) == DOT then - -- last dot in FQDNs must not be used for routing - return p:sub(1, -2) - end - - return p -end - - local function path_op_transform(path) return is_regex_magic(path) and OP_REGEX or OP_PREFIX end @@ -329,7 +358,7 @@ local function get_expression(route) expr_buf:reset() - local gen = gen_for_field("tls.sni", OP_EQUAL, snis, sni_val_transform) + local gen = gen_for_snis(snis) if gen then -- See #6425, if `net.protocol` is not `https` -- then SNI matching should simply not be considered 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 a3d551cab9c7..c2d0ee024413 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 @@ -1046,6 +1046,49 @@ describe("routes schema (flavor = " .. flavor .. ")", function() end end) + it("accepts leftmost wildcard", function() + for _, sni in ipairs({ "*.example.org", "*.foo.bar.test" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.is_nil(errs) + assert.truthy(ok) + end + end) + + it("accepts rightmost wildcard", function() + for _, sni in ipairs({ "example.*", "foo.bar.*" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.is_nil(errs) + assert.truthy(ok) + end + end) + + it("rejects invalid wildcard", function() + for _, sni in ipairs({ "foo.*.test", "foo*.test" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.falsy(ok) + assert.same({ + snis = { + "wildcard must be leftmost or rightmost character", + }, + }, errs) + end + end) + it("rejects specifying 'snis' if 'protocols' does not have 'https', 'tls' or 'tls_passthrough'", function() local route = Routes:process_auto_fields({ protocols = { "tcp", "udp" }, diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 86f28ac33ab3..da53e1a826cd 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -1732,6 +1732,67 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" end) end) + describe("[wildcard sni]", function() + local use_case, router + + lazy_setup(function() + use_case = { + { + service = service, + route = { + id = "e8fb37f1-102d-461e-9c51-6608a6bb8101", + snis = { "*.sni.test" }, + }, + }, + { + service = service, + route = { + id = "e8fb37f1-102d-461e-9c51-6608a6bb8102", + snis = { "sni.*" }, + }, + }, + } + + router = assert(new_router(use_case)) + end) + + it("matches leftmost wildcards", function() + for _, sni in ipairs({"foo.sni.test", "foo.bar.sni.test"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.truthy(match_t) + assert.same(use_case[1].route, match_t.route) + if flavor == "traditional" then + assert.same(use_case[1].route.snis[1], match_t.matches.sni) + end + assert.same(nil, match_t.matches.method) + assert.same(nil, match_t.matches.uri) + assert.same(nil, match_t.matches.uri_captures) + end + end) + + it("matches rightmost wildcards", function() + for _, sni in ipairs({"sni.foo", "sni.foo.bar"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.truthy(match_t) + assert.same(use_case[2].route, match_t.route) + if flavor == "traditional" then + assert.same(use_case[2].route.snis[1], match_t.matches.sni) + end + end + end) + + it("doesn't match wildcard", function() + for _, sni in ipairs({"bar.sni.foo", "foo.sni.test.bar"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.is_nil(match_t) + end + end) + end) + + if flavor ~= "traditional" then describe("incremental rebuild", function() local router diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index ba32c8e6846d..7378a8baab16 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -1312,6 +1312,20 @@ for _, strategy in helpers.each_strategy() do name = "service_behind_example.org" }, }, + { + protocols = { "https" }, + snis = { "*.foo.test" }, + service = { + name = "service_behind_wild.foo.test" + }, + }, + { + protocols = { "https" }, + snis = { "bar.*" }, + service = { + name = "service_behind_bar.wild" + }, + }, }) end) @@ -1368,6 +1382,40 @@ for _, strategy in helpers.each_strategy() do assert.equal("service_behind_example.org", res.headers["kong-service-name"]) end) + + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do + proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + assert.equal("service_behind_wild.foo.test", + res.headers["kong-service-name"]) + + proxy_ssl_client:close() + end + end) + + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"bar.x", "bar.y.z"}) do + proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + assert.equal("service_behind_bar.wild", + res.headers["kong-service-name"]) + + proxy_ssl_client:close() + end + end) end) describe("tls_passthrough", function() @@ -1396,6 +1444,26 @@ for _, strategy in helpers.each_strategy() do protocol = "tcp", }, }, + { + protocols = { "tls_passthrough" }, + snis = { "*.foo.test" }, + service = { + name = "service_behind_wild.foo.test", + host = helpers.mock_upstream_ssl_host, + port = helpers.mock_upstream_ssl_port, + protocol = "tcp", + }, + }, + { + protocols = { "tls_passthrough" }, + snis = { "bar.*" }, + service = { + name = "service_behind_bar.wild", + host = helpers.mock_upstream_ssl_host, + port = helpers.mock_upstream_ssl_port, + protocol = "tcp", + }, + }, }) end) @@ -1458,6 +1526,58 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end) + + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do + -- config propagates to stream subsystems not instantly + -- try up to 10 seconds with step of 2 seconds + -- in vagrant it takes around 6 seconds + helpers.wait_until(function() + proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) + local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify + if not ok then + proxy_ssl_client:close() + return false + end + return true + end, 10, 2) + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + + proxy_ssl_client:close() + end + end) + + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"bar.x", "bar.y.z"}) do + -- config propagates to stream subsystems not instantly + -- try up to 10 seconds with step of 2 seconds + -- in vagrant it takes around 6 seconds + helpers.wait_until(function() + proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) + local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify + if not ok then + proxy_ssl_client:close() + return false + end + return true + end, 10, 2) + + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + + proxy_ssl_client:close() + end + end) end) describe("[#headers]", function() @@ -1787,6 +1907,22 @@ for _, strategy in helpers.each_strategy() do url = helpers.grpcbin_ssl_url, }, }, + { + protocols = { "grpcs" }, + snis = { "*.grpcs_3.test" }, + service = { + name = "grpcs_3", + url = helpers.grpcbin_ssl_url, + }, + }, + { + protocols = { "grpcs" }, + snis = { "grpcs_4.*" }, + service = { + name = "grpcs_4", + url = helpers.grpcbin_ssl_url, + }, + }, }) end) @@ -1826,6 +1962,46 @@ for _, strategy in helpers.each_strategy() do assert.truthy(resp) assert.matches("kong-service-name: grpcs_2", resp, nil, true) end) + + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do + grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) + + local ok, resp = assert(grpcs_proxy_ssl_client({ + service = "hello.HelloService.SayHello", + body = { + greeting = "world!" + }, + opts = { + ["-H"] = "'kong-debug: 1'", + ["-v"] = true, -- verbose so we get response headers + } + })) + assert.truthy(ok) + assert.truthy(resp) + assert.matches("kong-service-name: grpcs_3", resp, nil, true) + end + end) + + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do + grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) + + local ok, resp = assert(grpcs_proxy_ssl_client({ + service = "hello.HelloService.SayHello", + body = { + greeting = "world!" + }, + opts = { + ["-H"] = "'kong-debug: 1'", + ["-v"] = true, -- verbose so we get response headers + } + })) + assert.truthy(ok) + assert.truthy(resp) + assert.matches("kong-service-name: grpcs_4", resp, nil, true) + end + end) end) end -- not enable_buffering From e1dab7b4f1a99aed341b01e958ed3a637e9ca510 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 11:07:28 +0800 Subject: [PATCH 02/18] update the changelog entry --- changelog/unreleased/kong/route-wildcard-snis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/route-wildcard-snis.yml b/changelog/unreleased/kong/route-wildcard-snis.yml index 4415c1da2645..ab9302213158 100644 --- a/changelog/unreleased/kong/route-wildcard-snis.yml +++ b/changelog/unreleased/kong/route-wildcard-snis.yml @@ -1,3 +1,3 @@ -message: extend `route.snis` to support wildcard. +message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. type: feature scope: Core From b6766b49fe8906b5e25d6c6a0ae6a61495fd9826 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 15:31:41 +0800 Subject: [PATCH 03/18] add compatible check code --- kong/clustering/compat/checkers.lua | 48 ++++++++++++++++++++++ spec/01-unit/19-hybrid/03-compat_spec.lua | 49 ++++++++++++++++++++++- 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index 2cc89cba3821..5836cc82cc3a 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -1,5 +1,6 @@ local ipairs = ipairs local type = type +local string_find = string.find local log_warn_message @@ -23,6 +24,53 @@ end local compatible_checkers = { + { 3007000000, --[[ 3.7.0.0 ]] + function(config_table, dp_version, log_suffix) + local has_update + + local flavor = kong and + kong.configuration and + kong.configuration.router_flavor + -- remove this once the `expressions` flavor supports `route.snis` + if flavor == "expressions" then + return nil + end + + for _, route in ipairs(config_table.routes or {}) do + local snis = route.snis + if not snis or #snis == 0 then + break + end + + local idx = 0 + local new_snis = {} + local local_has_update + for _, sni in ipairs(snis) do + -- remove the sni that contains wildcard + if string_find(sni, "*", nil, true) then + has_update = true + local_has_update = true + + else + idx = idx + 1 + new_snis[idx] = sni + end + end + + if local_has_update then + route.snis = new_snis + end + end + + if has_update then + log_warn_message("configuration 'route.snis' contains elements that contains wildcard character '*'", + "be removed", dp_version, log_suffix) + end + + return has_update + end + }, + { 3006000000, --[[ 3.6.0.0 ]] function(config_table, dp_version, log_suffix) local has_update diff --git a/spec/01-unit/19-hybrid/03-compat_spec.lua b/spec/01-unit/19-hybrid/03-compat_spec.lua index b2a0030aa0f0..a7231dc2c4bc 100644 --- a/spec/01-unit/19-hybrid/03-compat_spec.lua +++ b/spec/01-unit/19-hybrid/03-compat_spec.lua @@ -630,5 +630,52 @@ describe("kong.clustering.compat", function() assert.is_nil(assert(services[3]).ca_certificates) end) - end) + end) -- describe + + + describe("route entities compatible changes", function() + it("route.snis contain wildcard", function() + for _, flavor in ipairs({"traditional", "traditional_compatible"}) do + local _, db = helpers.get_db_utils(nil, { + "routes", + }) + _G.kong.db = db + _G.kong.configuration = { router_flavor = flavor } + + assert(declarative.load_into_db({ + routes = { + route1 = { + id = "00000000-0000-0000-0000-000000000001", + snis = { "*.foo.test", "normal.test", "bar.*", "good.test" }, + }, + route2 = { + id = "00000000-0000-0000-0000-000000000002", + snis = { "normal.test", "good.test" }, + }, + route3 = { + id = "00000000-0000-0000-0000-000000000003", + snis = { "*.foo.test", "bar.*", }, + }, + }, + }, { _transform = true })) + local config = { config_table = declarative.export_config() } + local has_update, result = compat.update_compatible_payload(config, "3.6.0", "test_") + assert.truthy(has_update) + result = cjson_decode(inflate_gzip(result)).config_table + + local routes = assert(assert(result).routes) + assert.equals(3, #routes) + for _, route in ipairs(routes) do + if route.id == "00000000-0000-0000-0000-000000000003" then + assert.equals(0, #route.snis) + + else + assert.equals(2, #route.snis) + assert.same({"normal.test", "good.test"}, route.snis) + end + end + end + end) + end) -- describe + end) From b403aa53e96b466e02a4a8bde3ac4c45c888745d Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 16:40:32 +0800 Subject: [PATCH 04/18] fixup --- kong/clustering/compat/checkers.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index 5836cc82cc3a..a8079c4037fb 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -38,7 +38,7 @@ local compatible_checkers = { for _, route in ipairs(config_table.routes or {}) do local snis = route.snis - if not snis or #snis == 0 then + if type(snis) ~= "table" or #snis == 0 then break end From 493cd87ce160fe8fd8dbbd13c8672a24cbe38f9d Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 16:55:46 +0800 Subject: [PATCH 05/18] apply comment --- kong/router/transform.lua | 78 +++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/kong/router/transform.lua b/kong/router/transform.lua index f92ee6e0dd2b..dbb693f394ee 100644 --- a/kong/router/transform.lua +++ b/kong/router/transform.lua @@ -124,7 +124,6 @@ local values_buf = buffer.new(64) local nets_buf = buffer.new(64) local expr_buf = buffer.new(64) local hosts_buf = buffer.new(64) -local snis_buf = buffer.new(64) local headers_buf = buffer.new(64) local single_header_buf = buffer.new(64) @@ -265,44 +264,6 @@ local function gen_for_nets(ip_field, port_field, vals) end -local function gen_for_snis(snis) - if is_empty_field(snis) then - return nil - end - - snis_buf:reset():put("(") - - for i, sni in ipairs(snis) do - local op = OP_EQUAL - if #sni > 1 and byte(sni, -1) == DOT then - -- last dot in FQDNs must not be used for routing - sni = sni:sub(1, -2) - end - - if byte(sni) == ASTERISK then - -- postfix matching - op = OP_POSTFIX - sni = sni:sub(2) - - elseif byte(sni, -1) == ASTERISK then - -- prefix matching - op = OP_PREFIX - sni = sni:sub(1, -2) - end - - local exp = "tls.sni ".. op .. " r#\"" .. sni .. "\"#" - expression_append(snis_buf, LOGICAL_OR, exp, i) - end -- for route.snis - - -- consume the whole buffer - -- returns a local variable instead of using a tail call - -- to avoid NYI - local str = snis_buf:put(")"):get() - - return str -end - - local is_stream_route do local is_stream_protocol = { @@ -322,6 +283,43 @@ do end +local function sni_op_transform(sni) + local op = OP_EQUAL + + if byte(sni) == ASTERISK then + -- postfix matching + op = OP_POSTFIX + + elseif byte(sni, -1) == ASTERISK then + -- prefix matching + op = OP_PREFIX + end + + return op +end + + +local function sni_val_transform(op, sni) + -- prefix matching + if op == OP_PREFIX then + sni = sni:sub(1, -2) + + else + if #sni > 1 and byte(sni, -1) == DOT then + -- last dot in FQDNs must not be used for routing + sni = sni:sub(1, -2) + end + + -- postfix matching + if op == OP_POSTFIX then + sni = sni:sub(2) + end + end + + return sni +end + + local function path_op_transform(path) return is_regex_magic(path) and OP_REGEX or OP_PREFIX end @@ -358,7 +356,7 @@ local function get_expression(route) expr_buf:reset() - local gen = gen_for_snis(snis) + local gen = gen_for_field("tls.sni", sni_op_transform, snis, sni_val_transform) if gen then -- See #6425, if `net.protocol` is not `https` -- then SNI matching should simply not be considered From 82d139924fa49d73cc6a2daac0884370da2d09eb Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 17:51:10 +0800 Subject: [PATCH 06/18] apply comments --- kong/clustering/compat/checkers.lua | 4 +++- spec/01-unit/19-hybrid/03-compat_spec.lua | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index a8079c4037fb..14b82310d1bd 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -39,7 +39,7 @@ local compatible_checkers = { for _, route in ipairs(config_table.routes or {}) do local snis = route.snis if type(snis) ~= "table" or #snis == 0 then - break + goto continue end local idx = 0 @@ -60,6 +60,8 @@ local compatible_checkers = { if local_has_update then route.snis = new_snis end + + ::continue:: end if has_update then diff --git a/spec/01-unit/19-hybrid/03-compat_spec.lua b/spec/01-unit/19-hybrid/03-compat_spec.lua index a7231dc2c4bc..d60f9f970dd6 100644 --- a/spec/01-unit/19-hybrid/03-compat_spec.lua +++ b/spec/01-unit/19-hybrid/03-compat_spec.lua @@ -633,9 +633,9 @@ describe("kong.clustering.compat", function() end) -- describe - describe("route entities compatible changes", function() - it("route.snis contain wildcard", function() - for _, flavor in ipairs({"traditional", "traditional_compatible"}) do + for _, flavor in ipairs({"traditional", "traditional_compatible"}) do + describe("Router (flavor = " .. flavor .. ") route entities compatible changes", function() + it("route.snis contain wildcard", function() local _, db = helpers.get_db_utils(nil, { "routes", }) @@ -674,8 +674,8 @@ describe("kong.clustering.compat", function() assert.same({"normal.test", "good.test"}, route.snis) end end - end - end) - end) -- describe + end) + end) -- describe + end end) From 42cf7d8cca092cb17f81e7a60fd45e92c0aad082 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Mon, 1 Apr 2024 17:55:54 +0800 Subject: [PATCH 07/18] remove the flavor check logic --- kong/clustering/compat/checkers.lua | 8 -------- 1 file changed, 8 deletions(-) diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index 14b82310d1bd..8baa5adeb234 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -28,14 +28,6 @@ local compatible_checkers = { function(config_table, dp_version, log_suffix) local has_update - local flavor = kong and - kong.configuration and - kong.configuration.router_flavor - -- remove this once the `expressions` flavor supports `route.snis` - if flavor == "expressions" then - return nil - end - for _, route in ipairs(config_table.routes or {}) do local snis = route.snis if type(snis) ~= "table" or #snis == 0 then From bb3ef2a3ac27b59583f0329dd7789df666f942a0 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Sun, 7 Apr 2024 15:46:10 +0800 Subject: [PATCH 08/18] remove the comopatibility check code --- kong/clustering/compat/checkers.lua | 42 ------------------- spec/01-unit/19-hybrid/03-compat_spec.lua | 49 +---------------------- 2 files changed, 1 insertion(+), 90 deletions(-) diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index 8baa5adeb234..2cc89cba3821 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -1,6 +1,5 @@ local ipairs = ipairs local type = type -local string_find = string.find local log_warn_message @@ -24,47 +23,6 @@ end local compatible_checkers = { - { 3007000000, --[[ 3.7.0.0 ]] - function(config_table, dp_version, log_suffix) - local has_update - - for _, route in ipairs(config_table.routes or {}) do - local snis = route.snis - if type(snis) ~= "table" or #snis == 0 then - goto continue - end - - local idx = 0 - local new_snis = {} - local local_has_update - for _, sni in ipairs(snis) do - -- remove the sni that contains wildcard - if string_find(sni, "*", nil, true) then - has_update = true - local_has_update = true - - else - idx = idx + 1 - new_snis[idx] = sni - end - end - - if local_has_update then - route.snis = new_snis - end - - ::continue:: - end - - if has_update then - log_warn_message("configuration 'route.snis' contains elements that contains wildcard character '*'", - "be removed", dp_version, log_suffix) - end - - return has_update - end - }, - { 3006000000, --[[ 3.6.0.0 ]] function(config_table, dp_version, log_suffix) local has_update diff --git a/spec/01-unit/19-hybrid/03-compat_spec.lua b/spec/01-unit/19-hybrid/03-compat_spec.lua index d60f9f970dd6..b2a0030aa0f0 100644 --- a/spec/01-unit/19-hybrid/03-compat_spec.lua +++ b/spec/01-unit/19-hybrid/03-compat_spec.lua @@ -630,52 +630,5 @@ describe("kong.clustering.compat", function() assert.is_nil(assert(services[3]).ca_certificates) end) - end) -- describe - - - for _, flavor in ipairs({"traditional", "traditional_compatible"}) do - describe("Router (flavor = " .. flavor .. ") route entities compatible changes", function() - it("route.snis contain wildcard", function() - local _, db = helpers.get_db_utils(nil, { - "routes", - }) - _G.kong.db = db - _G.kong.configuration = { router_flavor = flavor } - - assert(declarative.load_into_db({ - routes = { - route1 = { - id = "00000000-0000-0000-0000-000000000001", - snis = { "*.foo.test", "normal.test", "bar.*", "good.test" }, - }, - route2 = { - id = "00000000-0000-0000-0000-000000000002", - snis = { "normal.test", "good.test" }, - }, - route3 = { - id = "00000000-0000-0000-0000-000000000003", - snis = { "*.foo.test", "bar.*", }, - }, - }, - }, { _transform = true })) - local config = { config_table = declarative.export_config() } - local has_update, result = compat.update_compatible_payload(config, "3.6.0", "test_") - assert.truthy(has_update) - result = cjson_decode(inflate_gzip(result)).config_table - - local routes = assert(assert(result).routes) - assert.equals(3, #routes) - for _, route in ipairs(routes) do - if route.id == "00000000-0000-0000-0000-000000000003" then - assert.equals(0, #route.snis) - - else - assert.equals(2, #route.snis) - assert.same({"normal.test", "good.test"}, route.snis) - end - end - end) - end) -- describe - end - + end) end) From 6dc0d9186462e069abe22c33e9bad126d8c53631 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 11:09:56 +0800 Subject: [PATCH 09/18] remove the traditional router change, only support wildcard for traditional_compatible flavor --- kong/router/traditional.lua | 134 +++++++++--------------------------- 1 file changed, 31 insertions(+), 103 deletions(-) diff --git a/kong/router/traditional.lua b/kong/router/traditional.lua index 204e365c5387..70086db27f42 100644 --- a/kong/router/traditional.lua +++ b/kong/router/traditional.lua @@ -204,7 +204,6 @@ local MATCH_SUBRULES = { HAS_REGEX_URI = 0x01, PLAIN_HOSTS_ONLY = 0x02, HAS_WILDCARD_HOST_PORT = 0x04, - PLAIN_SNIS_ONLY = 0x08, } @@ -298,7 +297,7 @@ local function marshall_route(r) local methods_t = {} local sources_t = { [0] = 0 } local destinations_t = { [0] = 0 } - local snis_t = { [0] = 0 } + local snis_t = {} -- hosts @@ -481,55 +480,30 @@ local function marshall_route(r) -- snis - if snis then if type(snis) ~= "table" then return nil, "snis field must be a table" end - local has_sni_wildcard - local has_sni_plain - - for i = 1, #snis do - local sni = snis[i] - if type(sni) ~= "string" then - return nil, "snis values must be strings" - end - - if #sni > 1 and byte(sni, -1) == DOT then - -- last dot in FQDNs must not be used for routing - sni = sub(sni, 1, -2) - end - - if find(sni, "*", nil, true) then - -- wildcard sni matching - has_sni_wildcard = true + local count = #snis + if count > 0 then + match_rules = bor(match_rules, MATCH_RULES.SNI) + match_weight = match_weight + 1 - local wildcard_sni_regex = sni:gsub("%.", "\\.") - :gsub("%*", ".+") .. "$" + for i = 1, count do + local sni = snis[i] + if type(sni) ~= "string" then + return nil, "sni elements must be strings" + end - append(snis_t, { - wildcard = true, - value = sni, - regex = wildcard_sni_regex, - }) + if #sni > 1 and byte(sni, -1) == DOT then + -- last dot in FQDNs must not be used for routing + sni = sub(sni, 1, -2) + end - else - -- plain sni matching - has_sni_plain = true - append(snis_t, { value = sni }) snis_t[sni] = sni end end - - if has_sni_plain or has_sni_wildcard then - match_rules = bor(match_rules, MATCH_RULES.SNI) - match_weight = match_weight + 1 - end - - if not has_sni_wildcard then - submatch_weight = bor(submatch_weight, MATCH_SUBRULES.PLAIN_SNIS_ONLY) - end end @@ -648,7 +622,7 @@ end local function index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, - wildcard_hosts, wildcard_snis, src_trust_funcs, dst_trust_funcs) + wildcard_hosts, src_trust_funcs, dst_trust_funcs) for i = 1, route_t.hosts[0] do local host_t = route_t.hosts[i] if host_t.wildcard then @@ -683,14 +657,8 @@ local function index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, plain_indexes.methods[method] = true end - for i = 1, route_t.snis[0] do - local sni_t = route_t.snis[i] - if sni_t.wildcard then - append(wildcard_snis, sni_t) - - else - plain_indexes.snis[sni_t.value] = true - end + for sni in pairs(route_t.snis) do + plain_indexes.snis[sni] = true end index_src_dst(route_t.sources, plain_indexes.sources, src_trust_funcs) @@ -808,7 +776,7 @@ local function sort_src_dst(source, func) end -local function categorize_hosts_headers_uris_snis(route_t, source, category, key) +local function categorize_hosts_headers_uris(route_t, source, category, key) for i = 1, source[0] do local value = source[i][key or "value"] if category[value] then @@ -821,7 +789,7 @@ local function categorize_hosts_headers_uris_snis(route_t, source, category, key end -local function categorize_methods(route_t, source, category) +local function categorize_methods_snis(route_t, source, category) for key in pairs(source) do if category[key] then append(category[key], route_t) @@ -879,7 +847,7 @@ local function categorize_route_t(route_t, bit_category, categories) routes_by_methods = {}, routes_by_sources = {}, routes_by_destinations = {}, - routes_by_snis = {}, + routes_by_sni = {}, all = { [0] = 0 }, } @@ -887,11 +855,11 @@ local function categorize_route_t(route_t, bit_category, categories) end append(category.all, route_t) - categorize_hosts_headers_uris_snis(route_t, route_t.hosts, category.routes_by_hosts) - categorize_hosts_headers_uris_snis(route_t, route_t.headers, category.routes_by_headers, "name") - categorize_hosts_headers_uris_snis(route_t, route_t.uris, category.routes_by_uris) - categorize_hosts_headers_uris_snis(route_t, route_t.snis, category.routes_by_snis) - categorize_methods(route_t, route_t.methods, category.routes_by_methods) + categorize_hosts_headers_uris(route_t, route_t.hosts, category.routes_by_hosts) + categorize_hosts_headers_uris(route_t, route_t.headers, category.routes_by_headers, "name") + categorize_hosts_headers_uris(route_t, route_t.uris, category.routes_by_uris) + categorize_methods_snis(route_t, route_t.methods, category.routes_by_methods) + categorize_methods_snis(route_t, route_t.snis, category.routes_by_sni) categorize_src_dst(route_t, route_t.sources, category.routes_by_sources) categorize_src_dst(route_t, route_t.destinations, category.routes_by_destinations) end @@ -1090,28 +1058,10 @@ do end, [MATCH_RULES.SNI] = function(route_t, ctx) - local snis = route_t.snis - local sni = ctx.sni - if ctx.req_scheme == "http" or snis[sni] then - ctx.matches.sni = sni + if ctx.req_scheme == "http" or route_t.snis[ctx.sni] then + ctx.matches.sni = ctx.sni return true end - - for i = 1, snis[0] do - local sni_t = snis[i] - if sni_t.wildcard then - local from, _, err = re_find(ctx.sni, sni_t.regex, "ajo") - if err then - log(ERR, "could not evaluate wildcard sni regex: ", err) - return - end - - if from then - ctx.matches.sni = sni_t.value - return true - end - end - end end, [MATCH_RULES.SRC] = function(route_t, ctx) @@ -1181,7 +1131,7 @@ do end, [MATCH_RULES.SNI] = function(category, ctx) - return category.routes_by_snis[ctx.hits.sni or ctx.sni] + return category.routes_by_sni[ctx.sni] end, [MATCH_RULES.SRC] = function(category, ctx) @@ -1418,7 +1368,6 @@ function _M.new(routes, cache, cache_neg) local prefix_uris = { [0] = 0 } -- will be sorted by length local regex_uris = { [0] = 0 } local wildcard_hosts = { [0] = 0 } - local wildcard_snis = { [0] = 0 } local src_trust_funcs = { [0] = 0 } local dst_trust_funcs = { [0] = 0 } @@ -1499,7 +1448,7 @@ function _M.new(routes, cache, cache_neg) local route_t = marshalled_routes[i] categorize_route_t(route_t, route_t.match_rules, categories) index_route_t(route_t, plain_indexes, prefix_uris, regex_uris, - wildcard_hosts, wildcard_snis, src_trust_funcs, dst_trust_funcs) + wildcard_hosts, src_trust_funcs, dst_trust_funcs) end end @@ -1560,7 +1509,6 @@ function _M.new(routes, cache, cache_neg) local match_uris = not isempty(plain_indexes.uris) local match_methods = not isempty(plain_indexes.methods) local match_snis = not isempty(plain_indexes.snis) - local match_wildcard_snis = not isempty(wildcard_snis) local match_sources = not isempty(plain_indexes.sources) local match_destinations = not isempty(plain_indexes.destinations) @@ -1745,30 +1693,10 @@ function _M.new(routes, cache, cache_neg) -- sni match - if sni ~= "" then - if match_snis and plain_indexes.snis[sni] - then - req_category = bor(req_category, MATCH_RULES.SNI) - - elseif match_wildcard_snis then - for i = 1, wildcard_snis[0] do - local sni_t = wildcard_snis[i] - local from, _, err = re_find(sni, sni_t.regex, "ajo") - if err then - log(ERR, "could not match wildcard sni: ", err) - return - end - - if from then - hits.sni = sni_t.value - req_category = bor(req_category, MATCH_RULES.SNI) - break - end - end - end + if match_snis and plain_indexes.snis[sni] then + req_category = bor(req_category, MATCH_RULES.SNI) end - -- src match if match_sources and match_src_dst(plain_indexes.sources, src_ip, src_port, src_trust_funcs) then From f720d60e11f859af28cc77db6a5b318a8ca3732f Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Wed, 10 Apr 2024 11:15:41 +0800 Subject: [PATCH 10/18] change corresponding tests to traditional_compatible only --- .../01-db/01-schema/06-routes_spec.lua | 90 ++++++++-------- spec/01-unit/08-router_spec.lua | 100 +++++++++--------- .../05-proxy/02-router_spec.lua | 59 ++++++++--- 3 files changed, 145 insertions(+), 104 deletions(-) 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 c2d0ee024413..7f532157c0ea 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 @@ -1046,49 +1046,6 @@ describe("routes schema (flavor = " .. flavor .. ")", function() end end) - it("accepts leftmost wildcard", function() - for _, sni in ipairs({ "*.example.org", "*.foo.bar.test" }) do - local route = Routes:process_auto_fields({ - protocols = { "https" }, - snis = { sni }, - service = s, - }, "insert") - local ok, errs = Routes:validate(route) - assert.is_nil(errs) - assert.truthy(ok) - end - end) - - it("accepts rightmost wildcard", function() - for _, sni in ipairs({ "example.*", "foo.bar.*" }) do - local route = Routes:process_auto_fields({ - protocols = { "https" }, - snis = { sni }, - service = s, - }, "insert") - local ok, errs = Routes:validate(route) - assert.is_nil(errs) - assert.truthy(ok) - end - end) - - it("rejects invalid wildcard", function() - for _, sni in ipairs({ "foo.*.test", "foo*.test" }) do - local route = Routes:process_auto_fields({ - protocols = { "https" }, - snis = { sni }, - service = s, - }, "insert") - local ok, errs = Routes:validate(route) - assert.falsy(ok) - assert.same({ - snis = { - "wildcard must be leftmost or rightmost character", - }, - }, errs) - end - end) - it("rejects specifying 'snis' if 'protocols' does not have 'https', 'tls' or 'tls_passthrough'", function() local route = Routes:process_auto_fields({ protocols = { "tcp", "udp" }, @@ -1536,6 +1493,53 @@ describe("routes schema (flavor = " .. flavor .. ")", function() assert.truthy(ok) assert.is_nil(errs) end) + + describe("'snis' matching attribute (wildcard)", function() + local s = { id = "a4fbd24e-6a52-4937-bd78-2536713072d2" } + + it("accepts leftmost wildcard", function() + for _, sni in ipairs({ "*.example.org", "*.foo.bar.test" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.is_nil(errs) + assert.truthy(ok) + end + end) + + it("accepts rightmost wildcard", function() + for _, sni in ipairs({ "example.*", "foo.bar.*" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.is_nil(errs) + assert.truthy(ok) + end + end) + + it("rejects invalid wildcard", function() + for _, sni in ipairs({ "foo.*.test", "foo*.test" }) do + local route = Routes:process_auto_fields({ + protocols = { "https" }, + snis = { sni }, + service = s, + }, "insert") + local ok, errs = Routes:validate(route) + assert.falsy(ok) + assert.same({ + snis = { + "wildcard must be leftmost or rightmost character", + }, + }, errs) + end + end) + end) end) end -- flavor in ipairs({ "traditional_compatible", "expressions" }) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index da53e1a826cd..4171c376f005 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -1732,65 +1732,67 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" end) end) - describe("[wildcard sni]", function() - local use_case, router + if flavor == "tradition_compatible" then + describe("[wildcard sni]", function() + local use_case, router - lazy_setup(function() - use_case = { - { - service = service, - route = { - id = "e8fb37f1-102d-461e-9c51-6608a6bb8101", - snis = { "*.sni.test" }, + lazy_setup(function() + use_case = { + { + service = service, + route = { + id = "e8fb37f1-102d-461e-9c51-6608a6bb8101", + snis = { "*.sni.test" }, + }, }, - }, - { - service = service, - route = { - id = "e8fb37f1-102d-461e-9c51-6608a6bb8102", - snis = { "sni.*" }, + { + service = service, + route = { + id = "e8fb37f1-102d-461e-9c51-6608a6bb8102", + snis = { "sni.*" }, + }, }, - }, - } + } - router = assert(new_router(use_case)) - end) + router = assert(new_router(use_case)) + end) - it("matches leftmost wildcards", function() - for _, sni in ipairs({"foo.sni.test", "foo.bar.sni.test"}) do - local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, - sni) - assert.truthy(match_t) - assert.same(use_case[1].route, match_t.route) - if flavor == "traditional" then - assert.same(use_case[1].route.snis[1], match_t.matches.sni) + it("matches leftmost wildcards", function() + for _, sni in ipairs({"foo.sni.test", "foo.bar.sni.test"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.truthy(match_t) + assert.same(use_case[1].route, match_t.route) + if flavor == "traditional" then + assert.same(use_case[1].route.snis[1], match_t.matches.sni) + end + assert.same(nil, match_t.matches.method) + assert.same(nil, match_t.matches.uri) + assert.same(nil, match_t.matches.uri_captures) end - assert.same(nil, match_t.matches.method) - assert.same(nil, match_t.matches.uri) - assert.same(nil, match_t.matches.uri_captures) - end - end) + end) - it("matches rightmost wildcards", function() - for _, sni in ipairs({"sni.foo", "sni.foo.bar"}) do - local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, - sni) - assert.truthy(match_t) - assert.same(use_case[2].route, match_t.route) - if flavor == "traditional" then - assert.same(use_case[2].route.snis[1], match_t.matches.sni) + it("matches rightmost wildcards", function() + for _, sni in ipairs({"sni.foo", "sni.foo.bar"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.truthy(match_t) + assert.same(use_case[2].route, match_t.route) + if flavor == "traditional" then + assert.same(use_case[2].route.snis[1], match_t.matches.sni) + end end - end - end) + end) - it("doesn't match wildcard", function() - for _, sni in ipairs({"bar.sni.foo", "foo.sni.test.bar"}) do - local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, - sni) - assert.is_nil(match_t) - end + it("doesn't match wildcard", function() + for _, sni in ipairs({"bar.sni.foo", "foo.sni.test.bar"}) do + local match_t = router:select("GET", "/", "any.test", "https", nil, nil, nil, nil, + sni) + assert.is_nil(match_t) + end + end) end) - end) + end if flavor ~= "traditional" then diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 7378a8baab16..4c8050a16e46 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -2,6 +2,7 @@ local admin_api = require "spec.fixtures.admin_api" local helpers = require "spec.helpers" local cjson = require "cjson" local path_handling_tests = require "spec.fixtures.router_path_handling_tests" +local table_insert = table.insert local tonumber = tonumber @@ -130,6 +131,7 @@ for _, strategy in helpers.each_strategy() do local proxy_ssl_client local bp local it_trad_only = (flavor == "traditional") and it or pending + local it_trad_comp_only = (flavor == "traditional_compatible") and it or pending lazy_setup(function() local fixtures = { @@ -1297,7 +1299,7 @@ for _, strategy in helpers.each_strategy() do local proxy_ssl_client lazy_setup(function() - routes = insert_routes(bp, { + local configs = { { protocols = { "https" }, snis = { "www.example.org" }, @@ -1312,6 +1314,9 @@ for _, strategy in helpers.each_strategy() do name = "service_behind_example.org" }, }, + } + + local trad_comp_configs = { { protocols = { "https" }, snis = { "*.foo.test" }, @@ -1326,7 +1331,15 @@ for _, strategy in helpers.each_strategy() do name = "service_behind_bar.wild" }, }, - }) + } + + if flavor == "traditional_compatible" then + for _, v in ipairs(trad_comp_configs) do + table_insert(configs, v) + end + end + + routes = insert_routes(bp, configs) end) lazy_teardown(function() @@ -1383,7 +1396,7 @@ for _, strategy in helpers.each_strategy() do res.headers["kong-service-name"]) end) - it("matches a Route based on its leftmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1400,7 +1413,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1423,7 +1436,7 @@ for _, strategy in helpers.each_strategy() do local proxy_ssl_client lazy_setup(function() - routes = insert_routes(bp, { + local configs = { { protocols = { "tls_passthrough" }, snis = { "www.example.org" }, @@ -1444,6 +1457,9 @@ for _, strategy in helpers.each_strategy() do protocol = "tcp", }, }, + } + + local trad_comp_configs = { { protocols = { "tls_passthrough" }, snis = { "*.foo.test" }, @@ -1464,7 +1480,15 @@ for _, strategy in helpers.each_strategy() do protocol = "tcp", }, }, - }) + } + + if flavor == "traditional_compatible" then + for _, v in ipairs(trad_comp_configs) do + table_insert(configs, v) + end + end + + routes = insert_routes(bp, configs) end) lazy_teardown(function() @@ -1527,7 +1551,7 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end) - it("matches a Route based on its leftmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1553,7 +1577,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1890,7 +1914,7 @@ for _, strategy in helpers.each_strategy() do local grpcs_proxy_ssl_client lazy_setup(function() - routes = insert_routes(bp, { + local configs = { { protocols = { "grpcs" }, snis = { "grpcs_1.test" }, @@ -1907,6 +1931,9 @@ for _, strategy in helpers.each_strategy() do url = helpers.grpcbin_ssl_url, }, }, + } + + local trad_comp_configs = { { protocols = { "grpcs" }, snis = { "*.grpcs_3.test" }, @@ -1923,7 +1950,15 @@ for _, strategy in helpers.each_strategy() do url = helpers.grpcbin_ssl_url, }, }, - }) + } + + if flavor == "traditional_compatible" then + for _, v in ipairs(trad_comp_configs) do + table_insert(configs, v) + end + end + + routes = insert_routes(bp, configs) end) lazy_teardown(function() @@ -1963,7 +1998,7 @@ for _, strategy in helpers.each_strategy() do assert.matches("kong-service-name: grpcs_2", resp, nil, true) end) - it("matches a Route based on its leftmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) @@ -1983,7 +2018,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) From 505ebc63e2983efa03c447af37fca5aac6d9da4b Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Wed, 10 Apr 2024 11:58:33 +0800 Subject: [PATCH 11/18] update changelog --- changelog/unreleased/kong/route-wildcard-snis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/route-wildcard-snis.yml b/changelog/unreleased/kong/route-wildcard-snis.yml index ab9302213158..9de5209f2525 100644 --- a/changelog/unreleased/kong/route-wildcard-snis.yml +++ b/changelog/unreleased/kong/route-wildcard-snis.yml @@ -1,3 +1,3 @@ -message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. +message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character for `traditional_compatible` flavor. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. type: feature scope: Core From 16f2b1d97997f946e340c53866d883169b7f1ec7 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Wed, 10 Apr 2024 14:16:08 +0800 Subject: [PATCH 12/18] update changelog --- changelog/unreleased/kong/route-wildcard-snis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/route-wildcard-snis.yml b/changelog/unreleased/kong/route-wildcard-snis.yml index 9de5209f2525..8b1b2dcfc28d 100644 --- a/changelog/unreleased/kong/route-wildcard-snis.yml +++ b/changelog/unreleased/kong/route-wildcard-snis.yml @@ -1,3 +1,3 @@ -message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character for `traditional_compatible` flavor. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. +message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character when `router_flavor` is `traditional_compatible`. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. type: feature scope: Core From 49f6df826f2d458281537eb69a5e29ab301f64ea Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Fri, 12 Apr 2024 12:02:22 +0800 Subject: [PATCH 13/18] remove the changelog as this is to be a hidden feature --- changelog/unreleased/kong/route-wildcard-snis.yml | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 changelog/unreleased/kong/route-wildcard-snis.yml diff --git a/changelog/unreleased/kong/route-wildcard-snis.yml b/changelog/unreleased/kong/route-wildcard-snis.yml deleted file mode 100644 index 8b1b2dcfc28d..000000000000 --- a/changelog/unreleased/kong/route-wildcard-snis.yml +++ /dev/null @@ -1,3 +0,0 @@ -message: extend the field `snis` of the entity `routes` to support wildcard for the leftmost or the rightmost character when `router_flavor` is `traditional_compatible`. Note only the leftmost OR rightmost character can be the wildcard, otherwise the validation will fail. -type: feature -scope: Core From 298a8273076a6054ce63e862f4ec64ea7f962e7e Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 12:50:31 +0800 Subject: [PATCH 14/18] exclude traditional flavor --- kong/db/schema/entities/routes.lua | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kong/db/schema/entities/routes.lua b/kong/db/schema/entities/routes.lua index 0d5813dd7d53..0d3558557d4f 100644 --- a/kong/db/schema/entities/routes.lua +++ b/kong/db/schema/entities/routes.lua @@ -34,6 +34,11 @@ local entity_checks = { }}, } +local snis_elements_type = typedefs.wildcard_host + +if kong_router_flavor == "traditional" then + snis_elements_type = typedefs.sni +end -- works with both `traditional_compatible` and `expressions` routes local validate_route @@ -159,7 +164,7 @@ local routes = { { snis = { type = "set", description = "A list of SNIs that match this Route.", - elements = typedefs.wildcard_host }, }, + elements = snis_elements_type }, }, { sources = typedefs.sources }, { destinations = typedefs.destinations }, From dc84b125a02fbd4d12b6a9aa25a327526e49d821 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 12:51:10 +0800 Subject: [PATCH 15/18] add expressions flavor in tests --- spec/01-unit/08-router_spec.lua | 2 +- .../05-proxy/02-router_spec.lua | 32 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 4171c376f005..0cd78c65679c 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -1732,7 +1732,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" end) end) - if flavor == "tradition_compatible" then + if flavor ~= "traditional" then describe("[wildcard sni]", function() local use_case, router diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 4c8050a16e46..30c83a4be28e 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -131,7 +131,7 @@ for _, strategy in helpers.each_strategy() do local proxy_ssl_client local bp local it_trad_only = (flavor == "traditional") and it or pending - local it_trad_comp_only = (flavor == "traditional_compatible") and it or pending + local it_not_trad = (flavor ~= "traditional") and it or pending lazy_setup(function() local fixtures = { @@ -1316,7 +1316,7 @@ for _, strategy in helpers.each_strategy() do }, } - local trad_comp_configs = { + local not_trad_configs = { { protocols = { "https" }, snis = { "*.foo.test" }, @@ -1333,8 +1333,8 @@ for _, strategy in helpers.each_strategy() do }, } - if flavor == "traditional_compatible" then - for _, v in ipairs(trad_comp_configs) do + if flavor ~= "traditional" then + for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end end @@ -1396,7 +1396,7 @@ for _, strategy in helpers.each_strategy() do res.headers["kong-service-name"]) end) - it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() + it_not_trad("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1413,7 +1413,7 @@ for _, strategy in helpers.each_strategy() do end end) - it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() + it_not_trad("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1459,7 +1459,7 @@ for _, strategy in helpers.each_strategy() do }, } - local trad_comp_configs = { + local not_trad_configs = { { protocols = { "tls_passthrough" }, snis = { "*.foo.test" }, @@ -1482,8 +1482,8 @@ for _, strategy in helpers.each_strategy() do }, } - if flavor == "traditional_compatible" then - for _, v in ipairs(trad_comp_configs) do + if flavor ~= "traditional" then + for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end end @@ -1551,7 +1551,7 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end) - it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() + it_not_trad("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1577,7 +1577,7 @@ for _, strategy in helpers.each_strategy() do end end) - it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() + it_not_trad("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1933,7 +1933,7 @@ for _, strategy in helpers.each_strategy() do }, } - local trad_comp_configs = { + local not_trad_configs = { { protocols = { "grpcs" }, snis = { "*.grpcs_3.test" }, @@ -1952,8 +1952,8 @@ for _, strategy in helpers.each_strategy() do }, } - if flavor == "traditional_compatible" then - for _, v in ipairs(trad_comp_configs) do + if flavor ~= "traditional" then + for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end end @@ -1998,7 +1998,7 @@ for _, strategy in helpers.each_strategy() do assert.matches("kong-service-name: grpcs_2", resp, nil, true) end) - it_trad_comp_only("matches a Route based on its leftmost wildcard sni", function() + it_not_trad("matches a Route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) @@ -2018,7 +2018,7 @@ for _, strategy in helpers.each_strategy() do end end) - it_trad_comp_only("matches a Route based on its rightmost wildcard sni", function() + it_not_trad("matches a Route based on its rightmost wildcard sni", function() for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) From f9157c53f6fb2d4fd64f5fd45c78cc96ef829cdf Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 14:25:54 +0800 Subject: [PATCH 16/18] adjust the tests --- .../05-proxy/02-router_spec.lua | 335 +++++++++--------- 1 file changed, 170 insertions(+), 165 deletions(-) diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 30c83a4be28e..79d66ab237be 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -131,7 +131,6 @@ for _, strategy in helpers.each_strategy() do local proxy_ssl_client local bp local it_trad_only = (flavor == "traditional") and it or pending - local it_not_trad = (flavor ~= "traditional") and it or pending lazy_setup(function() local fixtures = { @@ -1316,24 +1315,24 @@ for _, strategy in helpers.each_strategy() do }, } - local not_trad_configs = { - { - protocols = { "https" }, - snis = { "*.foo.test" }, - service = { - name = "service_behind_wild.foo.test" + if flavor ~= "traditional" then + local not_trad_configs = { + { + protocols = { "https" }, + snis = { "*.foo.test" }, + service = { + name = "service_behind_wild.foo.test" + }, }, - }, - { - protocols = { "https" }, - snis = { "bar.*" }, - service = { - name = "service_behind_bar.wild" + { + protocols = { "https" }, + snis = { "bar.*" }, + service = { + name = "service_behind_bar.wild" + }, }, - }, - } + } - if flavor ~= "traditional" then for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end @@ -1396,39 +1395,41 @@ for _, strategy in helpers.each_strategy() do res.headers["kong-service-name"]) end) - it_not_trad("matches a Route based on its leftmost wildcard sni", function() - for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do - proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) + if flavor ~= "traditional" then + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do + proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) - local res = assert(proxy_ssl_client:send { - method = "GET", - path = "/status/200", - headers = { ["kong-debug"] = 1 }, - }) - assert.res_status(200, res) - assert.equal("service_behind_wild.foo.test", - res.headers["kong-service-name"]) + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + assert.equal("service_behind_wild.foo.test", + res.headers["kong-service-name"]) - proxy_ssl_client:close() - end - end) + proxy_ssl_client:close() + end + end) - it_not_trad("matches a Route based on its rightmost wildcard sni", function() - for _, sni in ipairs({"bar.x", "bar.y.z"}) do - proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"bar.x", "bar.y.z"}) do + proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) - local res = assert(proxy_ssl_client:send { - method = "GET", - path = "/status/200", - headers = { ["kong-debug"] = 1 }, - }) - assert.res_status(200, res) - assert.equal("service_behind_bar.wild", - res.headers["kong-service-name"]) + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) + assert.equal("service_behind_bar.wild", + res.headers["kong-service-name"]) - proxy_ssl_client:close() - end - end) + proxy_ssl_client:close() + end + end) + end end) describe("tls_passthrough", function() @@ -1459,30 +1460,30 @@ for _, strategy in helpers.each_strategy() do }, } - local not_trad_configs = { - { - protocols = { "tls_passthrough" }, - snis = { "*.foo.test" }, - service = { - name = "service_behind_wild.foo.test", - host = helpers.mock_upstream_ssl_host, - port = helpers.mock_upstream_ssl_port, - protocol = "tcp", + if flavor ~= "traditional" then + local not_trad_configs = { + { + protocols = { "tls_passthrough" }, + snis = { "*.foo.test" }, + service = { + name = "service_behind_wild.foo.test", + host = helpers.mock_upstream_ssl_host, + port = helpers.mock_upstream_ssl_port, + protocol = "tcp", + }, }, - }, - { - protocols = { "tls_passthrough" }, - snis = { "bar.*" }, - service = { - name = "service_behind_bar.wild", - host = helpers.mock_upstream_ssl_host, - port = helpers.mock_upstream_ssl_port, - protocol = "tcp", + { + protocols = { "tls_passthrough" }, + snis = { "bar.*" }, + service = { + name = "service_behind_bar.wild", + host = helpers.mock_upstream_ssl_host, + port = helpers.mock_upstream_ssl_port, + protocol = "tcp", + }, }, - }, - } + } - if flavor ~= "traditional" then for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end @@ -1551,57 +1552,59 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end) - it_not_trad("matches a Route based on its leftmost wildcard sni", function() - for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do - -- config propagates to stream subsystems not instantly - -- try up to 10 seconds with step of 2 seconds - -- in vagrant it takes around 6 seconds - helpers.wait_until(function() - proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) - local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify - if not ok then - proxy_ssl_client:close() - return false - end - return true - end, 10, 2) + if flavor ~= "traditional" then + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do + -- config propagates to stream subsystems not instantly + -- try up to 10 seconds with step of 2 seconds + -- in vagrant it takes around 6 seconds + helpers.wait_until(function() + proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) + local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify + if not ok then + proxy_ssl_client:close() + return false + end + return true + end, 10, 2) - local res = assert(proxy_ssl_client:send { - method = "GET", - path = "/status/200", - headers = { ["kong-debug"] = 1 }, - }) - assert.res_status(200, res) + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) - proxy_ssl_client:close() - end - end) + proxy_ssl_client:close() + end + end) - it_not_trad("matches a Route based on its rightmost wildcard sni", function() - for _, sni in ipairs({"bar.x", "bar.y.z"}) do - -- config propagates to stream subsystems not instantly - -- try up to 10 seconds with step of 2 seconds - -- in vagrant it takes around 6 seconds - helpers.wait_until(function() - proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) - local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify - if not ok then - proxy_ssl_client:close() - return false - end - return true - end, 10, 2) + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"bar.x", "bar.y.z"}) do + -- config propagates to stream subsystems not instantly + -- try up to 10 seconds with step of 2 seconds + -- in vagrant it takes around 6 seconds + helpers.wait_until(function() + proxy_ssl_client = helpers.http_client("127.0.0.1", stream_tls_listen_port) + local ok = proxy_ssl_client:ssl_handshake(nil, sni, false) -- explicit no-verify + if not ok then + proxy_ssl_client:close() + return false + end + return true + end, 10, 2) - local res = assert(proxy_ssl_client:send { - method = "GET", - path = "/status/200", - headers = { ["kong-debug"] = 1 }, - }) - assert.res_status(200, res) + local res = assert(proxy_ssl_client:send { + method = "GET", + path = "/status/200", + headers = { ["kong-debug"] = 1 }, + }) + assert.res_status(200, res) - proxy_ssl_client:close() - end - end) + proxy_ssl_client:close() + end + end) + end end) describe("[#headers]", function() @@ -1933,26 +1936,26 @@ for _, strategy in helpers.each_strategy() do }, } - local not_trad_configs = { - { - protocols = { "grpcs" }, - snis = { "*.grpcs_3.test" }, - service = { - name = "grpcs_3", - url = helpers.grpcbin_ssl_url, + if flavor ~= "traditional" then + local not_trad_configs = { + { + protocols = { "grpcs" }, + snis = { "*.grpcs_3.test" }, + service = { + name = "grpcs_3", + url = helpers.grpcbin_ssl_url, + }, }, - }, - { - protocols = { "grpcs" }, - snis = { "grpcs_4.*" }, - service = { - name = "grpcs_4", - url = helpers.grpcbin_ssl_url, + { + protocols = { "grpcs" }, + snis = { "grpcs_4.*" }, + service = { + name = "grpcs_4", + url = helpers.grpcbin_ssl_url, + }, }, - }, - } + } - if flavor ~= "traditional" then for _, v in ipairs(not_trad_configs) do table_insert(configs, v) end @@ -1998,45 +2001,47 @@ for _, strategy in helpers.each_strategy() do assert.matches("kong-service-name: grpcs_2", resp, nil, true) end) - it_not_trad("matches a Route based on its leftmost wildcard sni", function() - for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do - grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) + if flavor ~= "traditional" then + it("matches a Route based on its leftmost wildcard sni", function() + for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do + grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) - local ok, resp = assert(grpcs_proxy_ssl_client({ - service = "hello.HelloService.SayHello", - body = { - greeting = "world!" - }, - opts = { - ["-H"] = "'kong-debug: 1'", - ["-v"] = true, -- verbose so we get response headers - } - })) - assert.truthy(ok) - assert.truthy(resp) - assert.matches("kong-service-name: grpcs_3", resp, nil, true) - end - end) - - it_not_trad("matches a Route based on its rightmost wildcard sni", function() - for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do - grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) + local ok, resp = assert(grpcs_proxy_ssl_client({ + service = "hello.HelloService.SayHello", + body = { + greeting = "world!" + }, + opts = { + ["-H"] = "'kong-debug: 1'", + ["-v"] = true, -- verbose so we get response headers + } + })) + assert.truthy(ok) + assert.truthy(resp) + assert.matches("kong-service-name: grpcs_3", resp, nil, true) + end + end) - local ok, resp = assert(grpcs_proxy_ssl_client({ - service = "hello.HelloService.SayHello", - body = { - greeting = "world!" - }, - opts = { - ["-H"] = "'kong-debug: 1'", - ["-v"] = true, -- verbose so we get response headers - } - })) - assert.truthy(ok) - assert.truthy(resp) - assert.matches("kong-service-name: grpcs_4", resp, nil, true) - end - end) + it("matches a Route based on its rightmost wildcard sni", function() + for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do + grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) + + local ok, resp = assert(grpcs_proxy_ssl_client({ + service = "hello.HelloService.SayHello", + body = { + greeting = "world!" + }, + opts = { + ["-H"] = "'kong-debug: 1'", + ["-v"] = true, -- verbose so we get response headers + } + })) + assert.truthy(ok) + assert.truthy(resp) + assert.matches("kong-service-name: grpcs_4", resp, nil, true) + end + end) + end end) end -- not enable_buffering From 51bccf1a273da7c3afbb5bf6ccb1a1ec1ed1773b Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 14:43:59 +0800 Subject: [PATCH 17/18] adjust tests --- spec/01-unit/08-router_spec.lua | 8 +------- spec/02-integration/05-proxy/02-router_spec.lua | 6 +++--- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 0cd78c65679c..1ae16657fe9b 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -1763,9 +1763,6 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" sni) assert.truthy(match_t) assert.same(use_case[1].route, match_t.route) - if flavor == "traditional" then - assert.same(use_case[1].route.snis[1], match_t.matches.sni) - end assert.same(nil, match_t.matches.method) assert.same(nil, match_t.matches.uri) assert.same(nil, match_t.matches.uri_captures) @@ -1778,9 +1775,6 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" sni) assert.truthy(match_t) assert.same(use_case[2].route, match_t.route) - if flavor == "traditional" then - assert.same(use_case[2].route.snis[1], match_t.matches.sni) - end end end) @@ -1792,7 +1786,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" end end) end) - end + end -- if flavor ~= "traditional" then if flavor ~= "traditional" then diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 79d66ab237be..2e5166370fed 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -1429,7 +1429,7 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end end) - end + end -- if flavor ~= "traditional" then end) describe("tls_passthrough", function() @@ -1604,7 +1604,7 @@ for _, strategy in helpers.each_strategy() do proxy_ssl_client:close() end end) - end + end -- if flavor ~= "traditional" then end) describe("[#headers]", function() @@ -2041,7 +2041,7 @@ for _, strategy in helpers.each_strategy() do assert.matches("kong-service-name: grpcs_4", resp, nil, true) end end) - end + end -- if flavor ~= "traditional" then end) end -- not enable_buffering From 96e5d056cfe371b927473fff73308407e61bdc01 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 18 Apr 2024 15:25:17 +0800 Subject: [PATCH 18/18] update test descriptions --- .../02-integration/05-proxy/02-router_spec.lua | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 2e5166370fed..aa52eaa13c6b 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -1351,7 +1351,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its 'snis' attribute", function() + it("matches a route based on its 'snis' attribute", function() proxy_ssl_client = helpers.proxy_ssl_client(nil, "www.example.org") local res = assert(proxy_ssl_client:send { @@ -1396,7 +1396,7 @@ for _, strategy in helpers.each_strategy() do end) if flavor ~= "traditional" then - it("matches a Route based on its leftmost wildcard sni", function() + it("matches a route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1413,7 +1413,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it("matches a route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do proxy_ssl_client = helpers.proxy_ssl_client(nil, sni) @@ -1502,7 +1502,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its 'snis' attribute", function() + it("matches a route based on its 'snis' attribute", function() -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds -- in vagrant it takes around 6 seconds @@ -1553,7 +1553,7 @@ for _, strategy in helpers.each_strategy() do end) if flavor ~= "traditional" then - it("matches a Route based on its leftmost wildcard sni", function() + it("matches a route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.foo.test", "a.b.foo.test"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1579,7 +1579,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it("matches a route based on its rightmost wildcard sni", function() for _, sni in ipairs({"bar.x", "bar.y.z"}) do -- config propagates to stream subsystems not instantly -- try up to 10 seconds with step of 2 seconds @@ -1968,7 +1968,7 @@ for _, strategy in helpers.each_strategy() do remove_routes(strategy, routes) end) - it("matches a Route based on its 'snis' attribute", function() + it("matches a route based on its 'snis' attribute", function() grpcs_proxy_ssl_client = helpers.proxy_client_grpcs("grpcs_1.test") local ok, resp = assert(grpcs_proxy_ssl_client({ @@ -2002,7 +2002,7 @@ for _, strategy in helpers.each_strategy() do end) if flavor ~= "traditional" then - it("matches a Route based on its leftmost wildcard sni", function() + it("matches a route based on its leftmost wildcard sni", function() for _, sni in ipairs({"a.grpcs_3.test", "a.b.grpcs_3.test"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni) @@ -2022,7 +2022,7 @@ for _, strategy in helpers.each_strategy() do end end) - it("matches a Route based on its rightmost wildcard sni", function() + it("matches a route based on its rightmost wildcard sni", function() for _, sni in ipairs({"grpcs_4.x", "grpcs_4.y.z"}) do grpcs_proxy_ssl_client = helpers.proxy_client_grpcs(sni)