[resty.resolver] properly cache queries with scope

queries made by scope search would not be cached properly
as the cache would ask only for the query without the scope

so now try to retrive from cache during each lookup step

Author: mikz

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Handle partial credentials [PR #409](https://github.com/3scale/apicast/pull/409)
 - Crash when configuration endpoint was missing [PR #417](https://github.com/3scale/apicast/pull/417)
 - Fix double queries to not fully qualified domains [PR #419](https://github.com/3scale/apicast/pull/419)
+- Fix caching DNS queries with scope (like on OpenShift) [PR #420](https://github.com/3scale/apicast/pull/420)
 
 ### Changed
 

apicast/src/resty/resolver.lua

@@ -4,6 +4,7 @@ local open = io.open
 local gmatch = string.gmatch
 local match = string.match
 local format = string.format
+local find = string.find
 local rep = string.rep
 local unpack = unpack
 local insert = table.insert
@@ -197,6 +198,10 @@ local function is_ip(address)
   end
 end
 
+local function is_fqdn(name)
+  return find(name, '.', 1, true)
+end
+
 local servers_mt = {
   __tostring = function(t)
     return format(rep('%s', #t, ' '), unpack(t))
@@ -217,7 +222,36 @@ end
 
 local empty = {}
 
-local function lookup(dns, qname, search, options)
+local function valid_answers(answers)
+  return answers and not answers.errcode and #answers > 0
+end
+
+local function search_dns(search, dns, cache, qname, options, stale)
+  local answers, err
+
+  for i=1, #search do
+    local query = qname .. '.' .. search[i]
+    ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' search: ', search[i], ' query: ', query)
+
+    answers, err = cache:get(query, stale)
+    if valid_answers(answers) then break end
+
+    answers, err = dns:query(query, options)
+    if valid_answers(answers) then
+      cache:save(answers)
+      break
+    end
+  end
+
+  return answers, err
+end
+
+function _M.lookup(self, qname, stale)
+  local search = self.search
+  local options = self.options
+  local cache = self.cache
+  local dns = self.dns
+
   ngx.log(ngx.DEBUG, 'resolver query: ', qname)
 
   local answers, err
@@ -226,18 +260,19 @@ local function lookup(dns, qname, search, options)
     ngx.log(ngx.DEBUG, 'host is ip address: ', qname)
     answers = { new_answer(qname) }
   else
-    for i=1, #search do
-      local query = qname .. '.' .. search[i]
-      ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' search: ', search[i], ' query: ', query)
-      answers, err = dns:query(query, options)
-
-      if answers and not answers.errcode and #answers > 0 then
-        break
-      end
+    if is_fqdn(qname) then
+      answers, err = cache:get(qname, stale)
     end
+
+    if not valid_answers(answers) then
+      answers, err = search_dns(self.search, self.dns, cache, qname, self.options, stale)
+    end
+
   end
 
   ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
+
+
   return answers, err
 end
 
@@ -253,20 +288,12 @@ function _M.get_servers(self, qname, opts)
     return nil, 'query missing'
   end
 
-  local cache = self.cache
-  local search = self.search or _M.search
-
   -- TODO: pass proper options to dns resolver (like SRV query type)
 
   local sema, key = synchronization:acquire(format('qname:%s:qtype:%s', qname, 'A'))
   local ok = sema:wait(0)
 
-  local answers, err = cache:get(qname, not ok)
-
-  if not answers or err or #answers.addresses == 0 then
-    answers, err = lookup(dns, qname, search, self.options)
-    cache:save(answers)
-  end
+  local answers, err = self:lookup(qname, not ok)
 
   if ok then
     -- cleanup the key so we don't have unbounded growth of this table

spec/resty/resolver/http_spec.lua

@@ -13,7 +13,8 @@ describe('resty.resolver.http', function()
   describe(':connect', function()
     it('resolves localhost', function()
       local client = _M.new()
-      client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown', ttl = 1800 } })
+      client:set_timeout(1000)
+      client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
       assert(client:connect('unknown', 1984))
       assert.equal('unknown', client.host)
       assert.equal(1984, client.port)

spec/resty/resolver/socket_spec.lua

@@ -12,8 +12,11 @@ describe('resty.resolver.socket', function()
 
   describe(':connect', function()
     it('resolves localhost', function()
-      local wrapper = _M.new(ngx.socket.tcp())
-      wrapper.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown', ttl = 1800 } })
+      local sock = ngx.socket.tcp()
+      sock:settimeout(1000)
+      local wrapper = _M.new(sock)
+
+      wrapper.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
       assert(wrapper:connect('unknown', 1984))
       assert.equal('unknown', wrapper.host)
       assert.equal(1984, wrapper.port)

t/001-management.t

@@ -153,7 +153,7 @@ Could not resolve GET /foobar - nil
 === TEST 8: boot
 exposes boot function
 --- main_config
-env THREESCALE_PORTAL_ENDPOINT=http://localhost:$TEST_NGINX_SERVER_PORT/config/;
+env THREESCALE_PORTAL_ENDPOINT=http://localhost.local:$TEST_NGINX_SERVER_PORT/config/;
 env RESOLVER=127.0.0.1:1953;
 env APICAST_MANAGEMENT_API=debug;
 --- http_config
@@ -170,14 +170,14 @@ POST /boot
 --- error_code: 200
 --- udp_listen: 1953
 --- udp_reply eval
-$::dns->("localhost", "127.0.0.1", 60)
+$::dns->("localhost.local", "127.0.0.1", 60)
 --- no_error_log
 [error]
 
 === TEST 9: boot called twice
 keeps the same configuration
 --- main_config
-env THREESCALE_PORTAL_ENDPOINT=http://localhost:$TEST_NGINX_SERVER_PORT/config/;
+env THREESCALE_PORTAL_ENDPOINT=http://localhost.local:$TEST_NGINX_SERVER_PORT/config/;
 env RESOLVER=127.0.0.1:1953;
 env APICAST_MANAGEMENT_API=debug;
 --- http_config
@@ -199,7 +199,7 @@ POST /test
 --- error_code: 200
 --- udp_listen: 1953
 --- udp_reply eval
-$::dns->("localhost", "127.0.0.1", 60)
+$::dns->("localhost.local", "127.0.0.1", 60)
 --- no_error_log
 [error]