rpardini / docker-registry-proxy

An HTTPS Proxy for Docker providing centralized configuration and caching of any registry (quay.io, DockerHub, k8s.gcr.io)
Apache License 2.0
912 stars 170 forks source link

Add support for IPv6 #76

Closed mkowalski closed 3 years ago

mkowalski commented 3 years ago

I have tested it by running the proxy on a dual-stack machine, using IPv6-only client and pulling an image. Connectivity between client and proxy going via IPv6 and between proxy and dockerhub via IPv4 - this scenario works as expected

rpardini commented 3 years ago

Hmm. I wonder why we had ipv6=off in there to begin with. There was something, but it was years ago, and it could have been registry-specific. Did you run an IPv4-only run with this patch as well?

mkowalski commented 3 years ago

Ah yeah, the very basic scenario of everything being IPv4 also works, so no regression in this field

rpardini commented 3 years ago

I'm gonna merge, this really shouldn't break anything. (famous last words?)

gw0 commented 3 years ago

It seems to break ill-configured destination servers:

[crit] 66#66: *116 connect() to [2606:4700:90:0:f22e:abcd:abcd:abcd]:443 failed (99: Address not available) while connecting to upstream, client: 127.0.0.1, server: proxy_caching_
salanki commented 3 years ago

Yes, this is giving me major beef on ipv4 only nodes.

mkowalski commented 3 years ago

@salanki I no longer work for the company where we needed this, but can you show ip -4 a and ip -6 a from the affected node? My bet is that [::]:port is a problem if IPv6 stack is not existing at all on a server (who would do it in 2021 anyway… pipe dream about people going v6)

thedeadliestcatch commented 7 months ago

@mkowalski @salanki This PR should not have been merged without a conditional and further testing.

Yes, it is broken for any non-v6 enabled hosts, if the DNS resolver synthesizes or returns AAAA records from A records, for example, because the client will attempt/defer to the v6 addresses, which will only work if there is actual v6 addressing (routing) working.

You might want to disable AAAA records in your resolver if you are not using v6, and especially synthesizing v6 addresses from A (v4) records.

mkowalski commented 7 months ago

You might want to disable AAAA records in your resolver if you are not using v6

This is fundamentally "the wrong direction" of doing things between client and DNS server. You should never reconfigure DNS server based on what your client can or cannot do. If your DNS server returns you both AAAA and A record, and you have only IPv4 stack available, it's up to you as a client to ignore AAAA and consume A. Otherwise we end up in a situation where you'd suggest having separate DNS server for IPv6 and IPv4 clients.

I am not saying this PR has been crafted in the best possible way, but I am saying that the above comment is suggesting something that no one ever should do.