Merge pull request #420 from 3scale/resty-resolver-cache-search

[resty.resolver] properly cache queries with scope

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/.s2i/bin/run

@@ -34,7 +34,12 @@ else
   apicast=apicast
 fi
 
-dnsmasq --listen-address=127.0.0.1 --all-servers --no-host --no-hosts --log-facility=- --cache-size=1000 --port=5353 --no-negcache --server="${RESOLVER:-}"
+dnsmasq --listen-address=127.0.0.1 --port=5353 \
+  --all-servers --no-host --no-hosts \
+  --cache-size=1000 --no-negcache \
+  --server="${RESOLVER:-}" \
+  --log-facility=- ${DNSMASQ_OPTIONS:-} \
+
 export RESOLVER=127.0.0.1:5353
 
 exec "${apicast}" "$@"

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,38 @@ end
 
 local empty = {}
 
-local function lookup(dns, qname, search, options)
+local function valid_answers(answers)
+  return answers and not answers.errcode and #answers > 0 and (not answers.addresses or #answers.addresses > 0)
+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
+
+  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 cache = self.cache
+
   ngx.log(ngx.DEBUG, 'resolver query: ', qname)
 
   local answers, err
@@ -226,18 +262,18 @@ 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, qname, stale)
     end
+
   end
 
   ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
+
   return answers, err
 end
 
@@ -253,20 +289,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)

spec/resty/resolver_spec.lua

@@ -125,6 +125,10 @@ describe('resty.resolver', function()
     end)
   end)
 
+  describe(':lookup', function()
+    pending('does query when cached cname missing address')
+  end)
+
   describe('.parse_nameservers', function()
     local tmpname
 

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]