diff --git a/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.md b/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.md new file mode 100644 index 000000000000..91f7e6557547 --- /dev/null +++ b/changelog/unreleased/kong/fix-grpc-web-and-gateway-trailers.md @@ -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..6319eff9b2fd 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,51 @@ 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"] + ngx.exit(200, "ok") + } + } + } + ]] + + 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 +104,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..dd2fa1f53b57 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,31 @@ 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"] + ngx.exit(200, "ok") + } + } + } + ]] + + 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 +121,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", {