From af91334a9fd10ad1da5006e3dee6a506d09fa7cd Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Wed, 29 May 2024 17:52:10 +0800 Subject: [PATCH 1/9] fix(conf): remove `CNAME` from default `dns_order` option Because DNS servers are capable of performing recursive lookups on behalf of clients, it's often unnecessary for the client resolver to directly query CNAME records. KAG-4606 --- .../kong/remove_cname_from_dns_order_default_value.yml | 3 +++ kong.conf.default | 5 ++--- kong/templates/kong_defaults.lua | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml diff --git a/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml b/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml new file mode 100644 index 000000000000..83d301e9c64c --- /dev/null +++ b/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml @@ -0,0 +1,3 @@ +message: Removed `CNAME` from the default value of the `dns_order` option +type: bugfix +scope: Configuration diff --git a/kong.conf.default b/kong.conf.default index 74a1cf33deff..dc03eb89b439 100644 --- a/kong.conf.default +++ b/kong.conf.default @@ -1496,8 +1496,7 @@ # overridden by the environment variables `LOCALDOMAIN` and `RES_OPTIONS` if # they have been set. # -# Kong will resolve hostnames as either `SRV` or `A` records (in that order, and -# `CNAME` records will be dereferenced in the process). +# Kong will resolve hostnames as either `SRV` or `A` records. # In case a name was resolved as an `SRV` record it will also override any given # port number by the `port` field contents received from the DNS server. # @@ -1523,7 +1522,7 @@ # To read the file again after modifying it, # Kong must be reloaded. -#dns_order = LAST,SRV,A,CNAME # The order in which to resolve different +#dns_order = LAST,SRV,A # The order in which to resolve different # record types. The `LAST` type means the # type of the last successful lookup (for the # specified name). The format is a (case diff --git a/kong/templates/kong_defaults.lua b/kong/templates/kong_defaults.lua index ce532fd4b7ca..ca260d5661f4 100644 --- a/kong/templates/kong_defaults.lua +++ b/kong/templates/kong_defaults.lua @@ -161,7 +161,7 @@ db_cache_warmup_entities = services dns_resolver = NONE dns_hostsfile = /etc/hosts -dns_order = LAST,SRV,A,CNAME +dns_order = LAST,SRV,A dns_valid_ttl = NONE dns_stale_ttl = 3600 dns_cache_size = 10000 From ad498187a9166de40ee8a1beffbceaa257f5a76a Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 30 May 2024 10:39:45 +0800 Subject: [PATCH 2/9] change type from bugfix to breaking_change --- .../kong/remove_cname_from_dns_order_default_value.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml b/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml index 83d301e9c64c..1ead48d795ba 100644 --- a/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml +++ b/changelog/unreleased/kong/remove_cname_from_dns_order_default_value.yml @@ -1,3 +1,3 @@ -message: Removed `CNAME` from the default value of the `dns_order` option -type: bugfix +message: Removed `CNAME` from default `dns_order` option +type: breaking_change scope: Configuration From eef24f99244c06487a1b944ac446a7f2a76acbe6 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 30 May 2024 10:50:32 +0800 Subject: [PATCH 3/9] fix test case: 03-conf_loader_spec.lua --- spec/01-unit/03-conf_loader_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/03-conf_loader_spec.lua b/spec/01-unit/03-conf_loader_spec.lua index 10e0403d254e..c83e25055f0a 100644 --- a/spec/01-unit/03-conf_loader_spec.lua +++ b/spec/01-unit/03-conf_loader_spec.lua @@ -641,7 +641,7 @@ describe("Configuration loader", function() it("infer arrays (comma-separated strings)", function() local conf = assert(conf_loader()) assert.same({"bundled"}, conf.plugins) - assert.same({"LAST", "SRV", "A", "CNAME"}, conf.dns_order) + assert.same({"LAST", "SRV", "A"}, conf.dns_order) assert.is_nil(getmetatable(conf.plugins)) assert.is_nil(getmetatable(conf.dns_order)) end) From b3649ea10adef7b84ed227f344530b0752c7565c Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 31 May 2024 15:40:34 +0800 Subject: [PATCH 4/9] fix 05-dns_spec.lua: append ".com" to the domain to avoid dns server failure --- spec/02-integration/05-proxy/05-dns_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/02-integration/05-proxy/05-dns_spec.lua b/spec/02-integration/05-proxy/05-dns_spec.lua index 9607352a26ce..82125b567557 100644 --- a/spec/02-integration/05-proxy/05-dns_spec.lua +++ b/spec/02-integration/05-proxy/05-dns_spec.lua @@ -108,7 +108,7 @@ for _, strategy in helpers.each_strategy() do local service = bp.services:insert { name = "tests-retries", - host = "nowthisdoesnotexistatall", + host = "nowthisdoesnotexistatall.com", path = "/exist", port = 80, protocol = "http" From 2ec7da72aa4ed862048e283e906811e592d30051 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 31 May 2024 16:17:55 +0800 Subject: [PATCH 5/9] fix test cases --- spec/01-unit/14-dns_spec.lua | 2 +- spec/02-integration/05-proxy/05-dns_spec.lua | 8 +++++--- .../14-tracing/01-instrumentations_spec.lua | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/01-unit/14-dns_spec.lua b/spec/01-unit/14-dns_spec.lua index fda591d4df64..4518af584b7f 100644 --- a/spec/01-unit/14-dns_spec.lua +++ b/spec/01-unit/14-dns_spec.lua @@ -99,7 +99,7 @@ describe("DNS", function() resolver.new = old_new end) - it("returns an error and 503 on a Name Error (3)", function() + it("returns an error and 503 on a Name Error (3) #ttt", function() setup_it_block() mock_records = { ["konghq.com:" .. resolver.TYPE_A] = { errcode = 3, errstr = "name error" }, diff --git a/spec/02-integration/05-proxy/05-dns_spec.lua b/spec/02-integration/05-proxy/05-dns_spec.lua index 82125b567557..6bc69d9a68ec 100644 --- a/spec/02-integration/05-proxy/05-dns_spec.lua +++ b/spec/02-integration/05-proxy/05-dns_spec.lua @@ -108,7 +108,7 @@ for _, strategy in helpers.each_strategy() do local service = bp.services:insert { name = "tests-retries", - host = "nowthisdoesnotexistatall.com", + host = "nowthisdoesnotexistatall", path = "/exist", port = 80, protocol = "http" @@ -134,7 +134,7 @@ for _, strategy in helpers.each_strategy() do helpers.stop_kong() end) - it("fails with 503", function() + it("fails with 500", function() local r = proxy_client:send { method = "GET", path = "/", @@ -142,7 +142,9 @@ for _, strategy in helpers.each_strategy() do host = "retries.test" } } - assert.response(r).has.status(503) + -- The DNS server will reply "(2) server failure" for the domain in A + -- type without dot, like "nowthisdoesnotexistatall" + assert.response(r).has.status(500) end) end) diff --git a/spec/02-integration/14-tracing/01-instrumentations_spec.lua b/spec/02-integration/14-tracing/01-instrumentations_spec.lua index 781c85cd8fb2..fad7866d4533 100644 --- a/spec/02-integration/14-tracing/01-instrumentations_spec.lua +++ b/spec/02-integration/14-tracing/01-instrumentations_spec.lua @@ -546,7 +546,9 @@ for _, strategy in helpers.each_strategy() do method = "GET", path = "/test", }) - assert.res_status(503, r) + -- The DNS server will reply "(2) server failure" for the domain in A + -- type without dot, like "really-inexist-host" + assert.res_status(500, r) -- Getting back the TCP server input local ok, res = thread:join() From d5e0fd60562e4cec28c6c6a222aa452a57b39b98 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Fri, 31 May 2024 16:18:51 +0800 Subject: [PATCH 6/9] remove unnecessary tag #ttt --- spec/01-unit/14-dns_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/14-dns_spec.lua b/spec/01-unit/14-dns_spec.lua index 4518af584b7f..fda591d4df64 100644 --- a/spec/01-unit/14-dns_spec.lua +++ b/spec/01-unit/14-dns_spec.lua @@ -99,7 +99,7 @@ describe("DNS", function() resolver.new = old_new end) - it("returns an error and 503 on a Name Error (3) #ttt", function() + it("returns an error and 503 on a Name Error (3)", function() setup_it_block() mock_records = { ["konghq.com:" .. resolver.TYPE_A] = { errcode = 3, errstr = "name error" }, From c34179fdfc78d71bf7b2cc56a76c6b77d93b9d2b Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 3 Jun 2024 10:05:07 +0800 Subject: [PATCH 7/9] using .test domain instead of verifying 500 --- spec/02-integration/05-proxy/05-dns_spec.lua | 6 ++---- spec/02-integration/14-tracing/01-instrumentations_spec.lua | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/02-integration/05-proxy/05-dns_spec.lua b/spec/02-integration/05-proxy/05-dns_spec.lua index 6bc69d9a68ec..03ce29309ed7 100644 --- a/spec/02-integration/05-proxy/05-dns_spec.lua +++ b/spec/02-integration/05-proxy/05-dns_spec.lua @@ -108,7 +108,7 @@ for _, strategy in helpers.each_strategy() do local service = bp.services:insert { name = "tests-retries", - host = "nowthisdoesnotexistatall", + host = "nowthisdoesnotexistatall.test", path = "/exist", port = 80, protocol = "http" @@ -142,9 +142,7 @@ for _, strategy in helpers.each_strategy() do host = "retries.test" } } - -- The DNS server will reply "(2) server failure" for the domain in A - -- type without dot, like "nowthisdoesnotexistatall" - assert.response(r).has.status(500) + assert.response(r).has.status(503) end) end) diff --git a/spec/02-integration/14-tracing/01-instrumentations_spec.lua b/spec/02-integration/14-tracing/01-instrumentations_spec.lua index fad7866d4533..82c97f807211 100644 --- a/spec/02-integration/14-tracing/01-instrumentations_spec.lua +++ b/spec/02-integration/14-tracing/01-instrumentations_spec.lua @@ -524,7 +524,7 @@ for _, strategy in helpers.each_strategy() do -- intentionally trigger a DNS query error local service = bp.services:insert({ name = "inexist-host-service", - host = "really-inexist-host", + host = "really-inexist-host.test", port = 80, }) @@ -546,9 +546,7 @@ for _, strategy in helpers.each_strategy() do method = "GET", path = "/test", }) - -- The DNS server will reply "(2) server failure" for the domain in A - -- type without dot, like "really-inexist-host" - assert.res_status(500, r) + assert.res_status(503, r) -- Getting back the TCP server input local ok, res = thread:join() From fc90e032d693e2cd95efa1ff89c2a3e992a09d02 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 3 Jun 2024 10:06:29 +0800 Subject: [PATCH 8/9] recover test case name --- spec/02-integration/05-proxy/05-dns_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/02-integration/05-proxy/05-dns_spec.lua b/spec/02-integration/05-proxy/05-dns_spec.lua index 03ce29309ed7..3e2c9475723c 100644 --- a/spec/02-integration/05-proxy/05-dns_spec.lua +++ b/spec/02-integration/05-proxy/05-dns_spec.lua @@ -134,7 +134,7 @@ for _, strategy in helpers.each_strategy() do helpers.stop_kong() end) - it("fails with 500", function() + it("fails with 503", function() local r = proxy_client:send { method = "GET", path = "/", From 2891b65093218f746ccc5f6c5deaecb6499e45e3 Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 3 Jun 2024 10:35:36 +0800 Subject: [PATCH 9/9] fix typo: really-inexist-host -> really-inexist-host.test --- spec/02-integration/14-tracing/01-instrumentations_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/02-integration/14-tracing/01-instrumentations_spec.lua b/spec/02-integration/14-tracing/01-instrumentations_spec.lua index 82c97f807211..0d9af1927995 100644 --- a/spec/02-integration/14-tracing/01-instrumentations_spec.lua +++ b/spec/02-integration/14-tracing/01-instrumentations_spec.lua @@ -558,7 +558,7 @@ for _, strategy in helpers.each_strategy() do local dns_spans = assert_has_spans("kong.dns", spans) local upstream_dns for _, dns_span in ipairs(dns_spans) do - if dns_span.attributes["dns.record.domain"] == "really-inexist-host" then + if dns_span.attributes["dns.record.domain"] == "really-inexist-host.test" then upstream_dns = dns_span break end