Merge pull request #87117 from aojea/proxyv6LB

kube-proxy crash when load balancers use a different IP family

pkg/proxy/service.go

@@ -140,8 +140,6 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 		port:                   int(port.Port),
 		protocol:               port.Protocol,
 		nodePort:               int(port.NodePort),
-		// Deep-copy in case the service instance changes
-		loadBalancerStatus:     *service.Status.LoadBalancer.DeepCopy(),
 		sessionAffinityType:    service.Spec.SessionAffinity,
 		stickyMaxAgeSeconds:    stickyMaxAgeSeconds,
 		onlyNodeLocalEndpoints: onlyNodeLocalEndpoints,
@@ -153,9 +151,11 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 		info.loadBalancerSourceRanges = make([]string, len(service.Spec.LoadBalancerSourceRanges))
 		copy(info.loadBalancerSourceRanges, service.Spec.LoadBalancerSourceRanges)
 		copy(info.externalIPs, service.Spec.ExternalIPs)
+		// Deep-copy in case the service instance changes
+		info.loadBalancerStatus = *service.Status.LoadBalancer.DeepCopy()
 	} else {
 		// Filter out the incorrect IP version case.
-		// If ExternalIPs and LoadBalancerSourceRanges on service contains incorrect IP versions,
+		// If ExternalIPs, LoadBalancerSourceRanges and LoadBalancerStatus Ingress on service contains incorrect IP versions,
 		// only filter out the incorrect ones.
 		var incorrectIPs []string
 		info.externalIPs, incorrectIPs = utilproxy.FilterIncorrectIPVersion(service.Spec.ExternalIPs, *sct.isIPv6Mode)
@@ -166,6 +166,22 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 		if len(incorrectIPs) > 0 {
 			utilproxy.LogAndEmitIncorrectIPVersionEvent(sct.recorder, "loadBalancerSourceRanges", strings.Join(incorrectIPs, ","), service.Namespace, service.Name, service.UID)
 		}
+		// Obtain Load Balancer Ingress IPs
+		var ips []string
+		for _, ing := range service.Status.LoadBalancer.Ingress {
+			ips = append(ips, ing.IP)
+		}
+		if len(ips) > 0 {
+			correctIPs, incorrectIPs := utilproxy.FilterIncorrectIPVersion(ips, *sct.isIPv6Mode)
+			if len(incorrectIPs) > 0 {
+				utilproxy.LogAndEmitIncorrectIPVersionEvent(sct.recorder, "Load Balancer ingress IPs", strings.Join(incorrectIPs, ","), service.Namespace, service.Name, service.UID)
+			}
+			// Create the LoadBalancerStatus with the filtererd IPs
+			for _, ip := range correctIPs {
+				info.loadBalancerStatus.Ingress = append(info.loadBalancerStatus.Ingress, v1.LoadBalancerIngress{IP: ip})
+
+			}
+		}
 	}
 
 	if apiservice.NeedsHealthCheck(service) {

pkg/proxy/service_test.go

@@ -18,6 +18,7 @@ package proxy
 
 import (
 	"net"
+	"reflect"
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
@@ -166,15 +167,15 @@ func TestServiceToServiceMap(t *testing.T) {
 				svc.Spec.LoadBalancerIP = "5.6.7.8"
 				svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port3", "UDP", 8675, 30061, 7000)
 				svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port4", "UDP", 8676, 30062, 7001)
-				svc.Status.LoadBalancer = v1.LoadBalancerStatus{
+				svc.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
-					Ingress: []v1.LoadBalancerIngress{
-						{IP: "10.1.2.4"},
-					},
-				}
 			}),
 			expected: map[ServicePortName]*BaseServiceInfo{
-				makeServicePortName("ns1", "load-balancer", "port3", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8675, "UDP", 0),
+				makeServicePortName("ns1", "load-balancer", "port3", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8675, "UDP", 0, func(info *BaseServiceInfo) {
-				makeServicePortName("ns1", "load-balancer", "port4", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8676, "UDP", 0),
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
+				}),
+				makeServicePortName("ns1", "load-balancer", "port4", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8676, "UDP", 0, func(info *BaseServiceInfo) {
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
+				}),
 			},
 		},
 		{
@@ -185,17 +186,17 @@ func TestServiceToServiceMap(t *testing.T) {
 				svc.Spec.LoadBalancerIP = "5.6.7.8"
 				svc.Spec.Ports = addTestPort(svc.Spec.Ports, "portx", "UDP", 8677, 30063, 7002)
 				svc.Spec.Ports = addTestPort(svc.Spec.Ports, "porty", "UDP", 8678, 30064, 7003)
-				svc.Status.LoadBalancer = v1.LoadBalancerStatus{
+				svc.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
-					Ingress: []v1.LoadBalancerIngress{
-						{IP: "10.1.2.3"},
-					},
-				}
 				svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
 				svc.Spec.HealthCheckNodePort = 345
 			}),
 			expected: map[ServicePortName]*BaseServiceInfo{
-				makeServicePortName("ns1", "only-local-load-balancer", "portx", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8677, "UDP", 345),
+				makeServicePortName("ns1", "only-local-load-balancer", "portx", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8677, "UDP", 345, func(info *BaseServiceInfo) {
-				makeServicePortName("ns1", "only-local-load-balancer", "porty", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8678, "UDP", 345),
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
+				}),
+				makeServicePortName("ns1", "only-local-load-balancer", "porty", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8678, "UDP", 345, func(info *BaseServiceInfo) {
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
+				}),
 			},
 		},
 		{
@@ -225,6 +226,14 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			isIPv6Mode: &falseVal,
 		},
@@ -245,6 +254,14 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			isIPv6Mode: &trueVal,
 		},
@@ -267,11 +284,20 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			expected: map[ServicePortName]*BaseServiceInfo{
 				makeServicePortName("test", "validIPv4", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(info *BaseServiceInfo) {
 					info.externalIPs = []string{testExternalIPv4}
 					info.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv4}}
 				}),
 			},
 			isIPv6Mode: &falseVal,
@@ -295,11 +321,20 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			expected: map[ServicePortName]*BaseServiceInfo{
 				makeServicePortName("test", "validIPv6", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(info *BaseServiceInfo) {
 					info.externalIPs = []string{testExternalIPv6}
 					info.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv6}}
 				}),
 			},
 			isIPv6Mode: &trueVal,
@@ -323,11 +358,20 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			expected: map[ServicePortName]*BaseServiceInfo{
 				makeServicePortName("test", "filterIPv6InIPV4Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(info *BaseServiceInfo) {
 					info.externalIPs = []string{testExternalIPv4}
 					info.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv4}}
 				}),
 			},
 			isIPv6Mode: &falseVal,
@@ -351,11 +395,20 @@ func TestServiceToServiceMap(t *testing.T) {
 						},
 					},
 				},
+				Status: v1.ServiceStatus{
+					LoadBalancer: v1.LoadBalancerStatus{
+						Ingress: []v1.LoadBalancerIngress{
+							{IP: testExternalIPv4},
+							{IP: testExternalIPv6},
+						},
+					},
+				},
 			},
 			expected: map[ServicePortName]*BaseServiceInfo{
 				makeServicePortName("test", "filterIPv4InIPV6Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(info *BaseServiceInfo) {
 					info.externalIPs = []string{testExternalIPv6}
 					info.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
+					info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv6}}
 				}),
 			},
 			isIPv6Mode: &trueVal,
@@ -377,7 +430,8 @@ func TestServiceToServiceMap(t *testing.T) {
 				svcInfo.protocol != expectedInfo.protocol ||
 				svcInfo.healthCheckNodePort != expectedInfo.healthCheckNodePort ||
 				!sets.NewString(svcInfo.externalIPs...).Equal(sets.NewString(expectedInfo.externalIPs...)) ||
-				!sets.NewString(svcInfo.loadBalancerSourceRanges...).Equal(sets.NewString(expectedInfo.loadBalancerSourceRanges...)) {
+				!sets.NewString(svcInfo.loadBalancerSourceRanges...).Equal(sets.NewString(expectedInfo.loadBalancerSourceRanges...)) ||
+				!reflect.DeepEqual(svcInfo.loadBalancerStatus, expectedInfo.loadBalancerStatus) {
 				t.Errorf("[%s] expected new[%v]to be %v, got %v", tc.desc, svcKey, expectedInfo, *svcInfo)
 			}
 		}

pkg/proxy/util/utils_test.go

@@ -537,3 +537,96 @@ func TestShuffleStrings(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterIncorrectIPVersion(t *testing.T) {
+	testCases := []struct {
+		desc            string
+		ipString        []string
+		wantIPv6        bool
+		expectCorrect   []string
+		expectIncorrect []string
+	}{
+		{
+			desc:            "empty input IPv4",
+			ipString:        []string{},
+			wantIPv6:        false,
+			expectCorrect:   nil,
+			expectIncorrect: nil,
+		},
+		{
+			desc:            "empty input IPv6",
+			ipString:        []string{},
+			wantIPv6:        true,
+			expectCorrect:   nil,
+			expectIncorrect: nil,
+		},
+		{
+			desc:            "want IPv4 and receive IPv6",
+			ipString:        []string{"fd00:20::1"},
+			wantIPv6:        false,
+			expectCorrect:   nil,
+			expectIncorrect: []string{"fd00:20::1"},
+		},
+		{
+			desc:            "want IPv6 and receive IPv4",
+			ipString:        []string{"192.168.200.2"},
+			wantIPv6:        true,
+			expectCorrect:   nil,
+			expectIncorrect: []string{"192.168.200.2"},
+		},
+		{
+			desc:            "want IPv6 and receive IPv4 and IPv6",
+			ipString:        []string{"192.168.200.2", "192.1.34.23", "fd00:20::1", "2001:db9::3"},
+			wantIPv6:        true,
+			expectCorrect:   []string{"fd00:20::1", "2001:db9::3"},
+			expectIncorrect: []string{"192.168.200.2", "192.1.34.23"},
+		},
+		{
+			desc:            "want IPv4 and receive IPv4 and IPv6",
+			ipString:        []string{"192.168.200.2", "192.1.34.23", "fd00:20::1", "2001:db9::3"},
+			wantIPv6:        false,
+			expectCorrect:   []string{"192.168.200.2", "192.1.34.23"},
+			expectIncorrect: []string{"fd00:20::1", "2001:db9::3"},
+		},
+		{
+			desc:            "want IPv4 and receive IPv4 only",
+			ipString:        []string{"192.168.200.2", "192.1.34.23"},
+			wantIPv6:        false,
+			expectCorrect:   []string{"192.168.200.2", "192.1.34.23"},
+			expectIncorrect: nil,
+		},
+		{
+			desc:            "want IPv6 and receive IPv4 only",
+			ipString:        []string{"192.168.200.2", "192.1.34.23"},
+			wantIPv6:        true,
+			expectCorrect:   nil,
+			expectIncorrect: []string{"192.168.200.2", "192.1.34.23"},
+		},
+		{
+			desc:            "want IPv4 and receive IPv6 only",
+			ipString:        []string{"fd00:20::1", "2001:db9::3"},
+			wantIPv6:        false,
+			expectCorrect:   nil,
+			expectIncorrect: []string{"fd00:20::1", "2001:db9::3"},
+		},
+		{
+			desc:            "want IPv6 and receive IPv6 only",
+			ipString:        []string{"fd00:20::1", "2001:db9::3"},
+			wantIPv6:        true,
+			expectCorrect:   []string{"fd00:20::1", "2001:db9::3"},
+			expectIncorrect: nil,
+		},
+	}
+
+	for _, testcase := range testCases {
+		t.Run(testcase.desc, func(t *testing.T) {
+			correct, incorrect := FilterIncorrectIPVersion(testcase.ipString, testcase.wantIPv6)
+			if !reflect.DeepEqual(testcase.expectCorrect, correct) {
+				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, correct)
+			}
+			if !reflect.DeepEqual(testcase.expectIncorrect, incorrect) {
+				t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectIncorrect, incorrect)
+			}
+		})
+	}
+}