From 5bad24d7038648d4055c0396997dab1991e3076b Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Fri, 8 Sep 2017 16:05:04 +0700 Subject: [PATCH] [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 --- CHANGELOG.md | 1 + apicast/src/resty/resolver.lua | 65 +++++++++++++++++++++-------- spec/resty/resolver/http_spec.lua | 3 +- spec/resty/resolver/socket_spec.lua | 7 +++- t/001-management.t | 8 ++-- 5 files changed, 59 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c522d686..922a0a858 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apicast/src/resty/resolver.lua b/apicast/src/resty/resolver.lua index 4490c7ffe..60cb9dade 100644 --- a/apicast/src/resty/resolver.lua +++ b/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 +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,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, qname, stale) end + end ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers') + + return answers, err end @@ -253,20 +290,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 diff --git a/spec/resty/resolver/http_spec.lua b/spec/resty/resolver/http_spec.lua index ccd406cf1..87c43ed20 100644 --- a/spec/resty/resolver/http_spec.lua +++ b/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) diff --git a/spec/resty/resolver/socket_spec.lua b/spec/resty/resolver/socket_spec.lua index 8c6d6056f..f41b05df7 100644 --- a/spec/resty/resolver/socket_spec.lua +++ b/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) diff --git a/t/001-management.t b/t/001-management.t index c921cd514..e9ec089c8 100644 --- a/t/001-management.t +++ b/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]