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

feat(dns): function to force no sync toip() #12739

Merged
merged 3 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions changelog/unreleased/kong/force-no_sync-noip.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
message: |
Added a new function to bypass the Kong's DNS client synchronization option
when resolving hostnames.
type: feature
scope: Core
38 changes: 28 additions & 10 deletions kong/resty/dns/client.lua
Original file line number Diff line number Diff line change
Expand Up @@ -914,8 +914,10 @@ end
-- @param dnsCacheOnly if true, no active lookup is done when there is no (stale)
-- data. In that case an error is returned (as a dns server failure table).
-- @param try_list the try_list object to add to
-- @param force_no_sync force noSyncronisation
-- @return `entry + nil + try_list`, or `nil + err + try_list`
local function lookup(qname, r_opts, dnsCacheOnly, try_list)
local function lookup(qname, r_opts, dnsCacheOnly, try_list, force_no_sync)
local no_sync = noSynchronisation or force_no_sync or false
local entry = cachelookup(qname, r_opts.qtype)
if not entry then
--not found in cache
Expand All @@ -933,7 +935,7 @@ local function lookup(qname, r_opts, dnsCacheOnly, try_list)
-- perform a sync lookup, as we have no stale data to fall back to
try_list = try_add(try_list, qname, r_opts.qtype, "cache-miss")
-- while kong is exiting, we cannot use timers and hence we run all our queries without synchronization
if noSynchronisation then
if no_sync then
return individualQuery(qname, r_opts, try_list)
elseif ngx.worker and ngx.worker.exiting() then
log(DEBUG, PREFIX, "DNS query not synchronized because the worker is shutting down")
Expand Down Expand Up @@ -1144,8 +1146,9 @@ end
-- The field `additional_section` will default to `true` instead of `false`.
-- @param dnsCacheOnly Only check the cache, won't do server lookups
-- @param try_list (optional) list of tries to add to
-- @param force_no_sync force noSynchronisation
-- @return `list of records + nil + try_list`, or `nil + err + try_list`.
local function resolve(qname, r_opts, dnsCacheOnly, try_list)
local function resolve(qname, r_opts, dnsCacheOnly, try_list, force_no_sync)
qname = string_lower(qname)
local qtype = (r_opts or EMPTY).qtype
local err, records
Expand Down Expand Up @@ -1187,7 +1190,7 @@ local function resolve(qname, r_opts, dnsCacheOnly, try_list)
if (records[1] or EMPTY).type == _M.TYPE_CNAME and qtype ~= _M.TYPE_CNAME then
opts.qtype = nil
add_status_to_try_list(try_list, "dereferencing CNAME")
return resolve(records[1].cname, opts, dnsCacheOnly, try_list)
return resolve(records[1].cname, opts, dnsCacheOnly, try_list, force_no_sync)
end

-- return the shortname cache hit
Expand Down Expand Up @@ -1230,7 +1233,7 @@ local function resolve(qname, r_opts, dnsCacheOnly, try_list)

-- go look it up
opts.qtype = try_type
records, err, try_list = lookup(try_name, opts, dnsCacheOnly, try_list)
records, err, try_list = lookup(try_name, opts, dnsCacheOnly, try_list, force_no_sync)
if not records then
-- An error has occurred, terminate the lookup process. We don't want to try other record types because
-- that would potentially cause us to respond with wrong answers (i.e. the contents of an A record if the
Expand Down Expand Up @@ -1287,7 +1290,7 @@ local function resolve(qname, r_opts, dnsCacheOnly, try_list)
if records[1].type == _M.TYPE_CNAME and qtype ~= _M.TYPE_CNAME then
opts.qtype = nil
add_status_to_try_list(try_list, "dereferencing CNAME")
return resolve(records[1].cname, opts, dnsCacheOnly, try_list)
return resolve(records[1].cname, opts, dnsCacheOnly, try_list, force_no_sync)
end

return records, nil, try_list
Expand Down Expand Up @@ -1487,17 +1490,18 @@ end
-- representing the entire resolution history for a call. To prevent unnecessary
-- string concatenations on a hot code path, it is not logged in this module.
-- If you need to log it, just log `tostring(try_list)` from the caller code.
-- @function toip
-- @function execute_toip
-- @param qname hostname to resolve
-- @param port (optional) default port number to return if none was found in
-- the lookup chain (only SRV records carry port information, SRV with `port=0` will be ignored)
-- @param dnsCacheOnly Only check the cache, won't do server lookups (will
-- not invalidate any ttl expired data and will hence possibly return expired data)
-- @param try_list (optional) list of tries to add to
-- @param force_no_sync force noSynchronisation = true for a single call
-- @return `ip address + port + try_list`, or in case of an error `nil + error + try_list`
local function toip(qname, port, dnsCacheOnly, try_list)
local function execute_toip(qname, port, dnsCacheOnly, try_list, force_no_sync)
local rec, err
rec, err, try_list = resolve(qname, nil, dnsCacheOnly, try_list)
rec, err, try_list = resolve(qname, nil, dnsCacheOnly, try_list, force_no_sync)
if err then
return nil, err, try_list
end
Expand All @@ -1507,13 +1511,26 @@ local function toip(qname, port, dnsCacheOnly, try_list)
-- our SRV entry might still contain a hostname, so recurse, with found port number
local srvport = (entry.port ~= 0 and entry.port) or port -- discard port if it is 0
add_status_to_try_list(try_list, "dereferencing SRV")
return toip(entry.target, srvport, dnsCacheOnly, try_list)
return execute_toip(entry.target, srvport, dnsCacheOnly, try_list)
end

-- must be A or AAAA
return rec[roundRobin(rec)].address, port, try_list
end

-- @see execute_toip
local function toip(...)
return execute_toip(...)
end


-- This function calls execute_toip() forcing it to always execute an individual
-- query for each resolve, ignoring the noSynchronisation option.
-- @see execute_toip
local function individual_toip(qname, port, dnsCacheOnly, try_list)
return execute_toip(qname, port, dnsCacheOnly, try_list, true)
end


--- Socket functions
-- @section sockets
Expand Down Expand Up @@ -1566,6 +1583,7 @@ end
-- export local functions
_M.resolve = resolve
_M.toip = toip
_M.individual_toip = individual_toip
_M.connect = connect
_M.setpeername = setpeername

Expand Down
50 changes: 50 additions & 0 deletions spec/01-unit/21-dns-client/02-client_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,56 @@ describe("[DNS client]", function()
assert.is_nil(ip)
assert.are.equal("recursion detected", port)
end)

it("individual_noip - force no sync", function()
local resolve_count = 10
assert(client.init({
noSynchronisation = false,
}))

local callcount = 0
query_func = function(self, original_query_func, name, options)
callcount = callcount + 1
return original_query_func(self, name, options)
end

-- assert synchronisation is working
local threads = {}
for i=1,resolve_count do
threads[i] = ngx.thread.spawn(function()
local ip = client.toip("smtp." .. TEST_DOMAIN)
assert.is_string(ip)
end)
end

-- only one thread must have called the query_func
assert.are.equal(1, callcount,
"synchronisation failed - out of " .. resolve_count .. " toip() calls " .. callcount ..
" queries were made")

for i=1,#threads do
ngx.thread.wait(threads[i])
end

callcount = 0
threads = {}
for i=1,resolve_count do
threads[i] = ngx.thread.spawn(function()
local ip = client.individual_toip("atest." .. TEST_DOMAIN)
assert.is_string(ip)
end)
end

-- all threads must have called the query_func
assert.are.equal(resolve_count, callcount,
"force no sync failed - out of " .. resolve_count .. " toip() calls" ..
callcount .. " queries were made")

for i=1,#threads do
ngx.thread.wait(threads[i])
end
end)

end)


Expand Down
Loading