[Bugfix] Check if port is used in publish flag [duplicate of #2190]

cmd/nerdctl/container/container_run_network_linux_test.go

@@ -349,6 +349,61 @@ func TestUniqueHostPortAssignement(t *testing.T) {
 	}
 }
 
+func TestHostPortAlreadyInUse(t *testing.T) {
+	testCases := []struct {
+		hostPort      string
+		containerPort string
+	}{
+		{
+			hostPort:      "5000",
+			containerPort: "80/tcp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/tcp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/udp",
+		},
+		{
+			hostPort:      "5000",
+			containerPort: "80/sctp",
+		},
+	}
+
+	tID := testutil.Identifier(t)
+
+	for i, tc := range testCases {
+		tc := tc
+		tcName := fmt.Sprintf("%+v", tc)
+		t.Run(tcName, func(t *testing.T) {
+			if strings.Contains(tc.containerPort, "sctp") && rootlessutil.IsRootless() {
+				t.Skip("sctp is not supported in rootless mode")
+			}
+			testContainerName1 := fmt.Sprintf("%s-%d-1", tID, i)
+			testContainerName2 := fmt.Sprintf("%s-%d-2", tID, i)
+			base := testutil.NewBase(t)
+			t.Cleanup(func() {
+				base.Cmd("rm", "-f", testContainerName1, testContainerName2).AssertOK()
+			})
+			pFlag := fmt.Sprintf("%s:%s", tc.hostPort, tc.containerPort)
+			cmd1 := base.Cmd("run", "-d",
+				"--name", testContainerName1, "-p",
+				pFlag,
+				testutil.NginxAlpineImage)
+
+			cmd2 := base.Cmd("run", "-d",
+				"--name", testContainerName2, "-p",
+				pFlag,
+				testutil.NginxAlpineImage)
+
+			cmd1.AssertOK()
+			cmd2.AssertFail()
+		})
+	}
+}
+
 func TestRunPort(t *testing.T) {
 	baseTestRunPort(t, testutil.NginxAlpineImage, testutil.NginxAlpineIndexHTMLSnippet, true)
 }

pkg/portutil/port_allocate_linux.go

@@ -42,24 +42,56 @@ func filter(ss []procnet.NetworkDetail, filterFunc func(detail procnet.NetworkDe
 }
 
 func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, error) {
-	netprocData, err := procnet.ReadStatsFileData(protocol)
+	usedPort, err := getUsedPorts(ip, protocol)
 	if err != nil {
 		return 0, 0, err
 	}
-	netprocItems := procnet.Parse(netprocData)
+
+	start := uint64(allocateStart)
+	if count > uint64(allocateEnd-allocateStart+1) {
+		return 0, 0, fmt.Errorf("can not allocate %d ports", count)
+	}
+	for start < allocateEnd {
+		needReturn := true
+		for i := start; i < start+count; i++ {
+			if _, ok := usedPort[i]; ok {
+				needReturn = false
+				break
+			}
+		}
+		if needReturn {
+			allocateStart = int(start + count)
+			return start, start + count - 1, nil
+		}
+		start += count
+	}
+	return 0, 0, fmt.Errorf("there is not enough %d free ports", count)
+}
+
+func getUsedPorts(ip string, protocol string) (map[uint64]bool, error) {
+	netprocItems := []procnet.NetworkDetail{}
+
+	if protocol == "tcp" || protocol == "udp" {
+		netprocData, err := procnet.ReadStatsFileData(protocol)
+		if err != nil {
+			return nil, err
+		}
+		netprocItems = append(netprocItems, procnet.Parse(netprocData)...)
+	}
+
 	// In some circumstances, when we bind address like "0.0.0.0:80", we will get the formation of ":::80" in /proc/net/tcp6.
 	// So we need some trick to process this situation.
 	if protocol == "tcp" {
 		tempTCPV6Data, err := procnet.ReadStatsFileData("tcp6")
 		if err != nil {
-			return 0, 0, err
+			return nil, err
 		}
 		netprocItems = append(netprocItems, procnet.Parse(tempTCPV6Data)...)
 	}
 	if protocol == "udp" {
 		tempUDPV6Data, err := procnet.ReadStatsFileData("udp6")
 		if err != nil {
-			return 0, 0, err
+			return nil, err
 		}
 		netprocItems = append(netprocItems, procnet.Parse(tempUDPV6Data)...)
 	}
@@ -78,31 +110,13 @@ func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, err
 
 	ipTableItems, err := iptable.ReadIPTables("nat")
 	if err != nil {
-		return 0, 0, err
+		return nil, err
 	}
 	destinationPorts := iptable.ParseIPTableRules(ipTableItems)
 
 	for _, port := range destinationPorts {
 		usedPort[port] = true
 	}
 
-	start := uint64(allocateStart)
-	if count > uint64(allocateEnd-allocateStart+1) {
-		return 0, 0, fmt.Errorf("can not allocate %d ports", count)
-	}
-	for start < allocateEnd {
-		needReturn := true
-		for i := start; i < start+count; i++ {
-			if _, ok := usedPort[i]; ok {
-				needReturn = false
-				break
-			}
-		}
-		if needReturn {
-			allocateStart = int(start + count)
-			return start, start + count - 1, nil
-		}
-		start += count
-	}
-	return 0, 0, fmt.Errorf("there is not enough %d free ports", count)
+	return usedPort, nil
 }

pkg/portutil/port_allocate_other.go

@@ -23,3 +23,7 @@ import "fmt"
 func portAllocate(protocol string, ip string, count uint64) (uint64, uint64, error) {
 	return 0, 0, fmt.Errorf("auto port allocate are not support Non-Linux platform yet")
 }
+
+func getUsedPorts(ip string, protocol string) (map[uint64]bool, error) {
+	return nil, nil
+}

pkg/portutil/portutil.go

@@ -59,6 +59,7 @@ func ParseFlagP(s string) ([]cni.PortMapping, error) {
 	case 2:
 		proto = strings.ToLower(splitBySlash[1])
 		switch proto {
+		// sctp is not a supported protocol
 		case "tcp", "udp", "sctp":
 		default:
 			return nil, fmt.Errorf("invalid protocol %q", splitBySlash[1])
@@ -101,6 +102,15 @@ func ParseFlagP(s string) ([]cni.PortMapping, error) {
 		if err != nil {
 			return nil, fmt.Errorf("invalid hostPort: %s", hostPort)
 		}
+		usedPorts, err := getUsedPorts(ip, proto)
+		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 hostPort != "" && (endPort-startPort) != (endHostPort-startHostPort) {
 		if endPort != startPort {