diff --git a/CHANGELOG.md b/CHANGELOG.md index 32518d8bb..afcc29c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed - descriptions in `oneOf`s in policy manifests have been replaced with titles [PR #663](https://github.com/3scale/apicast/pull/663) +- `resty.balancer` doesn't fall back to the port `80` by default. If the port is missing, `apicast.balancer` sets the default port for the scheme of the `proxy_pass` URL [PR #662](https://github.com/3scale/apicast/pull/662) ## [3.2.0-beta3] - 2018-03-20 diff --git a/gateway/src/apicast/balancer.lua b/gateway/src/apicast/balancer.lua index c85559539..1b3a110a6 100644 --- a/gateway/src/apicast/balancer.lua +++ b/gateway/src/apicast/balancer.lua @@ -1,19 +1,52 @@ local round_robin = require 'resty.balancer.round_robin' +local resty_url = require 'resty.url' +local empty = {} local _M = { default_balancer = round_robin.new() } +local function get_default_port(upstream_url) + local url = resty_url.split(upstream_url) or empty + local scheme = url[1] or 'http' + return resty_url.default_port(scheme) +end + +local function exit_service_unavailable() + ngx.status = ngx.HTTP_SERVICE_UNAVAILABLE + ngx.exit(ngx.status) +end + function _M.call(_, _, balancer) balancer = balancer or _M.default_balancer local host = ngx.var.proxy_host -- NYI: return to lower frame local peers = balancer:peers(ngx.ctx[host]) - local peer, err = balancer:set_peer(peers) + local peer, err = balancer:select_peer(peers) if not peer then - ngx.status = ngx.HTTP_SERVICE_UNAVAILABLE - ngx.log(ngx.ERR, "failed to set current backend peer: ", err) - ngx.exit(ngx.status) + ngx.log(ngx.ERR, 'could not select peer: ', err) + return exit_service_unavailable() + end + + local address, port = peer[1], peer[2] + + if not address then + ngx.log(ngx.ERR, 'peer missing address') + return exit_service_unavailable() end + + if not port then + port = get_default_port(ngx.var.proxy_pass) + end + + local ok + ok, err = balancer.balancer.set_current_peer(address, port) + + if not ok then + ngx.log(ngx.ERR, 'failed to set current backend peer: ', err) + return exit_service_unavailable() + end + + ngx.log(ngx.INFO, 'balancer set peer ', address, ':', port) end return _M diff --git a/gateway/src/apicast/policy/token_introspection/token_introspection.lua b/gateway/src/apicast/policy/token_introspection/token_introspection.lua index 472413601..67df4b63d 100644 --- a/gateway/src/apicast/policy/token_introspection/token_introspection.lua +++ b/gateway/src/apicast/policy/token_introspection/token_introspection.lua @@ -44,7 +44,7 @@ local function introspect_token(self, token) local token_info, decode_err = cjson.decode(res.body) if type(token_info) == 'table' then return token_info - else + else ngx.log(ngx.ERR, 'failed to parse token introspection response:', decode_err) return { active = false } end diff --git a/gateway/src/resty/balancer.lua b/gateway/src/resty/balancer.lua index 96cd5a041..7625919f4 100644 --- a/gateway/src/resty/balancer.lua +++ b/gateway/src/resty/balancer.lua @@ -30,7 +30,7 @@ local function new_peer(server, port) return { address, - tonumber(server.port or port or 80, 10) + tonumber(server.port or port, 10) } end @@ -41,7 +41,7 @@ local function convert_servers(servers, port) for i =1, #servers do local peer = new_peer(servers[i], port) - if peer and #peer == 2 then + if peer then insert(peers, peer) else ngx.log(ngx.INFO, 'skipping peer because it misses address or port') diff --git a/spec/balancer_spec.lua b/spec/balancer_spec.lua new file mode 100644 index 000000000..e644b1541 --- /dev/null +++ b/spec/balancer_spec.lua @@ -0,0 +1,23 @@ +local apicast_balancer = require 'apicast.balancer' +local resty_balancer = require 'resty.balancer' + +describe('apicast.balancer', function() + + describe('.call', function() + local b = resty_balancer.new(function(peers) return peers[1] end) + b.balancer = { } + + ngx.var = { + proxy_host = 'upstream' + } + + it('sets default port from scheme if no port is specified for peers', function() + b.peers = function() return { { '127.0.0.2' } } end + ngx.var.proxy_pass = 'https://example.com' + + b.balancer.set_current_peer = spy.new(function() return true end) + apicast_balancer:call({},b) + assert.spy(b.balancer.set_current_peer).was.called_with('127.0.0.2', 443) + end) + end) +end) diff --git a/t/apicast-upstream-balancer.t b/t/apicast-upstream-balancer.t index a285e2ca6..2c869323c 100644 --- a/t/apicast-upstream-balancer.t +++ b/t/apicast-upstream-balancer.t @@ -51,21 +51,12 @@ GET /t server 0.0.0.1:1; balancer_by_lua_block { - local round_robin = require 'resty.balancer.round_robin' + local balancer = require 'apicast.balancer' - local balancer = round_robin.new() - local servers = { { address = '127.0.0.1', port = $TEST_NGINX_SERVER_PORT } } - local peers = balancer:peers(servers) + ngx.ctx.upstream = { { address = '127.0.0.1', port = $TEST_NGINX_SERVER_PORT } } - local ok, err = balancer:set_peer(peers) - - if not ok then - ngx.status = ngx.HTTP_SERVICE_UNAVAILABLE - ngx.log(ngx.ERR, "failed to set current peer: "..err) - ngx.exit(ngx.status) - end + local peers = balancer:call() } - keepalive 32; } --- config @@ -91,18 +82,8 @@ yay server 0.0.0.1:1; balancer_by_lua_block { - local round_robin = require 'resty.balancer.round_robin' - - local balancer = round_robin.new() - local peers = balancer:peers(ngx.ctx.upstream) - - local peer, err = balancer:set_peer(peers) - - if not peer then - ngx.status = ngx.HTTP_SERVICE_UNAVAILABLE - ngx.log(ngx.ERR, "failed to set current peer: "..err) - ngx.exit(ngx.status) - end + local balancer = require 'apicast.balancer' + local peers = balancer:call() } keepalive 32; @@ -134,3 +115,35 @@ GET /t --- response_body yay --- error_code: 200 + + + +=== TEST 3: unsupported scheme +Using an unsopported URI scheme causes an exception +--- http_config + lua_package_path "$TEST_NGINX_LUA_PATH"; + + upstream upstream { + server 0.0.0.1:1; + + balancer_by_lua_block { + local balancer = require 'apicast.balancer' + ngx.ctx.upstream = { { address = '127.0.0.1', port = $TEST_NGINX_SERVER_PORT } } + local peers = balancer:call() + } + + keepalive 32; + } +--- config +location /api { + echo 'yay'; +} + +location /t { + set $proxy_pass "unsupported://upstream/api"; + proxy_pass $proxy_pass; +} +--- request +GET /t +--- error_code: 500 +--- error_log: invalid URL prefix in