Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router/expressions): mapping protocols in expression #11082

Merged
merged 20 commits into from Jul 3, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -72,6 +72,9 @@
[#11135](https://github.com/Kong/kong/pull/11135)
- Fix a bug that caused the router to fail in `traditional_compatible` mode when a route with multiple paths and no service was created.
[#11158](https://github.com/Kong/kong/pull/11158)
- Fix an issue where the router of flavor `expressions` can not work correctly
when `route.protocols` is set to `grpc` or `grpcs`.
[#11082](https://github.com/Kong/kong/pull/11082)

#### Admin API

Expand Down
19 changes: 17 additions & 2 deletions kong/router/expressions.lua
Expand Up @@ -6,20 +6,35 @@ local gen_for_field = atc.gen_for_field


local OP_EQUAL = "=="


local LOGICAL_AND = atc.LOGICAL_AND


local ngx_log = ngx.log
local ngx_ERR = ngx.ERR


-- map to normal protocol
local PROTOCOLS_OVERRIDE = {
tls_passthrough = "tcp",
grpc = "http",
grpcs = "https",
}


local function get_exp_and_priority(route)
local exp = route.expression
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 router_flavor")
"Likely it's a misconfiguration. Please check the 'router_flavor' config in kong.conf")
return
end

local gen = gen_for_field("net.protocol", OP_EQUAL, route.protocols)
local gen = gen_for_field("net.protocol", OP_EQUAL, route.protocols,
function(_, p)
return PROTOCOLS_OVERRIDE[p] or p
end)
if gen then
exp = exp .. LOGICAL_AND .. gen
end
Expand Down
76 changes: 63 additions & 13 deletions spec/02-integration/05-proxy/19-grpc_proxy_spec.lua
@@ -1,19 +1,60 @@
local helpers = require "spec.helpers"
local cjson = require "cjson"
local pl_path = require "pl.path"
local atc_compat = require "kong.router.compat"

local FILE_LOG_PATH = os.tmpname()


local function reload_router(flavor)
_G.kong = {
configuration = {
router_flavor = flavor,
},
}

helpers.setenv("KONG_ROUTER_FLAVOR", flavor)

package.loaded["spec.helpers"] = nil
package.loaded["kong.db"] = nil
package.loaded["kong.db.schema.entities.routes"] = nil
package.loaded["kong.db.schema.entities.routes_subschemas"] = nil

helpers = require "spec.helpers"

helpers.unsetenv("KONG_ROUTER_FLAVOR")
end


local function gen_route(flavor, r)
if flavor ~= "expressions" then
return r
end

r.expression = atc_compat.get_expression(r)
r.priority = tonumber(atc_compat._get_priority(r))

r.hosts = nil
r.paths = nil
r.snis = nil

return r
end


for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do
for _, strategy in helpers.each_strategy() do

describe("gRPC Proxying [#" .. strategy .. "]", function()
describe("gRPC Proxying [#" .. strategy .. ", flavor = " .. flavor .. "]", function()
local proxy_client_grpc
local proxy_client_grpcs
local proxy_client
local proxy_client_ssl
local proxy_client_h2c
local proxy_client_h2

reload_router(flavor)

lazy_setup(function()
local bp = helpers.get_db_utils(strategy, {
"routes",
Expand Down Expand Up @@ -54,38 +95,38 @@ for _, strategy in helpers.each_strategy() do
target = "127.0.0.1:8765",
})

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpc" },
hosts = { "grpc" },
service = service1,
})
})))

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpcs" },
hosts = { "grpcs" },
service = service2,
})
})))

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpc" },
hosts = { "grpc_authority_1.example" },
service = mock_grpc_service,
preserve_host = true,
})
})))

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpc" },
hosts = { "grpc_authority_2.example" },
service = mock_grpc_service,
preserve_host = false,
})
})))

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpc" },
hosts = { "grpc_authority_retry.example" },
service = mock_grpc_service_retry,
preserve_host = false,
})
})))

assert(bp.plugins:insert {
service = mock_grpc_service_retry,
Expand Down Expand Up @@ -115,6 +156,7 @@ for _, strategy in helpers.each_strategy() do
]]

assert(helpers.start_kong({
router_flavor = flavor,
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
}, nil, nil, fixtures))
Expand Down Expand Up @@ -357,9 +399,17 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.falsy(ok)
assert.matches("Code: Canceled", resp, nil, true)
assert.matches("Message: gRPC request matched gRPCs route", resp, nil, true)

if flavor == "expressions" then
assert.matches("Code: NotFound", resp, nil, true)
assert.matches("Message: NotFound", resp, nil, true)

else
assert.matches("Code: Canceled", resp, nil, true)
assert.matches("Message: gRPC request matched gRPCs route", resp, nil, true)
end
end)
end)
end)
end
end -- flavor
51 changes: 46 additions & 5 deletions spec/02-integration/05-proxy/21-grpc_plugins_triggering_spec.lua
@@ -1,10 +1,46 @@
local helpers = require "spec.helpers"
local pl_file = require "pl.file"
local atc_compat = require "kong.router.compat"


local TEST_CONF = helpers.test_conf


local function reload_router(flavor)
_G.kong = {
configuration = {
router_flavor = flavor,
},
}

helpers.setenv("KONG_ROUTER_FLAVOR", flavor)

package.loaded["spec.helpers"] = nil
package.loaded["kong.db"] = nil
package.loaded["kong.db.schema.entities.routes"] = nil
package.loaded["kong.db.schema.entities.routes_subschemas"] = nil

helpers = require "spec.helpers"

helpers.unsetenv("KONG_ROUTER_FLAVOR")
end


local function gen_route(flavor, r)
if flavor ~= "expressions" then
return r
end

r.expression = atc_compat.get_expression(r)
r.priority = tonumber(atc_compat._get_priority(r))

r.hosts = nil
r.paths = nil
r.snis = nil

return r
end


local function find_in_file(pat, cnt)
local f = assert(io.open(TEST_CONF.prefix .. "/" .. TEST_CONF.proxy_error_log, "r"))
Expand Down Expand Up @@ -94,13 +130,16 @@ local function assert_phases(phrases)
end
end

for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do
for _, strategy in helpers.each_strategy() do

describe("gRPC Proxying [#" .. strategy .. "]", function()
describe("gRPC Proxying [#" .. strategy .. ", flavor = " .. flavor .. "]", function()
local grpc_client
local grpcs_client
local bp

reload_router(flavor)

before_each(function()
bp = helpers.get_db_utils(strategy, {
"routes",
Expand All @@ -120,23 +159,24 @@ for _, strategy in helpers.each_strategy() do
url = helpers.grpcbin_ssl_url,
})

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpc" },
hosts = { "grpc" },
service = service1,
})
})))

assert(bp.routes:insert {
assert(bp.routes:insert(gen_route(flavor, {
protocols = { "grpcs" },
hosts = { "grpcs" },
service = service2,
})
})))

assert(bp.plugins:insert {
name = "logger",
})

assert(helpers.start_kong {
router_flavor = flavor,
database = strategy,
plugins = "logger",
})
Expand Down Expand Up @@ -209,3 +249,4 @@ for _, strategy in helpers.each_strategy() do
end)
end)
end
end -- flavor