Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
fix(cache) do not set success types for records we're not resolving
Browse files Browse the repository at this point in the history
If the additional section would contain a TXT record, then that would
be set as the success type, and `last` would try and look for that.
The cache lookup would succeed, but since it doesn't have a `address`
field, `toip()` would return `nil`.

fixes Kong/kong#3210
  • Loading branch information
Tieske committed Feb 12, 2018
1 parent fd2ab4d commit 5ca5ee1
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 4 deletions.
115 changes: 115 additions & 0 deletions spec/client_cache_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,119 @@ describe("DNS client cache", function()

end)

-- ==============================================
-- success type caching
-- ==============================================


describe("success types", function()

local lrucache, mock_records, config
before_each(function()
config = {
nameservers = { "8.8.8.8" },
ndots = 1,
search = { "domain.com" },
hosts = {},
resolvConf = {},
order = { "LAST", "SRV", "A", "AAAA", "CNAME" },
badTtl = 0.5,
staleTtl = 0.5,
enable_ipv6 = false,
}
assert(client.init(config))
lrucache = client.getcache()

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:" .. client.TYPE_SRV] = {
{
type = client.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 = client.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,
},
}
}
client.toip("demo.service.consul")
local success = client.getcache():get("192.168.5.232.node.api_test.consul")
assert.not_equal(client.TYPE_TXT, success)
end)

it("in add. section are stored for listed types", function()
mock_records = {
["demo.service.consul:" .. client.TYPE_SRV] = {
{
type = client.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 = client.TYPE_A, -- In configured `order` !
class = 1,
name = "192.168.5.232.node.api_test.consul",
address = "192.168.5.232",
ttl = 0,
}, {
type = client.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,
},
}
}
client.toip("demo.service.consul")
local success = client.getcache():get("192.168.5.232.node.api_test.consul")
assert.equal(client.TYPE_A, success)
end)

it("are not overwritten by add. section info", function()
mock_records = {
["demo.service.consul:" .. client.TYPE_SRV] = {
{
type = client.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 = client.TYPE_A, -- In configured `order` !
class = 1,
name = "another.name.consul",
address = "192.168.5.232",
ttl = 0,
},
}
}
client.getcache():set("another.name.consul", client.TYPE_AAAA)
client.toip("demo.service.consul")
local success = client.getcache():get("another.name.consul")
assert.equal(client.TYPE_AAAA, success)
end)

end)

end)
25 changes: 21 additions & 4 deletions src/resty/dns/client.lua
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ local staleTtl -- ttl (in seconds) to serve stale data (while new lo
local cacheSize -- size of the lru cache
local noSynchronisation
local orderValids = {"LAST", "SRV", "A", "AAAA", "CNAME"} -- default order to query
local typeOrder -- array with order of types to try

for _,v in ipairs(orderValids) do orderValids[v:upper()] = v end

-- create module table
Expand Down Expand Up @@ -201,10 +203,26 @@ local function cachegetsuccess(qname)
end

-- Sets the last succesful query type.
-- @qparam name resolved
-- @qtype query/record type to set, or ˋnilˋ to clear
-- @return ˋtrueˋ
-- Only if the type provided is in the list of types to try.
-- @param qname name resolved
-- @param qtype query/record type to set, or ˋnilˋ to clear
-- @return `true` if set, or `false` if not
local function cachesetsuccess(qname, qtype)

-- Test whether the qtype value is in our search/order list
local validType = false
for _, t in ipairs(typeOrder) do
if t == qtype then
validType = true
break
end
end
if not validType then
-- the qtype is not in the list, so we're not setting it as the
-- success type
return false
end

dnscache:set(qname, qtype)
return true
end
Expand Down Expand Up @@ -289,7 +307,6 @@ end
-- @section resolving


local typeOrder
local poolMaxWait
local poolMaxRetry

Expand Down

0 comments on commit 5ca5ee1

Please sign in to comment.