fix(cache) do not set success types for records we're not resolving

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

README.md

@@ -44,6 +44,8 @@ Versioning is strictly based on [Semantic Versioning](https://semver.org/)
   the `host:getPeer()` function signature changed, this is a breaking change.
 - Fix: do not fail initialization without nameservers. 
 - Fix: properly recognize IPv6 in square brackets from the /etc/hosts file.
+- Fix: do not set success-type to types we're not looking for. Fixes
+  [Kong issue #3210](https://github.com/Kong/kong/issues/3210).
 
 ### 1.0.0 (14-Dec-2017) Fixes and IPv6
 

spec/client_cache_spec.lua

@@ -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)

src/resty/dns/client.lua

@@ -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
@@ -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
@@ -289,7 +307,6 @@ end
 -- @section resolving
 
 
-local typeOrder
 local poolMaxWait
 local poolMaxRetry