From f7d86e8b1cdd88e524b6eb9c837790487acc0464 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Mon, 31 Aug 2020 18:59:55 +0200 Subject: [PATCH 1/2] 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 } From ad4776ba544e8763b9954edecb6b40f85682d0e2 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 4 Feb 2021 10:03:24 +0100 Subject: [PATCH 2/2] dockershim hostport respect IPFamily --- .../network/hostport/fake_iptables.go | 15 +++--- .../dockershim/network/hostport/hostport.go | 27 +++++++--- .../network/hostport/hostport_manager.go | 52 ++++++++++++++----- .../network/hostport/hostport_manager_test.go | 39 ++++++++------ .../network/hostport/hostport_test.go | 14 +++-- 5 files changed, 101 insertions(+), 46 deletions(-) diff --git a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go index 9080a58b3d5..9d19e90e05e 100644 --- a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go +++ b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go @@ -148,14 +148,14 @@ func (f *fakeIPTables) ensureRule(position utiliptables.RulePosition, tableName return true, nil } - if position == utiliptables.Prepend { + switch position { + case utiliptables.Prepend: chain.rules = append([]string{rule}, chain.rules...) - } else if position == utiliptables.Append { + case utiliptables.Append: chain.rules = append(chain.rules, rule) - } else { + default: return false, fmt.Errorf("unknown position argument %q", position) } - return false, nil } @@ -185,7 +185,7 @@ func normalizeRule(rule string) (string, error) { // Normalize un-prefixed IP addresses like iptables does if net.ParseIP(arg) != nil { - arg = arg + "/32" + arg += "/32" } if len(normalized) > 0 { @@ -281,7 +281,10 @@ func (f *fakeIPTables) restore(restoreTableName utiliptables.Table, data []byte, if strings.HasPrefix(line, ":") { chainName := utiliptables.Chain(strings.Split(line[1:], " ")[0]) if flush == utiliptables.FlushTables { - table, chain, _ := f.getChain(tableName, chainName) + table, chain, err := f.getChain(tableName, chainName) + if err != nil { + return err + } if chain != nil { delete(table.chains, string(chainName)) } diff --git a/pkg/kubelet/dockershim/network/hostport/hostport.go b/pkg/kubelet/dockershim/network/hostport/hostport.go index 9e2f39b614c..c9f60bf5946 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport.go @@ -54,7 +54,17 @@ type PodPortMapping struct { IP net.IP } +// ipFamily refers to a specific family if not empty, i.e. "4" or "6". +type ipFamily string + +// Constants for valid IPFamily: +const ( + IPv4 ipFamily = "4" + IPv6 ipFamily = "6" +) + type hostport struct { + ipFamily ipFamily ip string port int32 protocol string @@ -84,17 +94,19 @@ func openLocalPort(hp *hostport) (closeable, error) { address := net.JoinHostPort(hp.ip, strconv.Itoa(int(hp.port))) switch hp.protocol { case "tcp": - listener, err := net.Listen("tcp", address) + network := "tcp" + string(hp.ipFamily) + listener, err := net.Listen(network, address) if err != nil { return nil, err } socket = listener case "udp": - addr, err := net.ResolveUDPAddr("udp", address) + network := "udp" + string(hp.ipFamily) + addr, err := net.ResolveUDPAddr(network, address) if err != nil { return nil, err } - conn, err := net.ListenUDP("udp", addr) + conn, err := net.ListenUDP(network, addr) if err != nil { return nil, err } @@ -107,8 +119,9 @@ func openLocalPort(hp *hostport) (closeable, error) { } // portMappingToHostport creates hostport structure based on input portmapping -func portMappingToHostport(portMapping *PortMapping) hostport { +func portMappingToHostport(portMapping *PortMapping, family ipFamily) hostport { return hostport{ + ipFamily: family, ip: portMapping.HostIP, port: portMapping.HostPort, protocol: strings.ToLower(string(portMapping.Protocol)), @@ -129,9 +142,11 @@ func ensureKubeHostportChains(iptables utiliptables.Interface, natInterfaceName {utiliptables.TableNAT, utiliptables.ChainOutput}, {utiliptables.TableNAT, utiliptables.ChainPrerouting}, } - args := []string{"-m", "comment", "--comment", "kube hostport portals", + args := []string{ + "-m", "comment", "--comment", "kube hostport portals", "-m", "addrtype", "--dst-type", "LOCAL", - "-j", string(kubeHostportsChain)} + "-j", string(kubeHostportsChain), + } for _, tc := range tableChainsNeedJumpServices { // KUBE-HOSTPORTS chain needs to be appended to the system chains. // This ensures KUBE-SERVICES chain gets processed first. diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_manager.go b/pkg/kubelet/dockershim/network/hostport/hostport_manager.go index 744781983ef..a9c099e596d 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_manager.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_manager.go @@ -59,6 +59,7 @@ type hostportManager struct { mu sync.Mutex } +// NewHostportManager creates a new HostPortManager func NewHostportManager(iptables utiliptables.Interface) HostPortManager { h := &hostportManager{ hostPortMap: make(map[hostport]closeable), @@ -78,13 +79,6 @@ func (hm *hostportManager) Add(id string, podPortMapping *PodPortMapping, natInt return nil } podFullName := getPodFullName(podPortMapping) - - // skip if there is no hostport needed - hostportMappings := gatherHostportMappings(podPortMapping) - if len(hostportMappings) == 0 { - return nil - } - // IP.To16() returns nil if IP is not a valid IPv4 or IPv6 address if podPortMapping.IP.To16() == nil { return fmt.Errorf("invalid or missing IP of pod %s", podFullName) @@ -92,11 +86,17 @@ func (hm *hostportManager) Add(id string, podPortMapping *PodPortMapping, natInt podIP := podPortMapping.IP.String() isIPv6 := utilnet.IsIPv6(podPortMapping.IP) + // skip if there is no hostport needed + hostportMappings := gatherHostportMappings(podPortMapping, isIPv6) + if len(hostportMappings) == 0 { + return nil + } + if isIPv6 != hm.iptables.IsIPv6() { return fmt.Errorf("HostPortManager IP family mismatch: %v, isIPv6 - %v", podIP, isIPv6) } - if err = ensureKubeHostportChains(hm.iptables, natInterfaceName); err != nil { + if err := ensureKubeHostportChains(hm.iptables, natInterfaceName); err != nil { return err } @@ -205,8 +205,8 @@ func (hm *hostportManager) Remove(id string, podPortMapping *PodPortMapping) (er return nil } - hostportMappings := gatherHostportMappings(podPortMapping) - if len(hostportMappings) <= 0 { + hostportMappings := gatherHostportMappings(podPortMapping, hm.iptables.IsIPv6()) + if len(hostportMappings) == 0 { return nil } @@ -238,6 +238,12 @@ func (hm *hostportManager) Remove(id string, podPortMapping *PodPortMapping) (er } } + // exit if there is nothing to remove + // don´t forget to clean up opened pod host ports + if len(existingChainsToRemove) == 0 { + return hm.closeHostports(hostportMappings) + } + natChains := bytes.NewBuffer(nil) natRules := bytes.NewBuffer(nil) writeLine(natChains, "*nat") @@ -252,7 +258,7 @@ func (hm *hostportManager) Remove(id string, podPortMapping *PodPortMapping) (er } writeLine(natRules, "COMMIT") - if err = hm.syncIPTables(append(natChains.Bytes(), natRules.Bytes()...)); err != nil { + if err := hm.syncIPTables(append(natChains.Bytes(), natRules.Bytes()...)); err != nil { return err } @@ -286,7 +292,12 @@ func (hm *hostportManager) openHostports(podPortMapping *PodPortMapping) (map[ho continue } - hp := portMappingToHostport(pm) + // HostIP IP family is not handled by this port opener + if pm.HostIP != "" && utilnet.IsIPv6String(pm.HostIP) != hm.iptables.IsIPv6() { + continue + } + + hp := portMappingToHostport(pm, hm.getIPFamily()) socket, err := hm.portOpener(&hp) if err != nil { retErr = fmt.Errorf("cannot open hostport %d for pod %s: %v", pm.HostPort, getPodFullName(podPortMapping), err) @@ -311,7 +322,7 @@ func (hm *hostportManager) openHostports(podPortMapping *PodPortMapping) (map[ho func (hm *hostportManager) closeHostports(hostportMappings []*PortMapping) error { errList := []error{} for _, pm := range hostportMappings { - hp := portMappingToHostport(pm) + hp := portMappingToHostport(pm, hm.getIPFamily()) if socket, ok := hm.hostPortMap[hp]; ok { klog.V(2).Infof("Closing host port %s", hp.String()) if err := socket.Close(); err != nil { @@ -326,6 +337,15 @@ func (hm *hostportManager) closeHostports(hostportMappings []*PortMapping) error return utilerrors.NewAggregate(errList) } +// getIPFamily returns the hostPortManager IP family +func (hm *hostportManager) getIPFamily() ipFamily { + family := IPv4 + if hm.iptables.IsIPv6() { + family = IPv6 + } + return family +} + // getHostportChain takes id, hostport and protocol for a pod and returns associated iptables chain. // This is computed by hashing (sha256) then encoding to base32 and truncating with the prefix // "KUBE-HP-". We do this because IPTables Chain Names must be <= 28 chars long, and the longer @@ -339,12 +359,16 @@ func getHostportChain(id string, pm *PortMapping) utiliptables.Chain { } // gatherHostportMappings returns all the PortMappings which has hostport for a pod -func gatherHostportMappings(podPortMapping *PodPortMapping) []*PortMapping { +// it filters the PortMappings that use HostIP and doesn't match the IP family specified +func gatherHostportMappings(podPortMapping *PodPortMapping, isIPv6 bool) []*PortMapping { mappings := []*PortMapping{} for _, pm := range podPortMapping.PortMappings { if pm.HostPort <= 0 { continue } + if pm.HostIP != "" && utilnet.IsIPv6String(pm.HostIP) != isIPv6 { + continue + } mappings = append(mappings, pm) } return mappings diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go index d14e3f1e5ab..1613b0d633a 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go @@ -128,6 +128,7 @@ func TestOpenCloseHostports(t *testing.T) { } iptables := NewFakeIPTables() + iptables.protocol = utiliptables.ProtocolIPv4 portOpener := NewFakeSocketManager() manager := &hostportManager{ hostPortMap: make(map[hostport]closeable), @@ -151,7 +152,7 @@ func TestOpenCloseHostports(t *testing.T) { countSctp := 0 for _, pm := range tc.podPortMapping.PortMappings { if pm.Protocol == v1.ProtocolSCTP { - countSctp += 1 + countSctp++ } } assert.EqualValues(t, len(mapping), len(tc.podPortMapping.PortMappings)-countSctp) @@ -211,7 +212,8 @@ func TestOpenCloseHostports(t *testing.T) { { portMappings: []*PortMapping{ {HostPort: 9999, Protocol: v1.ProtocolTCP}, - {HostPort: 9999, Protocol: v1.ProtocolUDP}}, + {HostPort: 9999, Protocol: v1.ProtocolUDP}, + }, }, } @@ -230,6 +232,7 @@ func TestOpenCloseHostports(t *testing.T) { func TestHostportManager(t *testing.T) { iptables := NewFakeIPTables() + iptables.protocol = utiliptables.ProtocolIPv4 portOpener := NewFakeSocketManager() manager := &hostportManager{ hostPortMap: make(map[hostport]closeable), @@ -237,7 +240,6 @@ func TestHostportManager(t *testing.T) { portOpener: portOpener.openFakeSocket, execer: exec.New(), } - testCases := []struct { mapping *PodPortMapping expectError bool @@ -318,7 +320,7 @@ func TestHostportManager(t *testing.T) { mapping: &PodPortMapping{ Name: "pod3", Namespace: "ns1", - IP: net.ParseIP("2001:beef::2"), + IP: net.ParseIP("192.168.12.12"), HostNetwork: false, PortMappings: []*PortMapping{ { @@ -330,7 +332,7 @@ func TestHostportManager(t *testing.T) { }, expectError: true, }, - // fail HostPort with PodIP and HostIP using different families + // skip HostPort with PodIP and HostIP using different families { mapping: &PodPortMapping{ Name: "pod4", @@ -346,7 +348,7 @@ func TestHostportManager(t *testing.T) { }, }, }, - expectError: true, + expectError: false, }, // open same HostPort on different IP @@ -408,9 +410,15 @@ func TestHostportManager(t *testing.T) { } // Check port opened - expectedPorts := []hostport{{"", 8080, "tcp"}, - {"", 8081, "udp"}, {"", 8443, "tcp"}, {"127.0.0.1", 8888, "tcp"}, - {"127.0.0.2", 8888, "tcp"}, {"", 9999, "tcp"}, {"", 9999, "udp"}} + expectedPorts := []hostport{ + {IPv4, "", 8080, "tcp"}, + {IPv4, "", 8081, "udp"}, + {IPv4, "", 8443, "tcp"}, + {IPv4, "127.0.0.1", 8888, "tcp"}, + {IPv4, "127.0.0.2", 8888, "tcp"}, + {IPv4, "", 9999, "tcp"}, + {IPv4, "", 9999, "udp"}, + } openedPorts := make(map[hostport]bool) for hp, port := range portOpener.mem { if !port.closed { @@ -499,8 +507,10 @@ func TestHostportManager(t *testing.T) { remainingChains[strings.TrimSpace(line)] = true } } - expectDeletedChains := []string{"KUBE-HP-4YVONL46AKYWSKS3", "KUBE-HP-7THKRFSEH4GIIXK7", "KUBE-HP-5N7UH5JAXCVP5UJR", - "KUBE-HP-TUKTZ736U5JD5UTK", "KUBE-HP-CAAJ45HDITK7ARGM", "KUBE-HP-WFUNFVXVDLD5ZVXN", "KUBE-HP-4MFWH2F2NAOMYD6A"} + 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) @@ -537,7 +547,6 @@ func TestHostportManagerIPv6(t *testing.T) { portOpener: portOpener.openFakeSocket, execer: exec.New(), } - testCases := []struct { mapping *PodPortMapping expectError bool @@ -639,7 +648,7 @@ func TestHostportManagerIPv6(t *testing.T) { } // Check port opened - expectedPorts := []hostport{{"", 8080, "tcp"}, {"", 8081, "udp"}, {"", 8443, "tcp"}} + expectedPorts := []hostport{{IPv6, "", 8080, "tcp"}, {IPv6, "", 8081, "udp"}, {IPv6, "", 8443, "tcp"}} openedPorts := make(map[hostport]bool) for hp, port := range portOpener.mem { if !port.closed { @@ -657,7 +666,7 @@ func TestHostportManagerIPv6(t *testing.T) { err := iptables.SaveInto(utiliptables.TableNAT, raw) assert.NoError(t, err) - lines := strings.Split(string(raw.Bytes()), "\n") + lines := strings.Split(raw.String(), "\n") expectedLines := map[string]bool{ `*nat`: true, `:KUBE-HOSTPORTS - [0:0]`: true, @@ -704,7 +713,7 @@ func TestHostportManagerIPv6(t *testing.T) { raw.Reset() err = iptables.SaveInto(utiliptables.TableNAT, raw) assert.NoError(t, err) - lines = strings.Split(string(raw.Bytes()), "\n") + lines = strings.Split(raw.String(), "\n") remainingChains := make(map[string]bool) for _, line := range lines { if strings.HasPrefix(line, ":") { diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_test.go index 29418ca0f5c..c3f36852836 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_test.go @@ -27,15 +27,15 @@ import ( ) type fakeSocket struct { - ip string + closed bool port int32 protocol string - closed bool + ip string } func (f *fakeSocket) Close() error { if f.closed { - return fmt.Errorf("Socket %q.%s already closed!", f.port, f.protocol) + return fmt.Errorf("socket %q.%s already closed", f.port, f.protocol) } f.closed = true return nil @@ -53,7 +53,12 @@ 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.ip, hp.port, hp.protocol, false} + fs := &fakeSocket{ + port: hp.port, + protocol: hp.protocol, + closed: false, + ip: hp.ip, + } f.mem[*hp] = fs return fs, nil } @@ -81,5 +86,4 @@ func TestEnsureKubeHostportChains(t *testing.T) { assert.EqualValues(t, len(chain.rules), 1) assert.Contains(t, chain.rules[0], jumpRule) } - }