From f7d86e8b1cdd88e524b6eb9c837790487acc0464 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Mon, 31 Aug 2020 18:59:55 +0200 Subject: [PATCH] dockershim hostport manager use HostIP the hostport manager was not taking into consideration the hostIP when binding the socket of the hostPort, causing that the same HostPort can not be used with different IP addresses. --- .../dockershim/network/hostport/hostport.go | 9 +- .../network/hostport/hostport_manager.go | 19 +- .../network/hostport/hostport_manager_test.go | 180 ++++++++++++++++-- .../network/hostport/hostport_test.go | 3 +- 4 files changed, 183 insertions(+), 28 deletions(-) diff --git a/pkg/kubelet/dockershim/network/hostport/hostport.go b/pkg/kubelet/dockershim/network/hostport/hostport.go index 9f3735f16f8..9e2f39b614c 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport.go @@ -21,6 +21,7 @@ package hostport import ( "fmt" "net" + "strconv" "strings" "k8s.io/klog/v2" @@ -54,6 +55,7 @@ type PodPortMapping struct { } type hostport struct { + ip string port int32 protocol string } @@ -78,15 +80,17 @@ func openLocalPort(hp *hostport) (closeable, error) { // bind()ed but not listen()ed, and at least the default debian netcat // has no way to avoid about 10 seconds of retries. var socket closeable + // open the socket on the HostIP and HostPort specified + address := net.JoinHostPort(hp.ip, strconv.Itoa(int(hp.port))) switch hp.protocol { case "tcp": - listener, err := net.Listen("tcp", fmt.Sprintf(":%d", hp.port)) + listener, err := net.Listen("tcp", address) if err != nil { return nil, err } socket = listener case "udp": - addr, err := net.ResolveUDPAddr("udp", fmt.Sprintf(":%d", hp.port)) + addr, err := net.ResolveUDPAddr("udp", address) if err != nil { return nil, err } @@ -105,6 +109,7 @@ func openLocalPort(hp *hostport) (closeable, error) { // portMappingToHostport creates hostport structure based on input portmapping func portMappingToHostport(portMapping *PortMapping) hostport { return hostport{ + ip: portMapping.HostIP, port: portMapping.HostPort, protocol: strings.ToLower(string(portMapping.Protocol)), } diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_manager.go b/pkg/kubelet/dockershim/network/hostport/hostport_manager.go index 9eb7d30121e..744781983ef 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_manager.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_manager.go @@ -152,10 +152,17 @@ func (hm *hostportManager) Add(id string, podPortMapping *PodPortMapping, natInt // DNAT to the podIP:containerPort hostPortBinding := net.JoinHostPort(podIP, strconv.Itoa(int(pm.ContainerPort))) - writeLine(natRules, "-A", string(chain), - "-m", "comment", "--comment", fmt.Sprintf(`"%s hostport %d"`, podFullName, pm.HostPort), - "-m", protocol, "-p", protocol, - "-j", "DNAT", fmt.Sprintf("--to-destination=%s", hostPortBinding)) + if pm.HostIP == "" || pm.HostIP == "0.0.0.0" || pm.HostIP == "::" { + writeLine(natRules, "-A", string(chain), + "-m", "comment", "--comment", fmt.Sprintf(`"%s hostport %d"`, podFullName, pm.HostPort), + "-m", protocol, "-p", protocol, + "-j", "DNAT", fmt.Sprintf("--to-destination=%s", hostPortBinding)) + } else { + writeLine(natRules, "-A", string(chain), + "-m", "comment", "--comment", fmt.Sprintf(`"%s hostport %d"`, podFullName, pm.HostPort), + "-m", protocol, "-p", protocol, "-d", pm.HostIP, + "-j", "DNAT", fmt.Sprintf("--to-destination=%s", hostPortBinding)) + } } // getHostportChain should be able to provide unique hostport chain name using hash @@ -312,6 +319,8 @@ func (hm *hostportManager) closeHostports(hostportMappings []*PortMapping) error continue } delete(hm.hostPortMap, hp) + } else { + klog.V(5).Infof("host port %s does not have an open socket", hp.String()) } } return utilerrors.NewAggregate(errList) @@ -324,7 +333,7 @@ func (hm *hostportManager) closeHostports(hostportMappings []*PortMapping) error // WARNING: Please do not change this function. Otherwise, HostportManager may not be able to // identify existing iptables chains. func getHostportChain(id string, pm *PortMapping) utiliptables.Chain { - hash := sha256.Sum256([]byte(id + strconv.Itoa(int(pm.HostPort)) + string(pm.Protocol))) + hash := sha256.Sum256([]byte(id + strconv.Itoa(int(pm.HostPort)) + string(pm.Protocol) + pm.HostIP)) encoded := base32.StdEncoding.EncodeToString(hash[:]) return utiliptables.Chain(kubeHostportChainPrefix + encoded[:16]) } diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go index b41e7908c0b..d14e3f1e5ab 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go @@ -35,6 +35,7 @@ func TestOpenCloseHostports(t *testing.T) { podPortMapping *PodPortMapping expectError bool }{ + // no portmaps { &PodPortMapping{ Namespace: "ns1", @@ -42,6 +43,7 @@ func TestOpenCloseHostports(t *testing.T) { }, false, }, + // allocate port 80/TCP, 8080/TCP and 443/TCP { &PodPortMapping{ Namespace: "ns1", @@ -54,6 +56,7 @@ func TestOpenCloseHostports(t *testing.T) { }, false, }, + // fail to allocate port previously allocated 80/TCP { &PodPortMapping{ Namespace: "ns1", @@ -64,6 +67,7 @@ func TestOpenCloseHostports(t *testing.T) { }, true, }, + // fail to allocate port previously allocated 8080/TCP { &PodPortMapping{ Namespace: "ns1", @@ -75,6 +79,7 @@ func TestOpenCloseHostports(t *testing.T) { }, true, }, + // allocate port 8081/TCP { &PodPortMapping{ Namespace: "ns1", @@ -85,6 +90,7 @@ func TestOpenCloseHostports(t *testing.T) { }, false, }, + // allocate port 7777/SCTP { &PodPortMapping{ Namespace: "ns1", @@ -95,6 +101,30 @@ func TestOpenCloseHostports(t *testing.T) { }, false, }, + // same HostPort different HostIP + { + &PodPortMapping{ + Namespace: "ns1", + Name: "n5", + PortMappings: []*PortMapping{ + {HostPort: 8888, Protocol: v1.ProtocolUDP, HostIP: "127.0.0.1"}, + {HostPort: 8888, Protocol: v1.ProtocolUDP, HostIP: "127.0.0.2"}, + }, + }, + false, + }, + // same HostPort different protocol + { + &PodPortMapping{ + Namespace: "ns1", + Name: "n6", + PortMappings: []*PortMapping{ + {HostPort: 9999, Protocol: v1.ProtocolTCP}, + {HostPort: 9999, Protocol: v1.ProtocolUDP}, + }, + }, + false, + }, } iptables := NewFakeIPTables() @@ -106,13 +136,18 @@ func TestOpenCloseHostports(t *testing.T) { execer: exec.New(), } + // open all hostports defined in the test cases for _, tc := range openPortCases { mapping, err := manager.openHostports(tc.podPortMapping) + for hostport, socket := range mapping { + manager.hostPortMap[hostport] = socket + } if tc.expectError { assert.Error(t, err) continue } assert.NoError(t, err) + // SCTP ports are not allocated countSctp := 0 for _, pm := range tc.podPortMapping.PortMappings { if pm.Protocol == v1.ProtocolSCTP { @@ -122,7 +157,9 @@ func TestOpenCloseHostports(t *testing.T) { assert.EqualValues(t, len(mapping), len(tc.podPortMapping.PortMappings)-countSctp) } - // We have 4 ports: 80, 443, 8080, 8081 open now. + // We have following ports open: 80/TCP, 443/TCP, 8080/TCP, 8081/TCP, + // 127.0.0.1:8888/TCP, 127.0.0.2:8888/TCP, 9999/TCP and 9999/UDP open now. + assert.EqualValues(t, len(manager.hostPortMap), 8) closePortCases := []struct { portMappings []*PortMapping expectError bool @@ -165,8 +202,20 @@ func TestOpenCloseHostports(t *testing.T) { {HostPort: 7777, Protocol: v1.ProtocolSCTP}, }, }, + { + portMappings: []*PortMapping{ + {HostPort: 8888, Protocol: v1.ProtocolUDP, HostIP: "127.0.0.1"}, + {HostPort: 8888, Protocol: v1.ProtocolUDP, HostIP: "127.0.0.2"}, + }, + }, + { + portMappings: []*PortMapping{ + {HostPort: 9999, Protocol: v1.ProtocolTCP}, + {HostPort: 9999, Protocol: v1.ProtocolUDP}}, + }, } + // close all the hostports opened in previous step for _, tc := range closePortCases { err := manager.closeHostports(tc.portMappings) if tc.expectError { @@ -175,7 +224,7 @@ func TestOpenCloseHostports(t *testing.T) { } assert.NoError(t, err) } - // Clear all elements in hostPortMap + // assert all elements in hostPortMap were cleared assert.Zero(t, len(manager.hostPortMap)) } @@ -193,6 +242,7 @@ func TestHostportManager(t *testing.T) { mapping *PodPortMapping expectError bool }{ + // open HostPorts 8080/TCP, 8081/UDP and 8083/SCTP { mapping: &PodPortMapping{ Name: "pod1", @@ -219,6 +269,7 @@ func TestHostportManager(t *testing.T) { }, expectError: false, }, + // fail to open HostPort due to conflict 8083/SCTP { mapping: &PodPortMapping{ Name: "pod2", @@ -245,6 +296,7 @@ func TestHostportManager(t *testing.T) { }, expectError: true, }, + // open port 443 { mapping: &PodPortMapping{ Name: "pod3", @@ -261,6 +313,7 @@ func TestHostportManager(t *testing.T) { }, expectError: false, }, + // fail to open HostPort 8443 already allocated { mapping: &PodPortMapping{ Name: "pod3", @@ -277,6 +330,71 @@ func TestHostportManager(t *testing.T) { }, expectError: true, }, + // fail HostPort with PodIP and HostIP using different families + { + mapping: &PodPortMapping{ + Name: "pod4", + Namespace: "ns1", + IP: net.ParseIP("2001:beef::2"), + HostNetwork: false, + PortMappings: []*PortMapping{ + { + HostPort: 8444, + ContainerPort: 444, + Protocol: v1.ProtocolTCP, + HostIP: "192.168.1.1", + }, + }, + }, + expectError: true, + }, + + // open same HostPort on different IP + { + mapping: &PodPortMapping{ + Name: "pod5", + Namespace: "ns5", + IP: net.ParseIP("10.1.1.5"), + HostNetwork: false, + PortMappings: []*PortMapping{ + { + HostPort: 8888, + ContainerPort: 443, + Protocol: v1.ProtocolTCP, + HostIP: "127.0.0.2", + }, + { + HostPort: 8888, + ContainerPort: 443, + Protocol: v1.ProtocolTCP, + HostIP: "127.0.0.1", + }, + }, + }, + expectError: false, + }, + // open same HostPort on different + { + mapping: &PodPortMapping{ + Name: "pod6", + Namespace: "ns1", + IP: net.ParseIP("10.1.1.2"), + HostNetwork: false, + PortMappings: []*PortMapping{ + { + HostPort: 9999, + ContainerPort: 443, + Protocol: v1.ProtocolTCP, + }, + { + HostPort: 9999, + ContainerPort: 443, + Protocol: v1.ProtocolUDP, + }, + }, + }, + expectError: false, + }, } // Add Hostports @@ -290,7 +408,9 @@ func TestHostportManager(t *testing.T) { } // Check port opened - expectedPorts := []hostport{{8080, "tcp"}, {8081, "udp"}, {8443, "tcp"}} + expectedPorts := []hostport{{"", 8080, "tcp"}, + {"", 8081, "udp"}, {"", 8443, "tcp"}, {"127.0.0.1", 8888, "tcp"}, + {"127.0.0.2", 8888, "tcp"}, {"", 9999, "tcp"}, {"", 9999, "udp"}} openedPorts := make(map[hostport]bool) for hp, port := range portOpener.mem { if !port.closed { @@ -319,24 +439,41 @@ func TestHostportManager(t *testing.T) { `:KUBE-HP-63UPIDJXVRSZGSUZ - [0:0]`: true, `:KUBE-HP-WFBOALXEP42XEMJK - [0:0]`: true, `:KUBE-HP-XU6AWMMJYOZOFTFZ - [0:0]`: true, - "-A KUBE-HOSTPORTS -m comment --comment \"pod3_ns1 hostport 8443\" -m tcp -p tcp --dport 8443 -j KUBE-HP-WFBOALXEP42XEMJK": true, - "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8081\" -m udp -p udp --dport 8081 -j KUBE-HP-63UPIDJXVRSZGSUZ": true, - "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8080\" -m tcp -p tcp --dport 8080 -j KUBE-HP-IJHALPHTORMHHPPK": true, - "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8083\" -m sctp -p sctp --dport 8083 -j KUBE-HP-XU6AWMMJYOZOFTFZ": true, - "-A OUTPUT -m comment --comment \"kube hostport portals\" -m addrtype --dst-type LOCAL -j KUBE-HOSTPORTS": true, - "-A PREROUTING -m comment --comment \"kube hostport portals\" -m addrtype --dst-type LOCAL -j KUBE-HOSTPORTS": true, - "-A POSTROUTING -m comment --comment \"SNAT for localhost access to hostports\" -o cbr0 -s 127.0.0.0/8 -j MASQUERADE": true, - "-A KUBE-HP-IJHALPHTORMHHPPK -m comment --comment \"pod1_ns1 hostport 8080\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, - "-A KUBE-HP-IJHALPHTORMHHPPK -m comment --comment \"pod1_ns1 hostport 8080\" -m tcp -p tcp -j DNAT --to-destination 10.1.1.2:80": true, - "-A KUBE-HP-63UPIDJXVRSZGSUZ -m comment --comment \"pod1_ns1 hostport 8081\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, - "-A KUBE-HP-63UPIDJXVRSZGSUZ -m comment --comment \"pod1_ns1 hostport 8081\" -m udp -p udp -j DNAT --to-destination 10.1.1.2:81": true, - "-A KUBE-HP-XU6AWMMJYOZOFTFZ -m comment --comment \"pod1_ns1 hostport 8083\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, - "-A KUBE-HP-XU6AWMMJYOZOFTFZ -m comment --comment \"pod1_ns1 hostport 8083\" -m sctp -p sctp -j DNAT --to-destination 10.1.1.2:83": true, - "-A KUBE-HP-WFBOALXEP42XEMJK -m comment --comment \"pod3_ns1 hostport 8443\" -s 10.1.1.4/32 -j KUBE-MARK-MASQ": true, - "-A KUBE-HP-WFBOALXEP42XEMJK -m comment --comment \"pod3_ns1 hostport 8443\" -m tcp -p tcp -j DNAT --to-destination 10.1.1.4:443": true, + `:KUBE-HP-TUKTZ736U5JD5UTK - [0:0]`: true, + `:KUBE-HP-CAAJ45HDITK7ARGM - [0:0]`: true, + `:KUBE-HP-WFUNFVXVDLD5ZVXN - [0:0]`: true, + `:KUBE-HP-4MFWH2F2NAOMYD6A - [0:0]`: true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod3_ns1 hostport 8443\" -m tcp -p tcp --dport 8443 -j KUBE-HP-WFBOALXEP42XEMJK": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8081\" -m udp -p udp --dport 8081 -j KUBE-HP-63UPIDJXVRSZGSUZ": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8080\" -m tcp -p tcp --dport 8080 -j KUBE-HP-IJHALPHTORMHHPPK": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod1_ns1 hostport 8083\" -m sctp -p sctp --dport 8083 -j KUBE-HP-XU6AWMMJYOZOFTFZ": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod5_ns5 hostport 8888\" -m tcp -p tcp --dport 8888 -j KUBE-HP-TUKTZ736U5JD5UTK": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod5_ns5 hostport 8888\" -m tcp -p tcp --dport 8888 -j KUBE-HP-CAAJ45HDITK7ARGM": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod6_ns1 hostport 9999\" -m udp -p udp --dport 9999 -j KUBE-HP-4MFWH2F2NAOMYD6A": true, + "-A KUBE-HOSTPORTS -m comment --comment \"pod6_ns1 hostport 9999\" -m tcp -p tcp --dport 9999 -j KUBE-HP-WFUNFVXVDLD5ZVXN": true, + "-A OUTPUT -m comment --comment \"kube hostport portals\" -m addrtype --dst-type LOCAL -j KUBE-HOSTPORTS": true, + "-A PREROUTING -m comment --comment \"kube hostport portals\" -m addrtype --dst-type LOCAL -j KUBE-HOSTPORTS": true, + "-A POSTROUTING -m comment --comment \"SNAT for localhost access to hostports\" -o cbr0 -s 127.0.0.0/8 -j MASQUERADE": true, + "-A KUBE-HP-IJHALPHTORMHHPPK -m comment --comment \"pod1_ns1 hostport 8080\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-IJHALPHTORMHHPPK -m comment --comment \"pod1_ns1 hostport 8080\" -m tcp -p tcp -j DNAT --to-destination 10.1.1.2:80": true, + "-A KUBE-HP-63UPIDJXVRSZGSUZ -m comment --comment \"pod1_ns1 hostport 8081\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-63UPIDJXVRSZGSUZ -m comment --comment \"pod1_ns1 hostport 8081\" -m udp -p udp -j DNAT --to-destination 10.1.1.2:81": true, + "-A KUBE-HP-XU6AWMMJYOZOFTFZ -m comment --comment \"pod1_ns1 hostport 8083\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-XU6AWMMJYOZOFTFZ -m comment --comment \"pod1_ns1 hostport 8083\" -m sctp -p sctp -j DNAT --to-destination 10.1.1.2:83": true, + "-A KUBE-HP-WFBOALXEP42XEMJK -m comment --comment \"pod3_ns1 hostport 8443\" -s 10.1.1.4/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-WFBOALXEP42XEMJK -m comment --comment \"pod3_ns1 hostport 8443\" -m tcp -p tcp -j DNAT --to-destination 10.1.1.4:443": true, + "-A KUBE-HP-TUKTZ736U5JD5UTK -m comment --comment \"pod5_ns5 hostport 8888\" -s 10.1.1.5/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-TUKTZ736U5JD5UTK -m comment --comment \"pod5_ns5 hostport 8888\" -m tcp -p tcp -d 127.0.0.1/32 -j DNAT --to-destination 10.1.1.5:443": true, + "-A KUBE-HP-CAAJ45HDITK7ARGM -m comment --comment \"pod5_ns5 hostport 8888\" -s 10.1.1.5/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-CAAJ45HDITK7ARGM -m comment --comment \"pod5_ns5 hostport 8888\" -m tcp -p tcp -d 127.0.0.2/32 -j DNAT --to-destination 10.1.1.5:443": true, + "-A KUBE-HP-WFUNFVXVDLD5ZVXN -m comment --comment \"pod6_ns1 hostport 9999\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-WFUNFVXVDLD5ZVXN -m comment --comment \"pod6_ns1 hostport 9999\" -m tcp -p tcp -j DNAT --to-destination 10.1.1.2:443": true, + "-A KUBE-HP-4MFWH2F2NAOMYD6A -m comment --comment \"pod6_ns1 hostport 9999\" -s 10.1.1.2/32 -j KUBE-MARK-MASQ": true, + "-A KUBE-HP-4MFWH2F2NAOMYD6A -m comment --comment \"pod6_ns1 hostport 9999\" -m udp -p udp -j DNAT --to-destination 10.1.1.2:443": true, `COMMIT`: true, } for _, line := range lines { + t.Logf("Line: %s", line) if len(strings.TrimSpace(line)) > 0 { _, ok := expectedLines[strings.TrimSpace(line)] assert.EqualValues(t, true, ok) @@ -362,7 +499,8 @@ func TestHostportManager(t *testing.T) { remainingChains[strings.TrimSpace(line)] = true } } - expectDeletedChains := []string{"KUBE-HP-4YVONL46AKYWSKS3", "KUBE-HP-7THKRFSEH4GIIXK7", "KUBE-HP-5N7UH5JAXCVP5UJR"} + expectDeletedChains := []string{"KUBE-HP-4YVONL46AKYWSKS3", "KUBE-HP-7THKRFSEH4GIIXK7", "KUBE-HP-5N7UH5JAXCVP5UJR", + "KUBE-HP-TUKTZ736U5JD5UTK", "KUBE-HP-CAAJ45HDITK7ARGM", "KUBE-HP-WFUNFVXVDLD5ZVXN", "KUBE-HP-4MFWH2F2NAOMYD6A"} for _, chain := range expectDeletedChains { _, ok := remainingChains[chain] assert.EqualValues(t, false, ok) @@ -372,6 +510,8 @@ func TestHostportManager(t *testing.T) { for _, port := range portOpener.mem { assert.EqualValues(t, true, port.closed) } + // Clear all elements in hostPortMap + assert.Zero(t, len(manager.hostPortMap)) } func TestGetHostportChain(t *testing.T) { @@ -499,7 +639,7 @@ func TestHostportManagerIPv6(t *testing.T) { } // Check port opened - expectedPorts := []hostport{{8080, "tcp"}, {8081, "udp"}, {8443, "tcp"}} + expectedPorts := []hostport{{"", 8080, "tcp"}, {"", 8081, "udp"}, {"", 8443, "tcp"}} openedPorts := make(map[hostport]bool) for hp, port := range portOpener.mem { if !port.closed { diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_test.go index 4721158f2a6..29418ca0f5c 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_test.go @@ -27,6 +27,7 @@ import ( ) type fakeSocket struct { + ip string port int32 protocol string closed bool @@ -52,7 +53,7 @@ func (f *fakeSocketManager) openFakeSocket(hp *hostport) (closeable, error) { if socket, ok := f.mem[*hp]; ok && !socket.closed { return nil, fmt.Errorf("hostport is occupied") } - fs := &fakeSocket{hp.port, hp.protocol, false} + fs := &fakeSocket{hp.ip, hp.port, hp.protocol, false} f.mem[*hp] = fs return fs, nil }