From d5361fd28803bbe8b6e45bdc7d649dd6a1110980 Mon Sep 17 00:00:00 2001 From: s3onghyun Date: Fri, 10 Jul 2026 03:15:25 +0900 Subject: [PATCH] Allocate a free host port from the range for a single container port When the container side is a single port and the host side is a range (e.g. `-p 3000-3001:8080`), nerdctl now treats the range as a pool and binds the container port to the first free host port in it, using getUsedPorts to skip ports already in use, matching Docker's behavior. Previously the extra host ports were silently dropped. Genuine range/range mismatches of unequal length are still rejected. The host IP is validated and normalized once, and the pool test occupies the first port of a range and asserts the free successor is chosen, so it fails without this change (Linux-only; skipped in rootless). Signed-off-by: s3onghyun --- pkg/portutil/portutil.go | 54 ++++++++++++++------- pkg/portutil/portutil_linux_test.go | 73 +++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 pkg/portutil/portutil_linux_test.go 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)) +}