Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: APISIX 2.10.5 (LTS) can't support IPv6 Upstream host IP #7563

Closed
topzyh opened this issue Jul 29, 2022 · 6 comments · Fixed by #7594
Closed

bug: APISIX 2.10.5 (LTS) can't support IPv6 Upstream host IP #7563

topzyh opened this issue Jul 29, 2022 · 6 comments · Fixed by #7594

Comments

@topzyh
Copy link
Contributor

topzyh commented Jul 29, 2022

Current Behavior

etcd data:

{
    "scheme":"http",
    "name":"safebox-esp-web_web-console-cns2waf_5009",
    "desc":"Created by apisix-ingress-controller, DO NOT modify it manually",
    "id":"a48bc649",
    "update_time":1659003639,
    "create_time":1659003457,
    "nodes":[
        {
            "weight":100,
            "priority":0,
            "host":"2409:XXXX:XXXX:2000:0:1:afd:7d20",
            "port":4009
        },
        {
            "weight":100,
            "priority":0,
            "host":"2409:XXXX:XXXX:2000:0:1:afd:7d68",
            "port":4009
        }
    ],
    "type":"roundrobin",
    "labels":{
        "managed-by":"apisix-ingress-controller"
    },
    "hash_on":"vars",
    "pass_host":"pass"
}

Expected Behavior

No response

Error Logs

No response

Steps to Reproduce

I had modify /usr/local/apisix/apisix/schema_def.lua LINE 40

-- local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"

AND /usr/local/apisix/apisix/balancer.lua LINE 294

            local has_ipv6 = string.find(server.host, ":", 1, true)
            if (has_ipv6 ~= nil) then
                local is_ipv6_plus = string.find(server.host, "[", 1, true)
                if (is_ipv6_plus == nil) then
                    server.host = "["..server.host.."]"
                end
            end

            local ok, err = balancer.set_current_peer(server.host, server.port, pool_opt)

When upstream hosts count =1,its working;
But upstream hosts number >=2 ,error log:

2022/07/28 10:07:09 [error] 51#51: *13796 [lua] balancer.lua:370: run(): failed to set server peer [[2409:XXXX:XXXX:2000:0:1:afd:7d65:4009]:nil] err: invalid IPv6 address while connecting to upstream, client: 10.253.91.92, server: _, request: "GET /web-console-cnswaf/ HTTP/1.1", host: "xxx.xxx.com"

server.host = 2409:XXXX:XXXX:2000:0:1:afd:7d65:4009 (4009 is port)
server.port = nil

Environment

  • APISIX version: 2.10.5uanme
  • Operating system: CentOS 7 , Linux 4.19
  • OpenResty version: 1.19.9.1
  • etcd version, if relevant: 3.4.0
@spacewander
Copy link
Member

@kingluo
Would you like to take a look?

@kingluo
Copy link
Contributor

kingluo commented Jul 31, 2022

@topzyh Why the address in your test output doesn't match the configuration in etcd?
That is, 2409:XXXX:XXXX:2000:0:1:afd:7d65 doesn't match those two addresses in your configuration.
And it's werid that the server.port is nil.
Could you give test output with correct corresponding configuration?

You should not quote the host with brackets to invoke set_current_peer, which should cause error to parse the address.
[2409:XXXX:XXXX:2000:0:1:afd:7d65:4009] is invalid, for nginx, it should be [2409:XXXX:XXXX:2000:0:1:afd:7d65]:4009.

Instead, I think the ingress controller should quote the server.host with brackets for ipv6 address. Without brackets, nginx will treat it as ipv4 address and makes it wrong.

@spacewander
Copy link
Member

spacewander commented Jul 31, 2022

@kingluo
Thanks for your analysis.

Is it possible to support IPv6 without bracket? Support this format will be great so people don't need to deal with the Nginx's special behavior.

@kingluo
Copy link
Contributor

kingluo commented Jul 31, 2022

@kingluo Thanks for your analysis.

Is it possible to support IPv6 without bracket? Support this format will be great so people don't need to deal with the Nginx's special behavior.

No, it's not nginx specific. In convention, to concatent with the port, the ipv6 must be quoted with bracket. So nginx does the right thing.

In fact, here the issue is not triggered by nginx only.

When apisix constructs the balancer, it would merge the address and port of each upstream as server url. After the server picker picks a server, it would parse the server url back into host and port. But since the address part of the server url is not quoted with brackets, it would fail to parse the ipv6 address, then leave the host value as the same server url value (2409:XXXX:XXXX:2000:0:1:afd:7d65:4009), and port is nil!

Check https://github.com/apache/apisix/blob/master/apisix/balancer.lua#L193 and https://github.com/apache/apisix/blob/master/apisix/core/utils.lua#L156 for codes.
Then when it passes the host to nginx, nginx would treat it as ipv4 address and raise error.

In the output by @topzyh, he modifies the codes to quote the host with brackets in the wrong place, then it comes in another story: nginx try to parse it as ipv6 address, and find it's in wrong format (with extra port in tail).

And in fact, in our ipv6 test, the input host is quoted with brackets, so it passes.

So all in all, in my opinion, the solution is, either we adjust the admin API to quote the ipv6 address automatically, or we require the user to quote the ipv6 address with brackets. It's a bug in our codes.

kingluo added a commit to kingluo/apisix that referenced this issue Aug 2, 2022
kingluo added a commit to kingluo/apisix that referenced this issue Aug 2, 2022
@topzyh
Copy link
Contributor Author

topzyh commented Aug 3, 2022

Thanks @kingluo @spacewander

  1. 这个不带方括号的 IPv6 nodes 是来自 apisix-ingress-controller 调用 APISIX admin-api 写入的,这边的 K8S Pods 全部都是 IPv6 (是不是需要在apisix-ingress-controller上加上方括号?);

  2. server.host = 2409:XXXX:XXXX:2000:0:1:afd:7d65:4009 ; server.port = nil 似乎是我在 balancer.set_current_peer 之前 core.log.info( server ) 出现的,即使我没改lua代码;

  3. @kingluo 看到您提交了 fix,新版本是支持自动加上方括号的么?

再次感谢您的解答。

@kingluo
Copy link
Contributor

kingluo commented Aug 3, 2022

  1. @kingluo 看到您提交了 fix,新版本是支持自动加上方括号的么?

Yes, the bug is fixed.

There are two formats of nodes. One is hash table, where each item is address with optional port, the other is node array. The ingress uses the latter one

For the hash table case, if the node is ipv6 with port, then the ipv6 address must be quoted with square brackets, otherwise it's impossible to distingush the port from node.

For node array, since the address and port are specified seperately, the bugfix would quote the address if the address is ipv6 automatically.

spacewander pushed a commit that referenced this issue Aug 4, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this issue Nov 4, 2022
spacewander pushed a commit to spacewander/incubator-apisix that referenced this issue Nov 9, 2022
spacewander pushed a commit that referenced this issue Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants