Skip to content

Commit

Permalink
[balancer] resty.balancer doesn't fall back to port 80, apicast.balan…
Browse files Browse the repository at this point in the history
…cer sets default port by scheme
  • Loading branch information
mayorova committed Apr 4, 2018
1 parent fbaeeb9 commit d1a0937
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
41 changes: 37 additions & 4 deletions gateway/src/apicast/balancer.lua
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions gateway/src/resty/balancer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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')
Expand Down
23 changes: 23 additions & 0 deletions spec/balancer_spec.lua
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit d1a0937

Please sign in to comment.