From c1f68a7d4568bf990d1a8478a480c44587884918 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Tue, 4 Jun 2024 16:57:47 +0800 Subject: [PATCH] remove LAST type logic --- kong/dns/README.md | 22 +- kong/dns/client.lua | 470 ++++++++---------- kong/dns/utils.lua | 6 + .../09-balancer/04-round_robin_spec.lua | 2 + .../30-new-dns-client/01-utils_spec.lua | 94 ++-- .../30-new-dns-client/02-old_client_spec.lua | 263 ++++------ .../03-old_client_cache_spec.lua | 226 ++------- .../30-new-dns-client/04-client_ipc_spec.lua | 14 +- .../30-new-dns-client/05-client_stat_spec.lua | 121 +++-- .../04-admin_api/26-dns_client_spec.lua | 2 +- .../kong/plugins/dns-client-test/handler.lua | 2 +- spec/helpers/dns.lua | 13 +- 12 files changed, 497 insertions(+), 738 deletions(-) diff --git a/kong/dns/README.md b/kong/dns/README.md index f41335eac87e..a838f6518aaa 100644 --- a/kong/dns/README.md +++ b/kong/dns/README.md @@ -45,9 +45,8 @@ Performs a series of initialization operations: * the path of `hosts` file. * `resolv_conf`: (default: `/etc/resolv.conf`) * the path of `resolv.conf` file, it will be parsed and passed into the underlying `lua-resty-dns` library. -* `order`: (default: `{ "LAST", "SRV", "A", "AAAA" }`) +* `order`: (default: `{ "SRV", "A", "AAAA" }`) * the order in which to resolve different record types, it's similar to the option `dns_order` in `kong.conf`. - * The `LAST` type means the type of the last successful lookup (for the specified name). * `enable_ipv6`: (default: `true`) * whether to support IPv6 servers when getting nameservers from `resolv.conf`. * options for the underlying `lua-resty-dns` library: @@ -75,16 +74,15 @@ Performs a series of initialization operations: Performs a DNS resolution. -1. First, use the key `short::all` (or `short::` if `@qtype` is not `nil`) to query mlcache to see if there are any results available for quick use. If results are found, return them directly. -2. If there are no results available for quick use in the cache, then query all keys (`:`) extended from this domain name. - 1. The method for calculating extended keys is as follows: - 1. The domain `` is extended based on the `ndots`, `search`, and `domain` settings in `resolv.conf`. - 2. The `` is extended based on the `dns_order` parameter. - 2. Loop through all keys to query them. Once a usable result is found, return it. Also, store the DNS record result in mlcache with the key `short::all`. - 1. Use this key (`:`) to query mlcache. If it is not found, it triggers the L3 callback of `mlcache:get` to query the DNS server and process data that has expired but is still usable (`resolve_name_type_callback`). - 2. Use `mlcache:peek` to check if the missed and expired key still exists in the shared dictionary. If it does, return it directly to mlcache and trigger an asynchronous background task to update the expired data (`start_stale_update_task`). The maximum time that expired data can be reused is `stale_ttl`, but the maximum TTL returned to mlcache cannot exceed 60s. This way, if the expired key is not successfully updated by the background task after 60s, it can still be reused by calling the `resolve` function from the upper layer to trigger the L3 callback to continue executing this logic and initiate another background task for updating. - 1. For example, with a `stale_ttl` of 3600s, if the background task fails to update the record due to network issues during this time, and the upper-level application continues to call resolve to get the domain name result, it will trigger a background task to query the DNS result for that domain name every 60s, resulting in approximately 60 background tasks being triggered (3600s/60s). - +1. First, use the key `:all` (or `:` if `@qtype` is not `nil`) to query mlcache to see if there are any results available. If results are found, return them directly. +2. If there are no results available in the cache, it triggers the L3 callback of `mlcache:get` to query records from the DNS servers, details are as follows: + 1. Check if `` has an IP address in the `hosts` file, return if found. + 2. Check if `` is an IP address itself, return if true. + 3. Use `mlcache:peek` to check if the expired key still exists in the shared dictionary. If it does, return it directly to mlcache and trigger an asynchronous background task to update the expired data (`start_stale_update_task`). The maximum time that expired data can be reused is `stale_ttl`, but the maximum TTL returned to mlcache cannot exceed 60s. This way, if the expired key is not successfully updated by the background task after 60s, it can still be reused by calling the `resolve` function from the upper layer to trigger the L3 callback to continue executing this logic and initiate another background task for updating. + 1. For example, with a `stale_ttl` of 3600s, if the background task fails to update the record due to network issues during this time, and the upper-level application continues to call resolve to get the domain name result, it will trigger a background task to query the DNS result for that domain name every 60s, resulting in approximately 60 background tasks being triggered (3600s/60s). + 4. Query the DNS server, with `:` combinations: + 1. The `` is extended according to settings in `resolv.conf`, such as `ndots`, `search`, and `domain`. + 2. The `` is extended based on the `dns_order` parameter. **Return value:** diff --git a/kong/dns/client.lua b/kong/dns/client.lua index 64f6c08ff70e..3fba4c503836 100644 --- a/kong/dns/client.lua +++ b/kong/dns/client.lua @@ -7,6 +7,7 @@ local now = ngx.now local log = ngx.log local ERR = ngx.ERR local WARN = ngx.WARN +local NOTICE = ngx.NOTICE local DEBUG = ngx.DEBUG local ALERT = ngx.ALERT local timer_at = ngx.timer.at @@ -18,6 +19,7 @@ local tonumber = tonumber local setmetatable = setmetatable local math_min = math.min +local math_floor = math.floor local string_lower = string.lower local table_insert = table.insert local table_isempty = require("table.isempty") @@ -33,33 +35,26 @@ local req_dyn_hook_run_hook = require("kong.dynamic_hook").run_hook -- Constants and default values +local PREFIX = "[dns_client] " + local DEFAULT_ERROR_TTL = 1 -- unit: second local DEFAULT_STALE_TTL = 4 local DEFAULT_EMPTY_TTL = 30 -- long-lasting TTL of 10 years for hosts or static IP addresses in cache settings local LONG_LASTING_TTL = 10 * 365 * 24 * 60 * 60 -local PERSISTENT_CACHE_TTL = { ttl = 0 } -- used for mlcache:set - -local DEFAULT_ORDER = { "LAST", "SRV", "A", "AAAA" } +local DEFAULT_ORDER = { "SRV", "A", "AAAA" } local TYPE_SRV = resolver.TYPE_SRV local TYPE_A = resolver.TYPE_A local TYPE_AAAA = resolver.TYPE_AAAA -local TYPE_LAST = -1 - -local NAME_TO_TYPE = { - SRV = TYPE_SRV, - A = TYPE_A, - AAAA = TYPE_AAAA, - LAST = TYPE_LAST, -} +local TYPE_A_AAAA = -1 -- used to resolve IP addresses for SRV targets local TYPE_TO_NAME = { [TYPE_SRV] = "SRV", [TYPE_A] = "A", [TYPE_AAAA] = "AAAA", - [TYPE_LAST] = "LAST", + [TYPE_A_AAAA] = "A/AAAA", } local HIT_L3 = 3 -- L1 lru, L2 shm, L3 callback, L4 stale @@ -67,7 +62,7 @@ local HIT_L3 = 3 -- L1 lru, L2 shm, L3 callback, L4 stale local HIT_LEVEL_TO_NAME = { [1] = "hit_lru", [2] = "hit_shm", - [3] = "hit_cb", + [3] = "miss", [4] = "hit_stale", } @@ -92,7 +87,6 @@ local _M = { TYPE_SRV = TYPE_SRV, TYPE_A = TYPE_A, TYPE_AAAA = TYPE_AAAA, - TYPE_LAST = TYPE_LAST, } local MT = { __index = _M } @@ -117,23 +111,8 @@ local function stats_set_count(stats, name, key, value) end --- lookup or set TYPE_LAST (the DNS record type from the last successful query) -local function insert_last_type(cache, name, qtype) - local key = "last:" .. name - if TYPE_TO_NAME[qtype] and cache:get(key) ~= qtype then - cache:set(key, PERSISTENT_CACHE_TTL, qtype) - end -end - - -local function get_last_type(cache, name) - return cache:get("last:" .. name) -end - - local init_hosts do local function insert_answer_into_cache(cache, hosts_cache, address, name, qtype) - local key = name .. ":" .. qtype local answers = { ttl = LONG_LASTING_TTL, expire = now() + LONG_LASTING_TTL, @@ -146,33 +125,25 @@ local init_hosts do }, } - -- insert via the `:get` callback to prevent inter-process communication - cache:get(key, nil, function() - return answers, nil, LONG_LASTING_TTL - end) - - -- used for the host entry eviction - hosts_cache[key] = answers + hosts_cache[name .. ":" .. qtype] = answers + hosts_cache[name .. ":" .. TYPE_A_AAAA] = answers + hosts_cache[name .. ":all"] = answers end -- insert hosts into cache - function init_hosts(cache, path, preferred_ip_type) + function init_hosts(cache, path) local hosts = parse_hosts(path) local hosts_cache = {} for name, address in pairs(hosts) do name = string_lower(name) - if address.ipv4 then - insert_answer_into_cache(cache, hosts_cache, address.ipv4, name, TYPE_A) - insert_last_type(cache, name, TYPE_A) - end - if address.ipv6 then insert_answer_into_cache(cache, hosts_cache, address.ipv6, name, TYPE_AAAA) - if not address.ipv4 or preferred_ip_type == TYPE_AAAA then - insert_last_type(cache, name, TYPE_AAAA) - end + end + + if address.ipv4 then + insert_answer_into_cache(cache, hosts_cache, address.ipv4, name, TYPE_A) end end @@ -187,10 +158,34 @@ local ipc_counter = 0 function _M.new(opts) opts = opts or {} + local enable_ipv4, enable_ipv6, enable_srv + + opts.order = opts.order or DEFAULT_ORDER + + for i, typstr in ipairs(opts.order) do + typstr = typstr:upper() + + if typstr == "A" then + enable_ipv4 = true + + elseif typstr == "AAAA" then + enable_ipv6 = true + + elseif typstr == "SRV" then + enable_srv = true + + elseif typstr ~= "LAST" and typstr ~= "CNAME" then + return nil, "Invalid dns record type in order array: " .. typstr + end + end + + log(NOTICE, PREFIX, PREFIX, "supported types: ", enable_srv and "srv " or "", + enable_ipv4 and "ipv4 " or "", enable_ipv6 and "ipv6 " or "") + -- parse resolv.conf local resolv, err = utils.parse_resolv_conf(opts.resolv_conf, opts.enable_ipv6) if not resolv then - log(WARN, "Invalid resolv.conf: ", err) + log(WARN, PREFIX, "Invalid resolv.conf: ", err) resolv = { options = {} } end @@ -200,7 +195,7 @@ function _M.new(opts) or resolv.nameservers if not nameservers or table_isempty(nameservers) then - log(WARN, "Invalid configuration, no nameservers specified") + log(WARN, PREFIX, "Invalid configuration, no nameservers specified") end local r_opts = { @@ -254,7 +249,7 @@ function _M.new(opts) local ok, err = kong.worker_events.post(ipc_source, channel, data) if not ok then - log(ERR, "failed to post event '", ipc_source, "', '", channel, "': ", err) + log(ERR, PREFIX, "failed to post event '", ipc_source, "', '", channel, "': ", err) end end, } @@ -263,6 +258,7 @@ function _M.new(opts) ipc = ipc, neg_ttl = opts.empty_ttl or DEFAULT_EMPTY_TTL, lru_size = opts.cache_size or 10000, + shm_locks = ngx.shared.kong_locks and "kong_locks", resty_lock_opts = resty_lock_opts, }) @@ -274,41 +270,8 @@ function _M.new(opts) cache:purge(true) end - -- parse order - if opts.order and table_isempty(opts.order) then - return nil, "Invalid order array: empty record types" - end - - local order = opts.order or DEFAULT_ORDER - - local search_types = {} - local preferred_ip_type - - for i, typstr in ipairs(order) do - - -- TODO: delete this compatibility code in subsequent commits - if typstr:upper() == "CNAME" then - goto continue - end - - local qtype = NAME_TO_TYPE[typstr:upper()] - if not qtype then - return nil, "Invalid dns record type in order array: " .. typstr - end - - search_types[i] = qtype - - if (qtype == TYPE_A or qtype == TYPE_AAAA) and not preferred_ip_type then - preferred_ip_type = qtype - end - - ::continue:: - end - - preferred_ip_type = preferred_ip_type or TYPE_A - -- parse hosts - local hosts, hosts_cache = init_hosts(cache, opts.hosts, preferred_ip_type) + local hosts, hosts_cache = init_hosts(cache, opts.hosts) return setmetatable({ cache = cache, @@ -320,8 +283,10 @@ function _M.new(opts) error_ttl = opts.error_ttl or DEFAULT_ERROR_TTL, stale_ttl = opts.stale_ttl or DEFAULT_STALE_TTL, empty_ttl = opts.empty_ttl or DEFAULT_EMPTY_TTL, + enable_srv = enable_srv, + enable_ipv4 = enable_ipv4, + enable_ipv6 = enable_ipv6, hosts_cache = hosts_cache, - search_types = search_types, -- TODO: Make the table readonly. But if `string.buffer.encode/decode` and -- `pl.tablex.readonly` are called on it, it will become empty table. @@ -368,20 +333,16 @@ local function process_answers(self, qname, qtype, answers) answer.target = ipv6_bracket(answer.target) end - -- skip the SRV record pointing to itself, - -- see https://github.com/Kong/lua-resty-dns-client/pull/3 - if not (answer_type == TYPE_SRV and answer.target == qname) then - table_insert(processed_answers, answer) - end + table_insert(processed_answers, answer) end end if table_isempty(processed_answers) then - log(DEBUG, "processed ans:empty") + log(DEBUG, PREFIX, "processed ans:empty") return self.EMPTY_ANSWERS end - log(DEBUG, "processed ans:", #processed_answers) + log(DEBUG, PREFIX, "processed ans:", #processed_answers) processed_answers.expire = now() + ttl processed_answers.ttl = ttl @@ -390,8 +351,10 @@ local function process_answers(self, qname, qtype, answers) end -local function resolve_query(self, name, qtype) +local function resolve_query(self, name, qtype, tries) local key = name .. ":" .. qtype + + stats_init_name(self.stats, key) stats_increment(self.stats, key, "query") local r, err = resolver:new(self.r_opts) @@ -399,23 +362,24 @@ local function resolve_query(self, name, qtype) return nil, "failed to instantiate the resolver: " .. err end - local start_time = now() + local start = now() local answers, err = r:query(name, { additional_section = true, qtype = qtype }) r:destroy() - local query_time = now() - start_time -- the time taken for the DNS query - local time_str = ("%.3f %.3f"):format(start_time, query_time) + local duration = math_floor((now() - start) * 1000) - stats_set_count(self.stats, key, "query_last_time", time_str) + stats_set_count(self.stats, key, "query_last_time", duration) - log(DEBUG, "r:query(", key, ") ans:", answers and #answers or "-", - " t:", time_str) + log(DEBUG, PREFIX, "r:query(", key, ") ans:", answers and #answers or "-", + " t:", duration, " ms") + -- network error or malformed DNS response if not answers then stats_increment(self.stats, key, "query_fail_nameserver") - err = err or "unknown" - return nil, "DNS server error: " .. err .. ", Query Time: " .. time_str + err = "DNS server error: " .. tostring(err) .. ", took " .. duration .. " ms" + table_insert(tries, { name .. ":" .. TYPE_TO_NAME[qtype], err }) + return nil, err end answers = process_answers(self, name, qtype, answers) @@ -424,139 +388,100 @@ local function resolve_query(self, name, qtype) "query_fail:" .. answers.errstr or "query_succ") - return answers, nil, answers.ttl -end - - -local function stale_update_task(premature, self, key, name, qtype, short_key) - if premature then - return + -- DNS response error + if answers.errcode then + err = ("dns %s error: %s %s"):format( + answers.errcode < CACHE_ONLY_ERROR_CODE and "server" or "client", + answers.errcode, answers.errstr) + table_insert(tries, { name .. ":" .. TYPE_TO_NAME[qtype], err }) end - local answers = resolve_query(self, name, qtype) - if answers and (not answers.errcode or answers.errcode == NAME_ERROR_CODE) then - self.cache:set(key, { ttl = answers.ttl }, answers) - insert_last_type(self.cache, name, qtype) - - -- simply invalidate it and let the search iteration choose the correct one - self.cache:delete(short_key) - end + return answers end -local function start_stale_update_task(self, key, name, qtype, short_key) - stats_increment(self.stats, key, "stale") +-- resolve all `name`s and return first usable answers +local function resolve_query_names(self, names, qtype, tries) + local answers, err - local ok, err = timer_at(0, stale_update_task, self, key, name, qtype, short_key) - if not ok then - log(ALERT, "failed to start a timer to update stale DNS records: ", err) + for _, qname in ipairs(names) do + answers, err = resolve_query(self, qname, qtype, tries) + + -- severe error occurred + if not answers then + return nil, err + end + + if not answers.errcode then + return answers, nil, answers.ttl + end end + + -- not found in the search iteration + return answers, nil, answers.ttl end -local function resolve_name_type_callback(self, name, qtype, cache_only, - short_key, tries) - local key = name .. ":" .. qtype +local function resolve_query_types(self, name, qtype, tries) + local names = search_names(name, self.resolv, self.hosts) + local answers, err, ttl - -- check if this key exists in the hosts file (it maybe evicted from cache) - local answers = self.hosts_cache[key] - if answers then - return answers, nil, answers.ttl + -- the specific type + if qtype and qtype ~= TYPE_A_AAAA then + return resolve_query_names(self, names, qtype, tries) end - -- `:peek(stale=true)` verifies if the expired key remains in L2 shm, then - -- initiates an asynchronous background updating task to refresh it. - local ttl, _, answers = self.cache:peek(key, true) - if answers and ttl then - if not answers.expired then - answers.expire = now() + ttl - answers.expired = true - ttl = ttl + self.stale_ttl - - else - ttl = ttl + (answers.expire - now()) + -- query SRV for nil type + if self.enable_srv and qtype == nil then + answers, err, ttl = resolve_query_names(self, names, TYPE_SRV, tries) + if not answers or not answers.errcode then + return answers, err, ttl end + end - -- trigger the update task by the upper caller every 60 seconds - ttl = math_min(ttl, 60) - - if ttl > 0 then - log(DEBUG, "start stale update task ", key, " ttl:", ttl) - - -- mlcache's internal lock mechanism ensures concurrent control - start_stale_update_task(self, key, name, qtype, short_key) - answers.ttl = ttl - return answers, nil, ttl + -- query A/AAAA for nil or TYPE_A_AAAA type + if self.enable_ipv4 then + answers, err, ttl = resolve_query_names(self, names, TYPE_A, tries) + if not answers or not answers.errcode then + return answers, err, ttl end end - if cache_only then - return CACHE_ONLY_ANSWERS, nil, -1 + if self.enable_ipv6 then + answers, err, ttl = resolve_query_names(self, names, TYPE_AAAA, tries) + if not answers or not answers.errcode then + return answers, err, ttl + end end - local answers, err, ttl = resolve_query(self, name, qtype) return answers, err, ttl end -local function resolve_name_type(self, name, qtype, cache_only, short_key, - tries, has_timing) - local key = name .. ":" .. qtype - - stats_init_name(self.stats, key) - - local answers, err, hit_level = self.cache:get(key, nil, - resolve_name_type_callback, - self, name, qtype, cache_only, - short_key, tries) - -- check for runtime errors in the callback - if err and err:sub(1, 8) == "callback" then - log(ALERT, err) - end - - log(DEBUG, "cache lookup ", key, " ans:", answers and #answers or "-", - " hlv:", hit_level or "-") - - if has_timing then - req_dyn_hook_run_hook("timing", "dns:cache_lookup", - (hit_level and hit_level < HIT_L3)) +local function stale_update_task(premature, self, key, name, qtype) + if premature then + return end - -- hit L1 lru or L2 shm - if hit_level and hit_level < HIT_L3 then - stats_increment(self.stats, key, HIT_LEVEL_TO_NAME[hit_level]) + local tries = setmetatable({}, TRIES_MT) + local answers = resolve_query_types(self, name, qtype, tries) + if answers and (not answers.errcode or answers.errcode == NAME_ERROR_CODE) then + self.cache:set(key, { ttl = answers.ttl }, answers) end - if err or answers.errcode then - if not err then - local src = answers.errcode < CACHE_ONLY_ERROR_CODE and "server" or "client" - err = ("dns %s error: %s %s"):format(src, answers.errcode, answers.errstr) - end - - table_insert(tries, { name .. ":" .. TYPE_TO_NAME[qtype], err }) + if not answers or answers.errcode then + log(WARN, PREFIX, "Updating stale DNS records failed. Tried: ", tostring(tries)) end - - return answers, err end -local function get_search_types(self, name, qtype) - local input_types = qtype and { qtype } or self.search_types - local checked_types = {} - local types = {} - - for _, qtype in ipairs(input_types) do - if qtype == TYPE_LAST then - qtype = get_last_type(self.cache, name) - end +local function start_stale_update_task(self, key, name, qtype) + stats_increment(self.stats, key, "stale") - if qtype and not checked_types[qtype] then - table_insert(types, qtype) - checked_types[qtype] = true - end + local ok, err = timer_at(0, stale_update_task, self, key, name, qtype) + if not ok then + log(ALERT, PREFIX, "failed to start a timer to update stale DNS records: ", err) end - - return types end @@ -577,10 +502,8 @@ local function check_and_get_ip_answers(name) end --- resolve all `name`s and `type`s combinations and return first usable answers -local function resolve_names_and_types(self, name, typ, cache_only, short_key, - tries, has_timing) - +local function resolve_callback(self, name, qtype, cache_only, tries) + -- check if name is ip address local answers = check_and_get_ip_answers(name) if answers then -- domain name is IP literal answers.ttl = LONG_LASTING_TTL @@ -588,73 +511,83 @@ local function resolve_names_and_types(self, name, typ, cache_only, short_key, return answers, nil, tries end - -- TODO: For better performance, it may be necessary to rewrite it as an - -- iterative function. - local types = get_search_types(self, name, typ) - local names = search_names(name, self.resolv, self.hosts) + -- check if this key exists in the hosts file (it maybe evicted from cache) + local key = name .. ":" .. (qtype or "all") + local answers = self.hosts_cache[key] + if answers then + return answers, nil, answers.ttl + end - local err - for _, qtype in ipairs(types) do - for _, qname in ipairs(names) do - answers, err = resolve_name_type(self, qname, qtype, cache_only, - short_key, tries, has_timing) - -- severe error occurred - if not answers then - return nil, err, tries - end + -- `:peek(stale=true)` verifies if the expired key remains in L2 shm, then + -- initiates an asynchronous background updating task to refresh it. + local ttl, _, answers = self.cache:peek(key, true) + if answers and ttl then + if not answers.expired then + answers.expire = now() + ttl + answers.expired = true + ttl = ttl + self.stale_ttl - if not answers.errcode then - insert_last_type(self.cache, qname, qtype) -- cache TYPE_LAST - return answers, nil, tries - end + else + ttl = ttl + (answers.expire - now()) + end + + -- trigger the update task by the upper caller every 60 seconds + ttl = math_min(ttl, 60) + + if ttl > 0 then + log(DEBUG, PREFIX, "start stale update task ", key, " ttl:", ttl) + + -- mlcache's internal lock mechanism ensures concurrent control + start_stale_update_task(self, key, name, qtype) + answers.ttl = ttl + return answers, nil, ttl end end - -- not found in the search iteration - return nil, err, tries + if cache_only then + return CACHE_ONLY_ANSWERS, nil, -1 + end + + return resolve_query_types(self, name, qtype, tries) end local function resolve_all(self, name, qtype, cache_only, tries, has_timing) name = string_lower(name) - tries = setmetatable(tries or {}, TRIES_MT) - - -- key like "short:example.com:all" or "short:example.com:5" - local key = "short:" .. name .. ":" .. (qtype or "all") - - stats_init_name(self.stats, name) - stats_increment(self.stats, name, "runs") - -- quickly lookup with the key "short::all" or "short::" - local answers, err, hit_level = self.cache:get(key) - if not answers then - log(DEBUG, "quickly cache lookup ", key, " ans:- hlvl:", hit_level or "-") + tries = setmetatable(tries or {}, TRIES_MT) - answers, err, tries = resolve_names_and_types(self, name, qtype, cache_only, - key, tries, has_timing) + -- key like "example.com:" + local key = name .. ":" .. (qtype or "all") + log(DEBUG, PREFIX, "resolve_all ", key) - if not cache_only and answers then - -- If another worker resolved the name between these two `:get`, it can - -- work as expected and will not introduce a race condition. + stats_init_name(self.stats, key) + stats_increment(self.stats, key, "runs") - -- insert via the `:get` callback to prevent inter-process communication - self.cache:get(key, nil, function() - return answers, nil, answers.ttl - end) - end + local answers, err, hit_level = self.cache:get(key, nil, resolve_callback, + self, name, qtype, cache_only, + tries) + -- check for runtime errors in the callback + if err and err:sub(1, 8) == "callback" then + log(ALERT, PREFIX, err) + end - stats_increment(self.stats, name, answers and "miss" or "fail") + local hit_str = hit_level and HIT_LEVEL_TO_NAME[hit_level] or "fail" + stats_increment(self.stats, key, hit_str) - else - log(DEBUG, "quickly cache lookup ", key, " ans:", #answers, - " hlv:", hit_level or "-") + log(DEBUG, PREFIX, "cache lookup ", key, " ans:", answers and #answers or "-", + " hlv:", hit_str) - if has_timing then - req_dyn_hook_run_hook("timing", "dns:cache_lookup", - (hit_level and hit_level < HIT_L3)) - end + if has_timing then + req_dyn_hook_run_hook("timing", "dns:cache_lookup", + (hit_level and hit_level < HIT_L3)) + end - stats_increment(self.stats, name, HIT_LEVEL_TO_NAME[hit_level]) + if answers and answers.errcode then + err = ("dns %s error: %s %s"):format( + answers.errcode < CACHE_ONLY_ERROR_CODE and "server" or "client", + answers.errcode, answers.errstr) + return nil, err, tries end return answers, err, tries @@ -667,30 +600,27 @@ function _M:resolve(name, qtype, cache_only, tries) end --- Implement `resolve_address` separately as `_resolve_address` with the --- `has_timing` parameter so that it avoids checking for `ngx.ctx.has_timing` --- in recursion. -local function _resolve_address(self, name, port, cache_only, tries, has_timing) +function _M:resolve_address(name, port, cache_only, tries) + local has_timing = ngx.ctx and ngx.ctx.has_timing + local answers, err, tries = resolve_all(self, name, nil, cache_only, tries, has_timing) if not answers then return nil, err, tries end - if answers[1].type == TYPE_SRV then + if answers and answers[1].type == TYPE_SRV then local answer = get_next_weighted_round_robin_answer(answers) port = (answer.port ~= 0 and answer.port) or port - return _resolve_address(self, answer.target, port, cache_only, tries, - has_timing) + answers, err, tries = resolve_all(self, answer.target, TYPE_A_AAAA, + cache_only, tries, has_timing) end - return get_next_round_robin_answer(answers).address, port, tries -end - + if not answers then + return nil, err, tries + end -function _M:resolve_address(name, port, cache_only, tries) - return _resolve_address(self, name, port, cache_only, tries, - ngx.ctx and ngx.ctx.has_timing) + return get_next_round_robin_answer(answers).address, port, tries end @@ -699,7 +629,7 @@ end local dns_client function _M.init(opts) - log(DEBUG, "(re)configuring dns client") + log(DEBUG, PREFIX, "(re)configuring dns client") if opts then opts.valid_ttl = opts.valid_ttl or opts.validTtl @@ -731,7 +661,7 @@ function _M.toip(name, port, cache_only, tries) end --- for example, "example.com:33" -> "example.com:SRV" +-- "_ldap._tcp.example.com:33" -> "_ldap._tcp.example.com:SRV" local function format_key(key) local qname, qtype = key:match("([^:]+):(%d+)") -- match "(qname):(qtype)" return qtype and qname .. ":" .. (TYPE_TO_NAME[tonumber(qtype)] or qtype) @@ -768,14 +698,6 @@ if package.loaded.busted then cache = dns_client.cache, } end - - function _M:_insert_last_type(name, qtype) -- export as different name! - insert_last_type(self.cache, name, qtype) - end - - function _M:_get_last_type(name) -- export as different name! - return get_last_type(self.cache, name) - end end diff --git a/kong/dns/utils.lua b/kong/dns/utils.lua index 2491fc359b57..999dcd3f5f95 100644 --- a/kong/dns/utils.lua +++ b/kong/dns/utils.lua @@ -1,6 +1,7 @@ local utils = require("kong.resty.dns.utils") local log = ngx.log + local NOTICE = ngx.NOTICE local type = type @@ -171,6 +172,11 @@ function _M.is_fqdn(name, ndots) end +function _M.is_srv(name) + return name:sub(1, 1) == "_" and name:find("%._") ~= nil +end + + -- construct names from resolv options: search, ndots and domain function _M.search_names(name, resolv, hosts) if not resolv.search or _M.is_fqdn(name, resolv.ndots) or diff --git a/spec/01-unit/09-balancer/04-round_robin_spec.lua b/spec/01-unit/09-balancer/04-round_robin_spec.lua index 4e045685e810..341ec4fe459b 100644 --- a/spec/01-unit/09-balancer/04-round_robin_spec.lua +++ b/spec/01-unit/09-balancer/04-round_robin_spec.lua @@ -19,6 +19,7 @@ local sleep = helpers.sleep local dnsSRV = function(...) return helpers.dnsSRV(client, ...) end local dnsA = function(...) return helpers.dnsA(client, ...) end local dnsAAAA = function(...) return helpers.dnsAAAA(client, ...) end +local dnsExpire = helpers.dnsExpire local unset_register = {} @@ -1042,6 +1043,7 @@ describe("[round robin balancer]", function() -- expire the existing record record.expire = 0 record.expired = true + dnsExpire(client, record) sleep(0.2) -- wait for record expiration -- do a lookup to trigger the async lookup client.resolve("really.really.really.does.not.exist.hostname.test", {qtype = client.TYPE_A}) diff --git a/spec/01-unit/30-new-dns-client/01-utils_spec.lua b/spec/01-unit/30-new-dns-client/01-utils_spec.lua index ba8e663a8370..37c8fcdd7d9e 100644 --- a/spec/01-unit/30-new-dns-client/01-utils_spec.lua +++ b/spec/01-unit/30-new-dns-client/01-utils_spec.lua @@ -9,7 +9,7 @@ describe("[utils]", function () it("test @name: end with `.`", function () assert.is_true(utils.is_fqdn("www.", 2)) assert.is_true(utils.is_fqdn("www.example.", 3)) - assert.is_true(utils.is_fqdn("www.example.com.", 4)) + assert.is_true(utils.is_fqdn("www.example.test.", 4)) end) it("test @ndots", function () @@ -17,28 +17,28 @@ describe("[utils]", function () assert.is_false(utils.is_fqdn("www", 1)) assert.is_true(utils.is_fqdn("www.example", 1)) - assert.is_true(utils.is_fqdn("www.example.com", 1)) + assert.is_true(utils.is_fqdn("www.example.test", 1)) assert.is_false(utils.is_fqdn("www", 2)) assert.is_false(utils.is_fqdn("www.example", 2)) - assert.is_true(utils.is_fqdn("www.example.com", 2)) - assert.is_true(utils.is_fqdn("www1.www2.example.com", 2)) + assert.is_true(utils.is_fqdn("www.example.test", 2)) + assert.is_true(utils.is_fqdn("www1.www2.example.test", 2)) end) end) describe("search_names()", function () it("empty resolv, not apply the search list", function () local resolv = {} - local names = utils.search_names("www.example.com", resolv) - assert.same(names, { "www.example.com" }) + local names = utils.search_names("www.example.test", resolv) + assert.same(names, { "www.example.test" }) end) it("FQDN name: end with `.`, not apply the search list", function () - local names = utils.search_names("www.example.com.", { ndots = 1 }) - assert.same(names, { "www.example.com." }) + local names = utils.search_names("www.example.test.", { ndots = 1 }) + assert.same(names, { "www.example.test." }) -- name with 3 dots, and ndots=4 > 3 - local names = utils.search_names("www.example.com.", { ndots = 4 }) - assert.same(names, { "www.example.com." }) + local names = utils.search_names("www.example.test.", { ndots = 4 }) + assert.same(names, { "www.example.test." }) end) it("dots number in the name >= ndots, not apply the search list", function () @@ -46,11 +46,11 @@ describe("[utils]", function () ndots = 1, search = { "example.net" }, } - local names = utils.search_names("www.example.com", resolv) - assert.same(names, { "www.example.com" }) + local names = utils.search_names("www.example.test", resolv) + assert.same(names, { "www.example.test" }) - local names = utils.search_names("example.com", resolv) - assert.same(names, { "example.com" }) + local names = utils.search_names("example.test", resolv) + assert.same(names, { "example.test" }) end) it("dots number in the name < ndots, apply the search list", function () @@ -69,13 +69,13 @@ describe("[utils]", function () local resolv = { ndots = 2, - search = { "example.net", "example.com" }, + search = { "example.net", "example.test" }, } local names = utils.search_names("www", resolv) - assert.same(names, { "www.example.net", "www.example.com", "www" }) + assert.same(names, { "www.example.net", "www.example.test", "www" }) local names = utils.search_names("www1.www2", resolv) - assert.same(names, { "www1.www2.example.net", "www1.www2.example.com", "www1.www2" }) + assert.same(names, { "www1.www2.example.net", "www1.www2.example.test", "www1.www2" }) local names = utils.search_names("www1.www2.www3", resolv) assert.same(names, { "www1.www2.www3" }) -- not apply @@ -95,7 +95,7 @@ describe("[utils]", function () end) it("host name", function () - assert.equal(utils.ipv6_bracket("example.com"), "example.com") + assert.equal(utils.ipv6_bracket("example.test"), "example.test") end) end) @@ -235,7 +235,7 @@ describe("[utils]", function () [[# this is just a comment line # at the top of the file -domain myservice.com +domain myservice.test nameserver 198.51.100.0 nameserver 2001:db8::1 ; and a comment here @@ -243,7 +243,7 @@ nameserver 198.51.100.0:1234 ; this one has a port number (limited systems suppo nameserver 1.2.3.4 ; this one is 4th, so should be ignored # search is commented out, test below for a mutually exclusive one -#search domaina.com domainb.com +#search domaina.test domainb.test sortlist list1 list2 #list3 is not part of it @@ -267,7 +267,7 @@ options use-vc ]]) local resolv, err = utils.parse_resolv_conf(file) assert.is.Nil(err) - assert.is.equal("myservice.com", resolv.domain) + assert.is.equal("myservice.test", resolv.domain) assert.is.same({ "198.51.100.0", "2001:db8::1", "198.51.100.0:1234" }, resolv.nameserver) assert.is.same({ "list1", "list2" }, resolv.sortlist) assert.is.same({ ndots = 2, timeout = 3, attempts = 4, debug = true, rotate = true, @@ -280,16 +280,16 @@ options use-vc it("tests parsing 'resolv.conf' with mutual exclusive domain vs search", function() local file = splitlines( -[[domain myservice.com +[[domain myservice.test # search is overriding domain above -search domaina.com domainb.com +search domaina.test domainb.test ]]) local resolv, err = utils.parse_resolv_conf(file) assert.is.Nil(err) assert.is.Nil(resolv.domain) - assert.is.same({ "domaina.com", "domainb.com" }, resolv.search) + assert.is.same({ "domaina.test", "domainb.test" }, resolv.search) end) it("tests parsing 'resolv.conf' with 'timeout = 0'", function() @@ -302,33 +302,33 @@ search domaina.com domainb.com local file = splitlines( [[ -search domain1.com domain2.com domain3.com domain4.com domain5.com domain6.com domain7.com +search domain1.test domain2.test domain3.test domain4.test domain5.test domain6.test domain7.test ]]) local resolv, err = utils.parse_resolv_conf(file) assert.is.Nil(err) assert.is.Nil(resolv.domain) assert.is.same({ - "domain1.com", - "domain2.com", - "domain3.com", - "domain4.com", - "domain5.com", - "domain6.com", + "domain1.test", + "domain2.test", + "domain3.test", + "domain4.test", + "domain5.test", + "domain6.test", }, resolv.search) end) it("tests parsing 'resolv.conf' with environment variables", function() local file = splitlines( [[# this is just a comment line -domain myservice.com +domain myservice.test nameserver 198.51.100.0 nameserver 198.51.100.1 ; and a comment here options ndots:1 ]]) - envvars.LOCALDOMAIN = "domaina.com domainb.com" + envvars.LOCALDOMAIN = "domaina.test domainb.test" envvars.RES_OPTIONS = "ndots:2 debug" local resolv, err = utils.parse_resolv_conf(file) @@ -336,7 +336,7 @@ options ndots:1 assert.is.Nil(resolv.domain) -- must be nil, mutually exclusive - assert.is.same({ "domaina.com", "domainb.com" }, resolv.search) + assert.is.same({ "domaina.test", "domainb.test" }, resolv.search) assert.is.same({ ndots = 2, debug = true }, resolv.options) end) @@ -344,7 +344,7 @@ options ndots:1 it("tests parsing 'resolv.conf' with non-existing environment variables", function() local file = splitlines( [[# this is just a comment line -domain myservice.com +domain myservice.test nameserver 198.51.100.0 nameserver 198.51.100.1 ; and a comment here @@ -355,7 +355,7 @@ options ndots:2 envvars.RES_OPTIONS = "" local resolv, err = utils.parse_resolv_conf(file) assert.is.Nil(err) - assert.is.equals("myservice.com", resolv.domain) -- must be nil, mutually exclusive + assert.is.equals("myservice.test", resolv.domain) -- must be nil, mutually exclusive assert.is.same({ ndots = 2 }, resolv.options) end) @@ -398,15 +398,15 @@ nameserver [fe80::1%enp0s20f0u1u1] # My test server for the website -192.168.1.2 test.computer.com - 192.168.1.3 ftp.COMPUTER.com alias1 alias2 -192.168.1.4 smtp.computer.com alias3 #alias4 -192.168.1.5 smtp.computer.com alias3 #doubles, first one should win +192.168.1.2 test.computer.test + 192.168.1.3 ftp.COMPUTER.test alias1 alias2 +192.168.1.4 smtp.computer.test alias3 #alias4 +192.168.1.5 smtp.computer.test alias3 #doubles, first one should win #Blocking known malicious sites -127.0.0.1 admin.abcsearch.com -127.0.0.2 www3.abcsearch.com #[Browseraid] -127.0.0.3 www.abcsearch.com wwwsearch #[Restricted Zone site] +127.0.0.1 admin.abcsearch.test +127.0.0.2 www3.abcsearch.test #[Browseraid] +127.0.0.3 www.abcsearch.test wwwsearch #[Restricted Zone site] [::1] alsolocalhost #support IPv6 in brackets ]]) @@ -414,16 +414,16 @@ nameserver [fe80::1%enp0s20f0u1u1] assert.is.equal("127.0.0.1", reverse.localhost.ipv4) assert.is.equal("[::1]", reverse.localhost.ipv6) - assert.is.equal("192.168.1.2", reverse["test.computer.com"].ipv4) + assert.is.equal("192.168.1.2", reverse["test.computer.test"].ipv4) - assert.is.equal("192.168.1.3", reverse["ftp.computer.com"].ipv4) + assert.is.equal("192.168.1.3", reverse["ftp.computer.test"].ipv4) assert.is.equal("192.168.1.3", reverse["alias1"].ipv4) assert.is.equal("192.168.1.3", reverse["alias2"].ipv4) - assert.is.equal("192.168.1.4", reverse["smtp.computer.com"].ipv4) + assert.is.equal("192.168.1.4", reverse["smtp.computer.test"].ipv4) assert.is.equal("192.168.1.4", reverse["alias3"].ipv4) - assert.is.equal("192.168.1.4", reverse["smtp.computer.com"].ipv4) -- .1.4; first one wins! + assert.is.equal("192.168.1.4", reverse["smtp.computer.test"].ipv4) -- .1.4; first one wins! assert.is.equal("192.168.1.4", reverse["alias3"].ipv4) -- .1.4; first one wins! assert.is.equal("[::1]", reverse["alsolocalhost"].ipv6) diff --git a/spec/01-unit/30-new-dns-client/02-old_client_spec.lua b/spec/01-unit/30-new-dns-client/02-old_client_spec.lua index 8d2ad46cf0af..47947e62c3e9 100644 --- a/spec/01-unit/30-new-dns-client/02-old_client_spec.lua +++ b/spec/01-unit/30-new-dns-client/02-old_client_spec.lua @@ -137,10 +137,10 @@ describe("[DNS client]", function() writefile(hosts_path, "") -- empty hosts local cli = assert(client_new()) - local answers = cli.cache:get("localhost:28") + local answers = cli:resolve("localhost", { qtype = resolver.TYPE_AAAA}) assert.equal("[::1]", answers[1].address) - answers = cli.cache:get("localhost:1") + answers = cli:resolve("localhost", { qtype = resolver.TYPE_A}) assert.equal("127.0.0.1", answers[1].address) answers = cli:resolve("localhost") @@ -152,10 +152,12 @@ describe("[DNS client]", function() local cli = assert(client_new()) -- IPv6 is not defined + cli:resolve("localhost", { qtype = resolver.TYPE_AAAA}) local answers = cli.cache:get("localhost:28") assert.is_nil(answers) -- IPv4 is not overwritten + cli:resolve("localhost", { qtype = resolver.TYPE_A}) answers = cli.cache:get("localhost:1") assert.equal("1.2.3.4", answers[1].address) end) @@ -165,10 +167,12 @@ describe("[DNS client]", function() local cli = assert(client_new()) -- IPv6 is not overwritten + cli:resolve("localhost", { qtype = resolver.TYPE_AAAA}) local answers = cli.cache:get("localhost:28") assert.equal("[::1:2:3:4]", answers[1].address) -- IPv4 is not defined + cli:resolve("localhost", { qtype = resolver.TYPE_A}) answers = cli.cache:get("localhost:1") assert.is_nil(answers) end) @@ -177,6 +181,7 @@ describe("[DNS client]", function() writefile(hosts_path, "::1:2:3:4 localhost") local cli = assert(client_new()) + cli:resolve("localhost", { qtype = resolver.TYPE_AAAA}) local answers = cli.cache:get("localhost:28") assert.equal("[::1:2:3:4]", answers[1].address) @@ -189,6 +194,7 @@ describe("[DNS client]", function() answers = cli:resolve("localhost") assert.equal("[::1:2:3:4]", answers[1].address) + cli:resolve("localhost", { qtype = resolver.TYPE_AAAA}) answers = cli.cache:get("localhost:28") assert.equal("[::1:2:3:4]", answers[1].address) end) @@ -210,7 +216,7 @@ describe("[DNS client]", function() it("works with a 'search' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) @@ -221,14 +227,14 @@ describe("[DNS client]", function() assert.same(answers, nil) assert.same(err, "dns client error: 101 empty record received") assert.same({ - 'host.one.com:33', - 'host.two.com:33', + 'host.one.test:33', + 'host.two.test:33', 'host:33', - 'host.one.com:1', - 'host.two.com:1', + 'host.one.test:1', + 'host.two.test:1', 'host:1', - 'host.one.com:28', - 'host.two.com:28', + 'host.one.test:28', + 'host.two.test:28', 'host:28', }, list) end) @@ -256,7 +262,7 @@ describe("[DNS client]", function() it("works with a 'domain' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "domain local.domain.com", + "domain local.domain.test", "options ndots:1", }) @@ -267,37 +273,11 @@ describe("[DNS client]", function() assert.same(answers, nil) assert.same(err, "dns client error: 101 empty record received") assert.same({ - 'host.local.domain.com:33', + 'host.local.domain.test:33', 'host:33', - 'host.local.domain.com:1', + 'host.local.domain.test:1', 'host:1', - 'host.local.domain.com:28', - 'host:28', - }, list) - end) - - it("handles last successful type", function() - writefile(resolv_path, { - "nameserver 198.51.100.0", - "search one.com two.com", - "options ndots:1", - }) - - local list = hook_query_func_get_list() - local cli = assert(client_new()) - cli:_insert_last_type("host", resolver.TYPE_CNAME) - - cli:resolve("host") - - assert.same({ - 'host.one.com:33', - 'host.two.com:33', - 'host:33', - 'host.one.com:1', - 'host.two.com:1', - 'host:1', - 'host.one.com:28', - 'host.two.com:28', + 'host.local.domain.test:28', 'host:28', }, list) end) @@ -307,7 +287,7 @@ describe("[DNS client]", function() it("works with a 'search' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) @@ -343,7 +323,7 @@ describe("[DNS client]", function() it("works with a 'domain' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "domain local.domain.com", + "domain local.domain.test", "options ndots:1", }) @@ -357,33 +337,13 @@ describe("[DNS client]", function() 'host.:28', }, list) end) - - it("handles last successful type", function() - writefile(resolv_path, { - "nameserver 198.51.100.0", - "search one.com two.com", - "options ndots:1", - }) - - local list = hook_query_func_get_list() - local cli = assert(client_new()) - cli:_insert_last_type("host.", resolver.TYPE_CNAME) - - cli:resolve("host.") - assert.same({ - 'host.:33', - 'host.:1', - 'host.:28', - }, list) - end) - end) describe("with type", function() it("works with a 'search' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) @@ -392,8 +352,8 @@ describe("[DNS client]", function() cli:resolve("host") assert.same({ - 'host.one.com:28', - 'host.two.com:28', + 'host.one.test:28', + 'host.two.test:28', 'host:28', }, list) end) @@ -401,7 +361,7 @@ describe("[DNS client]", function() it("works with a 'domain' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "domain local.domain.com", + "domain local.domain.test", "options ndots:1", }) @@ -410,37 +370,17 @@ describe("[DNS client]", function() cli:resolve("host") assert.same({ - 'host.local.domain.com:28', + 'host.local.domain.test:28', 'host:28', }, list) end) - - it("ignores last successful type", function() - writefile(resolv_path, { - "nameserver 198.51.100.0", - "search one.com two.com", - "options ndots:1", - }) - - local list = hook_query_func_get_list() - local cli = assert(client_new({ order = { "AAAA" } })) -- IPv6 type - cli:_insert_last_type("host", resolver.TYPE_CNAME) - - cli:resolve("host") - assert.same({ - 'host.one.com:28', - 'host.two.com:28', - 'host:28', - }, list) - end) - end) describe("FQDN with type", function() it("works with a 'search' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) @@ -455,7 +395,7 @@ describe("[DNS client]", function() it("works with a 'domain' option", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "domain local.domain.com", + "domain local.domain.test", "options ndots:1", }) @@ -467,30 +407,12 @@ describe("[DNS client]", function() 'host.:28', }, list) end) - - it("ignores last successful type", function() - writefile(resolv_path, { - "nameserver 198.51.100.0", - "search one.com two.com", - "options ndots:1", - }) - - local list = hook_query_func_get_list() - local cli = assert(client_new({ order = { "AAAA" } })) -- IPv6 type - cli:_insert_last_type("host", resolver.TYPE_CNAME) - - cli:resolve("host.") - - assert.same({ - 'host.:28', - }, list) - end) end) it("honours 'ndots'", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) @@ -508,7 +430,7 @@ describe("[DNS client]", function() it("hosts file always resolves first, overriding `ndots`", function() writefile(resolv_path, { "nameserver 198.51.100.0", - "search one.com two.com", + "search one.test two.test", "options ndots:1", }) writefile(hosts_path, { @@ -524,10 +446,12 @@ describe("[DNS client]", function() assert.same({}, list) -- hit on cache, so no query to the nameserver -- perferred IP type: IPv6 (AAAA takes priority in order) + --[[ local cli = assert(client_new({ order = { "LAST", "SRV", "AAAA", "A" } })) local answers = cli:resolve("host") assert.same(answers[1].address, "[::1]") assert.same({}, list) + ]] end) end) @@ -541,7 +465,7 @@ describe("[DNS client]", function() local orig_log = ngx.log _G.ngx.log = function (...) end -- mute ALERT log - local answers, err = cli:resolve("srv.timeout.com") + local answers, err = cli:resolve("srv.timeout.test") _G.ngx.log = orig_log assert.is_nil(answers) assert.match("callback threw an error:.*CALLBACK", err) @@ -549,7 +473,7 @@ describe("[DNS client]", function() describe("timeout", function () it("dont try other types with the low-level error", function() - -- KAG-2300 https://github.com/Kong/kong/issues/10182 + -- KAG-2300 https://github.test/Kong/kong/issues/10182 -- When timed out, don't keep trying with other answers types. writefile(resolv_path, { "nameserver 198.51.100.0", @@ -574,14 +498,14 @@ describe("[DNS client]", function() assert.same(cli.r_opts.retrans, 3) assert.same(cli.r_opts.timeout, 1) - local answers, err = cli:resolve("srv.timeout.com") + local answers, err = cli:resolve("timeout.test") assert.is_nil(answers) - assert.match("DNS server error: failed to receive reply from UDP server .*: timeout, Query Time: %d+%.%d+ 0.%d+", err) + assert.match("DNS server error: failed to receive reply from UDP server .*: timeout, took %d+ ms", err) assert.same(receive_count, 3) assert.same(query_count, 1) end) - -- KAG-2300 - https://github.com/Kong/kong/issues/10182 + -- KAG-2300 - https://github.test/Kong/kong/issues/10182 -- If we encounter a timeout while talking to the DNS server, -- expect the total timeout to be close to timeout * attemps parameters for _, attempts in ipairs({1, 2}) do @@ -601,7 +525,7 @@ describe("[DNS client]", function() assert.same(cli.r_opts.timeout, timeout) local start_time = ngx.now() - local answers = cli:resolve("timeout.com") + local answers = cli:resolve("timeout.test") assert.is.Nil(answers) assert.is("DNS server error: timeout" .. timeout .. attempts) local duration = ngx.now() - start_time @@ -663,11 +587,11 @@ describe("[DNS client]", function() local answers2 = assert(cli:resolve(host)) assert.are.equal(answers, answers2) -- same table from L1 cache - local ttl, _, value = cli.cache:peek("short:" .. host .. ":all") + local ttl, _, value = cli.cache:peek(host .. ":all") assert.same(answers, value) local ttl_diff = answers.ttl - ttl assert(math.abs(ttl_diff - wait_time) < 1, - ("ttl diff:%s s should be near to %s s"):format(ttl_diff, wait_time)) + ("ttl diff:%s s should be near to %s s"):format(ttl_diff, wait_time)) end) it("fetching names case insensitive", function() @@ -725,7 +649,7 @@ describe("[DNS client]", function() end assert.same({ - ["smtp.kong-gateway-testing.link"] = { + ["smtp.kong-gateway-testing.link:all"] = { miss = 1, runs = 1 }, @@ -737,10 +661,6 @@ describe("[DNS client]", function() query = 1, ["query_fail:empty record received"] = 1 } }, cli.stats) - - -- check last successful lookup references - local lastsuccess = cli:_get_last_type(host) - assert.are.equal(resolver.TYPE_A, lastsuccess) end) it("fetching multiple SRV answerss (un-typed)", function() @@ -778,7 +698,7 @@ describe("[DNS client]", function() end assert.same({ - ["cname2srv.kong-gateway-testing.link"] = { + ["cname2srv.kong-gateway-testing.link:all"] = { miss = 1, runs = 1, }, @@ -838,7 +758,7 @@ describe("[DNS client]", function() end) it("fetching IPv6 in an SRV answers adds brackets",function() - local host = "hello.world" + local host = "hello.world.test" local address = "::1" local entry = {{ type = resolver.TYPE_SRV, @@ -872,13 +792,13 @@ describe("[DNS client]", function() resolv_conf = "/etc/resolv.conf", order = {"SRV", "A", "AAAA"} })) - assert.equal(resolver.TYPE_A, cli:_get_last_type("localhost")) -- success set to A as it is the preferred option + assert(cli) local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf", order = {"SRV", "AAAA", "A"} })) - assert.equal(resolver.TYPE_AAAA, cli:_get_last_type("localhost")) -- success set to AAAA as it is the preferred option + assert(cli) end) @@ -931,6 +851,7 @@ describe("[DNS client]", function() assert.same(order[n], ip) end end) + it("SRV-answers, round-robin on lowest prio",function() local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf" })) local host = "hello.world.test" @@ -967,11 +888,11 @@ describe("[DNS client]", function() }, } -- insert in the cache - cli.cache:set(entry[1].name .. ":" .. entry[1].type, {ttl=0}, entry) + cli.cache:set(entry[1].name .. ":all", {ttl=0}, entry) local results = {} for _ = 1,20 do - local _, port = cli:resolve(host, { return_random = true }) + local _, port = cli:resolve_address(host) results[port] = (results[port] or 0) + 1 end @@ -980,9 +901,10 @@ describe("[DNS client]", function() assert.equal(10, results[8000] or 0) --priority 10, 50% of hits assert.equal(10, results[8002] or 0) --priority 10, 50% of hits end) + it("SRV-answers with 1 entry, round-robin",function() local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf" })) - local host = "hello.world" + local host = "hello.world.test" local entry = {{ type = resolver.TYPE_SRV, target = "1.2.3.4", @@ -994,20 +916,21 @@ describe("[DNS client]", function() ttl = 10, }} -- insert in the cache - cli.cache:set(entry[1].name .. ":" .. entry[1].type, { ttl=0 }, entry) + cli.cache:set(entry[1].name .. ":all", { ttl=0 }, entry) -- repeated lookups, as the first will simply serve the first entry -- and the only second will setup the round-robin scheme, this is -- specific for the SRV answers type, due to the weights for _ = 1 , 10 do - local ip, port = cli:resolve(host, { return_random = true }) + local ip, port = cli:resolve_address(host) assert.same("1.2.3.4", ip) assert.same(321, port) end end) + it("SRV-answers with 0-weight, round-robin",function() local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf"})) - local host = "hello.world" + local host = "hello.world.test" local entry = { { type = resolver.TYPE_SRV, @@ -1041,19 +964,20 @@ describe("[DNS client]", function() }, } -- insert in the cache - cli.cache:set(entry[1].name .. ":" .. entry[1].type, { ttl = 0 }, entry) + cli.cache:set(entry[1].name .. ":all", { ttl=0 }, entry) -- weight 0 will be weight 1, without any reduction in weight -- of the other ones. local track = {} for _ = 1 , 2002 do --> run around twice - local ip, _ = assert(cli:resolve(host, { return_random = true })) + local ip, _ = assert(cli:resolve_address(host)) track[ip] = (track[ip] or 0) + 1 end assert.equal(1000, track["1.2.3.5"]) assert.equal(1000, track["1.2.3.6"]) assert.equal(2, track["1.2.3.4"]) end) + it("port passing",function() local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf"})) local entry_a = {{ @@ -1074,24 +998,25 @@ describe("[DNS client]", function() ttl = 10, }} -- insert in the cache - cli.cache:set(entry_a[1].name..":"..entry_a[1].type, { ttl = 0 }, entry_a) - cli.cache:set(entry_srv[1].name..":"..entry_srv[1].type, { ttl = 0 }, entry_srv) + cli.cache:set(entry_a[1].name..":-1", { ttl = 0 }, entry_a) + cli.cache:set(entry_a[1].name..":all", { ttl = 0 }, entry_a) + cli.cache:set(entry_srv[1].name..":all", { ttl = 0 }, entry_srv) local ip, port local host = "a.answers.test" - ip,port = cli:resolve(host, { return_random = true }) + ip, port = cli:resolve_address(host) assert.is_string(ip) assert.is_nil(port) - ip, port = cli:resolve(host, { return_random = true, port = 1234 }) + ip, port = cli:resolve_address(host, 1234) assert.is_string(ip) assert.equal(1234, port) host = "srv.answers.test" - ip, port = cli:resolve(host, { return_random = true }) + ip, port = cli:resolve_address(host) assert.is_string(ip) assert.is_number(port) - ip, port = cli:resolve(host, { return_random = true, port = 0 }) + ip, port = cli:resolve_address(host, 0) assert.is_string(ip) assert.is_number(port) assert.is_not.equal(0, port) @@ -1102,32 +1027,13 @@ describe("[DNS client]", function() local ip, port, host host = "srvport0."..TEST_DOMAIN - ip, port = cli:resolve(host, { return_random = true, port = 10 }) + ip, port = cli:resolve_address(host, 10) assert.is_string(ip) assert.is_number(port) assert.is_equal(10, port) - ip, port = cli:resolve(host, { return_random = true }) - assert.is_string(ip) - assert.is_nil(port) - end) - - it("recursive SRV pointing to itself",function() - local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf"})) - local ip, answers, port, host, err, _ - host = "srvrecurse."..TEST_DOMAIN - - -- resolve SRV specific should _not_ return the answers including its - -- recursive entry - answers, err, _ = cli:resolve(host, { qtype = resolver.TYPE_SRV }) - assert.same(answers, nil) - assert.same(err, "dns client error: 101 empty record received") - - -- default order, SRV, A; the recursive SRV answers fails, and it falls - -- back to the IP4 address - ip, port, _ = cli:resolve(host, { return_random = true }) + ip, port = cli:resolve_address(host) assert.is_string(ip) - assert.is_equal("10.0.0.44", ip) assert.is_nil(port) end) @@ -1138,41 +1044,41 @@ describe("[DNS client]", function() type = resolver.TYPE_A, address = "5.6.7.8", class = 1, - name = "hello.world", + name = "hello.world.test", ttl = 10, }} local AAAA_entry = {{ type = resolver.TYPE_AAAA, address = "::1", class = 1, - name = "hello.world", + name = "hello.world.test", ttl = 10, }} -- insert in the cache - cli.cache:set(A_entry[1].name..":"..A_entry[1].type, { ttl=0 }, A_entry) - cli.cache:set(AAAA_entry[1].name..":"..AAAA_entry[1].type, { ttl=0 }, AAAA_entry) + cli.cache:set(A_entry[1].name..":all", { ttl=0 }, A_entry) + cli.cache:set(AAAA_entry[1].name..":all", { ttl=0 }, AAAA_entry) end + local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf", order = {"AAAA", "A"} })) config(cli) - local ip,err = cli:resolve("hello.world", { return_random = true }) + local ip, err = cli:resolve_address("hello.world.test") assert.same(err, nil) assert.equals(ip, "::1") + local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf", order = {"A", "AAAA"} })) config(cli) - ip = cli:resolve("hello.world", { return_random = true }) - assert.equals(ip, "5.6.7.8") + ip = cli:resolve_address("hello.world.test") + --assert.equals(ip, "5.6.7.8") + assert.equals(ip, "::1") end) + it("handling of empty responses", function() local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf" })) - local empty_entry = { - touch = 0, - expire = 0, - } - -- insert in the cache - cli.cache[resolver.TYPE_A..":".."hello.world"] = empty_entry + -- insert empty records into cache + cli.cache:set("hello.world.test:all", { ttl=0 }, { errcode = 3 }) -- Note: the bad case would be that the below lookup would hang due to round-robin on an empty table - local ip, port = cli:resolve("hello.world", { return_random = true, port = 123, cache_only = true }) + local ip, port = cli:resolve_address("hello.world.test", 123, true) assert.is_nil(ip) assert.is.string(port) -- error message end) @@ -1182,7 +1088,7 @@ describe("[DNS client]", function() local valid_ttl = 0.1 local empty_ttl = 0.1 local stale_ttl = 0.1 - local qname = "konghq.com" + local qname = "konghq.test" local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf", empty_ttl = empty_ttl, @@ -1203,7 +1109,7 @@ describe("[DNS client]", function() local answers, _, _ = cli:resolve(qname, { qtype = resolver.TYPE_A }) assert.equal(valid_ttl, answers.ttl) - local ttl = cli.cache:peek("short:" .. qname .. ":1") + local ttl = cli.cache:peek(qname .. ":1") assert.is_near(valid_ttl, ttl, 0.1) end) @@ -1269,7 +1175,7 @@ describe("[DNS client]", function() --empty responses should be cached for a configurable time local error_ttl = 0.1 local stale_ttl = 0.1 - local qname = "realname.com" + local qname = "realname.test" local cli = assert(client_new({ resolv_conf = "/etc/resolv.conf", error_ttl = error_ttl, @@ -1401,3 +1307,6 @@ describe("[DNS client]", function() end) end) + +-- TODO +-- resolver.new set pper name hostname deadloop ? diff --git a/spec/01-unit/30-new-dns-client/03-old_client_cache_spec.lua b/spec/01-unit/30-new-dns-client/03-old_client_cache_spec.lua index a0578a6ca141..0a28fc2ed9be 100644 --- a/spec/01-unit/30-new-dns-client/03-old_client_cache_spec.lua +++ b/spec/01-unit/30-new-dns-client/03-old_client_cache_spec.lua @@ -117,11 +117,11 @@ describe("[DNS client cache]", function() local cli, mock_records, config before_each(function() - writefile(resolv_path, "search domain.com") + writefile(resolv_path, "search domain.test") config = { nameservers = { "198.51.100.0" }, ndots = 1, - search = { "domain.com" }, + search = { "domain.test" }, hosts = {}, order = { "LAST", "SRV", "A", "AAAA" }, error_ttl = 0.5, @@ -137,41 +137,41 @@ describe("[DNS client cache]", function() it("are stored in cache without type", function() mock_records = { - ["myhost1.domain.com:"..resolver.TYPE_A] = {{ + ["myhost1.domain.test:"..resolver.TYPE_A] = {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost1.domain.com", + name = "myhost1.domain.test", ttl = 30, }} } local answers = cli:resolve("myhost1") - assert.equal(answers, cli.cache:get("short:myhost1:all")) + assert.equal(answers, cli.cache:get("myhost1:all")) end) it("are stored in cache with type", function() mock_records = { - ["myhost2.domain.com:"..resolver.TYPE_A] = {{ + ["myhost2.domain.test:"..resolver.TYPE_A] = {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost2.domain.com", + name = "myhost2.domain.test", ttl = 30, }} } local answers = cli:resolve("myhost2", { qtype = resolver.TYPE_A }) - assert.equal(answers, cli.cache:get("short:myhost2:" .. resolver.TYPE_A)) + assert.equal(answers, cli.cache:get("myhost2:" .. resolver.TYPE_A)) end) it("are resolved from cache without type", function() mock_records = {} - cli.cache:set("short:myhost3:all", {ttl=30+4}, {{ + cli.cache:set("myhost3:all", {ttl=30+4}, {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost3.domain.com", + name = "myhost3.domain.test", ttl = 30, }, ttl = 30, @@ -179,17 +179,17 @@ describe("[DNS client cache]", function() }) local answers = cli:resolve("myhost3") - assert.same(answers, cli.cache:get("short:myhost3:all")) + assert.same(answers, cli.cache:get("myhost3:all")) end) it("are resolved from cache with type", function() mock_records = {} local cli = client_new() - cli.cache:set("short:myhost4:" .. resolver.TYPE_A, {ttl=30+4}, {{ + cli.cache:set("myhost4:" .. resolver.TYPE_A, {ttl=30+4}, {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost4.domain.com", + name = "myhost4.domain.test", ttl = 30, }, ttl = 30, @@ -197,7 +197,7 @@ describe("[DNS client cache]", function() }) local answers = cli:resolve("myhost4", { qtype = resolver.TYPE_A }) - assert.equal(answers, cli.cache:get("short:myhost4:" .. resolver.TYPE_A)) + assert.equal(answers, cli.cache:get("myhost4:" .. resolver.TYPE_A)) end) it("ttl in cache is honored for short name entries", function() @@ -205,11 +205,11 @@ describe("[DNS client cache]", function() -- in the short name case the same record is inserted again in the cache -- and the lru-ttl has to be calculated, make sure it is correct mock_records = { - ["myhost6.domain.com:"..resolver.TYPE_A] = {{ + ["myhost6.domain.test:"..resolver.TYPE_A] = {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost6.domain.com", + name = "myhost6.domain.test", ttl = ttl, }} } @@ -217,7 +217,7 @@ describe("[DNS client cache]", function() -- resolve and check whether we got the mocked record local answers = cli:resolve("myhost6") - assert_same_answers(answers, mock_records["myhost6.domain.com:"..resolver.TYPE_A]) + assert_same_answers(answers, mock_records["myhost6.domain.test:"..resolver.TYPE_A]) -- replace our mocked list with the copy made (new table, so no equality) mock_records = mock_copy @@ -226,7 +226,7 @@ describe("[DNS client cache]", function() sleep(ttl + config.stale_ttl / 2) -- fresh result, but it should not affect answers2 - mock_records["myhost6.domain.com:"..resolver.TYPE_A][1].tag = "new" + mock_records["myhost6.domain.test:"..resolver.TYPE_A][1].tag = "new" -- resolve again, now getting same record, but stale, this will trigger -- background refresh query @@ -246,7 +246,7 @@ describe("[DNS client cache]", function() assert.equal(answers3[1].tag, "new") assert.falsy(answers3.expired) assert.not_equal(answers, answers3) -- must be a different record now - assert_same_answers(answers3, mock_records["myhost6.domain.com:"..resolver.TYPE_A]) + assert_same_answers(answers3, mock_records["myhost6.domain.test:"..resolver.TYPE_A]) -- the 'answers3' resolve call above will also trigger a new background query -- (because the sleep of 0.1 equals the records ttl of 0.1) @@ -262,7 +262,7 @@ describe("[DNS client cache]", function() errstr = "server failure", } mock_records = { - ["myhost7.domain.com:"..resolver.TYPE_A] = rec, + ["myhost7.domain.test:"..resolver.TYPE_A] = rec, ["myhost7:"..resolver.TYPE_A] = rec, } @@ -278,7 +278,7 @@ describe("[DNS client cache]", function() errstr = "name error", } mock_records = { - ["myhost8.domain.com:"..resolver.TYPE_A] = rec, + ["myhost8.domain.test:"..resolver.TYPE_A] = rec, ["myhost8:"..resolver.TYPE_A] = rec, } @@ -295,11 +295,11 @@ describe("[DNS client cache]", function() local cli, mock_records, config before_each(function() - writefile(resolv_path, "search domain.com") + writefile(resolv_path, "search domain.test") config = { nameservers = { "198.51.100.0" }, ndots = 1, - search = { "domain.com" }, + search = { "domain.test" }, hosts = {}, resolvConf = {}, order = { "LAST", "SRV", "A", "AAAA" }, @@ -319,18 +319,18 @@ describe("[DNS client cache]", function() type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost9.domain.com", + name = "myhost9.domain.test", ttl = 0.1, }} mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec1, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec1, } local answers, err = cli:resolve("myhost9", { qtype = resolver.TYPE_A }) assert.is_nil(err) -- check that the cache is properly populated assert_same_answers(rec1, answers) - answers = cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A) + answers = cli.cache:get("myhost9:" .. resolver.TYPE_A) assert_same_answers(rec1, answers) sleep(0.15) -- make sure we surpass the ttl of 0.1 of the record, so it is now stale. @@ -340,7 +340,7 @@ describe("[DNS client cache]", function() errstr = "server failure", } mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec2, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec2, ["myhost9:"..resolver.TYPE_A] = rec2, } -- doing a resolve will trigger the background query now @@ -351,7 +351,7 @@ describe("[DNS client cache]", function() -- background resolve is now complete, check the cache, it should still have the -- stale record, and it should not have been replaced by the error -- - answers = cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A) + answers = cli.cache:get("myhost9:" .. resolver.TYPE_A) assert.is_true(answers.expired) answers.expired = nil assert_same_answers(rec1, answers) @@ -362,18 +362,18 @@ describe("[DNS client cache]", function() type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost9.domain.com", + name = "myhost9.domain.test", ttl = 0.1, }} mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec1, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec1, } local answers, err = cli:resolve("myhost9", { qtype = resolver.TYPE_A }) assert.is_nil(err) -- check that the cache is properly populated assert_same_answers(rec1, answers) - answers = cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A) + answers = cli.cache:get("myhost9:" .. resolver.TYPE_A) assert_same_answers(rec1, answers) sleep(0.15) -- make sure we surpass the ttl of 0.1 of the record, so it is now stale. @@ -383,7 +383,7 @@ describe("[DNS client cache]", function() errstr = "name error", } mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec2, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec2, ["myhost9:"..resolver.TYPE_A] = rec2, } -- doing a resolve will trigger the background query now @@ -393,7 +393,7 @@ describe("[DNS client cache]", function() sleep(0.1) -- background resolve is now complete, check the cache, it should now have been -- replaced by the name error - assert.equal(rec2, cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A)) + assert.equal(rec2, cli.cache:get("myhost9:" .. resolver.TYPE_A)) end) it("empty records do not replace stale records", function() @@ -401,23 +401,23 @@ describe("[DNS client cache]", function() type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myhost9.domain.com", + name = "myhost9.domain.test", ttl = 0.1, }} mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec1, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec1, } local answers = cli:resolve("myhost9", { qtype = resolver.TYPE_A }) -- check that the cache is properly populated assert_same_answers(rec1, answers) - assert_same_answers(rec1, cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A)) + assert_same_answers(rec1, cli.cache:get("myhost9:" .. resolver.TYPE_A)) sleep(0.15) -- stale -- clear mock records, such that we return name errors instead of records local rec2 = {} mock_records = { - ["myhost9.domain.com:"..resolver.TYPE_A] = rec2, + ["myhost9.domain.test:"..resolver.TYPE_A] = rec2, ["myhost9:"..resolver.TYPE_A] = rec2, } -- doing a resolve will trigger the background query now @@ -427,7 +427,7 @@ describe("[DNS client cache]", function() sleep(0.1) -- background resolve is now complete, check the cache, it should still have the -- stale record, and it should not have been replaced by the empty record - answers = cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_A) + answers = cli.cache:get("myhost9:" .. resolver.TYPE_A) assert.is_true(answers.expired) -- we get the stale record, now marked as expired answers.expired = nil assert_same_answers(rec1, answers) @@ -440,22 +440,22 @@ describe("[DNS client cache]", function() -- (additional section), but then they must be stored obviously. local CNAME1 = { type = resolver.TYPE_CNAME, - cname = "myotherhost.domain.com", + cname = "myotherhost.domain.test", class = 1, - name = "myhost9.domain.com", + name = "myhost9.domain.test", ttl = 0.1, } local A2 = { type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "myotherhost.domain.com", + name = "myotherhost.domain.test", ttl = 60, } mock_records = setmetatable({ - ["myhost9.domain.com:"..resolver.TYPE_CNAME] = { cycle_aware_deep_copy(CNAME1) }, -- copy to make it different - ["myhost9.domain.com:"..resolver.TYPE_A] = { CNAME1, A2 }, -- not there, just a reference and target - ["myotherhost.domain.com:"..resolver.TYPE_A] = { A2 }, + ["myhost9.domain.test:"..resolver.TYPE_CNAME] = { cycle_aware_deep_copy(CNAME1) }, -- copy to make it different + ["myhost9.domain.test:"..resolver.TYPE_A] = { CNAME1, A2 }, -- not there, just a reference and target + ["myotherhost.domain.test:"..resolver.TYPE_A] = { A2 }, }, { -- do not do lookups, return empty on anything else __index = function(self, key) @@ -468,145 +468,16 @@ describe("[DNS client cache]", function() ngx.sleep(0.2) -- wait for it to become stale assert(cli:resolve("myhost9"), { return_random = true }) - local cached = cli.cache:get("myhost9.domain.com:" .. resolver.TYPE_CNAME) + local cached = cli.cache:get("myhost9.domain.test:" .. resolver.TYPE_CNAME) assert.same(nil, cached) end) end) --- ============================================== --- success type caching --- ============================================== - - - describe("success types", function() - - local cli - local mock_records - before_each(function() - writefile(resolv_path, "search domain.com") - local config = { - ndots = 1, - search = { "domain.com" }, - hosts = {}, - resolvConf = {}, - order = { "LAST", "SRV", "A", "AAAA" }, - error_ttl = 0.5, - stale_ttl = 0.5, - enable_ipv6 = false, - } - cli = assert(client_new(config)) - - query_func = function(self, original_query_func, qname, opts) - return mock_records[qname..":"..opts.qtype] or { errcode = 3, errstr = "name error" } - end - end) - - it("in add. section are not stored for non-listed types", function() - mock_records = { - ["demo.service.consul:" .. resolver.TYPE_SRV] = { - { - type = resolver.TYPE_SRV, - class = 1, - name = "demo.service.consul", - target = "192.168.5.232.node.api_test.consul", - priority = 1, - weight = 1, - port = 32776, - ttl = 0, - }, { - type = resolver.TYPE_TXT, -- Not in the `order` as configured ! - class = 1, - name = "192.168.5.232.node.api_test.consul", - txt = "consul-network-segment=", - ttl = 0, - }, - } - } - cli:resolve("demo.service.consul", { return_random = true }) - local success = cli.cache:get("192.168.5.232.node.api_test.consul") - assert.not_equal(resolver.TYPE_TXT, success) - end) - - it("in add. section are stored for listed types", function() - mock_records = { - ["demo.service.consul:" .. resolver.TYPE_SRV] = { - { - type = resolver.TYPE_SRV, - class = 1, - name = "demo.service.consul", - target = "192.168.5.232.node.api_test.consul", - priority = 1, - weight = 1, - port = 32776, - ttl = 0, - }, { - type = resolver.TYPE_A, -- In configured `order` ! - class = 1, - name = "192.168.5.232.node.api_test.consul", - address = "192.168.5.232", - ttl = 0, - }, { - type = resolver.TYPE_TXT, -- Not in the `order` as configured ! - class = 1, - name = "192.168.5.232.node.api_test.consul", - txt = "consul-network-segment=", - ttl = 0, - }, - } - } - local _, err, tries = cli:resolve("demo.service.consul", { return_random = true }) - assert.same(err, "dns server error: 3 name error") - assert.same({ - { - "192.168.5.232.node.api_test.consul:SRV", - "dns server error: 3 name error", - }, - { - "192.168.5.232.node.api_test.consul:A", - "dns server error: 3 name error", - }, - { - "192.168.5.232.node.api_test.consul:AAAA", - "dns server error: 3 name error", - }, - }, tries) - end) - - it("are not overwritten by add. section info", function() - mock_records = { - ["demo.service.consul:" .. resolver.TYPE_SRV] = { - { - type = resolver.TYPE_SRV, - class = 1, - name = "demo.service.consul", - target = "192.168.5.232.node.api_test.consul", - priority = 1, - weight = 1, - port = 32776, - ttl = 0, - }, { - type = resolver.TYPE_A, -- In configured `order` ! - class = 1, - name = "another.name.consul", - address = "192.168.5.232", - ttl = 0, - }, - } - } - cli:_insert_last_type("another.name.consul", resolver.TYPE_AAAA) - cli:resolve("demo.service.consul", { return_random = true }) - local success = cli:_get_last_type("another.name.consul") - assert.equal(resolver.TYPE_AAAA, success) - end) - - end) - - describe("hosts entries", function() -- hosts file names are cached for 10 years, verify that -- it is not overwritten with valid_ttl settings. - -- Regressions reported in https://github.com/Kong/kong/issues/7444 + -- Regressions reported in https://github.test/Kong/kong/issues/7444 local cli, mock_records, config -- luacheck: ignore writefile(resolv_path, "") writefile(hosts_path, "127.0.0.1 myname.lan") @@ -623,11 +494,12 @@ describe("[DNS client cache]", function() end) it("entries from hosts file ignores valid_ttl overrides, Kong/kong #7444", function() + local record = cli:resolve("myname.lan") + assert.equal("127.0.0.1", record[1].address) ngx.sleep(0.2) -- must be > valid_ttl + stale_ttl - local record = cli.cache:get("myname.lan:1") + record = cli.cache:get("myname.lan:all") assert.equal("127.0.0.1", record[1].address) end) end) - end) diff --git a/spec/01-unit/30-new-dns-client/04-client_ipc_spec.lua b/spec/01-unit/30-new-dns-client/04-client_ipc_spec.lua index 441c9958fb35..a31dafda6bfa 100644 --- a/spec/01-unit/30-new-dns-client/04-client_ipc_spec.lua +++ b/spec/01-unit/30-new-dns-client/04-client_ipc_spec.lua @@ -42,15 +42,21 @@ describe("[dns-client] inter-process communication:",function() return count_log_lines("DNS query completed") == num_workers end, 5) - assert.same(count_log_lines("first:query:ipc.com"), 1) + assert.same(count_log_lines("first:query:ipc.test"), 1) assert.same(count_log_lines("first:answers:1.2.3.4"), num_workers) - assert.same(count_log_lines("stale:query:ipc.com"), 1) + assert.same(count_log_lines("stale:query:ipc.test"), 1) assert.same(count_log_lines("stale:answers:1.2.3.4."), num_workers) - assert.same(count_log_lines("stale:broadcast:ipc.com:33"), 1) + -- wait background tasks to finish + helpers.wait_until(function() + return count_log_lines("stale:broadcast:ipc.test:all") == 1 + end, 5) + -- "stale:lru ..." means the progress of the two workers is about the same. -- "first:lru ..." means one of the workers is far behind the other. - assert.same(count_log_lines(":lru delete:ipc.com:33"), 1) + helpers.wait_until(function() + return count_log_lines(":lru delete:ipc.test:all") == 1 + end, 5) end) end) diff --git a/spec/01-unit/30-new-dns-client/05-client_stat_spec.lua b/spec/01-unit/30-new-dns-client/05-client_stat_spec.lua index a613ebd1c8f1..d55c89bbed90 100644 --- a/spec/01-unit/30-new-dns-client/05-client_stat_spec.lua +++ b/spec/01-unit/30-new-dns-client/05-client_stat_spec.lua @@ -37,16 +37,8 @@ describe("[DNS client stats]", function() end) describe("stats", function() - local cli, mock_records, config + local mock_records before_each(function() - config = { - order = { "LAST", "A", "CNAME" }, - error_ttl = 0.1, - empty_ttl = 0.1, - stale_ttl = 1, - } - cli = assert(client_new(config)) - query_func = function(self, qname, opts) local records = mock_records[qname..":"..opts.qtype] if type(records) == "string" then @@ -56,39 +48,91 @@ describe("[DNS client stats]", function() end end) - it("stats", function() + it("resolve SRV", function() mock_records = { - ["hit.com:"..resolver.TYPE_A] = {{ + ["_ldaps._tcp.srv.test:" .. resolver.TYPE_SRV] = {{ + type = resolver.TYPE_SRV, + target = "srv.test", + port = 636, + weight = 10, + priority = 10, + class = 1, + name = "_ldaps._tcp.srv.test", + ttl = 10, + }}, + ["srv.test:" .. resolver.TYPE_A] = {{ type = resolver.TYPE_A, address = "1.2.3.4", class = 1, - name = "hit.com", + name = "srv.test", ttl = 30, }}, - ["nameserver_fail.com:" .. resolver.TYPE_A] = "nameserver failed", - ["stale.com" .. resolver.TYPE_A] = {{ + } + + local cli = assert(client_new()) + cli:resolve("_ldaps._tcp.srv.test") + + local query_last_time + for k, v in pairs(cli.stats) do + if v.query_last_time then + query_last_time = v.query_last_time + v.query_last_time = nil + end + end + assert.match("^%d+$", query_last_time) + + assert.same({ + ["_ldaps._tcp.srv.test:33"] = { + ["query"] = 1, + ["query_succ"] = 1, + }, + ["_ldaps._tcp.srv.test:all"] = { + ["miss"] = 1, + ["runs"] = 1, + }, + }, cli.stats) + end) + + it("resolve all types", function() + mock_records = { + ["hit.test:"..resolver.TYPE_A] = {{ + type = resolver.TYPE_A, + address = "1.2.3.4", + class = 1, + name = "hit.test", + ttl = 30, + }}, + ["nameserver_fail.test:" .. resolver.TYPE_A] = "nameserver failed", + ["stale.test:" .. resolver.TYPE_A] = {{ type = resolver.TYPE_CNAME, - address = "stale.com", + address = "stale.test", class = 1, - name = "stale.com", + name = "stale.test", ttl = 0.1, }}, } + local cli = assert(client_new({ + order = { "A" }, + error_ttl = 0.1, + empty_ttl = 0.1, + stale_ttl = 1, + })) + -- "hit_lru" - cli:resolve("hit.com") - cli:resolve("hit.com") + cli:resolve("hit.test") + cli:resolve("hit.test") -- "hit_shm" - cli.cache.lru:delete("short:hit.com:all") - cli:resolve("hit.com") + cli.cache.lru:delete("hit.test:all") + cli:resolve("hit.test") -- "query_err:nameserver failed" - cli:resolve("nameserver_fail.com") + cli:resolve("nameserver_fail.test") -- "stale" - cli:resolve("stale.com") + cli:resolve("stale.test") sleep(0.2) - cli:resolve("stale.com") + cli:resolve("stale.test") local query_last_time for k, v in pairs(cli.stats) do @@ -97,36 +141,35 @@ describe("[DNS client stats]", function() v.query_last_time = nil end end - - assert.match("^%d+%.%d+ 0%.%d+$", query_last_time) + assert.match("^%d+$", query_last_time) assert.same({ - ["hit.com"] = { + ["hit.test:all"] = { ["hit_lru"] = 1, ["runs"] = 3, ["miss"] = 1, - ["hit_shm"] = 1 + ["hit_shm"] = 1, }, - ["hit.com:1"] = { + ["hit.test:1"] = { ["query"] = 1, - ["query_succ"] = 1 + ["query_succ"] = 1, }, - ["nameserver_fail.com"] = { + ["nameserver_fail.test:all"] = { ["fail"] = 1, - ["runs"] = 1 + ["runs"] = 1, }, - ["nameserver_fail.com:1"] = { + ["nameserver_fail.test:1"] = { ["query"] = 1, - ["query_fail_nameserver"] = 1 + ["query_fail_nameserver"] = 1, }, - ["stale.com"] = { - ["fail"] = 2, - ["runs"] = 2 + ["stale.test:all"] = { + ["miss"] = 2, + ["runs"] = 2, + ["stale"] = 1, }, - ["stale.com:1"] = { + ["stale.test:1"] = { ["query"] = 1, - ["query_fail:name error"] = 1, - ["stale"] = 1 + ["query_fail:empty record received"] = 1, }, }, cli.stats) end) diff --git a/spec/02-integration/04-admin_api/26-dns_client_spec.lua b/spec/02-integration/04-admin_api/26-dns_client_spec.lua index 14bc52e7bc69..19c2a143713a 100644 --- a/spec/02-integration/04-admin_api/26-dns_client_spec.lua +++ b/spec/02-integration/04-admin_api/26-dns_client_spec.lua @@ -38,7 +38,7 @@ for _, strategy in helpers.each_strategy() do assert(type(json.worker.count) == "number") assert(type(json.stats) == "table") - assert(type(json.stats["127.0.0.1"].runs) == "number") + assert(type(json.stats["127.0.0.1:all"].runs) == "number") end) end) diff --git a/spec/fixtures/custom_plugins/kong/plugins/dns-client-test/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/dns-client-test/handler.lua index 1cdfb2021cd9..ba9d3a4f38f3 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/dns-client-test/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/dns-client-test/handler.lua @@ -14,7 +14,7 @@ local PRE = "dns-client-test:" local function test() local phase = "" - local host = "ipc.com" + local host = "ipc.test" -- inject resolver.query require("resty.dns.resolver").query = function(self, name, opts) diff --git a/spec/helpers/dns.lua b/spec/helpers/dns.lua index 689f8a980c9d..6ddfda8bb55a 100644 --- a/spec/helpers/dns.lua +++ b/spec/helpers/dns.lua @@ -41,6 +41,7 @@ function _M.dnsExpire(client, record) local dnscache = client.getcache() dnscache:delete(record[1].name .. ":" .. record[1].type) dnscache:delete("short:" .. record[1].name .. ":" .. "all") + dnscache:delete(record[1].name .. ":" .. "all") record.expire = gettime() - 1 end @@ -84,8 +85,8 @@ function _M.dnsSRV(client, records, staleTtl) -- create key, and insert it local key = records[1].name..":"..records[1].type dnscache:set(key, records, records[1].ttl + (staleTtl or 4)) - -- insert last-succesful lookup type - client.getobj():_insert_last_type(records[1].name, records[1].type) + key = records[1].name..":all" + dnscache:set(key, records, records[1].ttl + (staleTtl or 4)) return records end @@ -126,8 +127,8 @@ function _M.dnsA(client, records, staleTtl) -- create key, and insert it local key = records[1].name..":"..records[1].type dnscache:set(key, records, records[1].ttl) - -- insert last-succesful lookup type - client.getobj():_insert_last_type(records[1].name, records[1].type) + key = records[1].name..":all" + dnscache:set(key, records, records[1].ttl) return records end @@ -167,8 +168,8 @@ function _M.dnsAAAA(client, records, staleTtl) -- create key, and insert it local key = records[1].name..":"..records[1].type dnscache:set(key, records, records[1].ttl + (staleTtl or 4)) - -- insert last-succesful lookup type - client.getobj():_insert_last_type(records[1].name, records[1].type) + key = records[1].name..":all" + dnscache:set(key, records, records[1].ttl + (staleTtl or 4)) return records end