fix: allow binding containers on different IPs to the same port

Fixes #4786

Signed-off-by: Kay Yan <kay.yan@daocloud.io>

Author: yankay

pkg/portutil/iptable/iptables.go

@@ -22,26 +22,46 @@ import (
 	"strings"
 )
 
-// ParseIPTableRules takes a slice of iptables rules as input and returns a slice of
-// uint64 containing the parsed destination port numbers from the rules.
-func ParseIPTableRules(rules []string) []uint64 {
-	ports := []uint64{}
+type PortRule struct {
+	IP   string
+	Port uint64
+}
+
+// ParseIPTableRules takes a slice of iptables rules as input and returns a
+// slice of PortRule containing the parsed destination IP and port from the
+// rules. When a rule has no -d flag, IP is empty (meaning the rule applies to
+// all addresses).
+func ParseIPTableRules(rules []string) []PortRule {
+	portRules := []PortRule{}
 
-	// Regex to match the '--dports' option followed by the port number
-	dportRegex := regexp.MustCompile(`--dports ((,?\d+)+)`)
+	dportsRegex := regexp.MustCompile(`--dports ((,?\d+)+)`)
+	dportRegex := regexp.MustCompile(`--dport (\d+)`)
+	destRegex := regexp.MustCompile(`-d (\S+?)(?:/\d+)?\s`)
 
 	for _, rule := range rules {
-		matches := dportRegex.FindStringSubmatch(rule)
-		if len(matches) > 1 {
-			for _, _match := range strings.Split(matches[1], ",") {
-				port64, err := strconv.ParseUint(_match, 10, 16)
-				if err != nil {
-					continue
-				}
-				ports = append(ports, port64)
+		var ports []string
+
+		if matches := dportsRegex.FindStringSubmatch(rule); len(matches) > 1 {
+			ports = strings.Split(matches[1], ",")
+		} else if matches := dportRegex.FindStringSubmatch(rule); len(matches) > 1 {
+			ports = []string{matches[1]}
+		} else {
+			continue
+		}
+
+		var ip string
+		if destMatches := destRegex.FindStringSubmatch(rule); len(destMatches) > 1 {
+			ip = destMatches[1]
+		}
+
+		for _, portStr := range ports {
+			port64, err := strconv.ParseUint(portStr, 10, 16)
+			if err != nil {
+				continue
 			}
+			portRules = append(portRules, PortRule{IP: ip, Port: port64})
 		}
 	}
 
-	return ports
+	return portRules
 }

pkg/portutil/iptable/iptables_linux.go

@@ -17,24 +17,55 @@
 package iptable
 
 import (
+	"strings"
+
 	"github.com/coreos/go-iptables/iptables"
 )
 
 // Chain used for port forwarding rules: https://www.cni.dev/plugins/current/meta/portmap/#dnat
 const cniDnatChain = "CNI-HOSTPORT-DNAT"
 
+// cniDNChainPrefix is the prefix for per-container DNAT sub-chains created by
+// the CNI portmap plugin. These sub-chains contain the actual DNAT rules with
+// destination IP filtering (e.g. -d 192.168.1.141/32 --dport 80 -j DNAT).
+const cniDNChainPrefix = "CNI-DN-"
+
 func ReadIPTables(table string) ([]string, error) {
 	ipt, err := iptables.New()
 	if err != nil {
 		return nil, err
 	}
 
 	var rules []string
-	chainExists, _ := ipt.ChainExists(table, cniDnatChain)
-	if chainExists {
-		rules, err = ipt.List(table, cniDnatChain)
-		if err != nil {
-			return nil, err
+
+	// Read per-container DNAT sub-chains (CNI-DN-*) which contain the actual
+	// DNAT rules with both destination IP and port information.
+	// The parent chain (CNI-HOSTPORT-DNAT) only dispatches by port to these
+	// sub-chains and does not include destination IP filtering.
+	chains, err := ipt.ListChains(table)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, chain := range chains {
+		if strings.HasPrefix(chain, cniDNChainPrefix) {
+			subRules, err := ipt.List(table, chain)
+			if err != nil {
+				continue
+			}
+			rules = append(rules, subRules...)
+		}
+	}
+
+	// Fall back to reading the parent chain if no CNI-DN-* sub-chains were read.
+	if len(rules) == 0 {
+		chainExists, _ := ipt.ChainExists(table, cniDnatChain)
+		if chainExists {
+			parentRules, err := ipt.List(table, cniDnatChain)
+			if err != nil {
+				return nil, err
+			}
+			rules = append(rules, parentRules...)
 		}
 	}
 

pkg/portutil/iptable/iptables_test.go

@@ -24,27 +24,65 @@ func TestParseIPTableRules(t *testing.T) {
 	testCases := []struct {
 		name  string
 		rules []string
-		want  []uint64
+		want  []PortRule
 	}{
 		{
 			name:  "Empty input",
 			rules: []string{},
-			want:  []uint64{},
+			want:  []PortRule{},
 		},
 		{
 			name: "Single rule with single port",
 			rules: []string{
 				"-A CNI-HOSTPORT-DNAT -p tcp -m comment --comment \"dnat name: \"bridge\" id: \"some-id\"\" -m multiport --dports 8080 -j CNI-DN-some-hash",
 			},
-			want: []uint64{8080},
+			want: []PortRule{{IP: "", Port: 8080}},
 		},
 		{
 			name: "Multiple rules with multiple ports",
 			rules: []string{
 				"-A CNI-HOSTPORT-DNAT -p tcp -m comment --comment \"dnat name: \"bridge\" id: \"some-id\"\" -m multiport --dports 8080 -j CNI-DN-some-hash",
 				"-A CNI-HOSTPORT-DNAT -p tcp -m comment --comment \"dnat name: \"bridge\" id: \"some-id\"\" -m multiport --dports 9090 -j CNI-DN-some-hash",
 			},
-			want: []uint64{8080, 9090},
+			want: []PortRule{
+				{IP: "", Port: 8080},
+				{IP: "", Port: 9090},
+			},
+		},
+		{
+			name: "Single rule with comma-separated ports",
+			rules: []string{
+				"-A CNI-HOSTPORT-DNAT -p tcp -m comment --comment \"dnat name: \"bridge\" id: \"some-id\"\" -m multiport --dports 8080,9090 -j CNI-DN-some-hash",
+			},
+			want: []PortRule{
+				{IP: "", Port: 8080},
+				{IP: "", Port: 9090},
+			},
+		},
+		{
+			name: "Sub-chain DNAT rule with destination IP",
+			rules: []string{
+				"-A CNI-DN-some-hash -d 192.168.1.141/32 -p tcp -m tcp --dport 80 -j DNAT --to-destination 10.4.0.2:80",
+			},
+			want: []PortRule{{IP: "192.168.1.141", Port: 80}},
+		},
+		{
+			name: "Multiple sub-chain rules with different IPs same port",
+			rules: []string{
+				"-A CNI-DN-hash1 -d 192.168.1.141/32 -p tcp -m tcp --dport 80 -j DNAT --to-destination 10.4.0.2:80",
+				"-A CNI-DN-hash2 -d 192.168.1.142/32 -p tcp -m tcp --dport 80 -j DNAT --to-destination 10.4.0.3:80",
+			},
+			want: []PortRule{
+				{IP: "192.168.1.141", Port: 80},
+				{IP: "192.168.1.142", Port: 80},
+			},
+		},
+		{
+			name: "Sub-chain rule without CIDR suffix",
+			rules: []string{
+				"-A CNI-DN-hash1 -d 10.0.0.1 -p tcp -m tcp --dport 443 -j DNAT --to-destination 10.4.0.2:443",
+			},
+			want: []PortRule{{IP: "10.0.0.1", Port: 443}},
 		},
 	}
 
@@ -58,12 +96,12 @@ func TestParseIPTableRules(t *testing.T) {
 	}
 }
 
-func equal(a, b []uint64) bool {
+func equal(a, b []PortRule) bool {
 	if len(a) != len(b) {
 		return false
 	}
-	for i, v := range a {
-		if v != b[i] {
+	for i := range a {
+		if a[i] != b[i] {
 			return false
 		}
 	}

pkg/portutil/port_allocate_linux.go

@@ -123,10 +123,14 @@ func getUsedPorts(ip string, protocol string) (map[uint64]bool, error) {
 	if err != nil {
 		return nil, err
 	}
-	destinationPorts := iptable.ParseIPTableRules(ipTableItems)
+	portRules := iptable.ParseIPTableRules(ipTableItems)
 
-	for _, port := range destinationPorts {
-		usedPort[port] = true
+	requestedIsWildcard := ip == "" || ip == "0.0.0.0" || ip == "::"
+	for _, rule := range portRules {
+		ruleIsWildcard := rule.IP == "" || rule.IP == "0.0.0.0" || rule.IP == "::"
+		if requestedIsWildcard || ruleIsWildcard || rule.IP == ip {
+			usedPort[rule.Port] = true
+		}
 	}
 
 	return usedPort, nil