Skip to content

Commit

Permalink
Remove support to toggle response buffering
Browse files Browse the repository at this point in the history
Turning off proxy_buffering seems like a bad practice so this commit
is to remove response buffering support from the policy
  • Loading branch information
tkan145 committed Sep 28, 2023
1 parent 4375b15 commit cf4bcad
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 66 deletions.
33 changes: 0 additions & 33 deletions gateway/conf.d/apicast.conf
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,6 @@ location @upstream {
header_filter_by_lua_block { require('apicast.executor'):header_filter() }
}

location @upstream_unbuffered {
internal;

rewrite_by_lua_block {
require('resty.ctx').apply()
}

proxy_request_buffering off;
proxy_buffering off;
{% include "conf.d/upstream_shared.conf" %}

# these are duplicated so when request is redirected here those phases are executed
post_action @out_of_band_authrep_action;
body_filter_by_lua_block { require('apicast.executor'):body_filter() }
header_filter_by_lua_block { require('apicast.executor'):header_filter() }
}

location @upstream_request_unbuffered {
internal;

Expand All @@ -117,22 +100,6 @@ location @upstream_request_unbuffered {
header_filter_by_lua_block { require('apicast.executor'):header_filter() }
}

location @upstream_response_buffering {
internal;

rewrite_by_lua_block {
require('resty.ctx').apply()
}

proxy_buffering off;
{% include "conf.d/upstream_shared.conf" %}

# these are duplicated so when request is redirected here those phases are executed
post_action @out_of_band_authrep_action;
body_filter_by_lua_block { require('apicast.executor'):body_filter() }
header_filter_by_lua_block { require('apicast.executor'):header_filter() }
}

location / {
set $cached_key '';
set $credentials '';
Expand Down
11 changes: 3 additions & 8 deletions gateway/src/apicast/policy/buffering/apicast-policy.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"$schema": "http://apicast.io/policy-v1/schema#manifest#",
"name": "3scale Request/Response buffering",
"summary": "Controls how 3scale buffer request/response",
"name": "3scale Request buffering",
"summary": "Controls how 3scale buffer request",
"description": [
"Configures the buffering for request and response"
"Configures the buffering for request"
],
"version": "builtin",
"configuration": {
Expand All @@ -13,11 +13,6 @@
"description": "Request Buffering mode",
"type": "boolean",
"default": true
},
"response_buffering": {
"description": "Response Buffering mode",
"type": "boolean",
"default": true
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions gateway/src/apicast/policy/buffering/buffering.lua
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ local new = _M.new
function _M.new(config)
local self = new(config)
self.request_buffering = config.request_buffering
self.response_buffering = config.response_buffering
return self
end

function _M:export()
return {
request_buffering = self.request_buffering,
response_buffering = self.response_buffering
}
end

Expand Down
6 changes: 0 additions & 6 deletions gateway/src/apicast/upstream.lua
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,8 @@ local function get_upstream_location_name(context)
return context.upstream_location_name
end
if context.request_buffering == false then
if context.response_buffering == false then
return "@upstream_unbuffered"
end
return "@upstream_request_unbuffered"
end
if context.response_buffering == false then
return "@upstream_response_unbuffered"
end
end

--- Execute the upstream.
Expand Down
3 changes: 0 additions & 3 deletions spec/policy/buffering/buffering_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ describe('Buffering policy', function()
local exported = policy:export()

assert.same(true, exported.request_buffering)
assert.same(true, exported.response_buffering)
end)

it('accepts configuration', function()
local config_buffering = {
request_buffering = false,
response_buffering = false,
}
local policy = BufferingPolicy.new(config_buffering)
local exported = policy:export()

assert.same(false, exported.request_buffering)
assert.same(false, exported.response_buffering)
end)
end)
end)
14 changes: 0 additions & 14 deletions spec/upstream_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,9 @@ describe('Upstream', function()
it('executes the upstream location when request_buffering provided in the context', function()
local contexts = {
["buffered_request"] = {ctx={request_buffering=true}, upstream_location="@upstream"},
["buffered_response"] = {ctx={response_buffering=true}, upstream_location="@upstream"},
["unbuffered_request"] = {ctx={request_buffering=false}, upstream_location="@upstream_request_unbuffered"},
["unbuffered_response"] = {ctx={response_buffering=false}, upstream_location="@upstream_response_unbuffered"},
["unbuffered"] = {ctx={request_buffering=false, response_buffering=false}, upstream_location="@upstream"},
["upstream_location and buffered_request"] = {ctx={upstream_location_name="@grpc", request_buffering=true}, upstream_location="@grpc"},
["upstream_location and buffered_response"] = {ctx={upstream_location_name="@grpc", response_buffering=true}, upstream_location="@grpc"},
["upstream_location and unbuffered_request"] = {ctx={upstream_location_name="@grpc", request_buffering=false}, upstream_location="@grpc"},
["upstream_location and unbuffered_response"] = {ctx={upstream_location_name="@grpc", response_buffering=false}, upstream_location="@grpc"},
["upstream_location and unbuffered"] = {ctx={upstream_location_name="@grpc", request_buffering=false, response_buffering=false}, upstream_location="@grpc"}
}

for _, value in pairs(contexts) do
Expand All @@ -239,14 +233,6 @@ describe('Upstream', function()
end
end)

it('executes the upstream location when response_buffering provided in the context', function()
local upstream = Upstream.new('http://localhost')

upstream:call({response_buffering=true})

assert.spy(ngx.exec).was_called_with("@upstream")
end)

it('skips executing the upstream location when missing', function()
local upstream = Upstream.new('http://localhost')
upstream.location_name = nil
Expand Down

0 comments on commit cf4bcad

Please sign in to comment.