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

THREESCALE-10582 fix integration of upstream connection policy with camel policy #1443

Merged
merged 9 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Docker compose up instead of docker compose run [PR #1442](https://github.com/3scale/APIcast/pull/1442)

- Fix integration of upstream connection policy with camel policy [PR #1443](https://github.com/3scale/APIcast/pull/1443) [THREESCALE-10582](https://issues.redhat.com/browse/THREESCALE-10582)

### Added

- Detect number of CPU shares when running on Cgroups V2 [PR #1410](https://github.com/3scale/apicast/pull/1410) [THREESCALE-10167](https://issues.redhat.com/browse/THREESCALE-10167)
Expand Down
10 changes: 6 additions & 4 deletions gateway/src/apicast/http_proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ local DEFAULT_CHUNKSIZE = 32 * 1024

function _M.reset()
_M.resolver = resty_resolver
_M.http_backend = require('resty.http_ng.backend.resty')
_M.dns_resolution = 'apicast' -- can be set to 'proxy' to let proxy do the name resolution

return _M
Expand Down Expand Up @@ -146,10 +145,12 @@ local function forward_https_request(proxy_uri, uri, proxy_opts)
path = format('%s%s%s', ngx.var.uri, ngx.var.is_args, ngx.var.query_string or ''),
body = body,
proxy_uri = proxy_uri,
proxy_auth = opts.proxy_auth
proxy_auth = opts.proxy_auth,
upstream_connection_opts = opts.upstream_connection_opts,
skip_https_connect = opts.skip_https_connect
}

local httpc, err = http_proxy.new(request, opts.skip_https_connect)
local httpc, err = http_proxy.new(request)

if not httpc then
ngx.log(ngx.ERR, 'could not connect to proxy: ', proxy_uri, ' err: ', err)
Expand Down Expand Up @@ -226,7 +227,8 @@ function _M.request(upstream, proxy_uri)
local proxy_opts = {
proxy_auth = proxy_auth,
skip_https_connect = upstream.skip_https_connect,
request_unbuffered = upstream.request_unbuffered
request_unbuffered = upstream.request_unbuffered,
upstream_connection_opts = upstream.upstream_connection_opts
}

forward_https_request(proxy_uri, uri, proxy_opts)
Expand Down
1 change: 1 addition & 0 deletions gateway/src/apicast/upstream.lua
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ function _M:call(context)
end

self.request_unbuffered = context.request_unbuffered
self.upstream_connection_opts = context.upstream_connection_opts
http_proxy.request(self, proxy_uri)
else
local err = self:rewrite_request()
Expand Down
2 changes: 2 additions & 0 deletions gateway/src/resty/balancer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ function _M:set_timeouts(connect_timeout, send_timeout, read_timeout)

-- If one of the values is nil, the default applies:
-- https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/balancer.md#set_timeouts
ngx.log(ngx.DEBUG, 'setting timeouts (secs), connect_timeout: ', connect_timeout,
' send_timeout: ', send_timeout, ' read_timeout: ', read_timeout)
return ngx_balancer.set_timeouts(connect_timeout, send_timeout, read_timeout)
end

Expand Down
22 changes: 19 additions & 3 deletions gateway/src/resty/http/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ local function _connect_proxy_https(httpc, request, host, port)
return httpc
end

local function connect_proxy(httpc, request, skip_https_connect)
local function connect_proxy(httpc, request)
-- target server requires hostname not IP and DNS resolution is left to the proxy itself as specified in the RFC #7231
-- https://httpwg.org/specs/rfc7231.html#CONNECT
local uri = request.uri
local proxy_uri = request.proxy
local skip_https_connect = request.skip_https_connect

if proxy_uri.scheme ~= 'http' then
return nil, 'proxy connection supports only http'
Expand Down Expand Up @@ -131,15 +132,30 @@ local function find_proxy_url(request)
return request.proxy_uri or _M.find(uri)
end

local function connect(request, skip_https_connect)
local function connect(request)
request = request or { }
local httpc = http.new()

if request.upstream_connection_opts then
local con_opts = request.upstream_connection_opts
ngx.log(ngx.DEBUG, 'setting timeouts (secs), connect_timeout: ', con_opts.connect_timeout,
' send_timeout: ', con_opts.send_timeout, ' read_timeout: ', con_opts.read_timeout)
-- lua-resty-http uses nginx API for lua sockets
-- in milliseconds
-- https://github.com/openresty/lua-nginx-module?tab=readme-ov-file#tcpsocksettimeouts
local connect_timeout = con_opts.connect_timeout and con_opts.connect_timeout * 1000
local send_timeout = con_opts.send_timeout and con_opts.send_timeout * 1000
local read_timeout = con_opts.read_timeout and con_opts.read_timeout * 1000
httpc:set_timeouts(connect_timeout, send_timeout, read_timeout)
end

local proxy_uri = find_proxy_url(request)

request.ssl_verify = request.options and request.options.ssl and request.options.ssl.verify
request.proxy = proxy_uri

if proxy_uri then
return connect_proxy(httpc, request, skip_https_connect)
return connect_proxy(httpc, request)
else
return connect_direct(httpc, request)
end
Expand Down
52 changes: 52 additions & 0 deletions spec/http_proxy_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@

describe('http_proxy', function()
describe('.request', function()
local function stub_ngx_request()
ngx.var = { }

stub(ngx, 'exit')
stub(ngx.req, 'get_headers', function() return { } end)
stub(ngx.req, 'get_method', function() return 'GET' end)
end

local function stub_resty_http_proxy()
local httpc = {
}

local response = {}
stub(httpc, 'request', function() return response end)
stub(httpc, 'proxy_response')
stub(httpc, 'set_keepalive')

local resty_http_proxy = require 'resty.http.proxy'
stub(resty_http_proxy, 'new', function() return httpc end)
end

before_each(function()
stub_ngx_request()
stub_resty_http_proxy()
end)

describe('on https backend', function()
local upstream = {
uri = {
scheme = 'https'
},
request_unbuffered = false,
skip_https_connect = false
}
local proxy_uri = {
}

before_each(function()
stub(upstream, 'rewrite_request')
end)

it('terminates phase', function()
local http_proxy = require('apicast.http_proxy')
http_proxy.request(upstream, proxy_uri)
assert.spy(ngx.exit).was_called_with(ngx.OK)
end)
end)
end)
end)
37 changes: 29 additions & 8 deletions spec/resty/http/proxy_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,37 @@ describe('resty.http.proxy', function()
end)

context('.new', function()
it('connects to the #http_proxy', function()
_M:reset({ http_proxy = 'http://127.0.0.1:1984' })
before_each(function()
_M:reset({ http_proxy = 'http://127.0.0.1:1984' })
end)

local request = { url = 'http://upstream:8091/request', method = 'GET' }
local proxy = assert(_M.new(request))
it('connects to the #http_proxy', function()
local request = { url = 'http://upstream:8091/request', method = 'GET' }
local proxy = assert(_M.new(request))

local res = assert(proxy:request(request))
local res = assert(proxy:request(request))

assert.same(200, res.status)
assert.match('GET http://upstream:8091/request HTTP/1.1', res:read_body())
end)
assert.same(200, res.status)
assert.match('GET http://upstream:8091/request HTTP/1.1', res:read_body())
end)

it('connects to the #http_proxy with timeouts', function()
local request = {
url = 'http://upstream:8091/request',
method = 'GET',
upstream_connection_opts = {
connect_timeout = 1,
send_timeout = 1,
read_timeout = 1
}
}

local proxy = assert(_M.new(request))

local res = assert(proxy:request(request))

assert.same(200, res.status)
assert.match('GET http://upstream:8091/request HTTP/1.1', res:read_body())
end)
end)
end)
2 changes: 1 addition & 1 deletion t/apicast-policy-camel.t
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ run_tests();

__DATA__
=== TEST 1: API backend connection uses http proxy
=== TEST 1: API backend connection uses http proxy
--- configuration
{
"services": [
Expand Down
4 changes: 1 addition & 3 deletions t/apicast-policy-http-proxy.t
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,8 @@ ETag: foobar
--- error_code: 200
--- error_log env
proxy request: CONNECT test-upstream.lvh.me:$TEST_NGINX_RANDOM_PORT HTTP/1.1
--- user_files fixture=tls.pl eval
--- error_log env
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there cannot be two --- error_log directives. Only one of them is being applied.

using proxy: $TEST_NGINX_HTTPS_PROXY

--- user_files fixture=tls.pl eval

=== TEST 4: using HTTP proxy with Basic Auth
--- configuration
Expand Down
Loading
Loading