Skip to content

Commit 342d6d0

Browse files
committed
[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
1 parent cb53a7b commit 342d6d0

File tree

6 files changed

+62
-25
lines changed

6 files changed

+62
-25
lines changed

CHANGELOG.md

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

2021
### Changed
2122

apicast/src/resty/resolver.lua

+46-18
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ local open = io.open
44
local gmatch = string.gmatch
55
local match = string.match
66
local format = string.format
7+
local find = string.find
78
local rep = string.rep
89
local unpack = unpack
910
local insert = table.insert
@@ -197,6 +198,10 @@ local function is_ip(address)
197198
end
198199
end
199200

201+
local function is_fqdn(name)
202+
return find(name, '.', 1, true)
203+
end
204+
200205
local servers_mt = {
201206
__tostring = function(t)
202207
return format(rep('%s', #t, ' '), unpack(t))
@@ -217,7 +222,38 @@ end
217222

218223
local empty = {}
219224

220-
local function lookup(dns, qname, search, options)
225+
local function valid_answers(answers)
226+
return answers and not answers.errcode and #answers > 0 and (not answers.addresses or #answers.addresses > 0)
227+
end
228+
229+
local function search_dns(self, qname, stale)
230+
local search = self.search
231+
local dns = self.dns
232+
local options = self.options
233+
local cache = self.cache
234+
235+
local answers, err
236+
237+
for i=1, #search do
238+
local query = qname .. '.' .. search[i]
239+
ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' search: ', search[i], ' query: ', query)
240+
241+
answers, err = cache:get(query, stale)
242+
if valid_answers(answers) then break end
243+
244+
answers, err = dns:query(query, options)
245+
if valid_answers(answers) then
246+
cache:save(answers)
247+
break
248+
end
249+
end
250+
251+
return answers, err
252+
end
253+
254+
function _M.lookup(self, qname, stale)
255+
local cache = self.cache
256+
221257
ngx.log(ngx.DEBUG, 'resolver query: ', qname)
222258

223259
local answers, err
@@ -226,18 +262,18 @@ local function lookup(dns, qname, search, options)
226262
ngx.log(ngx.DEBUG, 'host is ip address: ', qname)
227263
answers = { new_answer(qname) }
228264
else
229-
for i=1, #search do
230-
local query = qname .. '.' .. search[i]
231-
ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' search: ', search[i], ' query: ', query)
232-
answers, err = dns:query(query, options)
233-
234-
if answers and not answers.errcode and #answers > 0 then
235-
break
236-
end
265+
if is_fqdn(qname) then
266+
answers, err = cache:get(qname, stale)
267+
end
268+
269+
if not valid_answers(answers) then
270+
answers, err = search_dns(self, qname, stale)
237271
end
272+
238273
end
239274

240275
ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
276+
241277
return answers, err
242278
end
243279

@@ -253,20 +289,12 @@ function _M.get_servers(self, qname, opts)
253289
return nil, 'query missing'
254290
end
255291

256-
local cache = self.cache
257-
local search = self.search or _M.search
258-
259292
-- TODO: pass proper options to dns resolver (like SRV query type)
260293

261294
local sema, key = synchronization:acquire(format('qname:%s:qtype:%s', qname, 'A'))
262295
local ok = sema:wait(0)
263296

264-
local answers, err = cache:get(qname, not ok)
265-
266-
if not answers or err or #answers.addresses == 0 then
267-
answers, err = lookup(dns, qname, search, self.options)
268-
cache:save(answers)
269-
end
297+
local answers, err = self:lookup(qname, not ok)
270298

271299
if ok then
272300
-- cleanup the key so we don't have unbounded growth of this table

spec/resty/resolver/http_spec.lua

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ describe('resty.resolver.http', function()
1313
describe(':connect', function()
1414
it('resolves localhost', function()
1515
local client = _M.new()
16-
client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown', ttl = 1800 } })
16+
client:set_timeout(1000)
17+
client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
1718
assert(client:connect('unknown', 1984))
1819
assert.equal('unknown', client.host)
1920
assert.equal(1984, client.port)

spec/resty/resolver/socket_spec.lua

+5-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ describe('resty.resolver.socket', function()
1212

1313
describe(':connect', function()
1414
it('resolves localhost', function()
15-
local wrapper = _M.new(ngx.socket.tcp())
16-
wrapper.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown', ttl = 1800 } })
15+
local sock = ngx.socket.tcp()
16+
sock:settimeout(1000)
17+
local wrapper = _M.new(sock)
18+
19+
wrapper.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
1720
assert(wrapper:connect('unknown', 1984))
1821
assert.equal('unknown', wrapper.host)
1922
assert.equal(1984, wrapper.port)

spec/resty/resolver_spec.lua

+4
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ describe('resty.resolver', function()
125125
end)
126126
end)
127127

128+
describe(':lookup', function()
129+
pending('does query when cached cname missing address')
130+
end)
131+
128132
describe('.parse_nameservers', function()
129133
local tmpname
130134

t/001-management.t

+4-4
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ Could not resolve GET /foobar - nil
153153
=== TEST 8: boot
154154
exposes boot function
155155
--- main_config
156-
env THREESCALE_PORTAL_ENDPOINT=http://localhost:$TEST_NGINX_SERVER_PORT/config/;
156+
env THREESCALE_PORTAL_ENDPOINT=http://localhost.local:$TEST_NGINX_SERVER_PORT/config/;
157157
env RESOLVER=127.0.0.1:1953;
158158
env APICAST_MANAGEMENT_API=debug;
159159
--- http_config
@@ -170,14 +170,14 @@ POST /boot
170170
--- error_code: 200
171171
--- udp_listen: 1953
172172
--- udp_reply eval
173-
$::dns->("localhost", "127.0.0.1", 60)
173+
$::dns->("localhost.local", "127.0.0.1", 60)
174174
--- no_error_log
175175
[error]
176176
177177
=== TEST 9: boot called twice
178178
keeps the same configuration
179179
--- main_config
180-
env THREESCALE_PORTAL_ENDPOINT=http://localhost:$TEST_NGINX_SERVER_PORT/config/;
180+
env THREESCALE_PORTAL_ENDPOINT=http://localhost.local:$TEST_NGINX_SERVER_PORT/config/;
181181
env RESOLVER=127.0.0.1:1953;
182182
env APICAST_MANAGEMENT_API=debug;
183183
--- http_config
@@ -199,7 +199,7 @@ POST /test
199199
--- error_code: 200
200200
--- udp_listen: 1953
201201
--- udp_reply eval
202-
$::dns->("localhost", "127.0.0.1", 60)
202+
$::dns->("localhost.local", "127.0.0.1", 60)
203203
--- no_error_log
204204
[error]
205205

0 commit comments

Comments
 (0)