From b247e0ae60250a170f8d1c213540d5243b8a7c29 Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Mon, 12 Aug 2024 12:41:32 +1000
Subject: [PATCH 1/6] Rename semaphore variable name for better readability

---
 gateway/src/resty/resolver.lua | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/gateway/src/resty/resolver.lua b/gateway/src/resty/resolver.lua
index 1b204b077..38a73886c 100644
--- a/gateway/src/resty/resolver.lua
+++ b/gateway/src/resty/resolver.lua
@@ -18,11 +18,9 @@ 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"
+local semaphore = require "ngx.semaphore".new(1)
 local synchronization = require('resty.synchronization').new(1)
 
-local init = semaphore.new(1)
-
 local default_resolver_port = 53
 
 local _M = {
@@ -171,14 +169,14 @@ function _M.init_nameservers(path)
 end
 
 function _M.nameservers()
-  local ok, _ = init:wait(0)
+  local ok, _ = semaphore:wait(0)
 
   if ok and #(_M._nameservers) == 0 then
     _M.init()
   end
 
   if ok then
-    init:post()
+    semaphore:post()
   end
 
   return _M._nameservers

From 809f033616f6c586498b7be61c56767813ac8f79 Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Fri, 16 Aug 2024 10:25:04 +1000
Subject: [PATCH 2/6] [script] Update outdated path in resolver script

---
 script/resolver | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/script/resolver b/script/resolver
index 1d20f52ad..b52e32242 100755
--- a/script/resolver
+++ b/script/resolver
@@ -1,3 +1,3 @@
 #!/usr/bin/env sh
 
-exec resty -I apicast/src script/resolver.lua "$@"
+exec resty -I gateway/src script/resolver.lua "$@"

From f1d4d009f89d7e5fa9356e4706c2e0c454aca2fd Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Tue, 22 Oct 2024 22:29:25 +1000
Subject: [PATCH 3/6] [resolver] Always lookup in the cache first

---
 gateway/src/resty/resolver.lua | 47 +++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/gateway/src/resty/resolver.lua b/gateway/src/resty/resolver.lua
index 38a73886c..3d933a7f7 100644
--- a/gateway/src/resty/resolver.lua
+++ b/gateway/src/resty/resolver.lua
@@ -284,6 +284,21 @@ local empty = {}
 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 resolve_upstream(qname)
+  local peers, err = upstream.get_primary_peers(qname)
+
+  if not peers then
+    return nil, err
+  end
+
+  for i=1, #peers do
+    local m = re.split(peers[i].name, ':', 'oj')
+
+    peers[i] = new_answer(m[1], m[2])
+  end
+
+  return peers
+end
 
 local function search_dns(self, qname, stale)
 
@@ -326,21 +341,6 @@ local function search_dns(self, qname, stale)
   return nil, err
 end
 
-local function resolve_upstream(qname)
-  local peers, err = upstream.get_primary_peers(qname)
-
-  if not peers then
-    return nil, err
-  end
-
-  for i=1, #peers do
-    local m = re.split(peers[i].name, ':', 'oj')
-
-    peers[i] = new_answer(m[1], m[2])
-  end
-
-  return peers
-end
 
 function _M.lookup(self, qname, stale)
   local cache = self.cache
@@ -353,20 +353,19 @@ function _M.lookup(self, qname, stale)
     ngx.log(ngx.DEBUG, 'host is ip address: ', qname)
     answers = { new_answer(qname) }
   else
-    if is_fqdn(qname) then
-      answers, err = cache:get(qname, stale)
-    else
+    answers, err = cache:get(qname, stale)
+    if valid_answers(answers) then
+      return answers, nil
+    end
+
+    if not is_fqdn(qname) then
       answers, err = resolve_upstream(qname)
     end
 
     if not valid_answers(answers) then
-      answers, err = search_dns(self, qname, stale)
+      return search_dns(self, qname, stale)
     end
-
   end
-
-  ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
-
   return answers, err
 end
 
@@ -389,6 +388,8 @@ function _M.get_servers(self, qname, opts)
 
   local answers, err = self:lookup(qname, not ok)
 
+  ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
+
   if ok then
     -- cleanup the key so we don't have unbounded growth of this table
     synchronization:release(key)

From ce6d24926be1b0d5c365116c9400d6b4ed46f3dc Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Tue, 22 Oct 2024 22:37:03 +1000
Subject: [PATCH 4/6] [resolver] refactor search list

---
 gateway/src/resty/resolver.lua | 44 +++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/gateway/src/resty/resolver.lua b/gateway/src/resty/resolver.lua
index 3d933a7f7..25fe12b59 100644
--- a/gateway/src/resty/resolver.lua
+++ b/gateway/src/resty/resolver.lua
@@ -20,6 +20,7 @@ local upstream = require 'ngx.upstream'
 local re = require('ngx.re')
 local semaphore = require "ngx.semaphore".new(1)
 local synchronization = require('resty.synchronization').new(1)
+local table_new = require("table.new")
 
 local default_resolver_port = 53
 
@@ -284,6 +285,7 @@ local empty = {}
 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 resolve_upstream(qname)
   local peers, err = upstream.get_primary_peers(qname)
 
@@ -300,15 +302,36 @@ local function resolve_upstream(qname)
   return peers
 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
+    local query = sub(qname, 1 ,-2)
+    return {query}
+  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 queries = search_list(search, qname)
+  local answers, err
 
-  local function get_answer(query)
-    local answers, err
+  for _, query in ipairs(queries) do
+    ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' query: ', query)
     answers, err = cache:get(query, stale)
     if valid_answers(answers) then
       return answers, err
@@ -319,23 +342,6 @@ local function search_dns(self, qname, stale)
       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

From 8d92b0d1f4146379c30a746be0d8a5501726f70e Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Wed, 23 Oct 2024 17:12:15 +1000
Subject: [PATCH 5/6] [resolver] Refactor cache handling

Previously, we iterated through the answer and used the name contained in the
answer as the cache key. The problem with this is that if search domain is
added to the search query, the next query lookup will not hit the cache until
the same search domain is appened to the query.

With this PR, we use a combination of original qname and qtype as cache key
instead
---
 gateway/src/resty/resolver.lua       | 30 +++++++++++++++++----------
 gateway/src/resty/resolver/cache.lua | 29 +++++++++++++++++++-------
 spec/resty/resolver/cache_spec.lua   | 31 +++++++++++++++++++---------
 spec/resty/resolver/http_spec.lua    |  2 +-
 spec/resty/resolver/socket_spec.lua  |  2 +-
 spec/resty/resolver_spec.lua         | 27 ++++++++++++++++++++++--
 6 files changed, 89 insertions(+), 32 deletions(-)

diff --git a/gateway/src/resty/resolver.lua b/gateway/src/resty/resolver.lua
index 25fe12b59..d2214a79d 100644
--- a/gateway/src/resty/resolver.lua
+++ b/gateway/src/resty/resolver.lua
@@ -28,6 +28,8 @@ local _M = {
   _VERSION = '0.1',
 }
 
+local TYPE_A = 1
+
 local mt = { __index = _M }
 
 local function read_resolv_conf(path)
@@ -321,25 +323,21 @@ local function search_list(search, qname)
   return names
 end
 
-local function search_dns(self, qname, stale)
+local function search_dns(self, qname)
 
   local search = self.search
   local dns = self.dns
   local options = self.options
-  local cache = self.cache
   local queries = search_list(search, qname)
   local answers, err
 
+  -- Nothing found, append search domain and query DNS server
+  -- Return the first valid answer
   for _, query in ipairs(queries) do
     ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' query: ', query)
-    answers, err = cache:get(query, stale)
-    if valid_answers(answers) then
-      return answers, err
-    end
 
     answers, err = dns:query(query, options)
     if valid_answers(answers) then
-      cache:save(answers)
       return answers, err
     end
   end
@@ -350,6 +348,8 @@ end
 
 function _M.lookup(self, qname, stale)
   local cache = self.cache
+  local options = self.options
+  local qtype = options.qtype or TYPE_A
 
   ngx.log(ngx.DEBUG, 'resolver query: ', qname)
 
@@ -359,19 +359,28 @@ function _M.lookup(self, qname, stale)
     ngx.log(ngx.DEBUG, 'host is ip address: ', qname)
     answers = { new_answer(qname) }
   else
-    answers, err = cache:get(qname, stale)
+    local key = qname .. ":" .. qtype
+
+    -- Check cache first
+    answers, err = cache:get(key, stale)
     if valid_answers(answers) then
       return answers, nil
     end
 
     if not is_fqdn(qname) then
       answers, err = resolve_upstream(qname)
+
+      if valid_answers(answers) then
+        return answers, nil
+      end
     end
 
-    if not valid_answers(answers) then
-      return search_dns(self, qname, stale)
+    answers, err = search_dns(self, qname)
+    if answers then
+      cache:save(qname, qtype, answers)
     end
   end
+
   return answers, err
 end
 
@@ -393,7 +402,6 @@ function _M.get_servers(self, qname, opts)
   local ok = sema:wait(0)
 
   local answers, err = self:lookup(qname, not ok)
-
   ngx.log(ngx.DEBUG, 'resolver query: ', qname, ' finished with ', #(answers or empty), ' answers')
 
   if ok then
diff --git a/gateway/src/resty/resolver/cache.lua b/gateway/src/resty/resolver/cache.lua
index c77ee9806..152490f86 100644
--- a/gateway/src/resty/resolver/cache.lua
+++ b/gateway/src/resty/resolver/cache.lua
@@ -12,6 +12,7 @@ local _M = {
   _VERSION = '0.1'
 }
 
+
 local mt = { __index = _M }
 
 local shared_lrucache = resty_lrucache.new(1000)
@@ -35,9 +36,9 @@ local function compact_answers(servers)
 
     if server then
       local name = server.name or server.address
+      local type = server.type
 
       local packed = hash[name]
-
       if packed then
         insert(packed, server)
         packed.ttl = min(packed.ttl, server.ttl)
@@ -45,7 +46,8 @@ local function compact_answers(servers)
         packed = {
           server,
           name = name,
-          ttl = server.ttl
+          ttl = server.ttl,
+          type = type,
         }
 
         insert(compact, packed)
@@ -57,7 +59,7 @@ local function compact_answers(servers)
   return compact
 end
 
-function _M.store(self, answer, force_ttl)
+function _M.store(self, qname, qtype, answer, force_ttl)
   local cache = self.cache
 
   if not cache then
@@ -71,7 +73,17 @@ function _M.store(self, answer, force_ttl)
     return nil, 'invalid answer'
   end
 
-  ngx.log(ngx.DEBUG, 'resolver cache write ', name, ' with TLL ', answer.ttl)
+  local type = answer.type
+
+  if not type then
+    ngx.log(ngx.WARN, 'resolver cache write refused invalid answer type ', inspect(answer))
+    return nil, 'invalid answer'
+  end
+
+  if type == qtype then
+    name = qname
+  end
+
 
   local ttl = force_ttl or answer.ttl
 
@@ -79,15 +91,18 @@ function _M.store(self, answer, force_ttl)
     ttl = nil
   end
 
-  return cache:set(name, answer, ttl)
+  local key = name .. ":" .. qtype
+  ngx.log(ngx.DEBUG, 'resolver cache write ', name, ' with TLL ', ttl)
+
+  return cache:set(key, answer, ttl)
 end
 
 
-function _M.save(self, answers)
+function _M.save(self, qname, qtype, answers)
   local ans = compact_answers(answers or {})
 
   for _, answer in pairs(ans) do
-    local _, err = self:store(answer)
+    local _, err = self:store(qname, qtype, answer)
 
     if err then
       return nil, err
diff --git a/spec/resty/resolver/cache_spec.lua b/spec/resty/resolver/cache_spec.lua
index cbee15d01..964024e59 100644
--- a/spec/resty/resolver/cache_spec.lua
+++ b/spec/resty/resolver/cache_spec.lua
@@ -39,7 +39,7 @@ describe('resty.resolver.cache', function()
     it('returns compacted answers', function()
       local keys = {}
 
-      for _,v in ipairs(c:save(answers)) do
+      for _,v in ipairs(c:save('www.example.com', 1, answers)) do
         table.insert(keys, v.name)
       end
 
@@ -51,7 +51,7 @@ describe('resty.resolver.cache', function()
     it('stores the result', function()
       c.store = spy.new(c.store)
 
-      c:save(answers)
+      c:save('eld.example.com', 1, answers)
 
       assert.spy(c.store).was.called(3) -- TODO: proper called_with(args)
     end)
@@ -63,40 +63,51 @@ describe('resty.resolver.cache', function()
 
     it('writes to the cache', function()
       local record = { 'someting' }
-      local answer = { record, ttl = 60, name = 'foo.example.com' }
+      local answer = { record, ttl = 60, name = 'foo.example.com', type = 1 }
       c.cache.set = spy.new(function(_, key, value, ttl)
-        assert.same('foo.example.com', key)
+        assert.same('foo.example.com:1', key)
         assert.same(answer, value)
         assert.same(60, ttl)
       end)
 
-      c:store(answer)
+      c:store('foo.example.com', 1, answer)
 
       assert.spy(c.cache.set).was.called(1)
     end)
 
     it('works with -1 ttl', function()
-      local answer = { { 'something' }, ttl = -1, name = 'foo.example.com' }
+      local answer = { { 'something' }, ttl = -1, name = 'foo.example.com', type = 1 }
 
       c.cache.set = spy.new(function(_, key, value, ttl)
-        assert.same('foo.example.com', key)
+        assert.same('foo.example.com:1', key)
         assert.same(answer, value)
         assert.same(nil, ttl)
       end)
 
-      c:store(answer)
+      c:store('foo.example.com', 1, answer)
 
       assert.spy(c.cache.set).was.called(1)
     end)
+
+    it('return error when name is missing', function()
+      local answer = { { 'something' }, ttl = -1 }
+      c.cache.set = spy.new(function(_, key, value, ttl)
+      end)
+
+      local _, err = c:store('something', 1, answer)
+
+      assert.same(err, "invalid answer")
+      assert.spy(c.cache.set).was_not_called()
+    end)
   end)
 
   describe('.get', function()
     local c = resolver_cache.new()
 
     it('returns answers', function()
-      c:save(answers)
+      c:save('www.example.com', 1, answers)
 
-      local ans = c:get('www.example.com')
+      local ans = c:get('www.example.com:1')
 
       assert.same({ "54.221.208.116", "54.221.221.16" }, ans.addresses)
     end)
diff --git a/spec/resty/resolver/http_spec.lua b/spec/resty/resolver/http_spec.lua
index fadd42330..bab71c252 100644
--- a/spec/resty/resolver/http_spec.lua
+++ b/spec/resty/resolver/http_spec.lua
@@ -14,7 +14,7 @@ describe('resty.resolver.http', function()
     it('resolves localhost', function()
       local client = _M.new()
       client:set_timeout(1000)
-      client.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
+      client.resolver.cache:save('unknown', 1, { { address = '127.0.0.1', name = 'unknown.', ttl = 1800 , type=1} })
       assert(client:connect({scheme="http", host='unknown', port=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 f41b05df7..b647b8916 100644
--- a/spec/resty/resolver/socket_spec.lua
+++ b/spec/resty/resolver/socket_spec.lua
@@ -16,7 +16,7 @@ describe('resty.resolver.socket', function()
       sock:settimeout(1000)
       local wrapper = _M.new(sock)
 
-      wrapper.resolver.cache:save({ { address = '127.0.0.1', name = 'unknown.', ttl = 1800 } })
+      wrapper.resolver.cache:save('unknown', 1, { { address = '127.0.0.1', name = 'unknown.', ttl = 1800, type = 1} })
       assert(wrapper:connect('unknown', 1984))
       assert.equal('unknown', wrapper.host)
       assert.equal(1984, wrapper.port)
diff --git a/spec/resty/resolver_spec.lua b/spec/resty/resolver_spec.lua
index e0e85893f..5e5c00651 100644
--- a/spec/resty/resolver_spec.lua
+++ b/spec/resty/resolver_spec.lua
@@ -55,8 +55,8 @@ describe('resty.resolver', function()
     it('skips answers with no address', function()
       dns.query = spy.new(function()
         return {
-          { name = 'www.3scale.net' , cname = '3scale.net' },
-          { name = '3scale.net' , address = '127.0.0.1' }
+          { name = 'www.3scale.net' , cname = '3scale.net', type = 5 },
+          { name = '3scale.net' , address = '127.0.0.1', type = 1 }
         }
       end)
 
@@ -151,6 +151,29 @@ describe('resty.resolver', function()
       assert.same(err, nil)
     end)
 
+    it("return cached value", function()
+      local dns = {
+        query = spy.new(function(_, name)
+            return { errcode = 3, errstr = 'name error' }
+        end)
+      }
+      local cache = resolver_cache.new()
+      local answer =  {{
+        address = "54.221.221.16",
+        class = 1,
+        name = "test.service.default.svc.cluster.local",
+        section = 1,
+        ttl = 59,
+        type = 1
+      }}
+
+      cache:save('test.service', 1, answer)
+      resolver = resty_resolver.new(dns, { cache = cache, search = {"default.svc.cluster.local"}})
+      local record, err = resolver:lookup('test.service')
+      assert.same("test.service.default.svc.cluster.local", record[1].name)
+      assert.same(err, nil)
+      assert.spy(dns.query).was.called(0)
+    end)
   end)
 
   describe('.parse_resolver', function()

From 30711affde30661673a4c8edd6b87bfe22bc3bf1 Mon Sep 17 00:00:00 2001
From: An Tran <tkan145@gmail.com>
Date: Thu, 24 Oct 2024 18:24:56 +1000
Subject: [PATCH 6/6] [resolver] Correct the key used for logging

---
 CHANGELOG.md                         | 1 +
 gateway/src/resty/resolver/cache.lua | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2385672a..c41588521 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,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 Mutual TLS between APIcast and the Backend API fails when using a Forward Proxy [PR #1499](https://github.com/3scale/APIcast/pull/1499) [THREESCALE-5105](https://issues.redhat.com/browse/THREESCALE-5105)
+- 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/cache.lua b/gateway/src/resty/resolver/cache.lua
index 152490f86..aa9fba5e3 100644
--- a/gateway/src/resty/resolver/cache.lua
+++ b/gateway/src/resty/resolver/cache.lua
@@ -92,7 +92,7 @@ function _M.store(self, qname, qtype, answer, force_ttl)
   end
 
   local key = name .. ":" .. qtype
-  ngx.log(ngx.DEBUG, 'resolver cache write ', name, ' with TLL ', ttl)
+  ngx.log(ngx.DEBUG, 'resolver cache write ', key, ' with TLL ', ttl)
 
   return cache:set(key, answer, ttl)
 end