diff --git a/pkg/portutil/portutil.go b/pkg/portutil/portutil.go index 73853767f16..e99c924f1ed 100644 --- a/pkg/portutil/portutil.go +++ b/pkg/portutil/portutil.go @@ -76,6 +76,17 @@ func ParseFlagP(s string) ([]cni.PortMapping, error) { ip, hostPort, containerPort := splitParts(splitBySlash[0]) + // Validate and normalize the host IP once. An empty IP is passed through to + // getUsedPorts below as "all interfaces"; for error messages and the resulting + // PortMapping it is normalized to 0.0.0.0. + if ip != "" && net.ParseIP(ip) == nil { + return nil, fmt.Errorf("invalid ip address: %s", ip) + } + hostIP := ip + if hostIP == "" { + hostIP = "0.0.0.0" + } + if containerPort == "" { return nil, fmt.Errorf("no port specified: %s", splitBySlash[0]) } @@ -107,32 +118,43 @@ func ParseFlagP(s string) ([]cni.PortMapping, error) { if err != nil { return nil, err } - for i := startHostPort; i <= endHostPort; i++ { - if usedPorts[i] { - return nil, fmt.Errorf("bind for %s:%d failed: port is already allocated", ip, i) + if startPort == endPort && startHostPort != endHostPort { + // Docker-compatible behavior: a single container port with a host port + // range (e.g. "3000-3001:8080") treats the range as a pool and binds the + // container port to the first free host port in it, rather than silently + // collapsing to the first port and dropping the rest of the range. + // https://github.com/moby/moby/blob/master/daemon/libnetwork/portallocator/portallocator.go + found := false + for p := startHostPort; p <= endHostPort; p++ { + if !usedPorts[p] { + startHostPort, endHostPort = p, p + found = true + break + } + } + if !found { + return nil, fmt.Errorf("bind for %s failed: all ports in range %s are already allocated", hostIP, hostPort) + } + } else { + for i := startHostPort; i <= endHostPort; i++ { + if usedPorts[i] { + return nil, fmt.Errorf("bind for %s:%d failed: port is already allocated", hostIP, i) + } } } } if hostPort != "" && (endPort-startPort) != (endHostPort-startHostPort) { - if endPort != startPort { - return nil, fmt.Errorf("invalid ranges specified for container and host Ports: %s and %s", containerPort, hostPort) - } + // Both container and host sides are ranges but of unequal length — a genuine + // mismatch (the single-container-port pool case above has already collapsed + // the host range to one port, so it does not reach here). + return nil, fmt.Errorf("invalid ranges specified for container and host Ports: %s and %s", containerPort, hostPort) } for i := int32(0); i <= (int32(endPort) - int32(startPort)); i++ { res.ContainerPort = int32(startPort) + i res.HostPort = int32(startHostPort) + i - if ip == "" { - //TODO handle ipv6 - res.HostIP = "0.0.0.0" - } else { - // TODO handle ipv6 - if net.ParseIP(ip) == nil { - return nil, fmt.Errorf("invalid ip address: %s", ip) - } - res.HostIP = ip - } + res.HostIP = hostIP mr = append(mr, res) } diff --git a/pkg/portutil/portutil_test.go b/pkg/portutil/portutil_test.go index 02f390bb9ab..b007843f23c 100644 --- a/pkg/portutil/portutil_test.go +++ b/pkg/portutil/portutil_test.go @@ -17,6 +17,8 @@ package portutil import ( + "fmt" + "net" "reflect" "runtime" "sort" @@ -359,3 +361,55 @@ func TestParseFlagP(t *testing.T) { }) } } + +// TestParseFlagPHostRangePool verifies the Docker-compatible behavior for a single +// container port with a host port range (e.g. "3000-3001:8080"): the container port +// is bound to one free host port from the range, not collapsed-and-dropped. The exact +// port chosen depends on what is free in the environment, so this asserts membership +// in the range rather than a specific port. +func TestParseFlagPHostRangePool(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("getUsedPorts is only implemented on Linux") + } + if rootlessutil.IsRootless() { + t.Skip("getUsedPorts relies on the host network namespace and is not reliable in rootless mode") + } + + // Occupy the first port of a two-port range and confirm that the single + // container port is bound to the next free host port, not collapsed onto the + // occupied first port. Without the pool fix this asserts the wrong port. + var occupied net.Listener + var first int + for attempt := 0; attempt < 50; attempt++ { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + continue + } + p := l.Addr().(*net.TCPAddr).Port + if p+1 > 65535 { + l.Close() + continue + } + // Ensure the successor port is currently free. + probe, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", p+1)) + if err != nil { + l.Close() + continue + } + probe.Close() + occupied, first = l, p + break + } + if occupied == nil { + t.Skip("could not find an occupied port with a free successor") + } + defer occupied.Close() + + got, err := ParseFlagP(fmt.Sprintf("127.0.0.1:%d-%d:8080/tcp", first, first+1)) + assert.NilError(t, err) + assert.Equal(t, len(got), 1) + assert.Equal(t, got[0].ContainerPort, int32(8080)) + assert.Equal(t, got[0].Protocol, "tcp") + assert.Equal(t, got[0].HostIP, "127.0.0.1") + assert.Equal(t, got[0].HostPort, int32(first+1)) +}