Skip to content

Commit

Permalink
refactor(runloop) do not select Services/Routes with nulls
Browse files Browse the repository at this point in the history
When reading from the proxy, we do not wish to retrieve unset columns as
`ngx.null` for backwards-compatibility reasons. That is now the default
for all plugins since 735314c (parent commit), for plugins, and it is
now for the runloop as well.

Context: retrieving unset columns as `nil` instead of `ngx.null` allows
for backwards-compatibility with existing plugins. An example of this is
authentication plugins retrieving the authenticated Consumer, and
injecting its fields as headers into the upstream request (e.g.
`X-Consumer-Username`). When such fields are unset, the following
headers were sent upstream: `X-Consumer-Username: userdata: NULL`
instead of no header at all (or worse, producing HTTP 500 errors).
  • Loading branch information
thibaultcha authored and hishamhm committed Aug 20, 2018
1 parent edd1ad2 commit 6aa3a7e
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions kong/runloop/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ local fmt = string.format
local sort = table.sort
local ngx = ngx
local log = ngx.log
local null = ngx.null
local ngx_now = ngx.now
local update_time = ngx.update_time
local re_match = ngx.re.match
Expand Down Expand Up @@ -86,7 +85,7 @@ end

local function build_router(db, version)
local routes, i = {}, 0
local routes_iterator = db.routes:each(nil, { nulls = true })
local routes_iterator = db.routes:each()

local route, err = routes_iterator()
while route do
Expand All @@ -99,7 +98,7 @@ local function build_router(db, version)
local service

-- TODO: db requests in loop, problem or not
service, err = db.services:select(service_pk, { nulls = true })
service, err = db.services:select(service_pk)
if not service then
return nil, "could not find service for route (" .. route.id .. "): " .. err
end
Expand All @@ -109,7 +108,7 @@ local function build_router(db, version)
service = service,
}

if route.hosts ~= null then
if route.hosts then
-- TODO: headers should probably be moved to route
r.headers = {
host = route.hosts,
Expand Down Expand Up @@ -543,7 +542,7 @@ return {
-- TODO: this is probably not optimal
do
local retries = service.retries or api.retries
if retries ~= null then
if retries then
balancer_data.retries = retries

else
Expand All @@ -552,7 +551,7 @@ return {

local connect_timeout = service.connect_timeout or
api.upstream_connect_timeout
if connect_timeout ~= null then
if connect_timeout then
balancer_data.connect_timeout = connect_timeout

else
Expand All @@ -561,7 +560,7 @@ return {

local send_timeout = service.write_timeout or
api.upstream_send_timeout
if send_timeout ~= null then
if send_timeout then
balancer_data.send_timeout = send_timeout

else
Expand All @@ -570,7 +569,7 @@ return {

local read_timeout = service.read_timeout or
api.upstream_read_timeout
if read_timeout ~= null then
if read_timeout then
balancer_data.read_timeout = read_timeout

else
Expand Down

0 comments on commit 6aa3a7e

Please sign in to comment.