Skip to content

Commit

Permalink
feat(dns): function to force no sync toip() (#12739)
Browse files Browse the repository at this point in the history
  • Loading branch information
locao authored Mar 22, 2024
1 parent 5bba619 commit 48cd3a0
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 10 deletions.
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

1 comment on commit 48cd3a0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Bazel Build

Docker image available kong/kong:48cd3a0270adf738af192243861af02a46ca194a
Artifacts available https://github.com/Kong/kong/actions/runs/8391936983

Please sign in to comment.