Merge pull request #4443 from ElvinEfendi/lua-resolv-conf-parser

Lua resolv conf parser

build/dev-env.sh

@@ -56,6 +56,8 @@ if ! kubectl get namespace "${NAMESPACE}"; then
   kubectl create namespace "${NAMESPACE}"
 fi
 
+kubectl get deploy nginx-ingress-controller -n ${NAMESPACE} && kubectl delete deploy nginx-ingress-controller -n ${NAMESPACE}
+
 ROOT=./deploy/minikube
 
 if [[ ${KUBE_CLIENT_VERSION} -lt 14 ]]; then

internal/ingress/controller/template/template.go

@@ -144,7 +144,6 @@ var (
 		"filterRateLimits":           filterRateLimits,
 		"buildRateLimitZones":        buildRateLimitZones,
 		"buildRateLimit":             buildRateLimit,
-		"buildResolversForLua":       buildResolversForLua,
 		"configForLua":               configForLua,
 		"locationConfigForLua":       locationConfigForLua,
 		"buildResolvers":             buildResolvers,
@@ -279,37 +278,6 @@ func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF
 	return strings.Join(out, ";\n\r") + ";"
 }
 
-func buildResolversForLua(res interface{}, disableIpv6 interface{}) string {
-	nss, ok := res.([]net.IP)
-	if !ok {
-		klog.Errorf("expected a '[]net.IP' type but %T was returned", res)
-		return ""
-	}
-	no6, ok := disableIpv6.(bool)
-	if !ok {
-		klog.Errorf("expected a 'bool' type but %T was returned", disableIpv6)
-		return ""
-	}
-
-	if len(nss) == 0 {
-		return ""
-	}
-
-	r := []string{}
-	for _, ns := range nss {
-		if ing_net.IsIPV6(ns) {
-			if no6 {
-				continue
-			}
-			r = append(r, fmt.Sprintf("\"[%v]\"", ns))
-		} else {
-			r = append(r, fmt.Sprintf("\"%v\"", ns))
-		}
-	}
-
-	return strings.Join(r, ", ")
-}
-
 // configForLua returns some general configuration as Lua table represented as string
 func configForLua(input interface{}) string {
 	all, ok := input.(config.TemplateConfig)

internal/ingress/controller/template/template_test.go

@@ -577,43 +577,6 @@ func TestBuildForwardedFor(t *testing.T) {
 	}
 }
 
-func TestBuildResolversForLua(t *testing.T) {
-
-	ipOne := net.ParseIP("192.0.0.1")
-	ipTwo := net.ParseIP("2001:db8:1234:0000:0000:0000:0000:0000")
-	ipList := []net.IP{ipOne, ipTwo}
-
-	invalidType := &ingress.Ingress{}
-	expected := ""
-	actual := buildResolversForLua(invalidType, false)
-
-	// Invalid Type for []net.IP
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
-	}
-
-	actual = buildResolversForLua(ipList, invalidType)
-
-	// Invalid Type for bool
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
-	}
-
-	expected = "\"192.0.0.1\", \"[2001:db8:1234::]\""
-	actual = buildResolversForLua(ipList, false)
-
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
-	}
-
-	expected = "\"192.0.0.1\""
-	actual = buildResolversForLua(ipList, true)
-
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
-	}
-}
-
 func TestBuildResolvers(t *testing.T) {
 	ipOne := net.ParseIP("192.0.0.1")
 	ipTwo := net.ParseIP("2001:db8:1234:0000:0000:0000:0000:0000")

rootfs/etc/nginx/lua/configuration.lua

@@ -4,9 +4,7 @@ local cjson = require("cjson.safe")
 local configuration_data = ngx.shared.configuration_data
 local certificate_data = ngx.shared.certificate_data
 
-local _M = {
+local _M = {}
-  nameservers = {}
-}
 
 function _M.get_backends_data()
   return configuration_data:get("backends")

rootfs/etc/nginx/lua/test/balancer_test.lua

@@ -315,8 +315,7 @@ describe("Balancer", function()
         }
       }
 
-      local dns_helper = require("test/dns_helper")
+      helpers.mock_resty_dns_query({
-      dns_helper.mock_dns_query({
         {
           name = "example.com",
           address = "192.168.1.1",

rootfs/etc/nginx/lua/test/configuration_test.lua

@@ -115,15 +115,20 @@ describe("Configuration", function()
                 end)
 
                 it("returns a status of 400", function()
+                    local original_io_open = _G.io.open
                     _G.io.open = function(filename, extension) return false end
                     assert.has_no.errors(configuration.call)
                     assert.equal(ngx.status, ngx.HTTP_BAD_REQUEST)
+                    _G.io.open = original_io_open
                 end)
 
                 it("logs 'dynamic-configuration: unable to read valid request body to stderr'", function()
+                    local original_io_open = _G.io.open
+                    _G.io.open = function(filename, extension) return false end
                     local s = spy.on(ngx, "log")
                     assert.has_no.errors(configuration.call)
                     assert.spy(s).was_called_with(ngx.ERR, "dynamic-configuration: unable to read valid request body")
+                    _G.io.open = original_io_open
                 end)
             end)
 

rootfs/etc/nginx/lua/test/dns_helper.lua

@@ -1,27 +0,0 @@
-local _M = {}
-
-local configuration = require("configuration")
-local resolver = require("resty.dns.resolver")
-local old_resolver_new = resolver.new
-
-local function reset(nameservers)
-  configuration.nameservers = nameservers or { "1.1.1.1" }
-end
-
-function _M.mock_new(func, nameservers)
-  reset(nameservers)
-  resolver.new = func
-end
-
-function _M.mock_dns_query(response, err)
-  reset()
-  resolver.new = function(self, options)
-    local r = old_resolver_new(self, options)
-    r.query = function(self, name, options, tries)
-      return response, err
-    end
-    return r
-  end
-end
-
-return _M

rootfs/etc/nginx/lua/test/helpers.lua

@@ -0,0 +1,47 @@
+local _M = {}
+
+local resty_dns_resolver = require("resty.dns.resolver")
+
+local original_resty_dns_resolver_new = resty_dns_resolver.new
+local original_io_open = io.open
+
+function _M.with_resolv_conf(content, func)
+  local new_resolv_conf_f = assert(io.tmpfile())
+  new_resolv_conf_f:write(content)
+  new_resolv_conf_f:seek("set", 0)
+
+  io.open = function(path, mode)
+    if path ~= "/etc/resolv.conf" then
+      error("expected '/etc/resolv.conf' as path but got: " .. tostring(path))
+    end
+    if mode ~= "r" then
+      error("expected 'r' as mode but got: " .. tostring(mode))
+    end
+
+    return new_resolv_conf_f, nil
+  end
+
+  func()
+
+  io.open = original_io_open
+
+  if io.type(new_resolv_conf_f) ~= "closed file" then
+    error("file was left open")
+  end
+end
+
+function _M.mock_resty_dns_new(func)
+  resty_dns_resolver.new = func
+end
+
+function _M.mock_resty_dns_query(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)
+      return response, err
+    end
+    return r
+  end
+end
+
+return _M

rootfs/etc/nginx/lua/test/run.lua

@@ -11,6 +11,7 @@ do
   -- if there's more constants need to be whitelisted for test runs, add here.
   local GLOBALS_ALLOWED_IN_TEST = {
     _TEST = true,
+    helpers = true,
   }
   local newindex = function(table, key, value)
     rawset(table, key, value)
@@ -33,6 +34,8 @@ do
   setmetatable(_G, { __newindex = newindex })
 end
 
+_G.helpers = require("test.helpers")
+
 
 local ffi = require("ffi")
 local lua_ingress = require("lua_ingress")

rootfs/etc/nginx/lua/test/util/dns_test.lua

@@ -1,41 +1,51 @@
+local conf = [===[
+nameserver 1.2.3.4
+nameserver 4.5.6.7
+search ingress-nginx.svc.cluster.local svc.cluster.local cluster.local
+options ndots:5
+]===]
+
+helpers.with_resolv_conf(conf, function()
+  require("util.resolv_conf")
+end)
+
 describe("resolve", function()
   local dns = require("util.dns")
-  local dns_helper = require("test/dns_helper")
 
   it("sets correct nameservers", function()
-    dns_helper.mock_new(function(self, options)
+    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, { "1.2.3.4", "4.5.6.7" })
+    end)
     dns.resolve("example.com")
   end)
 
   it("returns host when an error happens", function()
     local s_ngx_log = spy.on(ngx, "log")
 
-    dns_helper.mock_new(function(...) return nil, "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")
 
-    dns_helper.mock_dns_query(nil, "oops!")
+    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!")
 
-    dns_helper.mock_dns_query({ errcode = 1, errstr = "format error" })
+    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")
 
-    dns_helper.mock_dns_query({})
+    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")
 
-    dns_helper.mock_dns_query({ { name = "example.com", cname = "sub.example.com", ttl = 60 } })
+    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")
   end)
 
   it("resolves all A records of given host, caches them with minimal ttl and returns from cache next time", function()
-    dns_helper.mock_dns_query({
+    helpers.mock_resty_dns_query({
       {
         name = "example.com",
         address = "192.168.1.1",
@@ -66,7 +76,7 @@ describe("resolve", function()
 
     assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns.resolve("example.com"))
 
-    dns_helper.mock_new(function(...)
+    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"))

rootfs/etc/nginx/lua/test/util/resolv_conf_test.lua

@@ -0,0 +1,64 @@
+local original_io_open = io.open
+
+describe("resolv_conf", function()
+  after_each(function()
+    package.loaded["util.resolv_conf"] = nil
+    io.open = original_io_open
+  end)
+
+  it("errors when file can not be opened", function()
+    io.open = function(...)
+      return nil, "file does not exist"
+    end
+
+    assert.has_error(function() require("util.resolv_conf") end, "could not open /etc/resolv.conf: file does not exist")
+  end)
+
+  it("opens '/etc/resolv.conf' with mode 'r'", function()
+    io.open = function(path, mode)
+      assert.are.same("/etc/resolv.conf", path)
+      assert.are.same("r", mode)
+
+      return original_io_open(path, mode)
+    end
+
+    assert.has_no.errors(function() require("util.resolv_conf") end)
+  end)
+
+  it("correctly parses resolv.conf", function()
+    local conf = [===[
+# This is a comment
+nameserver 10.96.0.10
+nameserver 10.96.0.99
+search ingress-nginx.svc.cluster.local svc.cluster.local cluster.local
+options ndots:5
+    ]===]
+
+    helpers.with_resolv_conf(conf, function()
+      local resolv_conf = require("util.resolv_conf")
+      assert.are.same({
+        nameservers = { "10.96.0.10", "10.96.0.99" },
+        search = { "ingress-nginx.svc.cluster.local", "svc.cluster.local", "cluster.local" },
+        ndots = 5,
+      }, resolv_conf)
+    end)
+  end)
+
+  it("ignores options that it does not understand", function()
+    local conf = [===[
+nameserver 10.96.0.10
+search example.com
+options debug
+options ndots:3
+    ]===]
+
+    helpers.with_resolv_conf(conf, function()
+      local resolv_conf = require("util.resolv_conf")
+      assert.are.same({
+        nameservers = { "10.96.0.10" },
+        search = { "example.com" },
+        ndots = 3,
+      }, resolv_conf)
+    end)
+  end)
+end)

rootfs/etc/nginx/lua/util/dns.lua

@@ -1,7 +1,6 @@
 local resolver = require("resty.dns.resolver")
 local lrucache = require("resty.lrucache")
-local configuration = require("configuration")
+local resolv_conf = require("util.resolv_conf")
-local util = require("util")
 
 local _M = {}
 local CACHE_SIZE = 10000
@@ -59,7 +58,7 @@ function _M.resolve(host)
 
   local r
   r, err = resolver:new{
-    nameservers = util.deepcopy(configuration.nameservers),
+    nameservers = resolv_conf.nameservers,
     retrans = 5,
     timeout = 2000,  -- 2 sec
   }

rootfs/etc/nginx/lua/util/resolv_conf.lua

@@ -0,0 +1,79 @@
+local ngx_re_split = require("ngx.re").split
+
+local ngx_log = ngx.log
+local ngx_ERR = ngx.ERR
+
+local CONF_PATH = "/etc/resolv.conf"
+
+local nameservers, search, ndots = {}, {}, 1
+
+local function set_search(parts)
+  local length = #parts
+
+  for i = 2, length, 1 do
+    search[i-1] = parts[i]
+  end
+end
+
+local function set_ndots(parts)
+  local option = parts[2]
+  if not option then
+    return
+  end
+
+  local option_parts, err = ngx_re_split(option, ":")
+  if err then
+    ngx_log(ngx_ERR, err)
+    return
+  end
+
+  if option_parts[1] ~= "ndots" then
+    return
+  end
+
+  ndots = tonumber(option_parts[2])
+end
+
+local function is_comment(line)
+  return line:sub(1, 1) == "#"
+end
+
+local function parse_line(line)
+  if is_comment(line) then
+    return
+  end
+
+  local parts, err = ngx_re_split(line, "\\s+")
+  if err then
+    ngx_log(ngx_ERR, err)
+  end
+
+  local keyword, value = parts[1], parts[2]
+
+  if keyword == "nameserver" then
+    nameservers[#nameservers + 1] = value
+  elseif keyword == "search" then
+    set_search(parts)
+  elseif keyword == "options" then
+    set_ndots(parts)
+  end
+end
+
+do
+  local f, err = io.open(CONF_PATH, "r")
+  if not f then
+    error("could not open " .. CONF_PATH .. ": " .. tostring(err))
+  end
+
+  for line in f:lines() do
+    parse_line(line)
+  end
+
+  f:close()
+end
+
+return {
+  nameservers = nameservers,
+  search = search,
+  ndots = ndots,
+}

rootfs/etc/nginx/template/nginx.tmpl

@@ -77,7 +77,6 @@ http {
           error("require failed: " .. tostring(res))
         else
           configuration = res
-          configuration.nameservers = { {{ buildResolversForLua $cfg.Resolver $cfg.DisableIpv6DNS }} }
         end
 
         ok, res = pcall(require, "balancer")
@@ -623,7 +622,6 @@ stream {
           error("require failed: " .. tostring(res))
         else
           configuration = res
-          configuration.nameservers = { {{ buildResolversForLua $cfg.Resolver $cfg.DisableIpv6DNS }} }
         end
 
         ok, res = pcall(require, "tcp_udp_configuration")

test/e2e/lua/dynamic_configuration.go

@@ -20,7 +20,6 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/http"
-	"regexp"
 	"strings"
 	"time"
 
@@ -63,13 +62,6 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
 		})
 	})
 
-	It("sets nameservers for Lua", func() {
-		f.WaitForNginxConfiguration(func(cfg string) bool {
-			r := regexp.MustCompile(`configuration.nameservers = { [".,0-9a-zA-Z]+ }`)
-			return r.MatchString(cfg)
-		})
-	})
-
 	Context("Lua shared dict", func() {
 		It("update config", func() {