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 2022/11/07 14:13:54 UTC

[GitHub] [apisix] TenYearsIn opened a new pull request, #8270: feat: add method that parse ip from /etc/hosts in high priority

TenYearsIn opened a new pull request, #8270:
URL: https://github.com/apache/apisix/pull/8270

   ### Description
   
   Fixes #5233 
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] soulbird commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
soulbird commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1306444984

   Need test cases also.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025208268


##########
apisix/core/resolver.lua:
##########
@@ -42,6 +58,21 @@ end
 -- @usage
 -- local ip, err = core.resolver.parse_domain("apache.org") -- "198.18.10.114"
 function _M.parse_domain(host)
+    local rev = HOSTS_IP_MATCH_CACHE[host]
+    if rev then
+        -- use ipv4 in high priority
+        local ip = rev["ipv4"]
+        if not ip then
+            ip = rev["ipv6"]

Review Comment:
   I thought of this setting before, But I think when the default value of this setting is true , We also use ipv4 .  So  I think the  ip's mapping that in /etc/hosts decides, but I improve ipv4's priority



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1309848039

   The tests fail because there is no "dns resolver domain: xxx to yyy" in the error.log, as resolving host in /etc/hosts doesn't go through the DNS client. Maybe you can add a similar log to let test pass.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1318220902

   I analyze the failed test cases reported by the CI And I Find out that some test cases ignore can't match this feature. So I modify some test-case code . Please review my thoughts @spacewander 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1309872298

   > The tests fail because there is no "dns resolver domain: xxx to yyy" in the error.log, as resolving host in /etc/hosts doesn't go through the DNS client. Maybe you can add a similar log to let test pass.
   
   Thx,  How can i trigger ci locally.  I want to know the cause .


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] tzssangglass commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025939255


##########
apisix/core/resolver.lua:
##########
@@ -19,15 +19,31 @@
 --
 -- @module core.resolver
 
-local json = require("apisix.core.json")
-local log = require("apisix.core.log")
-local utils = require("apisix.core.utils")
+local json           = require("apisix.core.json")
+local log            = require("apisix.core.log")
+local utils          = require("apisix.core.utils")
+local dns_utils      = require("resty.dns.utils")
+
+
+local HOSTS_IP_MATCH_CACHE = {}
 
 
 local _M = {}
 
 
+local function init_hosts_ip()
+    local hosts, err = dns_utils.parseHosts()
+    if not hosts then
+        return hosts, err
+    end
+    HOSTS_IP_MATCH_CACHE = hosts

Review Comment:
   When will `HOSTS_IP_MATCH_CACHE` be cleaned? APISIX will not update it after it is read at startup. Unless APISIX is restarted.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1309720875

   I analyze the error test cases reported by the CI, but I find that It has nothing to do with the code I commit, what I should do 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] tokers commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1016031580


##########
apisix/core/resolver.lua:
##########
@@ -19,15 +19,67 @@
 --
 -- @module core.resolver
 
-local json = require("apisix.core.json")
-local log = require("apisix.core.log")
-local utils = require("apisix.core.utils")
+local json           = require("apisix.core.json")
+local log            = require("apisix.core.log")
+local utils          = require("apisix.core.utils")
+local string_match   = string.match
+local open           = io.open
+
+
+local DEFAULT_HOSTS = "/etc/hosts"
+local HOSTS_IP_MATCH_CACHE = {}
 
 
 local _M = {}
 
 
+local function guess_name_type(name)
+    if name:match("^[%d%.]+$") then
+        return "ipv4"
+    end
+
+    if name:find(":") then
+        return "ipv6"
+    end
+
+    return "hostname"
+end
+
+
+local function init_hosts_ip()
+    if not DEFAULT_HOSTS then
+        return
+    end
+
+    local file = open(DEFAULT_HOSTS)
+    if not file then
+        return

Review Comment:
   It's better to print the error reason here.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025082715


##########
t/node/upstream-node-dns.t:
##########
@@ -67,7 +67,7 @@ passed
 
     local utils = require("apisix.core.utils")
     utils.dns_parse = function (domain)  -- mock: DNS parser
-        if domain == "test.com" then
+        if domain == "test2.com" then

Review Comment:
   I mean replacing `test.com` in this test file.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1016210113


##########
apisix/core/resolver.lua:
##########
@@ -19,15 +19,67 @@
 --
 -- @module core.resolver
 
-local json = require("apisix.core.json")
-local log = require("apisix.core.log")
-local utils = require("apisix.core.utils")
+local json           = require("apisix.core.json")
+local log            = require("apisix.core.log")
+local utils          = require("apisix.core.utils")
+local string_match   = string.match
+local open           = io.open
+
+
+local DEFAULT_HOSTS = "/etc/hosts"
+local HOSTS_IP_MATCH_CACHE = {}
 
 
 local _M = {}
 
 
+local function guess_name_type(name)
+    if name:match("^[%d%.]+$") then
+        return "ipv4"
+    end
+
+    if name:find(":") then
+        return "ipv6"
+    end
+
+    return "hostname"
+end
+
+
+local function init_hosts_ip()

Review Comment:
   Could we use the method from https://github.com/Kong/lua-resty-dns-client/blob/fb1e686a8857c7492fdd30ca228d200afbde19aa/src/resty/dns/client.lua#L493



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1309635294

   please approve running workflows @spacewander 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025071241


##########
apisix/core/resolver.lua:
##########
@@ -42,6 +58,21 @@ end
 -- @usage
 -- local ip, err = core.resolver.parse_domain("apache.org") -- "198.18.10.114"
 function _M.parse_domain(host)
+    local rev = HOSTS_IP_MATCH_CACHE[host]
+    if rev then
+        -- use ipv4 in high priority
+        local ip = rev["ipv4"]
+        if not ip then
+            ip = rev["ipv6"]

Review Comment:
   We need to consider this setting:
   https://github.com/apache/apisix/blob/2f4a4dba2c0cbc426ae99133c2546112ea71dba8/conf/config-default.yaml#L35



##########
t/node/upstream-node-dns.t:
##########
@@ -67,7 +67,7 @@ passed
 
     local utils = require("apisix.core.utils")
     utils.dns_parse = function (domain)  -- mock: DNS parser
-        if domain == "test.com" then
+        if domain == "test2.com" then

Review Comment:
   Look like this test now will resolve with `test.com` and return "127.0.0.2" always.
   Maybe we can change `test.com` to `test1.com` so we can bypass the /etc/hosts in the CI env.



##########
t/core/resolver.t:
##########
@@ -0,0 +1,90 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: resolve host from /etc/hosts
+--- config
+    location /t {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local resolver = require("apisix.core.resolver")
+            local domain = "localhost"
+            local ip_info, err = resolver.parse_domain(domain)
+            if not ip_info then
+                core.log.error("failed to parse domain: ", domain, ", error: ",err)
+                return
+            end
+            ngx.say("ip_info: ", require("toolkit.json").encode(ip_info))
+        }
+    }
+--- request
+GET /t

Review Comment:
   We can set the common sections at the top of this file like
   https://github.com/apache/apisix/blob/2f4a4dba2c0cbc426ae99133c2546112ea71dba8/t/core/ctx2.t#L23
   
   So we don't need to repeat it in each test.



##########
t/core/resolver.t:
##########
@@ -0,0 +1,90 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: resolve host from /etc/hosts
+--- config
+    location /t {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local resolver = require("apisix.core.resolver")
+            local domain = "localhost"
+            local ip_info, err = resolver.parse_domain(domain)
+            if not ip_info then
+                core.log.error("failed to parse domain: ", domain, ", error: ",err)
+                return
+            end
+            ngx.say("ip_info: ", require("toolkit.json").encode(ip_info))
+        }
+    }
+--- request
+GET /t
+--- response_body
+ip_info: "127.0.0.1"
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: resolve host from dns
+--- config
+    location /t {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local resolver = require("apisix.core.resolver")
+            local domain = "apisix.apache.org"
+            local ip_info, err = resolver.parse_domain(domain)
+            if not ip_info then
+                core.log.error("failed to parse domain: ", domain, ", error: ",err)
+                return
+            end
+            ngx.say("ip_info: ", require("toolkit.json").encode(ip_info))
+        }
+    }
+--- request
+GET /t
+--- response_body
+ip_info: "151.101.2.132"

Review Comment:
   Better to use injected code to check it.
   The machine running apisix.apache.org is not owned by us and its IP may change.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025082715


##########
t/node/upstream-node-dns.t:
##########
@@ -67,7 +67,7 @@ passed
 
     local utils = require("apisix.core.utils")
     utils.dns_parse = function (domain)  -- mock: DNS parser
-        if domain == "test.com" then
+        if domain == "test2.com" then

Review Comment:
   I mean replacing `test.com` in the whole test file.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025941278


##########
apisix/core/resolver.lua:
##########
@@ -19,15 +19,31 @@
 --
 -- @module core.resolver
 
-local json = require("apisix.core.json")
-local log = require("apisix.core.log")
-local utils = require("apisix.core.utils")
+local json           = require("apisix.core.json")
+local log            = require("apisix.core.log")
+local utils          = require("apisix.core.utils")
+local dns_utils      = require("resty.dns.utils")
+
+
+local HOSTS_IP_MATCH_CACHE = {}
 
 
 local _M = {}
 
 
+local function init_hosts_ip()
+    local hosts, err = dns_utils.parseHosts()
+    if not hosts then
+        return hosts, err
+    end
+    HOSTS_IP_MATCH_CACHE = hosts

Review Comment:
   This kind of configuration is basically unchanged, so I didn't consider reloading logic at that time



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander merged pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander merged PR #8270:
URL: https://github.com/apache/apisix/pull/8270


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1016214323


##########
apisix/core/resolver.lua:
##########
@@ -19,15 +19,67 @@
 --
 -- @module core.resolver
 
-local json = require("apisix.core.json")
-local log = require("apisix.core.log")
-local utils = require("apisix.core.utils")
+local json           = require("apisix.core.json")
+local log            = require("apisix.core.log")
+local utils          = require("apisix.core.utils")
+local string_match   = string.match
+local open           = io.open
+
+
+local DEFAULT_HOSTS = "/etc/hosts"
+local HOSTS_IP_MATCH_CACHE = {}
 
 
 local _M = {}
 
 
+local function guess_name_type(name)
+    if name:match("^[%d%.]+$") then
+        return "ipv4"
+    end
+
+    if name:find(":") then
+        return "ipv6"
+    end
+
+    return "hostname"
+end
+
+
+local function init_hosts_ip()

Review Comment:
   Yeah,  I will replace 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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #8270:
URL: https://github.com/apache/apisix/pull/8270#issuecomment-1311271365

   > > The tests fail because there is no "dns resolver domain: xxx to yyy" in the error.log, as resolving host in /etc/hosts doesn't go through the DNS client. Maybe you can add a similar log to let test pass.
   > 
   > Thx, How can i trigger ci locally. I want to know the cause .
   
   You can submit the PR to your fork, where you have the right to rerun it if you want.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1026047536


##########
t/core/resolver.t:
##########
@@ -0,0 +1,97 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }

Review Comment:
   Let's enable the no_error_log by default too:
   https://github.com/apache/apisix/blob/ae400b95b91240fb867c3cedc9fa65a6b5593f36/t/core/ctx2.t#L34



##########
apisix/core/resolver.lua:
##########
@@ -42,6 +58,21 @@ end
 -- @usage
 -- local ip, err = core.resolver.parse_domain("apache.org") -- "198.18.10.114"
 function _M.parse_domain(host)
+    local rev = HOSTS_IP_MATCH_CACHE[host]
+    if rev then
+        -- use ipv4 in high priority
+        local ip = rev["ipv4"]
+        if not ip then
+            ip = rev["ipv6"]

Review Comment:
   When the users disable ipv6, we should not return the IPv6 address in /etc/hosts to them.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1025210110


##########
t/core/resolver.t:
##########
@@ -0,0 +1,90 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: resolve host from /etc/hosts
+--- config
+    location /t {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local resolver = require("apisix.core.resolver")
+            local domain = "localhost"
+            local ip_info, err = resolver.parse_domain(domain)
+            if not ip_info then
+                core.log.error("failed to parse domain: ", domain, ", error: ",err)
+                return
+            end
+            ngx.say("ip_info: ", require("toolkit.json").encode(ip_info))
+        }
+    }
+--- request
+GET /t
+--- response_body
+ip_info: "127.0.0.1"
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: resolve host from dns
+--- config
+    location /t {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local resolver = require("apisix.core.resolver")
+            local domain = "apisix.apache.org"
+            local ip_info, err = resolver.parse_domain(domain)
+            if not ip_info then
+                core.log.error("failed to parse domain: ", domain, ", error: ",err)
+                return
+            end
+            ngx.say("ip_info: ", require("toolkit.json").encode(ip_info))
+        }
+    }
+--- request
+GET /t
+--- response_body
+ip_info: "151.101.2.132"

Review Comment:
   OK,  Thanks !



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] TenYearsIn commented on a diff in pull request #8270: feat: add method that parse ip from /etc/hosts in high priority

Posted by GitBox <gi...@apache.org>.
TenYearsIn commented on code in PR #8270:
URL: https://github.com/apache/apisix/pull/8270#discussion_r1027600502


##########
t/core/resolver.t:
##########
@@ -0,0 +1,97 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }

Review Comment:
   Please Review ~ @spacewander 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org