fix(ipv6): allow disabling IPv6 resolve #6023
Conversation
apisix/cli/ops.lua
Outdated
| @@ -552,6 +552,7 @@ Please modify "admin_key" in conf/config.yaml . | |||
| admin_server_addr = admin_server_addr, | |||
| control_server_addr = control_server_addr, | |||
| prometheus_server_addr = prometheus_server_addr, | |||
| enable_ipv6 = yaml_conf.apisix.enable_ipv6, | |||
There was a problem hiding this comment.
The enable_ipv6 is already imported below
apisix/core/dns/client.lua
Outdated
| @@ -130,7 +130,7 @@ end | |||
|
|
|||
|
|
|||
| function _M.new(opts) | |||
| opts.ipv6 = true | |||
| opts.ipv6 = opts.ipv6 or true | |||
There was a problem hiding this comment.
Why not = local_conf.apisix.enable_ipv6? And we need a test for it.
There was a problem hiding this comment.
Why not = local_conf.apisix.enable_ipv6?
IMO, = local_conf.apisix.enable_ipv6 is a breaking change, Imagine a scenario where a third party or serverless plugin uses this function to parse ipv6, this change will causes an unexpected behavior, I wander if this is a fictitious demand.
Do let me know WDYT. Thanks.
And we need a test for it.
Sure, I will add it.
There was a problem hiding this comment.
We can use
if opts.ipv6 == nil then
...
end
so user can specify the ipv6 option to overwrite the default one.
AFAIK, we don't promise the DNS client always handles IPv6 query and the enable_ipv6 is enabled by default, so it's not a break change. (Even if users don't consider it is a bug fix)
There was a problem hiding this comment.
OK, I will add test case for it in next PR
apisix/core/dns/client.lua
Outdated
| opts.ipv6 = true | ||
| local local_conf = config_local.local_conf() | ||
|
|
||
| if opts.ipv6 == nil then |
There was a problem hiding this comment.
I spend some time digging into it. Look like it should be enable_ipv6, and it only works with the nameserver. See http://kong.github.io/lua-resty-dns-client/modules/resty.dns.client.html#init
This field is incorrect from the beginning...
spacewander
changed the title
What this PR does / why we need it:
Sync
apisix.enable_ipv6config to the generatednginx.confresolver sectionfix #6017
Pre-submission checklist: