You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/03/07 12:43:32 UTC
[GitHub] [incubator-apisix] qiujiayu opened a new pull request #1217: import
system dns resolver
qiujiayu opened a new pull request #1217: import system dns resolver
URL: https://github.com/apache/incubator-apisix/pull/1217
### Summary
auto import system DNS resolver
### Issues resolved
Fix #1164
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-596208120
@membphis @moonming I have finish this feature, check please.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-604180724
got it
Thanks,
Ming Wen
Twitter: _WenMing
YuanSheng Wang <no...@github.com> 于2020年3月26日周四 上午9:45写道：
> @moonming <https://github.com/moonming> 3. Use openresty's local = on
> feature, and patch tengine.
>
> we can not use local = on, we need to parse the user domain by DNS
> resolver at Lua land.
>
> For example:
> https://github.com/apache/incubator-apisix/blob/master/lua/apisix.lua#L183
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-apisix/pull/1217#issuecomment-604180006>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK3IE5QA54GW6DBM76DRJKXSVANCNFSM4LDPL4BQ>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397203146
##########
File path: conf/config.yaml
##########
@@ -80,7 +80,7 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS, If not set, read from `/etc/resolv.conf`
Review comment:
only keep
```
If not set, read from `/etc/resolv.conf`
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391703713
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
Review comment:
sorry, I don't know what the problem is here. This is just to check `sys_conf["enable_local_dns"] == true`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r393466662
##########
File path: lua/apisix.lua
##########
@@ -532,7 +531,7 @@ end
end -- do
-function _M.stream_init()
+function _M.stream_init(args)
Review comment:
should remove `args`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391693351
##########
File path: lua/apisix.lua
##########
@@ -532,8 +531,9 @@ end
end -- do
-function _M.stream_init()
+function _M.stream_init(args)
core.log.info("enter stream_init")
+ parse_args(args)
Review comment:
I check again the code, not need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-604178602
@moonming please take a view, I think we can merge this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-604180006
> @moonming 3\. Use openresty's `local = on` feature, and patch tengine.
we can not use `local = on`, we need to parse the user domain by DNS resolver at Lua land.
For example: https://github.com/apache/incubator-apisix/blob/master/lua/apisix.lua#L183
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391695637
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
+ end
+ else
+ sys_conf["dns_resolver"] = dns_addrs
Review comment:
There may do not configure 'dns_resolver' in `config.yaml`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391648553
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
+ end
+ else
+ sys_conf["dns_resolver"] = dns_addrs
Review comment:
the same as https://github.com/apache/incubator-apisix/pull/1167#discussion_r388035389
why we need this line?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391974988
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
+ end
+ else
+ sys_conf["dns_resolver"] = dns_addrs
Review comment:
no test cases cover
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397150826
##########
File path: lua/apisix.lua
##########
@@ -32,14 +32,21 @@ local pairs = pairs
local tostring = tostring
local load_balancer
-
+local dns_resolver
local parsed_domain
+local function parse_args(args)
+ if args and args["dns_resolver"] then
Review comment:
print some log, then we can check the log content when running the test case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397495748
##########
File path: lua/apisix.lua
##########
@@ -32,14 +32,20 @@ local pairs = pairs
local tostring = tostring
local load_balancer
-
+local dns_resolver
local parsed_domain
+local function parse_args(args)
+ dns_resolver = args and args["dns_resolver"]
+ core.log.info("dns resolver", core.json.delay_encode(dns_resolver, true))
Review comment:
Need a test case, we need to check it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-603820526
I think we should discuss the complete design on the mailing list or issue
then submit the pr，instead of discussing these basic issues over and over
again.
YuanSheng Wang <no...@github.com>于2020年3月25日 周三下午8:35写道：
> *@membphis* commented on this pull request.
> ------------------------------
>
> In conf/config.yaml
> <https://github.com/apache/incubator-apisix/pull/1217#discussion_r397817423>
> :
>
> > @@ -80,13 +80,13 @@ apisix:
> # udp: # UDP proxy port list
> # - 9200
> # - 9211
> - dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
> + dns_resolver: # If not set, read from `/etc/resolv.conf`
>
> I think it's easier to use extra separate fields to decide whether to use
> local / etc / resolv.conf.
>
> for example: dns_resolver_local: true
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-apisix/pull/1217#pullrequestreview-381102400>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK6SJIU36DOI4PUDWP3RJH3AVANCNFSM4LDPL4BQ>
> .
>
--
Thanks,
Ming Wen
Twitter: _WenMing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-598548718
> @qiujiayu I think the following design will be better. What do you think?
>
> 1. When the `dns_resolver` in `config.yaml` is not set any IP, then it will read the local `resolv.conf`. This is equivalent to a hidden switch.
> 2. Support IPv6, remove the current restrictions.
> 3. Use openresty's `local = on` feature, and patch tengine.
>
> The third can be TOOD.
1. Has been adjusted by the plan of what you said.
2. I didn't have an IPv6 environment to test.
3. Because APISIX resolves domain names using "resty.dns.resolver", DNS needs to be configured.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391981868
##########
File path: bin/apisix
##########
@@ -609,6 +632,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] == true then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
Review comment:
the `dns_resolver` used in `ngx_tpl`,
https://github.com/apache/incubator-apisix/pull/1217/files/ceee77794539d2b3dee3e90d41096aed2d976b51#diff-62edadffc237f13dc28a694080d293a8R120
https://github.com/apache/incubator-apisix/pull/1217/files/ceee77794539d2b3dee3e90d41096aed2d976b51#diff-62edadffc237f13dc28a694080d293a8R249
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391648199
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
Review comment:
the same as https://github.com/apache/incubator-apisix/pull/1167#discussion_r388035303
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-599152687
> 2\. I didn't have an IPv6 environment to test.
we can create a new issue about this, fix it later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391976952
##########
File path: bin/apisix
##########
@@ -609,6 +632,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] == true then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
Review comment:
`dns_resolver` is for what? not used after this line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-596264842
please take a look: https://github.com/apache/incubator-apisix/pull/1167#issuecomment-594976329
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396290270
##########
File path: bin/apisix
##########
@@ -114,6 +117,9 @@ stream {
.. [=[{*lua_cpath*};";
lua_socket_log_errors off;
+ resolver {% for _, dns_addr in ipairs(dns_resolver or {}) do %} {*dns_addr*} {% end %} valid={*dns_resolver_valid*} ipv6=off;
Review comment:
removed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397146090
##########
File path: conf/config.yaml
##########
@@ -80,7 +80,7 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS, If not set, read from `/etc/resolv.conf`
Review comment:
No more than 100 characters per line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r393467376
##########
File path: bin/apisix
##########
@@ -609,6 +632,15 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ local dns_resolver = sys_conf["dns_resolver"]
Review comment:
need to update doc of dns: https://github.com/apache/incubator-apisix/blob/master/conf/config.yaml#L83
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396884027
##########
File path: t/APISIX.pm
##########
@@ -161,7 +196,7 @@ _EOC_
lua_shared_dict upstream-healthcheck 32m;
lua_shared_dict worker-events 10m;
- resolver 8.8.8.8 114.114.114.114 ipv6=off;
+ resolver $dns_addrs_str ipv6=off;
Review comment:
remove all `ipv6=off`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397202735
##########
File path: bin/apisix
##########
@@ -260,7 +266,12 @@ http {
init_by_lua_block {
require "resty.core"
apisix = require("apisix")
- apisix.http_init()
+
+ local dns_resolver = { {% for _, dns_addr in ipairs(dns_resolver or {}) do %} "{*dns_addr*}", {% end %} }
+ local args = {
+ dns_resolver = dns_resolver,
+ }
+ apisix.http_init(args)
Review comment:
https://github.com/apache/incubator-apisix/pull/1217#discussion_r391652590
stream not need dns resolver, so removed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397817423
##########
File path: conf/config.yaml
##########
@@ -80,13 +80,13 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # If not set, read from `/etc/resolv.conf`
Review comment:
I think it's easier to use extra separate fields to decide whether to use local `/etc/resolv.conf`.
for example: `dns_resolver_local: true`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-598500231
@qiujiayu I think the following design will be better. What do you think?
1. When the `dns_resolver` in `config.yaml` is not set any IP, then it will read the local `resolv.conf`. This is equivalent to a hidden switch.
2. Support IPv6, remove the current restrictions.
3. Use openresty's `local = on` feature, and patch tengine.
The third can be TOOD.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397817423
##########
File path: conf/config.yaml
##########
@@ -80,13 +80,13 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # If not set, read from `/etc/resolv.conf`
Review comment:
I think it's easier to use extra separate fields to decide whether to use local `/ etc / resolv.conf`.
for example: `dns_resolver_local: true`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r398268856
##########
File path: conf/config.yaml
##########
@@ -80,13 +80,13 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # If not set, read from `/etc/resolv.conf`
Review comment:
After discussion on the mailing list, the current implementation is better.
https://lists.apache.org/thread.html/r78c7780ae94bb543d448125327c2038ca6bff328946120dd48a2bb5f%40%3Cdev.apisix.apache.org%3E
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391695637
##########
File path: bin/apisix
##########
@@ -609,6 +637,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
+ end
+ else
+ sys_conf["dns_resolver"] = dns_addrs
Review comment:
There may do not configure `dns_resolver` in `config.yaml`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r393468088
##########
File path: t/APISIX.pm
##########
@@ -37,6 +37,38 @@ sub read_file($) {
$data;
}
+sub local_dns_resolver() {
+ open my $in, "/etc/resolv.conf" or die "cannot open /etc/resolv.conf";
+ my @lines = <$in>;
+ my @dns_addrs = ();
+ foreach my $line (@lines){
+ $line =~ m/^nameserver\s+(\d+[.]\d+[.]\d+[.]\d+)\s*$/;
+ if ($1) {
+ push(@dns_addrs, $1);
+ }
+ }
+ close($in);
+ return @dns_addrs
+}
+
+
+my $dns_addrs_str = "";
+my $dns_addrs_tbl_str = "";
+my $enable_local_dns = 0;
+if ($enable_local_dns) {
+ my @dns_addrs = local_dns_resolver();
+ $dns_addrs_tbl_str = "{";
+ foreach my $addr (@dns_addrs){
+ $dns_addrs_str = "$dns_addrs_str $addr";
+ $dns_addrs_tbl_str = "$dns_addrs_tbl_str\"$addr\", ";
+ }
+ $dns_addrs_tbl_str = "$dns_addrs_tbl_str}";
+} else {
+ $dns_addrs_str = "8.8.8.8 114.114.114.114";
+ $dns_addrs_tbl_str = "{\"8.8.8.8\", \"114.114.114.114\"}";
+}
+
+
Review comment:
why modify `t/APISIX.pm`? which is just for test cases. And you hard-code `enable_local_dns=0`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397146805
##########
File path: lua/apisix.lua
##########
@@ -32,14 +32,21 @@ local pairs = pairs
local tostring = tostring
local load_balancer
-
+local dns_resolver
local parsed_domain
+local function parse_args(args)
+ if args and args["dns_resolver"] then
Review comment:
`dns_resolver = args and args["dns_resolver"]`
this style is simpler
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396290430
##########
File path: bin/apisix
##########
@@ -609,6 +632,15 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ local dns_resolver = sys_conf["dns_resolver"]
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-598182482
> please take a look: [#1167 (comment)](https://github.com/apache/incubator-apisix/pull/1167#issuecomment-594976329)
`local=on` it only valid in openresty. we can not use it in `tengine`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-598491735
> > please take a look: [#1167 (comment)](https://github.com/apache/incubator-apisix/pull/1167#issuecomment-594976329)
>
> `local=on` it only valid in openresty. we can not use it in `tengine`.
we can patch tengine as we do before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391977401
##########
File path: bin/apisix
##########
@@ -609,6 +632,22 @@ local function init()
sys_conf["worker_processes"] = "auto"
end
+ if sys_conf["enable_local_dns"] == true then
+ local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
+ if not dns_addrs then
+ error("failed to import local DNS: " .. err)
+ end
+
+ local dns_resolver = sys_conf["dns_resolver"]
+ if dns_resolver then
+ for _, addr in ipairs(dns_addrs) do
+ table.insert(dns_resolver, addr)
Review comment:
this is not a small PR, please add test cases, otherwise I can not merge it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis merged pull request #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r391652590
##########
File path: lua/apisix.lua
##########
@@ -532,8 +531,9 @@ end
end -- do
-function _M.stream_init()
+function _M.stream_init(args)
core.log.info("enter stream_init")
+ parse_args(args)
Review comment:
Is stream need dns resolver?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397145027
##########
File path: bin/apisix
##########
@@ -260,7 +266,12 @@ http {
init_by_lua_block {
require "resty.core"
apisix = require("apisix")
- apisix.http_init()
+
+ local dns_resolver = { {% for _, dns_addr in ipairs(dns_resolver or {}) do %} "{*dns_addr*}", {% end %} }
+ local args = {
+ dns_resolver = dns_resolver,
+ }
+ apisix.http_init(args)
Review comment:
we need to passed `args` to stream case
https://github.com/apache/incubator-apisix/blob/master/bin/apisix#L129-L133
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] membphis commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397142249
##########
File path: bin/apisix
##########
@@ -119,6 +122,9 @@ stream {
.. [=[{*lua_cpath*};";
lua_socket_log_errors off;
+ resolver {% for _, dns_addr in ipairs(dns_resolver or {}) do %} {*dns_addr*} {% end %} valid={*dns_resolver_valid*};
+ resolver_timeout 5;
Review comment:
We should put this configuration item in `conf / config.yaml`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396290873
##########
File path: t/APISIX.pm
##########
@@ -37,6 +37,38 @@ sub read_file($) {
$data;
}
+sub local_dns_resolver() {
+ open my $in, "/etc/resolv.conf" or die "cannot open /etc/resolv.conf";
+ my @lines = <$in>;
+ my @dns_addrs = ();
+ foreach my $line (@lines){
+ $line =~ m/^nameserver\s+(\d+[.]\d+[.]\d+[.]\d+)\s*$/;
+ if ($1) {
+ push(@dns_addrs, $1);
+ }
+ }
+ close($in);
+ return @dns_addrs
+}
+
+
+my $dns_addrs_str = "";
+my $dns_addrs_tbl_str = "";
+my $enable_local_dns = 0;
+if ($enable_local_dns) {
+ my @dns_addrs = local_dns_resolver();
+ $dns_addrs_tbl_str = "{";
+ foreach my $addr (@dns_addrs){
+ $dns_addrs_str = "$dns_addrs_str $addr";
+ $dns_addrs_tbl_str = "$dns_addrs_tbl_str\"$addr\", ";
+ }
+ $dns_addrs_tbl_str = "$dns_addrs_tbl_str}";
+} else {
+ $dns_addrs_str = "8.8.8.8 114.114.114.114";
+ $dns_addrs_tbl_str = "{\"8.8.8.8\", \"114.114.114.114\"}";
+}
+
+
Review comment:
had add a test case for enable local DNS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r393466809
##########
File path: bin/apisix
##########
@@ -114,6 +117,9 @@ stream {
.. [=[{*lua_cpath*};";
lua_socket_log_errors off;
+ resolver {% for _, dns_addr in ipairs(dns_resolver or {}) do %} {*dns_addr*} {% end %} valid={*dns_resolver_valid*} ipv6=off;
Review comment:
remove `ipv6=off`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396883369
##########
File path: conf/config.yaml
##########
@@ -80,7 +80,7 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS, remove this will enable local DNS
Review comment:
please use
```
If not set, read from `/etc/resolv.conf`
```
as comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on issue #1217: enable local
DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on issue #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#issuecomment-602956559
@moonming @membphis has any question for this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] moonming commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397203754
##########
File path: t/APISIX.pm
##########
@@ -196,7 +196,7 @@ _EOC_
lua_shared_dict upstream-healthcheck 32m;
lua_shared_dict worker-events 10m;
- resolver $dns_addrs_str ipv6=off;
+ resolver $dns_addrs_str;
Review comment:
remove ALL `ipv6=off`, please search this repo.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r396290223
##########
File path: lua/apisix.lua
##########
@@ -532,7 +531,7 @@ end
end -- do
-function _M.stream_init()
+function _M.stream_init(args)
Review comment:
removed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-apisix] qiujiayu commented on a change in pull request
#1217: enable local DNS
Posted by GitBox <gi...@apache.org>.
qiujiayu commented on a change in pull request #1217: enable local DNS
URL: https://github.com/apache/incubator-apisix/pull/1217#discussion_r397845171
##########
File path: conf/config.yaml
##########
@@ -80,13 +80,13 @@ apisix:
# udp: # UDP proxy port list
# - 9200
# - 9211
- dns_resolver: # default DNS resolver, with disable IPv6 and enable local DNS
+ dns_resolver: # If not set, read from `/etc/resolv.conf`
Review comment:
https://github.com/apache/incubator-apisix/pull/1217/commits/7ec7fd0d8596ea9002398d697f267f4313ba447b#diff-82e3bcd74a1f6e15d0577acef2d4fa8cR88
this commit add a swith for enable local DNS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services