feat(balancer): add use_srv_name field (Kong#9430)

add upstream use srv name config If set, the balancer will use SRV hostname(if DNS Answer has SRV record) as the proxy upstream host. FTI-4301
Benwhite112 · Sep 26, 2022 · 34e2ec1 · 34e2ec1
1 parent 56270cb
commit 34e2ec1
Show file tree

Hide file tree

Showing 10 changed files with 144 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -74,13 +74,16 @@
 - Fix issue where external plugins crashing with unhandled exceptions
   would cause high CPU utilization after the automatic restart.
   [#9384](https://github.com/Kong/kong/pull/9384)
+- Add `use_srv_name` options to upstream for balancer.
+  [#9430](https://github.com/Kong/kong/pull/9430)
 - Fix issue in `header_filter` instrumentation where the span was not
   correctly created.
   [#9434](https://github.com/Kong/kong/pull/9434)
 - Fix issue in router building where when field contains an empty table,
   the generated expression is invalid.
   [#9451](https://github.com/Kong/kong/pull/9451)
 
+
 ## [3.0.0]
 
 > Released 2022/09/12

diff --git a/autodoc/admin-api/data/admin-api.lua b/autodoc/admin-api/data/admin-api.lua
@@ -1693,6 +1693,7 @@ return {
         ["hash_fallback_uri_capture"] = { kind = "semi-optional", skip_in_example = true, description = [[The name of the route URI capture to take the value from as hash input. Only required when `hash_fallback` is set to `uri_capture`.]] },
         ["host_header"] = { description = [[The hostname to be used as `Host` header when proxying requests through Kong.]], example = "example.com", },
         ["client_certificate"] = { description = [[If set, the certificate to be used as client certificate while TLS handshaking to the upstream server.]] },
+        ["use_srv_name"] = { description = [[If set, the balancer will use SRV hostname(if DNS Answer has SRV record) as the proxy upstream `Host`.]] },
         ["healthchecks.active.timeout"] = { description = [[Socket timeout for active health checks (in seconds).]] },
         ["healthchecks.active.concurrency"] = { description = [[Number of targets to check concurrently in active health checks.]] },
         ["healthchecks.active.type"] = { description = [[Whether to perform active health checks using HTTP or HTTPS, or just attempt a TCP connection.]] },

diff --git a/kong-3.1.0-0.rockspec b/kong-3.1.0-0.rockspec
@@ -241,6 +241,7 @@ build = {
     ["kong.db.migrations.core.014_230_to_270"] = "kong/db/migrations/core/014_230_to_270.lua",
     ["kong.db.migrations.core.015_270_to_280"] = "kong/db/migrations/core/015_270_to_280.lua",
     ["kong.db.migrations.core.016_280_to_300"] = "kong/db/migrations/core/016_280_to_300.lua",
+    ["kong.db.migrations.core.017_300_to_310"] = "kong/db/migrations/core/017_300_to_310.lua",
     ["kong.db.migrations.operations.200_to_210"] = "kong/db/migrations/operations/200_to_210.lua",
     ["kong.db.migrations.operations.210_to_211"] = "kong/db/migrations/operations/210_to_211.lua",
     ["kong.db.migrations.operations.212_to_213"] = "kong/db/migrations/operations/212_to_213.lua",

diff --git a/kong/db/migrations/core/017_300_to_310.lua b/kong/db/migrations/core/017_300_to_310.lua
@@ -0,0 +1,20 @@
+return {
+    postgres = {
+      up = [[
+        DO $$
+            BEGIN
+            ALTER TABLE IF EXISTS ONLY "upstreams" ADD "use_srv_name"  BOOLEAN DEFAULT false;
+            EXCEPTION WHEN DUPLICATE_COLUMN THEN
+            -- Do nothing, accept existing state
+            END;
+        $$;
+      ]]
+    },
+
+    cassandra = {
+      up = [[
+        ALTER TABLE upstreams ADD use_srv_name boolean;
+      ]]
+    },
+  }
+
diff --git a/kong/db/migrations/core/init.lua b/kong/db/migrations/core/init.lua
@@ -14,4 +14,5 @@ return {
   "014_230_to_270",
   "015_270_to_280",
   "016_280_to_300",
+  "017_300_to_310",
 }
diff --git a/kong/db/schema/entities/upstreams.lua b/kong/db/schema/entities/upstreams.lua
@@ -202,6 +202,7 @@ local r =  {
     { tags = typedefs.tags },
     { host_header = typedefs.host_with_optional_port },
     { client_certificate = { type = "foreign", reference = "certificates" }, },
+    { use_srv_name =  { type = "boolean", default = false, }, },
   },
   entity_checks = {
     -- hash_on_header must be present when hashing on header

diff --git a/kong/runloop/balancer/balancers.lua b/kong/runloop/balancer/balancers.lua
@@ -130,7 +130,7 @@ local function create_balancer_exclusive(upstream)
     ttl0Interval = opts.ttl0 or TTL_0_RETRY, -- refreshing ttl=0 records
     healthy = false, -- initial healthstatus of the balancer
     healthThreshold = health_threshold or 0, -- % healthy weight for overall balancer health
-    useSRVname = not not opts.useSRVname, -- force to boolean
+    useSRVname = upstream.use_srv_name,
   }, balancer_mt)
 
   for _, target in ipairs(targets_list) do

diff --git a/spec/01-unit/01-db/01-schema/09-upstreams_spec.lua b/spec/01-unit/01-db/01-schema/09-upstreams_spec.lua
@@ -226,6 +226,20 @@ describe("load upstreams", function()
   end)
 
 
+  it("produces set use_srv_name flag", function()
+    local u = {
+      name = "www.example.com",
+      use_srv_name = true,
+    }
+    u = Upstreams:process_auto_fields(u, "insert")
+    local ok, err = Upstreams:validate(u)
+    assert.truthy(ok)
+    assert.is_nil(err)
+    assert.same(u.name, "www.example.com")
+    assert.same(u.use_srv_name, true)
+  end)
+
+
   it("produces defaults", function()
     local u = {
       name = "www.example.com",
@@ -240,6 +254,7 @@ describe("load upstreams", function()
     assert.same(u.hash_fallback, "none")
     assert.same(u.hash_on_cookie_path, "/")
     assert.same(u.slots, 10000)
+    assert.same(u.use_srv_name, false)
     assert.same(u.healthchecks, {
       active = {
         type = "http",

diff --git a/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua b/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua
@@ -1775,6 +1775,7 @@ describe("declarative config: flatten", function()
                 hash_fallback_query_arg = null,
                 hash_on_uri_capture = null,
                 hash_fallback_uri_capture = null,
+                use_srv_name = false,
                 healthchecks = {
                   active = {
                     concurrency = 10,
@@ -1831,6 +1832,7 @@ describe("declarative config: flatten", function()
                 hash_fallback_query_arg = null,
                 hash_on_uri_capture = null,
                 hash_fallback_uri_capture = null,
+                use_srv_name = false,
                 healthchecks = {
                   active = {
                     concurrency = 10,

diff --git a/spec/01-unit/09-balancer/01-generic_spec.lua b/spec/01-unit/09-balancer/01-generic_spec.lua
@@ -11,6 +11,39 @@ local utils = require "kong.tools.utils"
 
 local ws_id = utils.uuid()
 
+local hc_defaults = {
+  active = {
+    timeout = 1,
+    concurrency = 10,
+    http_path = "/",
+    healthy = {
+      interval = 0,  -- 0 = probing disabled by default
+      http_statuses = { 200, 302 },
+      successes = 0, -- 0 = disabled by default
+    },
+    unhealthy = {
+      interval = 0, -- 0 = probing disabled by default
+      http_statuses = { 429, 404,
+                        500, 501, 502, 503, 504, 505 },
+      tcp_failures = 0,  -- 0 = disabled by default
+      timeouts = 0,      -- 0 = disabled by default
+      http_failures = 0, -- 0 = disabled by default
+    },
+  },
+  passive = {
+    healthy = {
+      http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
+                        300, 301, 302, 303, 304, 305, 306, 307, 308 },
+      successes = 0,
+    },
+    unhealthy = {
+      http_statuses = { 429, 500, 503 },
+      tcp_failures = 0,  -- 0 = circuit-breaker disabled by default
+      timeouts = 0,      -- 0 = circuit-breaker disabled by default
+      http_failures = 0, -- 0 = circuit-breaker disabled by default
+    },
+  },
+}
 
 local unset_register = {}
 local function setup_block()
@@ -73,39 +106,6 @@ local upstream_index = 0
 local function new_balancer(algorithm)
   upstream_index = upstream_index + 1
   local upname="upstream_" .. upstream_index
-  local hc_defaults = {
-    active = {
-      timeout = 1,
-      concurrency = 10,
-      http_path = "/",
-      healthy = {
-        interval = 0,  -- 0 = probing disabled by default
-        http_statuses = { 200, 302 },
-        successes = 0, -- 0 = disabled by default
-      },
-      unhealthy = {
-        interval = 0, -- 0 = probing disabled by default
-        http_statuses = { 429, 404,
-                          500, 501, 502, 503, 504, 505 },
-        tcp_failures = 0,  -- 0 = disabled by default
-        timeouts = 0,      -- 0 = disabled by default
-        http_failures = 0, -- 0 = disabled by default
-      },
-    },
-    passive = {
-      healthy = {
-        http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
-                          300, 301, 302, 303, 304, 305, 306, 307, 308 },
-        successes = 0,
-      },
-      unhealthy = {
-        http_statuses = { 429, 500, 503 },
-        tcp_failures = 0,  -- 0 = circuit-breaker disabled by default
-        timeouts = 0,      -- 0 = circuit-breaker disabled by default
-        http_failures = 0, -- 0 = circuit-breaker disabled by default
-      },
-    },
-  }
   local my_upstream = { id=upname, name=upname, ws_id=ws_id, slots=10, healthchecks=hc_defaults, algorithm=algorithm }
   local b = (balancers.create_balancer(my_upstream, true))
 
@@ -1460,6 +1460,72 @@ for _, algorithm in ipairs{ "consistent-hashing", "least-connections", "round-ro
     end)
 
 
+    describe("getpeer() upstream use_srv_name = false", function()
+
+      local b
+
+      before_each(function()
+        upstream_index = upstream_index + 1
+        local upname="upstream_" .. upstream_index
+
+        local my_upstream = { id=upname, name=upname, ws_id=ws_id, slots=10, healthchecks=hc_defaults, algorithm=algorithm, use_srv_name = false }
+        b = (balancers.create_balancer(my_upstream, true))
+      end)
+
+      after_each(function()
+        b = nil
+      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 },
+        })
+        add_target(b, "konghq.com", 8000, 50)
+        local ip, port, hostname, handle = b:getPeer(true, nil, "a string")
+        assert.equal("1.2.3.4", ip)
+        assert.equal(2, port)
+        assert.equal("konghq.com", hostname)
+        assert.not_nil(handle)
+      end)
+    end)
+
+
+    describe("getpeer() upstream use_srv_name = true", function()
+
+      local b
+
+      before_each(function()
+        upstream_index = upstream_index + 1
+        local upname="upstream_" .. upstream_index
+        local my_upstream = { id=upname, name=upname, ws_id=ws_id, slots=10, healthchecks=hc_defaults, algorithm=algorithm, use_srv_name = true }
+        b = (balancers.create_balancer(my_upstream, true))
+      end)
+
+      after_each(function()
+        b = nil
+      end)
+
+
+      it("returns expected results/types when using SRV with name ('useSRVname=true')", function()
+        dnsA({
+          { name = "getkong.org", address = "1.2.3.4" },
+        })
+        dnsSRV({
+          { name = "konghq.com", target = "getkong.org", port = 2, weight = 3 },
+        })
+        add_target(b, "konghq.com", 8000, 50)
+        local ip, port, hostname, handle = b:getPeer(true, nil, "a string")
+        assert.equal("1.2.3.4", ip)
+        assert.equal(2, port)
+        assert.equal("getkong.org", hostname)
+        assert.not_nil(handle)
+      end)
+    end)
+
 
     describe("getpeer()", function()