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_linux_test.go b/pkg/portutil/portutil_linux_test.go new file mode 100644 index 00000000000..8d69aab179b --- /dev/null +++ b/pkg/portutil/portutil_linux_test.go @@ -0,0 +1,73 @@ +/* + Copyright The containerd Authors. + + Licensed 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. +*/ + +package portutil + +import ( + "fmt" + "net" + "testing" + + "gotest.tools/v3/assert" +) + +// 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 test +// occupies the first port of a two-port range and asserts that the container port is +// bound to the next free host port (first+1); without the pool-allocation fix it would +// be dropped onto the occupied first port and this assertion would fail. +// +// This lives in a _linux_test.go file because getUsedPorts is only implemented on Linux. +func TestParseFlagPHostRangePool(t *testing.T) { + // 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.Fatal("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)) +}