Skip to content

Commit

Permalink
feat(proxy) add service.request_buffering
Browse files Browse the repository at this point in the history
### Summary

By default Kong does buffer the request. This is not ideal when proxying
bigger payloads with HTTP 1.1 chunked encoding.

This commit adds a new property to service entity:
```
request_buffering = <true/false>
```

By default that is `true` (which is the same as before). Users can now
turn off the request buffering on service-by-service manner by setting
`service.request_buffering=false`.
  • Loading branch information
bungle committed Jun 26, 2020
1 parent 612c365 commit 03ad1c9
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 12 deletions.
7 changes: 7 additions & 0 deletions autodoc/data/admin-api.lua
Expand Up @@ -653,6 +653,13 @@ return {
for transmitting a request to the upstream server.
]]
},
request_buffering = {
description = [[
Whether to enable request body buffering or not. With HTTP 1.1, it
may make sense to turn this off on services that receive data with
chunked transfer encoding.
]]
},
client_certificate = {
description = [[
Certificate to be used as client certificate while TLS handshaking
Expand Down
1 change: 1 addition & 0 deletions kong-2.0.4-0.rockspec
Expand Up @@ -197,6 +197,7 @@ build = {
["kong.db.migrations.core.007_140_to_150"] = "kong/db/migrations/core/007_140_to_150.lua",
["kong.db.migrations.core.008_150_to_200"] = "kong/db/migrations/core/008_150_to_200.lua",
["kong.db.migrations.core.009_200_to_210"] = "kong/db/migrations/core/009_200_to_210.lua",
["kong.db.migrations.core.010_210_to_220"] = "kong/db/migrations/core/010_210_to_220.lua",
["kong.db.migrations.operations.200_to_210"] = "kong/db/migrations/operations/200_to_210.lua",

["kong.pdk"] = "kong/pdk/init.lua",
Expand Down
19 changes: 19 additions & 0 deletions kong/db/migrations/core/010_210_to_220.lua
@@ -0,0 +1,19 @@
return {
postgres = {
up = [[
DO $$
BEGIN
ALTER TABLE IF EXISTS ONLY "services" ADD "request_buffering" BOOLEAN;
EXCEPTION WHEN DUPLICATE_COLUMN THEN
-- Do nothing, accept existing state
END;
$$;
]],
},

cassandra = {
up = [[
ALTER TABLE services ADD request_buffering boolean;
]],
},
}
1 change: 1 addition & 0 deletions kong/db/migrations/core/init.lua
Expand Up @@ -7,4 +7,5 @@ return {
"007_140_to_150",
"008_150_to_200",
"009_200_to_210",
"010_210_to_220",
}
1 change: 1 addition & 0 deletions kong/db/schema/entities/services.lua
Expand Up @@ -38,6 +38,7 @@ return {
{ connect_timeout = nonzero_timeout { default = 60000 }, },
{ write_timeout = nonzero_timeout { default = 60000 }, },
{ read_timeout = nonzero_timeout { default = 60000 }, },
{ request_buffering = { type = "boolean", required = true, default = true }, },
{ tags = typedefs.tags },
{ client_certificate = { type = "foreign", reference = "certificates" }, },
{ tls_verify = { type = "boolean", }, },
Expand Down
5 changes: 3 additions & 2 deletions kong/init.lua
Expand Up @@ -648,8 +648,9 @@ end


function Kong.rewrite()
if var.kong_proxy_mode == "grpc" then
kong_resty_ctx.apply_ref() -- if kong_proxy_mode is gRPC, this is executing
local proxy_mode = var.kong_proxy_mode
if proxy_mode == "grpc" or proxy_mode == "unbuffered" then
kong_resty_ctx.apply_ref() -- if kong_proxy_mode is gRPC/unbuffered, this is executing
kong_resty_ctx.stash_ref() -- after an internal redirect. Restore (and restash)
-- context to avoid re-executing phases

Expand Down
2 changes: 1 addition & 1 deletion kong/reports.lua
Expand Up @@ -217,7 +217,7 @@ local function get_current_suffix(ctx)
local scheme = var.scheme
local proxy_mode = var.kong_proxy_mode
if scheme == "http" or scheme == "https" then
if proxy_mode == "http" then
if proxy_mode == "http" or proxy_mode == "unbuffered" then
local http_upgrade = var.http_upgrade
if http_upgrade and lower(http_upgrade) == "websocket" then
if scheme == "http" then
Expand Down
4 changes: 4 additions & 0 deletions kong/runloop/handler.lua
Expand Up @@ -1219,6 +1219,10 @@ return {
if service.protocol == "grpcs" then
return ngx.exec("@grpcs")
end
if service.request_buffering == false and http_version == 1.1 then
return ngx.exec("@unbuffered")
end
end
end,
-- Only executed if the `router` module found a route and allows nginx to proxy it.
Expand Down
33 changes: 32 additions & 1 deletion kong/templates/nginx_kong.lua
Expand Up @@ -137,7 +137,38 @@ server {
set $upstream_x_forwarded_prefix '';
set $kong_proxy_mode 'http';
proxy_http_version 1.1;
proxy_http_version 1.1;
proxy_request_buffering on;
proxy_set_header TE $upstream_te;
proxy_set_header Host $upstream_host;
proxy_set_header Upgrade $upstream_upgrade;
proxy_set_header Connection $upstream_connection;
proxy_set_header X-Forwarded-For $upstream_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $upstream_x_forwarded_proto;
proxy_set_header X-Forwarded-Host $upstream_x_forwarded_host;
proxy_set_header X-Forwarded-Port $upstream_x_forwarded_port;
proxy_set_header X-Forwarded-Prefix $upstream_x_forwarded_prefix;
proxy_set_header X-Real-IP $remote_addr;
proxy_pass_header Server;
proxy_pass_header Date;
proxy_ssl_name $upstream_host;
proxy_ssl_server_name on;
> if client_ssl then
proxy_ssl_certificate ${{CLIENT_SSL_CERT}};
proxy_ssl_certificate_key ${{CLIENT_SSL_CERT_KEY}};
> end
proxy_pass $upstream_scheme://kong_upstream$upstream_uri;
}
location @unbuffered {
internal;
default_type '';
set $kong_proxy_mode 'unbuffered';
proxy_http_version 1.1;
proxy_request_buffering off;
proxy_set_header TE $upstream_te;
proxy_set_header Host $upstream_host;
proxy_set_header Upgrade $upstream_upgrade;
Expand Down
26 changes: 19 additions & 7 deletions spec/01-unit/01-db/01-schema/05-services_spec.lua
Expand Up @@ -124,12 +124,13 @@ describe("services", function()
-- acceptance
it("should be greater than zero", function()
local service = {
host = "example.com",
port = 80,
protocol = "https",
connect_timeout = 1,
read_timeout = 10,
write_timeout = 100,
host = "example.com",
port = 80,
protocol = "https",
connect_timeout = 1,
read_timeout = 10,
write_timeout = 100,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand Down Expand Up @@ -240,6 +241,7 @@ describe("services", function()
host = "example.com",
path = "/",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -253,6 +255,7 @@ describe("services", function()
host = "example.com",
path = "/abcd~user~2",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -269,6 +272,7 @@ describe("services", function()
host = "example.com",
path = valid_paths[i],
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -283,6 +287,7 @@ describe("services", function()
host = "example.com",
path = "/ovo/",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand Down Expand Up @@ -395,6 +400,7 @@ describe("services", function()
protocol = "http",
host = valid_hosts[i],
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand Down Expand Up @@ -437,6 +443,7 @@ describe("services", function()
for i = 1, #invalid_names do
local service = {
name = invalid_names[i],
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand Down Expand Up @@ -465,7 +472,8 @@ describe("services", function()
protocol = "http",
host = "example.com",
port = 80,
name = valid_names[i]
name = valid_names[i],
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -481,6 +489,7 @@ describe("services", function()
protocol = "tcp",
host = "x.y",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -493,6 +502,7 @@ describe("services", function()
protocol = "tls",
host = "x.y",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -505,6 +515,7 @@ describe("services", function()
protocol = "grpc",
host = "x.y",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand All @@ -517,6 +528,7 @@ describe("services", function()
protocol = "grpcs",
host = "x.y",
port = 80,
request_buffering = true,
}

local ok, err = Services:validate(service)
Expand Down
Expand Up @@ -59,6 +59,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
_comment = "my comment",
_ignore = { { foo = "bar" } },
},
Expand All @@ -71,6 +72,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
_comment = "my comment",
_ignore = { { foo = "bar" } },
}
Expand Down Expand Up @@ -101,6 +103,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
}
}
}, config)
Expand Down Expand Up @@ -250,6 +253,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
plugins = {}
}
}
Expand Down Expand Up @@ -298,6 +302,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
_comment = "my comment",
_ignore = { { foo = "bar" } },
plugins = {
Expand Down Expand Up @@ -340,6 +345,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
plugins = {
{
name = "basic-auth",
Expand Down Expand Up @@ -391,6 +397,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {}
}
}
Expand Down Expand Up @@ -438,6 +445,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {
{
paths = { "/path" },
Expand Down Expand Up @@ -477,6 +485,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {
{
paths = { "/path" },
Expand Down Expand Up @@ -524,6 +533,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {
{
name = "foo",
Expand Down Expand Up @@ -587,6 +597,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {
{
name = "foo",
Expand Down Expand Up @@ -637,6 +648,7 @@ describe("declarative config: process_auto_fields", function()
read_timeout = 60000,
write_timeout = 60000,
retries = 5,
request_buffering = true,
routes = {
{
name = "bar",
Expand Down

0 comments on commit 03ad1c9

Please sign in to comment.