diff --git a/README.md b/README.md index 36e4c71..b683ec3 100644 --- a/README.md +++ b/README.md @@ -51,8 +51,15 @@ Release process: 4. commit and tag the release 5. upload rock to LuaRocks -### 4.2.x unreleased - +### x.x.x unreleased + +- BREAKING: `getPeer` now returns the host-header value instead of the hostname + that was used to add the address. This is only breaking if a host was added through + `addHost` with an ip-address. In that case `getPeer` will no longer return the + ip-address as the hostname, but will now return `nil`. +- Added: option `useSRVname`, if truthy then `getPeer` will return the name as found + in the SRV record, instead of the hostname as added to the balancer. +- Added: callback return an extra parameter; the host-header for the address added/removed. - Fix: using the module instance instead of the passed one for dns resolution in the balancer (only affected testing). See [PR 88](https://github.com/Kong/lua-resty-dns-client/pull/88). diff --git a/spec/balancer/base_spec.lua b/spec/balancer/base_spec.lua index da0062f..5a22349 100644 --- a/spec/balancer/base_spec.lua +++ b/spec/balancer/base_spec.lua @@ -157,7 +157,7 @@ describe("[balancer_base]", function() describe("callbacks", function() local list - local handler = function(balancer, eventname, address, ip, port, hostname) + local handler = function(balancer, eventname, address, ip, port, hostname, hostheader) assert(({ added = true, removed = true, @@ -174,7 +174,7 @@ describe("[balancer_base]", function() assert.is.equal(address.port, port) end list[#list + 1] = { - balancer, eventname, address, ip, port, hostname, + balancer, eventname, address, ip, port, hostname, hostheader, } end @@ -202,7 +202,8 @@ describe("[balancer_base]", function() assert.is.table(list[2][3]) assert.equal("127.0.0.1", list[2][4]) assert.equal(80, list[2][5]) - assert.equal("localhost", list[2][6]) + assert.equal("localhost", list[2][6]) -- hostname + assert.equal("localhost", list[2][7]) -- hostheader end) @@ -225,7 +226,8 @@ describe("[balancer_base]", function() assert.is.table(list[2][3]) assert.equal("127.0.0.1", list[2][4]) assert.equal(80, list[2][5]) - assert.equal("localhost", list[2][6]) + assert.equal("localhost", list[2][6]) -- hostname + assert.equal("localhost", list[2][7]) -- hostheader b:removeHost("localhost", 80) ngx.sleep(0.1) @@ -240,7 +242,8 @@ describe("[balancer_base]", function() assert.equal(list[2][3], list[4][3]) -- same address object as added assert.equal("127.0.0.1", list[4][4]) assert.equal(80, list[4][5]) - assert.equal("localhost", list[4][6]) + assert.equal("localhost", list[4][6]) -- hostname + assert.equal("localhost", list[4][7]) -- hostheader end) end) diff --git a/spec/balancer/generic_spec.lua b/spec/balancer/generic_spec.lua index 70b95a0..e3d5952 100644 --- a/spec/balancer/generic_spec.lua +++ b/spec/balancer/generic_spec.lua @@ -1507,6 +1507,7 @@ for algorithm, balancer_module in helpers.balancer_types() do b = balancer_module.new({ dns = client, healthThreshold = 50, + useSRVname = false, }) end) @@ -1515,7 +1516,7 @@ for algorithm, balancer_module in helpers.balancer_types() do end) - it("returns expected results/types when using SRV", function() + it("returns expected results/types when using SRV with IP", function() dnsSRV({ { name = "konghq.com", target = "1.1.1.1", port = 2, weight = 3 }, }) @@ -1528,6 +1529,40 @@ for algorithm, balancer_module in helpers.balancer_types() do end) + it("returns expected results/types when using SRV with name ('useSRVname=false')", function() + dnsA({ + { name = "getkong.org", address = "1.2.3.4" }, + }) + dnsSRV({ + { name = "konghq.com", target = "getkong.org", port = 2, weight = 3 }, + }) + b:addHost("konghq.com", 8000, 50) + local ip, port, hostname, handle = b:getPeer() + assert.equal("1.2.3.4", ip) + assert.equal(2, port) + assert.equal("konghq.com", hostname) + assert.equal("userdata", type(handle.__udata)) + end) + + + it("returns expected results/types when using SRV with name ('useSRVname=true')", function() + b.useSRVname = true -- override setting specified when creating + + dnsA({ + { name = "getkong.org", address = "1.2.3.4" }, + }) + dnsSRV({ + { name = "konghq.com", target = "getkong.org", port = 2, weight = 3 }, + }) + b:addHost("konghq.com", 8000, 50) + local ip, port, hostname, handle = b:getPeer() + assert.equal("1.2.3.4", ip) + assert.equal(2, port) + assert.equal("getkong.org", hostname) + assert.equal("userdata", type(handle.__udata)) + end) + + it("returns expected results/types when using A", function() dnsA({ { name = "getkong.org", address = "1.2.3.4" }, @@ -1546,7 +1581,7 @@ for algorithm, balancer_module in helpers.balancer_types() do local ip, port, hostname, handle = b:getPeer() assert.equal("4.3.2.1", ip) assert.equal(8000, port) - assert.equal("4.3.2.1", hostname) + assert.equal(nil, hostname) assert.equal("userdata", type(handle.__udata)) end) @@ -1556,7 +1591,7 @@ for algorithm, balancer_module in helpers.balancer_types() do local ip, port, hostname, handle = b:getPeer() assert.equal("[::1]", ip) assert.equal(8000, port) - assert.equal("::1", hostname) + assert.equal(nil, hostname) assert.equal("userdata", type(handle.__udata)) end) diff --git a/src/resty/dns/balancer/base.lua b/src/resty/dns/balancer/base.lua index 193735f..3f058a1 100644 --- a/src/resty/dns/balancer/base.lua +++ b/src/resty/dns/balancer/base.lua @@ -201,7 +201,7 @@ local mt_objBalancer = { __index = objBalancer } -- =========================================================================== -- Returns the peer info. --- @return ip-address, port and hostname of the target, or nil+err if unavailable +-- @return ip-address, port and hostheader for the target, or nil+err if unavailable -- or lookup error function objAddr:getPeer(cacheOnly) if not self.available then @@ -219,16 +219,13 @@ function objAddr:getPeer(cacheOnly) local ip, port, try_list = self.host.balancer.dns.toip(self.ip, self.port, cacheOnly) if not ip then port = tostring(port) .. ". Tried: " .. tostring(try_list) + return ip, port end - -- which is the proper name to return in this case? - -- `self.host.hostname`? or the named SRV entry: `self.ip`? - -- use our own hostname, as it might be used to mark this address - -- as unhealthy, so we must be able to find it - return ip, port, self.host.hostname - else - -- just an IP address - return self.ip, self.port, self.host.hostname + + return ip, port, self.hostHeader end + + return self.ip, self.port, self.hostHeader end -- disables an address object from the balancer. @@ -252,7 +249,7 @@ function objAddr:delete() " (host ", (self.host or EMPTY).hostname, ")") self.host.balancer:callback("removed", self, self.ip, - self.port, self.host.hostname) + self.port, self.host.hostname, self.hostHeader) self.host.balancer:removeAddress(self) self.host = nil end @@ -343,6 +340,24 @@ function objBalancer:newAddress(addr) addr.host:addWeight(addr.weight) + if addr.host.nameType ~= "name" then + -- hostname is an IP address + addr.hostHeader = nil + else + -- hostname is an actual name + if addr.ipType ~= "name" then + -- the address is an ip, so use the hostname as header value + addr.hostHeader = addr.host.hostname + else + -- the address itself is a nested name (SRV) + if addr.useSRVname then + addr.hostHeader = addr.ip + else + addr.hostHeader = addr.host.hostname + end + end + end + ngx_log(ngx_DEBUG, addr.host.log_prefix, "new address for host '", addr.host.hostname, "' created: ", addr.ip, ":", addr.port, " (weight ", addr.weight,")") @@ -650,6 +665,7 @@ function objHost:addAddress(entry) port = (entry.port ~= 0 and entry.port) or self.port, weight = weight or self.nodeWeight, host = self, + useSRVname = self.balancer.useSRVname, } end @@ -813,6 +829,7 @@ function objBalancer:newHost(host) host.lastSorted = nil -- last successful dns query, sorted for comparison host.addresses = {} -- list of addresses (address objects) this host resolves to host.expire = nil -- time when the dns query this host is based upon expires + host.nameType = dns_utils.hostnameType(host.hostname) -- 'ipv4', 'ipv6' or 'name' -- insert into our parent balancer before recalculating (in queryDns) @@ -963,7 +980,7 @@ function objBalancer:addAddress(address) local list = self.addresses assert(list[address] == nil, "Can't add address twice") - self:callback("added", address, address.ip, address.port, address.host.hostname) + self:callback("added", address, address.ip, address.port, address.host.hostname, address.hostHeader) list[#list + 1] = address self:onAddAddress(address) @@ -1063,16 +1080,16 @@ end -- retain some state over retries. See also `setAddressStatus`. -- @param hashValue (optional) number for consistent hashing, if supported by -- the algorithm. The hashValue must be an (evenly distributed) `integer >= 0`. --- @return `ip + port + hostname` + `handle`, or `nil+error` +-- @return `ip + port + hostheader` + `handle`, or `nil+error` -- @within User properties -- @usage -- -- get an IP address --- local ip, port, hostname, handle = b:getPeer() +-- local ip, port, hostheader, handle = b:getPeer() -- -- -- go do the connection stuff here... -- -- -- on a retry do: --- ip, port, hostname, handle = b:getPeer(true, handle) -- pass in previous 'handle' +-- ip, port, hostheader, handle = b:getPeer(true, handle) -- pass in previous 'handle' -- -- -- go try again -- @@ -1256,11 +1273,18 @@ end -- -- Signature of the callback is for address adding/removing: -- --- `function(balancer, "added"/"removed", address, ip, port, hostname)` +-- `function(balancer, "added"/"removed", address, ip, port, hostname, hostheader)` -- --- where `ip` might also --- be a hostname if the DNS resolution returns another name (usually in --- SRV records). +-- - `address` is the address object added +-- - `ip` is the IP address for this object, but might also be a hostname if +-- the DNS resolution returns another name (usually in SRV records) +-- - `port` is the port to use +-- - `hostname` is the hostname for which the address was added to the balancer +-- with `addHost` (resolving that name caused the creation of this address) +-- - `hostheader` is the hostheader to be used. This can have 3 values; 1) `nil` if the +-- `hostname` added was an ip-address to begin with, 2) it will be equal to the +-- name in `ip` if there is a named SRV entry, and `useSRVname == true`, 3) otherwise +-- it will be equal to `hostname` -- -- For health updates the signature is: -- @@ -1274,9 +1298,9 @@ end function objBalancer:setCallback(callback) assert(type(callback) == "function", "expected a callback function") - self.callback = function(balancer, action, address, ip, port, hostname) + self.callback = function(balancer, action, address, ip, port, hostname, hostheader) local ok, err = ngx.timer.at(0, function(premature) - callback(balancer, action, address, ip, port, hostname) + callback(balancer, action, address, ip, port, hostname, hostheader) end) if not ok then @@ -1371,6 +1395,10 @@ end -- - `healthThreshold` (optional) minimum percentage of the balancer weight that must -- be healthy/available for the whole balancer to be considered healthy. Defaults -- to 0% if omitted. +-- - `useSRVname` (optional) if truthy, then in case of the hostname resolving to +-- an SRV record with another level of names, the returned hostname by `getPeer` will +-- not be the name of the host as added, but the name of the entry in the SRV +-- record. -- @param opts table with options -- @return new balancer object or nil+error -- @within User properties @@ -1400,6 +1428,7 @@ _M.new = function(opts) ttl0Interval = opts.ttl0 or TTL_0_RETRY, -- refreshing ttl=0 records healthy = false, -- initial healthstatus of the balancer healthThreshold = opts.healthThreshold or 0, -- % healthy weight for overall balancer health + useSRVname = not not opts.useSRVname, -- force to boolean } self = setmetatable(self, mt_objBalancer) self.super = objBalancer