diff --git a/CHANGELOG.md b/CHANGELOG.md index c5356f746..fbe9d73d4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/gateway/src/resty/resolver.lua b/gateway/src/resty/resolver.lua index 38a73886c..8733f8149 100644 --- a/gateway/src/resty/resolver.lua +++ b/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,7 +355,7 @@ 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 @@ -342,6 +363,13 @@ local function resolve_upstream(qname) 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 diff --git a/spec/resty/resolver_spec.lua b/spec/resty/resolver_spec.lua index e0e85893f..fdedcd40e 100644 --- a/spec/resty/resolver_spec.lua +++ b/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()