From 42832e766643ff85ffcd7056c3d717fcaa57e644 Mon Sep 17 00:00:00 2001 From: m1093782566 Date: Mon, 13 Nov 2017 17:28:41 +0800 Subject: [PATCH] fix ipvs proxier getLocalIPs() error --- pkg/proxy/iptables/proxier.go | 2 +- pkg/proxy/iptables/proxier_test.go | 8 ++++++++ pkg/proxy/ipvs/proxier.go | 16 ++++++++++------ pkg/proxy/ipvs/proxier_test.go | 8 ++++++++ pkg/proxy/util/endpoints.go | 9 +++++++-- pkg/proxy/util/endpoints_test.go | 1 + 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 79802d635f8..e82f8c45822 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -798,7 +798,7 @@ func getLocalIPs(endpointsMap proxyEndpointsMap) map[types.NamespacedName]sets.S for svcPortName := range endpointsMap { for _, ep := range endpointsMap[svcPortName] { if ep.isLocal { - // If the endpoint has a bad format, ipPart() will log an + // If the endpoint has a bad format, utilproxy.IPPart() will log an // error and ep.IPPart() will return a null string. if ip := ep.IPPart(); ip != "" { nsn := svcPortName.NamespacedName diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 2e929347246..9aad141cd16 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -1264,6 +1264,14 @@ func Test_getLocalIPs(t *testing.T) { {Namespace: "ns2", Name: "ep2"}: sets.NewString("2.2.2.2", "2.2.2.22", "2.2.2.3"), {Namespace: "ns4", Name: "ep4"}: sets.NewString("4.4.4.4", "4.4.4.6"), }, + }, { + // Case[5]: named port local and bad endpoints IP + endpointsMap: map[proxy.ServicePortName][]*endpointsInfo{ + makeServicePortName("ns1", "ep1", "p11"): { + {endpoint: "bad ip:11", isLocal: true}, + }, + }, + expected: map[types.NamespacedName]sets.String{}, }} for tci, tc := range testCases { diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 8066f61003a..6f2c340c28d 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -1466,14 +1466,18 @@ func writeLine(buf *bytes.Buffer, words ...string) { func getLocalIPs(endpointsMap proxyEndpointsMap) map[types.NamespacedName]sets.String { localIPs := make(map[types.NamespacedName]sets.String) - for svcPort := range endpointsMap { - for _, ep := range endpointsMap[svcPort] { + for svcPortName := range endpointsMap { + for _, ep := range endpointsMap[svcPortName] { if ep.isLocal { - nsn := svcPort.NamespacedName - if localIPs[nsn] == nil { - localIPs[nsn] = sets.NewString() + // If the endpoint has a bad format, utilproxy.IPPart() will log an + // error and ep.IPPart() will return a null string. + if ip := ep.IPPart(); ip != "" { + nsn := svcPortName.NamespacedName + if localIPs[nsn] == nil { + localIPs[nsn] = sets.NewString() + } + localIPs[nsn].Insert(ip) } - localIPs[nsn].Insert(ep.IPPart()) // just the IP part } } } diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index b4147353d42..182d9cae487 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -2017,6 +2017,14 @@ func Test_getLocalIPs(t *testing.T) { {Namespace: "ns2", Name: "ep2"}: sets.NewString("2.2.2.2", "2.2.2.22", "2.2.2.3"), {Namespace: "ns4", Name: "ep4"}: sets.NewString("4.4.4.4", "4.4.4.6"), }, + }, { + // Case[5]: named port local and bad endpoints IP + endpointsMap: map[proxy.ServicePortName][]*endpointsInfo{ + makeServicePortName("ns1", "ep1", "p11"): { + {endpoint: "bad ip:11", isLocal: true}, + }, + }, + expected: map[types.NamespacedName]sets.String{}, }} for tci, tc := range testCases { diff --git a/pkg/proxy/util/endpoints.go b/pkg/proxy/util/endpoints.go index 32e770d4f94..0e8bdffc310 100644 --- a/pkg/proxy/util/endpoints.go +++ b/pkg/proxy/util/endpoints.go @@ -32,12 +32,17 @@ func IPPart(s string) string { return s } // Must be IP:port - ip, _, err := net.SplitHostPort(s) + host, _, err := net.SplitHostPort(s) if err != nil { glog.Errorf("Error parsing '%s': %v", s, err) return "" } - return ip + // Check if host string is a valid IP address + if ip := net.ParseIP(host); ip != nil { + glog.Errorf("invalid IP part '%s'", host) + return host + } + return "" } // ToCIDR returns a host address of the form /32 for diff --git a/pkg/proxy/util/endpoints_test.go b/pkg/proxy/util/endpoints_test.go index 618f59e96a8..bb28cbd714a 100644 --- a/pkg/proxy/util/endpoints_test.go +++ b/pkg/proxy/util/endpoints_test.go @@ -35,6 +35,7 @@ func TestIPPart(t *testing.T) { {"[2001:db8::2:2]:9999", "2001:db8::2:2", noError}, {"1.2.3.4::9999", "", "too many colons"}, {"1.2.3.4:[0]", "", "unexpected '[' in address"}, + {"1.2.3:8080", "", "invalid ip part"}, } for _, tc := range testCases {