Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for ExternalName service type in dynamic mode #2804

Merged
merged 2 commits into from
Jul 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions build/busted

This file was deleted.

2 changes: 1 addition & 1 deletion build/test-lua.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ resty \
--shdict "configuration_data 5M" \
--shdict "balancer_ewma 1M" \
--shdict "balancer_ewma_last_touched_at 1M" \
./build/busted ${BUSTED_ARGS} ./rootfs/etc/nginx/lua/test/
./rootfs/etc/nginx/lua/test/run.lua ${BUSTED_ARGS} ./rootfs/etc/nginx/lua/test/
2 changes: 2 additions & 0 deletions internal/ingress/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ func configureDynamically(pcfg *ingress.Configuration, port int) error {
backends := make([]*ingress.Backend, len(pcfg.Backends))

for i, backend := range pcfg.Backends {
service := &apiv1.Service{Spec: backend.Service.Spec}
luaBackend := &ingress.Backend{
Name: backend.Name,
Port: backend.Port,
Expand All @@ -749,6 +750,7 @@ func configureDynamically(pcfg *ingress.Configuration, port int) error {
SessionAffinity: backend.SessionAffinity,
UpstreamHashBy: backend.UpstreamHashBy,
LoadBalancing: backend.LoadBalancing,
Service: service,
}

var endpoints []ingress.Endpoint
Expand Down
28 changes: 28 additions & 0 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ var (
"filterRateLimits": filterRateLimits,
"buildRateLimitZones": buildRateLimitZones,
"buildRateLimit": buildRateLimit,
"buildResolversForLua": buildResolversForLua,
"buildResolvers": buildResolvers,
"buildUpstreamName": buildUpstreamName,
"isLocationInLocationList": isLocationInLocationList,
Expand Down Expand Up @@ -220,6 +221,33 @@ func buildLuaSharedDictionaries(s interface{}, dynamicConfigurationEnabled bool,
return strings.Join(out, ";\n\r") + ";"
}

func buildResolversForLua(res interface{}, disableIpv6 interface{}) string {
nss, ok := res.([]net.IP)
if !ok {
glog.Errorf("expected a '[]net.IP' type but %T was returned", res)
return ""
}
no6, ok := disableIpv6.(bool)
if !ok {
glog.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) && no6 {
continue
}
r = append(r, fmt.Sprintf("\"%v\"", ns))
}

return strings.Join(r, ", ")
}

// buildResolvers returns the resolvers reading the /etc/resolv.conf file
func buildResolvers(res interface{}, disableIpv6 interface{}) string {
// NGINX need IPV6 addresses to be surrounded by brackets
Expand Down
20 changes: 20 additions & 0 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,26 @@ 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}

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")
Expand Down
20 changes: 20 additions & 0 deletions rootfs/etc/nginx/lua/balancer.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
local ngx_balancer = require("ngx.balancer")
local json = require("cjson")
local util = require("util")
local dns_util = require("util.dns")
local configuration = require("configuration")
local round_robin = require("balancer.round_robin")
local chash = require("balancer.chash")
Expand Down Expand Up @@ -40,6 +42,19 @@ local function get_implementation(backend)
return implementation
end

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)
for _, ip in ipairs(ips) do
table.insert(endpoints, { address = ip, port = endpoint.port })
end
end
backend.endpoints = endpoints
return backend
end

local function sync_backend(backend)
local implementation = get_implementation(backend)
local balancer = balancers[backend.name]
Expand All @@ -59,6 +74,11 @@ local function sync_backend(backend)
return
end

local service_type = backend.service and backend.service.spec and backend.service.spec["type"]
if service_type == "ExternalName" then
backend = resolve_external_names(backend)
end

balancer:sync(backend)
end

Expand Down
4 changes: 3 additions & 1 deletion rootfs/etc/nginx/lua/configuration.lua
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
-- this is the Lua representation of Configuration struct in internal/ingress/types.go
local configuration_data = ngx.shared.configuration_data

local _M = {}
local _M = {
nameservers = {}
}

function _M.get_backends_data()
return configuration_data:get("backends")
Expand Down
38 changes: 38 additions & 0 deletions rootfs/etc/nginx/lua/test/balancer_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,44 @@ describe("Balancer", function()
assert.spy(s).was_called_with(implementation, backend)
end)

it("resolves external name to endpoints when service is of type External name", function()
backend = {
name = "exmaple-com", service = { spec = { ["type"] = "ExternalName" } },
endpoints = {
{ address = "example.com", port = "80", maxFails = 0, failTimeout = 0 }
}
}

local dns_helper = require("test/dns_helper")
dns_helper.mock_dns_query({
{
name = "example.com",
address = "192.168.1.1",
ttl = 3600,
},
{
name = "example.com",
address = "1.2.3.4",
ttl = 60,
}
})
expected_backend = {
name = "exmaple-com", service = { spec = { ["type"] = "ExternalName" } },
endpoints = {
{ address = "192.168.1.1", port = "80" },
{ address = "1.2.3.4", port = "80" },
}
}

local mock_instance = { sync = function(backend) end }
setmetatable(mock_instance, implementation)
implementation.new = function(self, backend) return mock_instance end
assert.has_no.errors(function() balancer.sync_backend(backend) end)
stub(mock_instance, "sync")
assert.has_no.errors(function() balancer.sync_backend(backend) end)
assert.stub(mock_instance.sync).was_called_with(mock_instance, expected_backend)
end)

it("replaces the existing balancer when load balancing config changes for backend", function()
assert.has_no.errors(function() balancer.sync_backend(backend) end)

Expand Down
27 changes: 27 additions & 0 deletions rootfs/etc/nginx/lua/test/dns_helper.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
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
32 changes: 32 additions & 0 deletions rootfs/etc/nginx/lua/test/run.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
local ffi = require("ffi")

-- without this we get errors such as "attempt to redefine XXX"
local old_cdef = ffi.cdef
local exists = {}
ffi.cdef = function(def)
if exists[def] then
return
end
exists[def] = true
return old_cdef(def)
end

local old_udp = ngx.socket.udp
ngx.socket.udp = function(...)
local socket = old_udp(...)
socket.send = function(...)
error("ngx.socket.udp:send please mock this to use in tests")
end
return socket
end

local old_tcp = ngx.socket.tcp
ngx.socket.tcp = function(...)
local socket = old_tcp(...)
socket.send = function(...)
error("ngx.socket.tcp:send please mock this to use in tests")
end
return socket
end

require "busted.runner"({ standalone = false })
74 changes: 74 additions & 0 deletions rootfs/etc/nginx/lua/test/util/dns_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
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)
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" })
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)
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!")
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: oops!")

dns_helper.mock_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, "server returned error code: 1: format error")

dns_helper.mock_dns_query({})
assert.are.same({ "example.com" }, dns.resolve("example.com"))
assert.spy(s_ngx_log).was_called_with(ngx.ERR, "no A record resolved")

dns_helper.mock_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, "no A 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({
{
name = "example.com",
address = "192.168.1.1",
ttl = 3600,
},
{
name = "example.com",
address = "1.2.3.4",
ttl = 60,
}
})

local lrucache = require("resty.lrucache")
local old_lrucache_new = lrucache.new
lrucache.new = function(...)
local cache = old_lrucache_new(...)

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

return cache
end

assert.are.same({ "192.168.1.1", "1.2.3.4" }, dns.resolve("example.com"))

dns_helper.mock_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"))
end)
end)
Loading