From 63040b79c3276892aa931aec035877e89848371c Mon Sep 17 00:00:00 2001 From: Guilherme Salazar Date: Fri, 20 Dec 2024 14:39:43 -0300 Subject: [PATCH] fix(plugins): grpc-web, grpc-gateway: TE trailers Ensure `TE` headers is properly sent to gRPC upstream server in request generated from Kong. Previously, the call to `kong.service.request.set_headers` was not taking effect as the `TE` headers cannot be set through normal OpenResty APIs; this PR ensures it's set in a similar way as the `:authority` pseudo-header. --- .../fix-grpc-web-and-gateway-trailers.yml | 3 + kong/pdk/service/request.lua | 18 ++++- .../28-grpc-gateway/01-proxy_spec.lua | 73 ++++++++++++++++++- spec/03-plugins/32-grpc-web/01-proxy_spec.lua | 70 +++++++++++++++++- 4 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.yml diff --git a/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.yml b/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.yml new file mode 100644 index 000000000000..a35547dd4dde --- /dev/null +++ b/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.yml @@ -0,0 +1,3 @@ +message: "**grpc-web** and **grpc-gateway**: Fixed a bug where the `TE` (transfer-encoding) header would not be sent to the upstream gRPC servers when `grpc-web` or `grpc-gateweay` are in use." +type: bugfix +scope: Plugin diff --git a/kong/pdk/service/request.lua b/kong/pdk/service/request.lua index f583d390fa14..dfc0e29ecc29 100644 --- a/kong/pdk/service/request.lua +++ b/kong/pdk/service/request.lua @@ -313,14 +313,24 @@ local function new(self) -- kong.service.request.set_header("X-Foo", "value") request.set_header = function(header, value) check_phase(access_rewrite_balancer) - validate_header(header, value) - if string_lower(header) == "host" then + local header_lower = string_lower(header) + + if header_lower == "host" then ngx_var.upstream_host = value - end + return + + elseif header_lower == "te" then + if (ngx_var.upstream_scheme == "grpc" or + ngx_var.upstream_scheme == "grpcs") and value ~= "trailers" then + return nil, "grpc requires TE to be set to trailers" + end + + ngx.var.upstream_te = value + return - if string_lower(header) == ":authority" then + elseif header_lower == ":authority" then if ngx_var.upstream_scheme == "grpc" or ngx_var.upstream_scheme == "grpcs" then diff --git a/spec/03-plugins/28-grpc-gateway/01-proxy_spec.lua b/spec/03-plugins/28-grpc-gateway/01-proxy_spec.lua index 0f5d9530fd1d..599bd01399c3 100644 --- a/spec/03-plugins/28-grpc-gateway/01-proxy_spec.lua +++ b/spec/03-plugins/28-grpc-gateway/01-proxy_spec.lua @@ -48,10 +48,50 @@ for _, strategy in helpers.each_strategy() do }, }) - assert(helpers.start_kong { + local mock_grpc_service = assert(bp.services:insert { + name = "mock_grpc_service", + url = "http://localhost:8765", + }) + + local mock_grpc_route = assert(bp.routes:insert { + protocols = { "http" }, + hosts = { "grpc_mock.example" }, + service = mock_grpc_service, + preserve_host = true, + }) + + assert(bp.plugins:insert { + route = mock_grpc_route, + name = "grpc-gateway", + config = { + proto = "./spec/fixtures/grpc/targetservice.proto", + }, + }) + + local fixtures = { + http_mock = {} + } + fixtures.http_mock.my_server_block = [[ + server { + server_name myserver; + listen 8765; + + location ~ / { + content_by_lua_block { + local headers = ngx.req.get_headers() + ngx.header.content_type = "application/grpc" + ngx.header.received_host = headers["Host"] + ngx.header.received_te = headers["te"] + } + } + } + ]] + + assert(helpers.start_kong({ database = strategy, plugins = "bundled,grpc-gateway", - }) + nginx_conf = "spec/fixtures/custom_nginx.template", + }, nil, nil, fixtures)) end) before_each(function() @@ -63,6 +103,35 @@ for _, strategy in helpers.each_strategy() do helpers.stop_grpc_target() end) + test("Sets 'TE: trailers'", function() + local res, err = proxy_client:post("/v1/echo", { + headers = { + ["Host"] = "grpc_mock.example", + ["Content-Type"] = "application/json", + }, + }) + + assert.equal("trailers", res.headers["received-te"]) + assert.is_nil(err) + end) + + test("Ignores user-agent TE", function() + -- in grpc-gateway, kong acts as a grpc client on behalf of the client + -- (which generally is a web-browser); as such, the Te header must be + -- set by kong, which will append trailers to the response body + local res, err = proxy_client:post("/v1/echo", { + headers = { + ["Host"] = "grpc_mock.example", + ["Content-Type"] = "application/json", + ["TE"] = "chunked", + }, + }) + + assert.equal("trailers", res.headers["received-te"]) + assert.is_nil(err) + end) + + test("main entrypoint", function() local res, err = proxy_client:get("/v1/messages/john_doe") diff --git a/spec/03-plugins/32-grpc-web/01-proxy_spec.lua b/spec/03-plugins/32-grpc-web/01-proxy_spec.lua index 8c37776204a3..6c17a306cbfe 100644 --- a/spec/03-plugins/32-grpc-web/01-proxy_spec.lua +++ b/spec/03-plugins/32-grpc-web/01-proxy_spec.lua @@ -49,6 +49,25 @@ for _, strategy in helpers.each_strategy() do service = service1, }) + local mock_grpc_service = assert(bp.services:insert { + name = "mock_grpc_service", + url = "http://localhost:8765", + }) + + local mock_grpc_route = assert(bp.routes:insert { + protocols = { "http" }, + hosts = { "grpc_mock.example" }, + service = mock_grpc_service, + preserve_host = true, + }) + + assert(bp.plugins:insert { + route = mock_grpc_route, + name = "grpc-web", + config = { + }, + }) + assert(bp.plugins:insert { route = route1, name = "grpc-web", @@ -66,10 +85,30 @@ for _, strategy in helpers.each_strategy() do }, }) - assert(helpers.start_kong { + local fixtures = { + http_mock = {} + } + fixtures.http_mock.my_server_block = [[ + server { + server_name myserver; + listen 8765; + + location ~ / { + content_by_lua_block { + local headers = ngx.req.get_headers() + ngx.header.content_type = "application/grpc" + ngx.header.received_host = headers["Host"] + ngx.header.received_te = headers["te"] + } + } + } + ]] + + assert(helpers.start_kong({ database = strategy, plugins = "bundled,grpc-web", - }) + nginx_conf = "spec/fixtures/custom_nginx.template", + }, nil, nil, fixtures)) end) before_each(function() @@ -81,6 +120,33 @@ for _, strategy in helpers.each_strategy() do helpers.stop_kong() end) + test("Sets 'TE: trailers'", function() + local res, err = proxy_client:post("/", { + headers = { + ["Host"] = "grpc_mock.example", + ["Content-Type"] = "application/grpc-web-text", + }, + }) + + assert.equal("trailers", res.headers["received-te"]) + assert.is_nil(err) + end) + + test("Ignores user-agent TE", function() + -- in grpc-web, kong acts as a grpc client on behalf of the client + -- (which generally is a web-browser); as such, the Te header must be + -- set by kong, which will append trailers to the response body + local res, err = proxy_client:post("/", { + headers = { + ["Host"] = "grpc_mock.example", + ["Content-Type"] = "application/grpc-web-text", + ["TE"] = "chunked", + }, + }) + + assert.equal("trailers", res.headers["received-te"]) + assert.is_nil(err) + end) test("Call gRCP-base64 via HTTP", function() local res, err = proxy_client:post("/hello.HelloService/SayHello", {