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

[t] use one base class #458

Merged
merged 4 commits into from
Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- URI params in POST requests are now taken into account when matching mapping rules [PR #437](https://github.com/3scale/apicast/pull/437)
- Increased number of background timers and connections in the cosocket pool [PR #290](https://github.com/3scale/apicast/pull/290)
- Make OAuth tokens TTL configurable [PR #448](https://github.com/3scale/apicast/pull/448)
- Detect when being executed in Test::Nginx and use default backend accordingly [PR #458](https://github.com/3scale/apicast/pull/458)

### Fixed

Expand Down
36 changes: 27 additions & 9 deletions apicast/src/configuration.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ local len = string.len
local format = string.format
local pairs = pairs
local type = type
local unpack = unpack
local error = error
local tostring = tostring
local next = next
Expand Down Expand Up @@ -129,12 +128,34 @@ end

local Service = require 'configuration.service'

local noop = function() end
local function readonly_table(table)
return setmetatable({}, { __newindex = noop, __index = table })
end

local empty_t = readonly_table()

local function backend_endpoint(proxy)
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation here is not consistent with the rest of the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Haven't configured my IDE and used the default 4 spaces indentation.
Must admit that I quite like it. Somehow it makes it more readable. Also the OpenResty code is indented with 4 spaces.
I'd consider switching to 4 spaces. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind as long as it's spaces :) and we are consistent across the codebase.
http://lua-users.org/wiki/LuaStyleGuide

local backend_endpoint_override = resty_url.parse(env.get("BACKEND_ENDPOINT_OVERRIDE"))
local backend = backend_endpoint_override or proxy.backend or empty_t

if backend == empty_t then
local test_nginx_server_port = env.get('TEST_NGINX_SERVER_PORT')
if test_nginx_server_port then
return { endpoint = 'http://127.0.0.1:' .. test_nginx_server_port }
else
return nil
end
else
return { endpoint = backend.endpoint or tostring(backend), host = backend.host }
end
end

function _M.parse_service(service)
local backend_version = tostring(service.backend_version)
local proxy = service.proxy or {}
local backend = proxy.backend or {}
local backend_endpoint_override = env.get("BACKEND_ENDPOINT_OVERRIDE")
local _, _, _, backend_host_override = unpack(resty_url.split(backend_endpoint_override) or {})
local proxy = service.proxy or empty_t
local backend = backend_endpoint(proxy)


return Service.new({
id = tostring(service.id or 'default'),
Expand All @@ -161,10 +182,7 @@ function _M.parse_service(service)
type = service.backend_authentication_type,
value = service.backend_authentication_value
},
backend = {
endpoint = backend_endpoint_override or backend.endpoint,
host = backend_host_override or backend.host
},
backend = backend,
oidc = {
issuer_endpoint = proxy.oidc_issuer_endpoint ~= ngx.null and proxy.oidc_issuer_endpoint
},
Expand Down
42 changes: 32 additions & 10 deletions spec/configuration_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ describe('Configuration object', function()
assert.same('example.com', config.hostname_rewrite)
end)

it('overrides backend endpoint from ENV', function()
env.set('BACKEND_ENDPOINT_OVERRIDE', 'https://backend.example.com')

local config = configuration.parse_service({ proxy = {
backend = { endpoint = 'http://example.com', host = 'foo.example.com' }
}})

assert.same('https://backend.example.com', config.backend.endpoint)
assert.same('backend.example.com', config.backend.host)
end)

it('has a default message, content-type, and status for the auth failed error', function()
local config = configuration.parse_service({})
Expand Down Expand Up @@ -67,6 +57,38 @@ describe('Configuration object', function()
assert.same('text/plain; charset=utf-8', config.limits_exceeded_headers)
assert.equals(429, config.limits_exceeded_status)
end)

describe('backend', function()
it('defaults to nothing', function()
local config = configuration.parse_service({ proxy = {
backend = nil
}})

assert.falsy(config.backend)
end)

it('is overriden from ENV', function()
env.set('BACKEND_ENDPOINT_OVERRIDE', 'https://backend.example.com')

local config = configuration.parse_service({ proxy = {
backend = { endpoint = 'http://example.com', host = 'foo.example.com' }
}})

assert.same('https://backend.example.com', config.backend.endpoint)
assert.same('backend.example.com', config.backend.host)
end)

it('detects TEST_NGINX_SERVER_PORT', function()
env.set('TEST_NGINX_SERVER_PORT', '1954')

local config = configuration.parse_service({ proxy = {
backend = nil
}})

assert.same('http://127.0.0.1:1954', config.backend.endpoint)
assert.falsy(config.backend.host)
end)
end)
end)

describe('.filter_services', function()
Expand Down