diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 5e605adbe08..afa9b06a7e4 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3509,8 +3509,12 @@ type LoadBalancerIngress struct { // +optional Hostname string - // IPMode specifies the IP mode to use for this ingress - // Defaults to `VIP` if IP is set, null otherwise + // IPMode specifies how the load-balancer's IP behaves. + // Setting this to "VIP" indicates that the traffic passing through + // this load-balancer is delivered with the destination IP and port set to the load-balancer's IP and port. + // Setting this to "Proxy" indicates that the load-balancer acts like a proxy, + // delivering traffic with the destination IP and port set to the node's IP and nodePort or to the pod's IP and targetPort. + // This field can only be set when the ip field is also set, and defaults to "VIP" if not specified. // +optional IPMode *LoadBalancerIPMode } @@ -3528,7 +3532,7 @@ const ( // is delivered with the destination IP set to the specified LoadBalancer IP LoadBalancerIPModeVIP LoadBalancerIPMode = "VIP" // LoadBalancerIPModeProxy indicates that the specified LoadBalancer acts like a proxy, - // changing the destination IP to the node IP and the source IP to the LoadBalaner (mostly private) IP + // changing the destination IP to the node IP and the source IP to the LoadBalancer (mostly private) IP LoadBalancerIPModeProxy LoadBalancerIPMode = "Proxy" ) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 154f18609c4..1b4399c1e68 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5989,18 +5989,22 @@ func ValidateLoadBalancerStatus(status *core.LoadBalancerStatus, fldPath *field. } if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) { - if len(ingress.IP) > 0 && ingress.IPMode != nil { - switch *ingress.IPMode { - case core.LoadBalancerIPModeVIP, core.LoadBalancerIPModeProxy: - break - default: - allErrs = append(allErrs, field.NotSupported(idxPath.Child("ipMode"), ingress.IPMode, supportedLoadBalancerIPMode.List())) - } - } else if len(ingress.IP) > 0 && ingress.IPMode == nil { + if len(ingress.IP) > 0 && ingress.IPMode == nil { allErrs = append(allErrs, field.Required(idxPath.Child("ipMode"), "must be specified when `ip` is set")) - } else if len(ingress.IP) == 0 && ingress.IPMode != nil { + } + } + + if ingress.IPMode != nil { + if len(ingress.IP) == 0 { allErrs = append(allErrs, field.Forbidden(idxPath.Child("ipMode"), "may not be used when `ip` is not set")) } + + switch *ingress.IPMode { + case core.LoadBalancerIPModeVIP, core.LoadBalancerIPModeProxy: + break + default: + allErrs = append(allErrs, field.NotSupported(idxPath.Child("ipMode"), ingress.IPMode, supportedLoadBalancerIPMode.List())) + } } if len(ingress.Hostname) > 0 { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 20276c41a22..7f73d3f6877 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11071,6 +11071,90 @@ func TestValidateServiceCreate(t *testing.T) { } } +func TestValidateLoadBalancerStatus(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LoadBalancerIPMode, true)() + + ipModeVIP := core.LoadBalancerIPModeVIP + ipModeProxy := core.LoadBalancerIPModeProxy + ipModeDummy := core.LoadBalancerIPMode("dummy") + + testCases := []struct { + name string + tweakLBStatus func(s *core.LoadBalancerStatus) + numErrs int + }{ + /* LoadBalancerIPMode*/ + { + name: `valid vip ipMode`, + tweakLBStatus: func(s *core.LoadBalancerStatus) { + s.Ingress = []core.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeVIP, + }, + } + }, + numErrs: 0, + }, + { + name: `valid proxy ipMode`, + tweakLBStatus: func(s *core.LoadBalancerStatus) { + s.Ingress = []core.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeProxy, + }, + } + }, + numErrs: 0, + }, + { + name: `invalid ipMode`, + tweakLBStatus: func(s *core.LoadBalancerStatus) { + s.Ingress = []core.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeDummy, + }, + } + }, + numErrs: 1, + }, + { + name: `missing ipMode`, + tweakLBStatus: func(s *core.LoadBalancerStatus) { + s.Ingress = []core.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + } + }, + numErrs: 1, + }, + { + name: `missing ip with ipMode present`, + tweakLBStatus: func(s *core.LoadBalancerStatus) { + s.Ingress = []core.LoadBalancerIngress{ + { + IPMode: &ipModeProxy, + }, + } + }, + numErrs: 1, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := core.LoadBalancerStatus{} + tc.tweakLBStatus(&s) + errs := ValidateLoadBalancerStatus(&s, field.NewPath("status")) + if len(errs) != tc.numErrs { + t.Errorf("Unexpected error list for case %q(expected:%v got %v) - Errors:\n %v", tc.name, tc.numErrs, len(errs), errs.ToAggregate()) + } + }) + } +} + func TestValidateServiceExternalTrafficFieldsCombination(t *testing.T) { testCases := []struct { name string diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 6b3bd48432b..e4ba681d2a9 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -1175,7 +1175,7 @@ func (proxier *Proxier) syncProxyRules() { // This currently works for loadbalancers that preserves source ips. // For loadbalancers which direct traffic to service NodePort, the firewall rules will not apply. - if !utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) || ingress.IPMode == nil || *ingress.IPMode == v1.LoadBalancerIPModeVIP { + if !utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) || *ingress.IPMode == v1.LoadBalancerIPModeVIP { args = append(args[:0], "-A", string(kubeServicesChain), "-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcNameString), diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 4a481a5b330..513a66ead51 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -1333,7 +1333,7 @@ func (proxier *Proxier) syncProxyRules() { // Capture load-balancer ingress. for _, ingress := range svcInfo.LoadBalancerIngress() { - if ingress.IP != "" && (!utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) || ingress.IPMode == nil || *ingress.IPMode == v1.LoadBalancerIPModeVIP) { + if ingress.IP != "" && (!utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) || *ingress.IPMode == v1.LoadBalancerIPModeVIP) { // ipset call entry = &utilipset.Entry{ IP: ingress.IP, diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 4557c283760..6cfce122911 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -163,26 +163,16 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic klog.V(4).Infof("service change tracker(%v) ignored the following load balancer source ranges(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(incorrectIPs, ","), service.Namespace, service.Name) } - // Obtain Load Balancer Ingress IPs - var allIncorrectIPs []string - for _, ing := range service.Status.LoadBalancer.Ingress { - // []string{ing.IP} have a len of 1, so len(correctIPs) + len(incorrectIPs) == 1 - correctIPs, incorrectIPs := utilproxy.FilterIncorrectIPVersion([]string{ing.IP}, sct.ipFamily) + correctIngresses, incorrectIngresses := utilproxy.FilterIncorrectLoadBalancerIngress(service.Status.LoadBalancer.Ingress, sct.ipFamily) - // len is either 1 or 0 - if len(correctIPs) == 1 { - // Update the LoadBalancerStatus with the filtered IP - info.loadBalancerStatus.Ingress = append(info.loadBalancerStatus.Ingress, ing) - continue + info.loadBalancerStatus.Ingress = correctIngresses + + if len(incorrectIngresses) > 0 { + var incorrectIPs []string + for _, incorrectIng := range incorrectIngresses { + incorrectIPs = append(incorrectIPs, incorrectIng.IP) } - - // here len(incorrectIPs) == 1 since len(correctIPs) == 0 - allIncorrectIPs = append(allIncorrectIPs, incorrectIPs[0]) - } - - if len(allIncorrectIPs) > 0 { klog.V(4).Infof("service change tracker(%v) ignored the following load balancer(%s) ingress ips for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(incorrectIPs, ","), service.Namespace, service.Name) - } if apiservice.NeedsHealthCheck(service) { diff --git a/pkg/proxy/util/utils.go b/pkg/proxy/util/utils.go index 2031cd1a09e..4fb49a97c53 100644 --- a/pkg/proxy/util/utils.go +++ b/pkg/proxy/util/utils.go @@ -403,3 +403,26 @@ func GetClusterIPByFamily(ipFamily v1.IPFamily, service *v1.Service) string { return "" } + +// FilterIncorrectLoadBalancerIngress filters out the ingresses with an IP version different from the given one +func FilterIncorrectLoadBalancerIngress(ingresses []v1.LoadBalancerIngress, ipFamily v1.IPFamily) ([]v1.LoadBalancerIngress, []v1.LoadBalancerIngress) { + var validIngresses []v1.LoadBalancerIngress + var invalidIngresses []v1.LoadBalancerIngress + + for _, ing := range ingresses { + // []string{ing.IP} have a len of 1, so len(correctIPs) + len(incorrectIPs) == 1 + correctIPs, _ := FilterIncorrectIPVersion([]string{ing.IP}, ipFamily) + + // len is either 1 or 0 + if len(correctIPs) == 1 { + // Update the LoadBalancerStatus with the filtered IP + validIngresses = append(validIngresses, ing) + continue + } + + // here len(incorrectIPs) == 1 since len(correctIPs) == 0 + invalidIngresses = append(invalidIngresses, ing) + } + + return validIngresses, invalidIngresses +} diff --git a/pkg/proxy/util/utils_test.go b/pkg/proxy/util/utils_test.go index f10a95c88b3..787f92f49c7 100644 --- a/pkg/proxy/util/utils_test.go +++ b/pkg/proxy/util/utils_test.go @@ -822,3 +822,179 @@ func TestGetClusterIPByFamily(t *testing.T) { } } + +func TestFilterIncorrectLoadBalancerIngress(t *testing.T) { + ipModeVIP := v1.LoadBalancerIPModeVIP + testCases := []struct { + name string + ingresses []v1.LoadBalancerIngress + ipFamily v1.IPFamily + expectedCorrect []v1.LoadBalancerIngress + expectedIncorrect []v1.LoadBalancerIngress + }{ + { + name: "IPv4 only valid ingresses", + ipFamily: v1.IPv4Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeVIP, + }, + { + IP: "1.2.3.5", + }, + }, + expectedCorrect: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeVIP, + }, + { + IP: "1.2.3.5", + }, + }, + expectedIncorrect: nil, + }, + { + name: "IPv4 some invalid ingresses", + ipFamily: v1.IPv4Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeVIP, + }, + { + IP: "2000::1", + }, + { + Hostname: "dummy", + }, + }, + expectedCorrect: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + IPMode: &ipModeVIP, + }, + { + Hostname: "dummy", // weirdly no IP is a valid IPv4 but invalid IPv6 + }, + }, + expectedIncorrect: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + }, + }, + }, + { + name: "IPv4 only invalid ingresses", + ipFamily: v1.IPv4Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + }, + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + }, + expectedCorrect: nil, + expectedIncorrect: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + }, + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + }, + }, + { + name: "IPv6 only valid ingresses", + ipFamily: v1.IPv6Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + { + IP: "2000::2", + }, + }, + expectedCorrect: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + { + IP: "2000::2", + }, + }, + expectedIncorrect: nil, + }, + { + name: "IPv6 some invalid ingresses", + ipFamily: v1.IPv6Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + { + IP: "1.2.3.4", + }, + { + Hostname: "dummy", + }, + }, + expectedCorrect: []v1.LoadBalancerIngress{ + { + IP: "2000::1", + IPMode: &ipModeVIP, + }, + }, + expectedIncorrect: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + { + Hostname: "dummy", // weirdly no IP is a valid IPv4 but invalid IPv6 + }, + }, + }, + { + name: "IPv6 only invalid ingresses", + ipFamily: v1.IPv6Protocol, + ingresses: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + { + IP: "1.2.3.5", + IPMode: &ipModeVIP, + }, + }, + expectedCorrect: nil, + expectedIncorrect: []v1.LoadBalancerIngress{ + { + IP: "1.2.3.4", + }, + { + IP: "1.2.3.5", + IPMode: &ipModeVIP, + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + correctIngresses, incorrectIngresses := FilterIncorrectLoadBalancerIngress(testCase.ingresses, testCase.ipFamily) + if !reflect.DeepEqual(correctIngresses, testCase.expectedCorrect) { + t.Errorf("Test %v failed: expected %v, got %v", testCase.name, testCase.expectedCorrect, correctIngresses) + } + if !reflect.DeepEqual(incorrectIngresses, testCase.expectedIncorrect) { + t.Errorf("Test %v failed: expected %v, got %v", testCase.name, testCase.expectedIncorrect, incorrectIngresses) + } + }) + } +} diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 301db96fc2b..4ee36bb1893 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3972,8 +3972,12 @@ type LoadBalancerIngress struct { // +optional Hostname string `json:"hostname,omitempty" protobuf:"bytes,2,opt,name=hostname"` - // IPMode specifies the IP mode to use for this ingress - // Defaults to `VIP` if IP is set, null otherwise + // IPMode specifies how the load-balancer's IP behaves. + // Setting this to "VIP" indicates that the traffic passing through + // this load-balancer is delivered with the destination IP and port set to the load-balancer's IP and port. + // Setting this to "Proxy" indicates that the load-balancer acts like a proxy, + // delivering traffic with the destination IP and port set to the node's IP and nodePort or to the pod's IP and targetPort. + // This field can only be set when the ip field is also set, and defaults to "VIP" if not specified. // +optional IPMode *LoadBalancerIPMode `json:"ipMode,omitempty" protobuf:"bytes,3,opt,name=ipMode"` } @@ -3991,7 +3995,7 @@ const ( // is delivered with the destination IP set to the specified LoadBalancer IP LoadBalancerIPModeVIP LoadBalancerIPMode = "VIP" // LoadBalancerIPModeProxy indicates that the specified LoadBalancer acts like a proxy, - // changing the destination IP to the node IP and the source IP to the LoadBalaner (mostly private) IP + // changing the destination IP to the node IP and the source IP to the LoadBalancer (mostly private) IP LoadBalancerIPModeProxy LoadBalancerIPMode = "Proxy" )