moby / libnetwork

networking for containers
Apache License 2.0
2.16k stars 881 forks source link

portmapper: do not run dummy proxy when requesting a specific port #2657

Closed vincentbernat closed 1 year ago

vincentbernat commented 2 years ago

When a specific port is requested, we do not start a dummy proxy to mark the port not available anymore. Doing so trigger numerous problems, notably with UDP.

This change requires altering tests. It also becomes possible to map to a port already used by the host, shadowing it. There is also the possibility of using a port in the dynamic range which is already in use by the host (but this case was not handled gracefully either, making the whole allocation fails instead of trying the next port).

Alternatively, the change could be done only for UDP where the problem is more apparent. Or it could be configurable.

Cc moby/moby#14856

AkihiroSuda commented 2 years ago

The repo was moved to https://github.com/moby/moby/tree/master/libnetwork

polarathene commented 1 year ago

Is the PR still relevant?

UDP was addressed here: https://github.com/moby/moby/pull/44742

akerouanton commented 1 year ago

As @polarathene pointed out, the UDP issue has been addressed in moby/moby#44742.

It also becomes possible to map to a port already used by the host, shadowing it.

We don't want that. When docker publishes a port, we want to make sure this port: a. isn't already in use ; b. won't be bound by another local process after dockerd created NAT rules for that port.

And finally, as pointed out by @AkihiroSuda, this repo was moved to https://github.com/moby/moby/tree/master/libnetwork.

I don't have permissions to close this PR, so I'll defer to @thaJeztah for doing so.