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

hotfix(cache): removes deadlocks when cache callbacks error #2197

Merged
merged 3 commits into from
Mar 21, 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
7 changes: 5 additions & 2 deletions kong/core/plugins_iterator.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ local function load_plugin_into_memory(api_id, consumer_id, plugin_name)
name = plugin_name
}
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end

if #rows > 0 then
Expand All @@ -37,8 +37,11 @@ end
-- @treturn table Plugin retrieved from the cache or database.
local function load_plugin_configuration(api_id, consumer_id, plugin_name)
local cache_key = cache.plugin_key(plugin_name, api_id, consumer_id)
local plugin = cache.get_or_set(cache_key, nil, load_plugin_into_memory,
local plugin, err = cache.get_or_set(cache_key, nil, load_plugin_into_memory,
api_id, consumer_id, plugin_name)
if err then
responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
if plugin ~= nil and plugin.enabled then
return plugin.config or {}
end
Expand Down
8 changes: 5 additions & 3 deletions kong/plugins/acl/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ end
local function load_acls_into_memory(consumer_id)
local results, err = singletons.dao.acls:find_all {consumer_id = consumer_id}
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return results
end
Expand All @@ -37,9 +37,11 @@ function ACLHandler:access(conf)
end

-- Retrieve ACL
local acls = cache.get_or_set(cache.acls_key(consumer_id), nil,
local acls, err = cache.get_or_set(cache.acls_key(consumer_id), nil,
load_acls_into_memory, consumer_id)

if err then
responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
if not acls then acls = {} end

local block
Expand Down
20 changes: 15 additions & 5 deletions kong/plugins/basic-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,20 @@ end
local function load_credential_into_memory(username)
local credentials, err = singletons.dao.basicauth_credentials:find_all {username = username}
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return credentials[1]
end

local function load_credential_from_db(username)
if not username then return end

return cache.get_or_set(cache.basicauth_credential_key(username),
local credential, err = cache.get_or_set(cache.basicauth_credential_key(username),
nil, load_credential_into_memory, username)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
return credential
end

local function load_consumer_into_memory(consumer_id, anonymous)
Expand All @@ -87,7 +91,7 @@ local function load_consumer_into_memory(consumer_id, anonymous)
if anonymous and not err then
err = 'anonymous consumer "'..consumer_id..'" not found'
end
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return result
end
Expand Down Expand Up @@ -132,8 +136,11 @@ local function do_authentication(conf)
end

-- Retrieve consumer
local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
nil, load_consumer_into_memory, credential.consumer_id, false)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

set_consumer(consumer, credential)

Expand All @@ -145,8 +152,11 @@ function _M.execute(conf)
if not ok then
if conf.anonymous ~= "" then
-- get anonymous user
local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
nil, load_consumer_into_memory, conf.anonymous, true)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
set_consumer(consumer, nil)
else
return responses.send(err.status, err.message)
Expand Down
23 changes: 17 additions & 6 deletions kong/plugins/hmac-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,22 @@ end
local function load_credential_into_memory(username)
local keys, err = singletons.dao.hmacauth_credentials:find_all { username = username }
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return keys[1]
end

local function load_credential(username)
local credential
local credential, err
if username then
credential = cache.get_or_set(cache.hmacauth_credential_key(username),
credential, err = cache.get_or_set(cache.hmacauth_credential_key(username),
nil, load_credential_into_memory, username)
end

if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

return credential
end

Expand Down Expand Up @@ -144,7 +149,7 @@ local function load_consumer_into_memory(consumer_id, anonymous)
if anonymous and not err then
err = 'anonymous consumer "'..consumer_id..'" not found'
end
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return result
end
Expand Down Expand Up @@ -197,8 +202,11 @@ local function do_authentication(conf)
end

-- Retrieve consumer
local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
nil, load_consumer_into_memory, credential.consumer_id)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

set_consumer(consumer, credential)

Expand All @@ -210,8 +218,11 @@ function _M.execute(conf)
if not ok then
if conf.anonymous ~= "" then
-- get anonymous user
local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
nil, load_consumer_into_memory, conf.anonymous, true)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
set_consumer(consumer, nil)
else
return responses.send(err.status, err.message)
Expand Down
19 changes: 14 additions & 5 deletions kong/plugins/jwt/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ end
local function load_credential(jwt_secret_key)
local rows, err = singletons.dao.jwt_secrets:find_all {key = jwt_secret_key}
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR()
return nil, err
end
return rows[1]
end
Expand All @@ -64,7 +64,7 @@ local function load_consumer(consumer_id, anonymous)
if anonymous and not err then
err = 'anonymous consumer "'..consumer_id..'" not found'
end
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return result
end
Expand Down Expand Up @@ -114,8 +114,11 @@ local function do_authentication(conf)
end

-- Retrieve the secret
local jwt_secret = cache.get_or_set(cache.jwtauth_credential_key(jwt_secret_key),
local jwt_secret, err = cache.get_or_set(cache.jwtauth_credential_key(jwt_secret_key),
nil, load_credential, jwt_secret_key)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

if not jwt_secret then
return false, {status = 403, message = "No credentials found for given '"..conf.key_claim_name.."'"}
Expand Down Expand Up @@ -149,8 +152,11 @@ local function do_authentication(conf)
end

-- Retrieve the consumer
local consumer = cache.get_or_set(cache.consumer_key(jwt_secret_key),
local consumer, err = cache.get_or_set(cache.consumer_key(jwt_secret_key),
nil, load_consumer, jwt_secret.consumer_id)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

-- However this should not happen
if not consumer then
Expand All @@ -169,8 +175,11 @@ function JwtHandler:access(conf)
if not ok then
if conf.anonymous ~= "" then
-- get anonymous user
local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
nil, load_consumer, conf.anonymous, true)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
set_consumer(consumer, nil)
else
return responses.send(err.status, err.message)
Expand Down
14 changes: 10 additions & 4 deletions kong/plugins/key-auth/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ local function load_credential(key)
key = key
}
if not creds then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return creds[1]
end
Expand All @@ -37,7 +37,7 @@ local function load_consumer(consumer_id, anonymous)
if anonymous and not err then
err = 'anonymous consumer "'..consumer_id..'" not found'
end
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return result
end
Expand Down Expand Up @@ -98,8 +98,11 @@ local function do_authentication(conf)
end

-- retrieve our consumer linked to this API key
local credential = cache.get_or_set(cache.keyauth_credential_key(key),
local credential, err = cache.get_or_set(cache.keyauth_credential_key(key),
nil, load_credential, key)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

-- no credential in DB, for this key, it is invalid, HTTP 403
if not credential then
Expand All @@ -111,8 +114,11 @@ local function do_authentication(conf)
-----------------------------------------

-- retrieve the consumer linked to this API key, to set appropriate headers
local consumer = cache.get_or_set(cache.consumer_key(credential.consumer_id),
local consumer, err = cache.get_or_set(cache.consumer_key(credential.consumer_id),
nil, load_consumer, credential.consumer_id)
if err then
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

set_consumer(consumer, credential)

Expand Down
15 changes: 10 additions & 5 deletions kong/plugins/ldap-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ local function ldap_authenticate(given_username, given_password, conf)
ok, err = sock:connect(conf.ldap_host, conf.ldap_port)
if not ok then
ngx_log(ngx_error, "[ldap-auth] failed to connect to "..conf.ldap_host..":"..tostring(conf.ldap_port)..": ", err)
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err, responses.status_codes.HTTP_INTERNAL_SERVER_ERROR
end

if conf.start_tls then
Expand All @@ -68,7 +68,8 @@ end
local function load_credential(given_username, given_password, conf)
ngx_log(ngx_debug, "[ldap-auth] authenticating user against LDAP server: "..conf.ldap_host..":"..conf.ldap_port)

local ok, err = ldap_authenticate(given_username, given_password, conf)
local ok, err, status = ldap_authenticate(given_username, given_password, conf)
if status ~= nil then return nil, err, status end
if err ~= nil then ngx_log(ngx_error, err) end
if not ok then
return nil
Expand All @@ -82,8 +83,9 @@ local function authenticate(conf, given_credentials)
return false
end

local credential = cache.get_or_set(cache.ldap_credential_key(ngx.ctx.api.id, given_username),
local credential, err, status = cache.get_or_set(cache.ldap_credential_key(ngx.ctx.api.id, given_username),
conf.cache_ttl, load_credential, given_username, given_password, conf)
if status then responses.send(status, err) end

return credential and credential.password == given_password, credential
end
Expand All @@ -94,7 +96,7 @@ local function load_consumer(consumer_id, anonymous)
if anonymous and not err then
err = 'anonymous consumer "'..consumer_id..'" not found'
end
return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
return nil, err
end
return result
end
Expand Down Expand Up @@ -151,8 +153,11 @@ function _M.execute(conf)
if not ok then
if conf.anonymous ~= "" then
-- get anonymous user
local consumer = cache.get_or_set(cache.consumer_key(conf.anonymous),
local consumer, err = cache.get_or_set(cache.consumer_key(conf.anonymous),
nil, load_consumer, conf.anonymous, true)
if err then
responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
set_consumer(consumer, nil)
else
return responses.send(err.status, err.message)
Expand Down
Loading