diff --git a/rootfs/etc/nginx/lua/balancer.lua b/rootfs/etc/nginx/lua/balancer.lua index 28312c344..ec8eb194b 100644 --- a/rootfs/etc/nginx/lua/balancer.lua +++ b/rootfs/etc/nginx/lua/balancer.lua @@ -1,7 +1,7 @@ local ngx_balancer = require("ngx.balancer") local cjson = require("cjson.safe") local util = require("util") -local dns_util = require("util.dns") +local dns_lookup = require("util.dns").lookup local configuration = require("configuration") local round_robin = require("balancer.round_robin") local chash = require("balancer.chash") @@ -52,7 +52,7 @@ local function resolve_external_names(original_backend) local backend = util.deepcopy(original_backend) local endpoints = {} for _, endpoint in ipairs(backend.endpoints) do - local ips = dns_util.resolve(endpoint.address) + local ips = dns_lookup(endpoint.address) for _, ip in ipairs(ips) do table.insert(endpoints, { address = ip, port = endpoint.port }) end diff --git a/rootfs/etc/nginx/lua/tcp_udp_balancer.lua b/rootfs/etc/nginx/lua/tcp_udp_balancer.lua index 7d9601ca8..6c623b483 100644 --- a/rootfs/etc/nginx/lua/tcp_udp_balancer.lua +++ b/rootfs/etc/nginx/lua/tcp_udp_balancer.lua @@ -1,7 +1,7 @@ local ngx_balancer = require("ngx.balancer") local cjson = require("cjson.safe") local util = require("util") -local dns_util = require("util.dns") +local dns_lookup = require("util.dns").lookup local configuration = require("tcp_udp_configuration") local round_robin = require("balancer.round_robin") @@ -34,7 +34,7 @@ local function resolve_external_names(original_backend) local backend = util.deepcopy(original_backend) local endpoints = {} for _, endpoint in ipairs(backend.endpoints) do - local ips = dns_util.resolve(endpoint.address) + local ips = dns_lookup(endpoint.address) for _, ip in ipairs(ips) do table.insert(endpoints, {address = ip, port = endpoint.port}) end diff --git a/rootfs/etc/nginx/lua/test/balancer_test.lua b/rootfs/etc/nginx/lua/test/balancer_test.lua index b9855cc7e..2851f5853 100644 --- a/rootfs/etc/nginx/lua/test/balancer_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer_test.lua @@ -315,7 +315,7 @@ describe("Balancer", function() } } - helpers.mock_resty_dns_query({ + helpers.mock_resty_dns_query(nil, { { name = "example.com", address = "192.168.1.1", diff --git a/rootfs/etc/nginx/lua/test/helpers.lua b/rootfs/etc/nginx/lua/test/helpers.lua index e115e528a..f21019410 100644 --- a/rootfs/etc/nginx/lua/test/helpers.lua +++ b/rootfs/etc/nginx/lua/test/helpers.lua @@ -34,10 +34,13 @@ function _M.mock_resty_dns_new(func) resty_dns_resolver.new = func end -function _M.mock_resty_dns_query(response, err) +function _M.mock_resty_dns_query(mocked_host, response, err) resty_dns_resolver.new = function(self, options) local r = original_resty_dns_resolver_new(self, options) - r.query = function(self, name, options, tries) + r.query = function(self, host, options, tries) + if mocked_host and mocked_host ~= host then + return error(tostring(host) .. " is not mocked") + end return response, err end return r diff --git a/rootfs/etc/nginx/lua/test/run.lua b/rootfs/etc/nginx/lua/test/run.lua index 3836dd56e..0dff725e6 100644 --- a/rootfs/etc/nginx/lua/test/run.lua +++ b/rootfs/etc/nginx/lua/test/run.lua @@ -35,7 +35,7 @@ do end _G.helpers = require("test.helpers") - +_G._TEST = true local ffi = require("ffi") local lua_ingress = require("lua_ingress") diff --git a/rootfs/etc/nginx/lua/test/util/dns_test.lua b/rootfs/etc/nginx/lua/test/util/dns_test.lua index 1895fcbbe..92df082d1 100644 --- a/rootfs/etc/nginx/lua/test/util/dns_test.lua +++ b/rootfs/etc/nginx/lua/test/util/dns_test.lua @@ -9,45 +9,81 @@ helpers.with_resolv_conf(conf, function() require("util.resolv_conf") end) -describe("resolve", function() - local dns = require("util.dns") +describe("dns.lookup", function() + local dns, dns_lookup, spy_ngx_log + + before_each(function() + spy_ngx_log = spy.on(ngx, "log") + dns = require("util.dns") + dns_lookup = dns.lookup + end) + + after_each(function() + package.loaded["util.dns"] = nil + end) it("sets correct nameservers", function() helpers.mock_resty_dns_new(function(self, options) assert.are.same({ nameservers = { "1.2.3.4", "4.5.6.7" }, retrans = 5, timeout = 2000 }, options) return nil, "" end) - dns.resolve("example.com") + dns_lookup("example.com") end) - it("returns host when an error happens", function() - local s_ngx_log = spy.on(ngx, "log") + describe("when there's an error", function() + it("returns host when resolver can not be instantiated", function() + helpers.mock_resty_dns_new(function(...) return nil, "an error" end) + assert.are.same({ "example.com" }, dns_lookup("example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to instantiate the resolver: an error") + end) - helpers.mock_resty_dns_new(function(...) return nil, "an error" end) - assert.are.same({ "example.com" }, dns.resolve("example.com")) - assert.spy(s_ngx_log).was_called_with(ngx.ERR, "failed to instantiate the resolver: an error") + it("returns host when the query returns nil", function() + helpers.mock_resty_dns_query(nil, nil, "oops!") + assert.are.same({ "example.com" }, dns_lookup("example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\noops!\noops!") + end) - helpers.mock_resty_dns_query(nil, "oops!") - assert.are.same({ "example.com" }, dns.resolve("example.com")) - assert.spy(s_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\noops!\noops!") + it("returns host when the query returns empty answer", function() + helpers.mock_resty_dns_query(nil, {}) + assert.are.same({ "example.com" }, dns_lookup("example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\nno A record resolved\nno AAAA record resolved") + end) - helpers.mock_resty_dns_query({ errcode = 1, errstr = "format error" }) - assert.are.same({ "example.com" }, dns.resolve("example.com")) - assert.spy(s_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\nserver returned error code: 1: format error\nserver returned error code: 1: format error") + it("returns host when there's answer but with error", function() + helpers.mock_resty_dns_query(nil, { errcode = 1, errstr = "format error" }) + assert.are.same({ "example.com" }, dns_lookup("example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\n" .. + "server returned error code: 1: format error\nserver returned error code: 1: format error") + end) - helpers.mock_resty_dns_query({}) - assert.are.same({ "example.com" }, dns.resolve("example.com")) - assert.spy(s_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\nno record resolved\nno record resolved") + it("retuns host when there's answer but no A/AAAA record in it", function() + helpers.mock_resty_dns_query(nil, { { name = "example.com", cname = "sub.example.com", ttl = 60 } }) + assert.are.same({ "example.com" }, dns_lookup("example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\nno A record resolved\nno AAAA record resolved") + end) - helpers.mock_resty_dns_query({ { name = "example.com", cname = "sub.example.com", ttl = 60 } }) - assert.are.same({ "example.com" }, dns.resolve("example.com")) - assert.spy(s_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\nno record resolved\nno record resolved") + it("returns host when the query returns nil and number of dots is not less than configured ndots", function() + helpers.mock_resty_dns_query(nil, nil, "oops!") + assert.are.same({ "a.b.c.d.example.com" }, dns_lookup("a.b.c.d.example.com")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\noops!\noops!") + end) + + it("returns host when the query returns nil for a fully qualified domain", function() + helpers.mock_resty_dns_query("example.com.", nil, "oops!") + assert.are.same({ "example.com." }, dns_lookup("example.com.")) + assert.spy(spy_ngx_log).was_called_with(ngx.ERR, "failed to query the DNS server:\noops!\noops!") + end) end) - it("resolves all A records of given host, caches them with minimal ttl and returns from cache next time", function() - helpers.mock_resty_dns_query({ + it("returns answer from cache if it exists without doing actual DNS query", function() + dns._cache:set("example.com", { "192.168.1.1" }) + assert.are.same({ "192.168.1.1" }, dns_lookup("example.com")) + end) + + it("resolves a fully qualified domain without looking at resolv.conf search and caches result", function() + helpers.mock_resty_dns_query("example.com.", { { - name = "example.com", + name = "example.com.", address = "192.168.1.1", ttl = 3600, }, @@ -57,28 +93,43 @@ describe("resolve", function() ttl = 60, } }) + assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns_lookup("example.com.")) + assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns._cache:get("example.com.")) + end) - local lrucache = require("resty.lrucache") - local old_lrucache_new = lrucache.new - lrucache.new = function(...) - local cache = old_lrucache_new(...) + it("starts with host itself when number of dots is not less than configured ndots", function() + local host = "a.b.c.d.example.com" + helpers.mock_resty_dns_query(host, { { name = host, address = "192.168.1.1", ttl = 3600, } } ) - local old_set = cache.set - cache.set = function(self, key, value, ttl) - assert.equal("example.com", key) - assert.are.same({ "192.168.1.1", "1.2.3.4" }, value) - assert.equal(60, ttl) - return old_set(self, key, value, ttl) - end + assert.are.same({ "192.168.1.1" }, dns_lookup(host)) + assert.are.same({ "192.168.1.1" }, dns._cache:get(host)) + end) - return cache - end + it("starts with first search entry when number of dots is less than configured ndots", function() + local host = "example.com.ingress-nginx.svc.cluster.local" + helpers.mock_resty_dns_query(host, { { name = host, address = "192.168.1.1", ttl = 3600, } } ) - assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns.resolve("example.com")) + assert.are.same({ "192.168.1.1" }, dns_lookup(host)) + assert.are.same({ "192.168.1.1" }, dns._cache:get(host)) + end) - helpers.mock_resty_dns_new(function(...) - error("expected to short-circuit and return response from cache") - end) - assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns.resolve("example.com")) + it("it caches with minimal ttl", function() + helpers.mock_resty_dns_query("example.com.", { + { + name = "example.com.", + address = "192.168.1.1", + ttl = 3600, + }, + { + name = "example.com.", + address = "1.2.3.4", + ttl = 60, + } + }) + + local spy_cache_set = spy.on(dns._cache, "set") + + assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns_lookup("example.com.")) + assert.spy(spy_cache_set).was_called_with(match.is_table(), "example.com.", { "192.168.1.1", "1.2.3.4" }, 60) end) end) diff --git a/rootfs/etc/nginx/lua/util/dns.lua b/rootfs/etc/nginx/lua/util/dns.lua index 94023fa1a..fcef83767 100644 --- a/rootfs/etc/nginx/lua/util/dns.lua +++ b/rootfs/etc/nginx/lua/util/dns.lua @@ -2,23 +2,48 @@ local resolver = require("resty.dns.resolver") local lrucache = require("resty.lrucache") local resolv_conf = require("util.resolv_conf") +local ngx_log = ngx.log +local ngx_INFO = ngx.INFO +local ngx_ERR = ngx.ERR +local string_format = string.format +local table_concat = table.concat +local table_insert = table.insert +local ipairs = ipairs +local tostring = tostring + local _M = {} local CACHE_SIZE = 10000 local MAXIMUM_TTL_VALUE = 2147483647 -- maximum value according to https://tools.ietf.org/html/rfc2181 +-- for every host we will try two queries for the following types with the order set here +local QTYPES_TO_CHECK = { resolver.TYPE_A, resolver.TYPE_AAAA } -local cache, err = lrucache.new(CACHE_SIZE) -if not cache then - return error("failed to create the cache: " .. (err or "unknown")) +local cache +do + local err + cache, err = lrucache.new(CACHE_SIZE) + if not cache then + return error("failed to create the cache: " .. (err or "unknown")) + end end -local function a_records_and_max_ttl(answers) +local function cache_set(host, addresses, ttl) + cache:set(host, addresses, ttl) + ngx_log(ngx_INFO, string_format("cache set for '%s' with value of [%s] and ttl of %s.", + host, table_concat(addresses, ", "), ttl)) +end + +local function is_fully_qualified(host) + return host:sub(-1) == "." +end + +local function a_records_and_min_ttl(answers) local addresses = {} local ttl = MAXIMUM_TTL_VALUE -- maximum value according to https://tools.ietf.org/html/rfc2181 for _, ans in ipairs(answers) do if ans.address then - table.insert(addresses, ans.address) - if ttl > ans.ttl then + table_insert(addresses, ans.address) + if ans.ttl < ttl then ttl = ans.ttl end end @@ -27,71 +52,109 @@ local function a_records_and_max_ttl(answers) return addresses, ttl end -local function resolve_host(host, r, qtype) - local answers - answers, err = r:query(host, { qtype = qtype }, {}) +local function resolve_host_for_qtype(r, host, qtype) + ngx_log(ngx_INFO, string_format("resolving %s with qtype %s", host, qtype)) + + local answers, err = r:query(host, { qtype = qtype }, {}) if not answers then - return nil, -1, tostring(err) + return nil, -1, err end if answers.errcode then - return nil, -1, string.format("server returned error code: %s: %s", answers.errcode, answers.errstr) + return nil, -1, string_format("server returned error code: %s: %s", answers.errcode, answers.errstr) end - local addresses, ttl = a_records_and_max_ttl(answers) + local addresses, ttl = a_records_and_min_ttl(answers) if #addresses == 0 then - return nil, -1, "no record resolved" + local msg = "no A record resolved" + if qtype == resolver.TYPE_AAAA then msg = "no AAAA record resolved" end + return nil, -1, msg end return addresses, ttl, nil end -function _M.resolve(host) +local function resolve_host(r, host) + local dns_errors = {} + + for _, qtype in ipairs(QTYPES_TO_CHECK) do + local addresses, ttl, err = resolve_host_for_qtype(r, host, qtype) + if addresses and #addresses > 0 then + return addresses, ttl, nil + end + table_insert(dns_errors, tostring(err)) + end + + return nil, nil, dns_errors +end + +function _M.lookup(host) local cached_addresses = cache:get(host) if cached_addresses then - local message = string.format( - "addresses %s for host %s was resolved from cache", - table.concat(cached_addresses, ", "), host) - ngx.log(ngx.INFO, message) return cached_addresses end - local r - r, err = resolver:new{ + local r, err = resolver:new{ nameservers = resolv_conf.nameservers, retrans = 5, timeout = 2000, -- 2 sec } if not r then - ngx.log(ngx.ERR, "failed to instantiate the resolver: " .. tostring(err)) + ngx_log(ngx_ERR, string_format("failed to instantiate the resolver: %s", err)) return { host } end - local dns_errors = {} + local addresses, ttl, dns_errors - local addresses, ttl - addresses, ttl, err = resolve_host(host, r, r.TYPE_A) - if not addresses then - table.insert(dns_errors, tostring(err)) - elseif #addresses > 0 then - cache:set(host, addresses, ttl) - return addresses + -- when the queried domain is fully qualified + -- then we don't go through resolv_conf.search + -- NOTE(elvinefendi): currently FQDN as externalName will be supported starting + -- with K8s 1.15: https://github.com/kubernetes/kubernetes/pull/78385 + if is_fully_qualified(host) then + addresses, ttl, dns_errors = resolve_host(r, host) + if addresses then + cache_set(host, addresses, ttl) + return addresses + end + + ngx_log(ngx_ERR, "failed to query the DNS server:\n" .. table_concat(dns_errors, "\n")) + + return { host } end - addresses, ttl, err = resolve_host(host, r, r.TYPE_AAAA) - if not addresses then - table.insert(dns_errors, tostring(err)) - elseif #addresses > 0 then - cache:set(host, addresses, ttl) - return addresses + -- for non fully qualified domains if number of dots in + -- the queried host is less than resolv_conf.ndots then we try + -- with all the entries in resolv_conf.search before trying the original host + -- + -- if number of dots is not less than resolv_conf.ndots then we start with + -- the original host and then try entries in resolv_conf.search + local _, host_ndots = host:gsub("%.", "") + local search_start, search_end = 0, #resolv_conf.search + if host_ndots < resolv_conf.ndots then + search_start = 1 + search_end = #resolv_conf.search + 1 + end + + for i = search_start, search_end, 1 do + local new_host = resolv_conf.search[i] and string_format("%s.%s", host, resolv_conf.search[i]) or host + + addresses, ttl, dns_errors = resolve_host(r, new_host) + if addresses then + cache_set(host, addresses, ttl) + return addresses + end end if #dns_errors > 0 then - ngx.log(ngx.ERR, "failed to query the DNS server:\n" .. table.concat(dns_errors, "\n")) + ngx_log(ngx_ERR, "failed to query the DNS server:\n" .. table_concat(dns_errors, "\n")) end return { host } end +if _TEST then + _M._cache = cache +end + return _M diff --git a/test/e2e/servicebackend/service_externalname.go b/test/e2e/servicebackend/service_externalname.go index 7ae65616d..4447f5e17 100644 --- a/test/e2e/servicebackend/service_externalname.go +++ b/test/e2e/servicebackend/service_externalname.go @@ -41,6 +41,40 @@ var _ = framework.IngressNginxDescribe("Service Type ExternalName", func() { AfterEach(func() { }) + It("works with external name set to incomplete fdqn", func() { + f.NewEchoDeployment() + + host := "echo" + + svc := &core.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httpbin", + Namespace: f.Namespace, + }, + Spec: corev1.ServiceSpec{ + ExternalName: "http-svc", + Type: corev1.ServiceTypeExternalName, + }, + } + + f.EnsureService(svc) + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, "httpbin", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "proxy_pass http://upstream_balancer;") + }) + + resp, _, errs := gorequest.New(). + Get(f.GetURL(framework.HTTP)+"/get"). + Set("Host", host). + End() + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(200)) + }) + It("should return 200 for service type=ExternalName without a port defined", func() { host := "echo"