[resolver] refactor search list

Now return the full list of names to search for. This allows us to iterate
over the list and check the cache entry before querying the DNS server.

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed Conditional policy evaluating incorrectly: second policy in policy chain that implement export() always triggers [PR #1485](https://github.com/3scale/APIcast/pull/1485) [THREESCALE-9320](https://issues.redhat.com/browse/THREESCALE-9320)
 - Fix APIcast using stale configuration for deleted products [PR #1488](https://github.com/3scale/APIcast/pull/1488) [THREESCALE-10130](https://issues.redhat.com/browse/THREESCALE-10130)
+- Fixed DNS cache miss [PR #1500](https://github.com/3scale/APIcast/pull/1500) [THEESCALE-9301](https://issues.redhat.com/browse/THREESCALE-9301)
 
 ### Added
 

gateway/src/resty/resolver.lua

@@ -1,3 +1,12 @@
+-- DNS Resolver
+--
+-- _NOTES_:
+--
+-- 1. Parsing the config file (/etc/resolv.conf) use blocking I/O, so it should
+--    be called in init phase. See `init` for details
+-- 2. TTL for records is the TTL returned by DNS server and won't be updated
+--    while the client serves the records from its cache
+
 local setmetatable = setmetatable
 local next = next
 local open = io.open
@@ -9,15 +18,17 @@ local find = string.find
 local insert = table.insert
 local concat = table.concat
 local io_type = io.type
+local table_new = require("table.new")
 
 require('resty.core.regex') -- to allow use of ngx.re.match in the init phase
 
+local ngx_re = require('ngx.re')
+local re_split = ngx_re.split
 local re_match = ngx.re.match
 local resolver_cache = require 'resty.resolver.cache'
 local dns_client = require 'resty.resolver.dns_client'
 local resty_env = require 'resty.env'
 local upstream = require 'ngx.upstream'
-local re = require('ngx.re')
 local semaphore = require "ngx.semaphore".new(1)
 local synchronization = require('resty.synchronization').new(1)
 
@@ -126,7 +137,7 @@ function _M.parse_nameservers(path)
     insert(nameservers, resolver)
   end
 
-  for _,line in ipairs(re.split(resolv_conf, "\n+")) do
+  for _,line in ipairs(re_split(resolv_conf, "\n+")) do
 
     local domains = match(line, '^search%s+([^\n]+)')
 
@@ -182,6 +193,8 @@ function _M.nameservers()
   return _M._nameservers
 end
 
+--- Initialize the resolver.
+-- @param path to resolve.conf file
 function _M.init(path)
   _M.init_nameservers(path)
 end
@@ -285,42 +298,50 @@ local function valid_answers(answers)
   return answers and not answers.errcode and #answers > 0 and (not answers.addresses or #answers.addresses > 0)
 end
 
+-- construct search list from resolv options: search
+-- @param search table of search domain
+-- @param qname the name to query for
+-- @return table with search names
+local function search_list(search, qname)
+  -- FQDN
+  if sub(qname, -1) == "." then
+    return {qname}
+  end
+
+  local names = table_new(#search +1, 0)
+  for i=1, #search do
+    names[i] = qname .. "." .. search[i]
+  end
+  return names
+end
+
 local function search_dns(self, qname, stale)
 
   local search = self.search
   local dns = self.dns
   local options = self.options
   local cache = self.cache
+  local answers, err
+  local names = search_list(search, qname)
 
-  local function get_answer(query)
-    local answers, err
+  -- Search the cache first
+  for _, query in ipairs(names) do
     answers, err = cache:get(query, stale)
     if valid_answers(answers) then
       return answers, err
     end
+  end
+
+  -- Nothing found, send DNS queries to resolve all `names` and return the
+  -- first valid answers
+  for _, query in ipairs(names) do
+    ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' query: ', query)
 
     answers, err = dns:query(query, options)
     if valid_answers(answers) then
       cache:save(answers)
       return answers, err
     end
-    return nil, err
-  end
-
-  if sub(qname, -1) == "." then
-    local query = sub(qname, 1 ,-2)
-    ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' query: ', query)
-    return get_answer(query)
-  end
-
-  local answer, err
-  for i=1, #search do
-    local query = qname .. '.' .. search[i]
-    ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' search: ', search[i], ' query: ', query)
-    answer, err = get_answer(query)
-    if answer then
-      return answer, err
-    end
   end
 
   return nil, err
@@ -334,14 +355,21 @@ local function resolve_upstream(qname)
   end
 
   for i=1, #peers do
-    local m = re.split(peers[i].name, ':', 'oj')
+    local m = re_split(peers[i].name, ':', 'oj')
 
     peers[i] = new_answer(m[1], m[2])
   end
 
   return peers
 end
 
+-- Queries the DNS for this qname. If nothing is in the cache, a new DNS query
+-- will be sent. If stale data is available in the cache, first query is going
+-- to refresh the query while all others use stale cache data.
+--
+-- @param qname the name to look for
+-- @param stale if true return stale data
+-- @return dns `answers + nil`, or `nil + error
 function _M.lookup(self, qname, stale)
   local cache = self.cache
 

spec/resty/resolver_spec.lua

@@ -1,5 +1,6 @@
 local resty_resolver = require 'resty.resolver'
 local resolver_cache = require 'resty.resolver.cache'
+local string_byte = string.byte
 
 describe('resty.resolver', function()
 
@@ -127,13 +128,23 @@ describe('resty.resolver', function()
 
   describe(':lookup', function()
     local  resolver
+    local dns
 
     before_each(function()
-      local dns = {
+      dns = {
         query = spy.new(function(_, name)
-          return {
-            { name = name , address = '127.0.0.1' }
-          }
+          -- FQDN, drop the last '.' as the answer does not contain it
+
+          if string_byte(name, -1) == string_byte(".") then
+            name = name:sub(1, -2)
+          end
+          if name == '3scale' then
+            return { errcode = 3, errstr = 'name error' }
+          else
+            return {
+              { name = name , address = '127.0.0.1' }
+            }
+          end
         end)
       }
       resolver = resty_resolver.new(dns, { cache = resolver_cache.new(), search = {"foo.com"}})
@@ -151,6 +162,38 @@ describe('resty.resolver', function()
       assert.same(err, nil)
     end)
 
+    it("search entry in cache before sending new request", function()
+      local dns = {
+        query = spy.new(function(_, name)
+          -- FQDN, drop the last '.' as the answer does not contain it
+
+          if string_byte(name, -1) == string_byte(".") then
+            name = name:sub(1, -2)
+          end
+          if name == '3scale' then
+            return { errcode = 3, errstr = 'name error' }
+          else
+            return {
+              { name = name , address = '127.0.0.1' }
+            }
+          end
+        end)
+      }
+      resolver = resty_resolver.new(dns, { cache = resolver_cache.new(), search = {"foo.com"}})
+      -- First populate the cache with new entry
+      local answer, err = resolver:lookup('3scale.foo.com.')
+      assert.same("3scale.foo.com", answer[1].name)
+      assert.same(err, nil)
+
+      -- Now search for shortname, it should append the name with search
+      -- domain and lookup in cache first.
+      local answer, err = resolver:lookup('3scale')
+      assert.same("3scale.foo.com", answer[1].name)
+      assert.same(err, nil)
+
+      -- Should only called 1 time
+      assert.spy(dns.query).was_called(1)
+    end)
   end)
 
   describe('.parse_resolver', function()