From f4521aa75a12210d325001d2e3aef0972bda463d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 1 Aug 2021 16:42:33 -0700 Subject: [PATCH] Fix validation on ETP: "" is not valid This was causing tests to pass which ought not be passing. This is not an API change because we default the value of it when needed. So we would never see this in the wild, but it makes the tests sloppy. --- pkg/apis/core/validation/validation.go | 76 +++++++++--------- pkg/apis/core/validation/validation_test.go | 78 ++++++++++++++++++- .../core/service/storage/rest_test.go | 1 + 3 files changed, 114 insertions(+), 41 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 696563223b8..528fe50e267 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4454,9 +4454,8 @@ func ValidateService(service *core.Service) field.ErrorList { // validate LoadBalancerClass field allErrs = append(allErrs, validateLoadBalancerClassField(nil, service)...) - // external traffic fields - allErrs = append(allErrs, validateServiceExternalTrafficFieldsValue(service)...) - allErrs = append(allErrs, validateServiceExternalTrafficFieldsCombination(service)...) + // external traffic policy fields + allErrs = append(allErrs, validateServiceExternalTrafficPolicy(service)...) // internal traffic policy field allErrs = append(allErrs, validateServiceInternalTrafficFieldsValue(service)...) @@ -4508,22 +4507,46 @@ func validateServicePort(sp *core.ServicePort, requireName, isHeadlessService bo return allErrs } -// validateServiceExternalTrafficFieldsValue validates ExternalTraffic related annotations -// have legal value. -func validateServiceExternalTrafficFieldsValue(service *core.Service) field.ErrorList { +func needsExternalTrafficPolicy(svc *api.Service) bool { + return svc.Spec.Type == core.ServiceTypeLoadBalancer || svc.Spec.Type == core.ServiceTypeNodePort +} + +var validExternalTrafficPolicies = sets.NewString( + string(core.ServiceExternalTrafficPolicyTypeCluster), + string(core.ServiceExternalTrafficPolicyTypeLocal)) + +func validateServiceExternalTrafficPolicy(service *core.Service) field.ErrorList { allErrs := field.ErrorList{} - // Check first class fields. - if service.Spec.ExternalTrafficPolicy != "" && - service.Spec.ExternalTrafficPolicy != core.ServiceExternalTrafficPolicyTypeCluster && - service.Spec.ExternalTrafficPolicy != core.ServiceExternalTrafficPolicyTypeLocal { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("externalTrafficPolicy"), service.Spec.ExternalTrafficPolicy, - fmt.Sprintf("ExternalTrafficPolicy must be empty, %v or %v", core.ServiceExternalTrafficPolicyTypeCluster, core.ServiceExternalTrafficPolicyTypeLocal))) + fldPath := field.NewPath("spec") + + if !needsExternalTrafficPolicy(service) { + if service.Spec.ExternalTrafficPolicy != "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("externalTrafficPolicy"), service.Spec.ExternalTrafficPolicy, + "may only be set when `type` is 'NodePort' or 'LoadBalancer'")) + } + } else { + if service.Spec.ExternalTrafficPolicy == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("externalTrafficPolicy"), "")) + } else if !validExternalTrafficPolicies.Has(string(service.Spec.ExternalTrafficPolicy)) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("externalTrafficPolicy"), + service.Spec.ExternalTrafficPolicy, validExternalTrafficPolicies.List())) + } } - if service.Spec.HealthCheckNodePort < 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("healthCheckNodePort"), service.Spec.HealthCheckNodePort, - "HealthCheckNodePort must be not less than 0")) + if !apiservice.NeedsHealthCheck(service) { + if service.Spec.HealthCheckNodePort != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("healthCheckNodePort"), service.Spec.HealthCheckNodePort, + "may only be set when `type` is 'LoadBalancer' and `externalTrafficPolicy` is 'Local'")) + } + } else { + if service.Spec.HealthCheckNodePort == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("healthCheckNodePort"), "")) + } else { + for _, msg := range validation.IsValidPortNum(int(service.Spec.HealthCheckNodePort)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("healthCheckNodePort"), service.Spec.HealthCheckNodePort, msg)) + } + } } return allErrs @@ -4559,29 +4582,6 @@ func validateServiceInternalTrafficFieldsValue(service *core.Service) field.Erro return allErrs } -// validateServiceExternalTrafficFieldsCombination validates if ExternalTrafficPolicy, -// HealthCheckNodePort and Type combination are legal. For update, it should be called -// after clearing externalTraffic related fields for the ease of transitioning between -// different service types. -func validateServiceExternalTrafficFieldsCombination(service *core.Service) field.ErrorList { - allErrs := field.ErrorList{} - - if service.Spec.Type != core.ServiceTypeLoadBalancer && - service.Spec.Type != core.ServiceTypeNodePort && - service.Spec.ExternalTrafficPolicy != "" { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "externalTrafficPolicy"), service.Spec.ExternalTrafficPolicy, - "ExternalTrafficPolicy can only be set on NodePort and LoadBalancer service")) - } - - if !apiservice.NeedsHealthCheck(service) && - service.Spec.HealthCheckNodePort != 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "healthCheckNodePort"), service.Spec.HealthCheckNodePort, - "HealthCheckNodePort can only be set on LoadBalancer service with ExternalTrafficPolicy=Local")) - } - - return allErrs -} - // ValidateServiceCreate validates Services as they are created. func ValidateServiceCreate(service *core.Service) field.ErrorList { return ValidateService(service) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 64182ef6ef4..64cdf96bd7a 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11003,6 +11003,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid load balancer protocol UDP 1", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports[0].Protocol = "UDP" }, @@ -11012,6 +11013,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid load balancer protocol UDP 2", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports[0] = core.ServicePort{Name: "q", Port: 12345, Protocol: "UDP", TargetPort: intstr.FromInt(12345)} }, @@ -11021,6 +11023,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "load balancer with mix protocol", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "UDP", TargetPort: intstr.FromInt(12345)}) }, @@ -11075,6 +11078,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type - loadbalancer", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 0, @@ -11083,6 +11087,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type - loadbalancer with allocateLoadBalancerNodePorts=false", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false) }, numErrs: 0, @@ -11091,6 +11096,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid type - missing AllocateLoadBalancerNodePorts for loadbalancer type", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster }, numErrs: 1, }, @@ -11098,6 +11104,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type loadbalancer 2 ports", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, @@ -11107,6 +11114,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid external load balancer 2 ports", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, @@ -11116,6 +11124,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "duplicate nodeports", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "r", Port: 2, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(2)}) }, @@ -11125,6 +11134,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "duplicate nodeports (different protocols)", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "r", Port: 2, Protocol: "UDP", NodePort: 1, TargetPort: intstr.FromInt(2)}) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "s", Port: 3, Protocol: "SCTP", NodePort: 1, TargetPort: intstr.FromInt(3)}) @@ -11161,6 +11171,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type - nodeport", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster }, numErrs: 0, }, @@ -11168,6 +11179,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type - loadbalancer with allocateLoadBalancerNodePorts=true", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 0, @@ -11176,6 +11188,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type loadbalancer 2 ports", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, @@ -11185,6 +11198,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type loadbalancer with NodePort", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345, TargetPort: intstr.FromInt(12345)}) }, @@ -11194,6 +11208,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type=NodePort service with NodePort", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", NodePort: 12345, TargetPort: intstr.FromInt(12345)}) }, numErrs: 0, @@ -11202,6 +11217,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type=NodePort service without NodePort", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, numErrs: 0, @@ -11226,6 +11242,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid public service with duplicate NodePort", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "p1", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "p2", Port: 2, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(2)}) }, @@ -11235,6 +11252,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid type=LoadBalancer", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 12345, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, @@ -11246,6 +11264,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid port type=LoadBalancer", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "kubelet", Port: 10250, Protocol: "TCP", TargetPort: intstr.FromInt(12345)}) }, @@ -11255,6 +11274,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid LoadBalancer source range annotation", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "1.2.3.4/8, 5.6.7.8/16" }, @@ -11264,6 +11284,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "empty LoadBalancer source range annotation", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "" }, @@ -11280,6 +11301,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid LoadBalancer source range annotation (invalid CIDR)", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "1.2.3.4/33" }, @@ -11296,6 +11318,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid LoadBalancer source range", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.LoadBalancerSourceRanges = []string{"1.2.3.4/8", "5.6.7.8/16"} }, @@ -11305,6 +11328,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "empty LoadBalancer source range", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.LoadBalancerSourceRanges = []string{" "} }, @@ -11314,6 +11338,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid LoadBalancer source range", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.LoadBalancerSourceRanges = []string{"foo.bar"} }, @@ -11369,6 +11394,7 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.ClusterIP = "None" s.Spec.ClusterIPs = []string{"None"} s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 1, @@ -11377,6 +11403,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid node port with clusterIP None", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeNodePort + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.Ports = append(s.Spec.Ports, core.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) s.Spec.ClusterIP = "None" s.Spec.ClusterIPs = []string{"None"} @@ -11463,6 +11490,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "sessionAffinityConfig can't be set when session affinity is None", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.SessionAffinity = core.ServiceAffinityNone s.Spec.SessionAffinityConfig = &core.SessionAffinityConfig{ @@ -11916,6 +11944,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "valid LoadBalancerClass when type is LoadBalancer", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-load-balancer-class") }, @@ -11925,6 +11954,7 @@ func TestValidateServiceCreate(t *testing.T) { name: "invalid LoadBalancerClass", tweakSvc: func(s *core.Service) { s.Spec.Type = core.ServiceTypeLoadBalancer + s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) s.Spec.LoadBalancerClass = utilpointer.StringPtr("Bad/LoadBalancerClass") }, @@ -11955,7 +11985,7 @@ func TestValidateServiceCreate(t *testing.T) { } } -func TestValidateServiceExternalTrafficFieldsCombination(t *testing.T) { +func TestValidateServiceExternalTrafficPolicy(t *testing.T) { testCases := []struct { name string tweakSvc func(svc *core.Service) // Given a basic valid service, each test case can customize it. @@ -12014,13 +12044,26 @@ func TestValidateServiceExternalTrafficFieldsCombination(t *testing.T) { }, numErrs: 2, }, + { + name: "externalTrafficPolicy is required on NodePort service", + tweakSvc: func(s *core.Service) { + s.Spec.Type = core.ServiceTypeNodePort + }, + numErrs: 1, + }, + { + name: "externalTrafficPolicy is required on LoadBalancer service", + tweakSvc: func(s *core.Service) { + s.Spec.Type = core.ServiceTypeLoadBalancer + }, + numErrs: 1, + }, } for _, tc := range testCases { svc := makeValidService() tc.tweakSvc(&svc) - // TODO: This test is probably insufficient for such a big function under test. - errs := validateServiceExternalTrafficFieldsCombination(&svc) + errs := validateServiceExternalTrafficPolicy(&svc) if len(errs) != tc.numErrs { t.Errorf("Unexpected error list for case %q: %v", tc.name, errs.ToAggregate()) } @@ -13511,6 +13554,7 @@ func TestValidateServiceUpdate(t *testing.T) { name: "change type", tweakSvc: func(oldSvc, newSvc *core.Service) { newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 0, @@ -13526,6 +13570,7 @@ func TestValidateServiceUpdate(t *testing.T) { name: "change type -> nodeport", tweakSvc: func(oldSvc, newSvc *core.Service) { newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster }, numErrs: 0, }, @@ -13535,6 +13580,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerSourceRanges = []string{"10.0.0.0/8"} }, @@ -13547,6 +13593,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.LoadBalancerSourceRanges = []string{"10.0.0.0/8"} newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerSourceRanges = []string{"10.100.0.0/16"} }, @@ -13558,6 +13605,7 @@ func TestValidateServiceUpdate(t *testing.T) { newSvc.Spec.ClusterIP = "None" newSvc.Spec.ClusterIPs = []string{"None"} newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 1, @@ -13631,6 +13679,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "1.2.3.4" oldSvc.Spec.ClusterIPs = []string{"1.2.3.4"} @@ -13645,6 +13694,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "" oldSvc.Spec.ClusterIPs = nil @@ -13659,6 +13709,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "1.2.3.4" @@ -13674,6 +13725,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "" @@ -13690,6 +13742,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false) }, numErrs: 0, @@ -13700,6 +13753,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false) newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) }, numErrs: 0, @@ -13709,6 +13763,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "1.2.3.4" oldSvc.Spec.ClusterIPs = []string{"1.2.3.4"} @@ -13723,6 +13778,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "" oldSvc.Spec.ClusterIPs = nil @@ -13765,6 +13821,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "1.2.3.4" @@ -13780,6 +13837,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "" @@ -13796,6 +13854,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "1.2.3.4" @@ -13812,6 +13871,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) oldSvc.Spec.ClusterIP = "" @@ -13858,6 +13918,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "1.2.3.4" oldSvc.Spec.ClusterIPs = []string{"1.2.3.4"} @@ -13873,6 +13934,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeLoadBalancer oldSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.ClusterIP = "" oldSvc.Spec.ClusterIPs = nil @@ -13915,6 +13977,7 @@ func TestValidateServiceUpdate(t *testing.T) { tweakSvc: func(oldSvc, newSvc *core.Service) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster oldSvc.Spec.Ports = append(oldSvc.Spec.Ports, core.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) newSvc.Spec.Ports = append(newSvc.Spec.Ports, core.ServicePort{Name: "q", Port: 1, Protocol: "TCP", NodePort: 1, TargetPort: intstr.FromInt(1)}) @@ -14367,6 +14430,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-old") newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-old") }, @@ -14380,6 +14444,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-old") newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-new") }, @@ -14393,6 +14458,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-old") newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = nil }, @@ -14406,6 +14472,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.LoadBalancerClass = nil newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-new") }, @@ -14417,6 +14484,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-load-balancer-class") }, @@ -14428,6 +14496,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = nil }, @@ -14439,6 +14508,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeClusterIP newSvc.Spec.Type = core.ServiceTypeLoadBalancer + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("Bad/LoadBalancerclass") }, @@ -14470,6 +14540,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.Type = core.ServiceTypeNodePort newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-load-balancer-class") }, numErrs: 2, @@ -14506,6 +14577,7 @@ func TestValidateServiceUpdate(t *testing.T) { oldSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-load-balancer-class") newSvc.Spec.Type = core.ServiceTypeNodePort + newSvc.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyTypeCluster newSvc.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test-load-balancer-class") }, numErrs: 2, diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index ae25c59c05c..affc06dbff1 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -597,6 +597,7 @@ func TestServiceRegistryUpdateLoadBalancerService(t *testing.T) { // Modify to be loadbalancer. svc2 := obj.(*api.Service).DeepCopy() svc2.Spec.Type = api.ServiceTypeLoadBalancer + svc2.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster svc2.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) obj, _, err = storage.Update(ctx, svc2.Name, rest.DefaultUpdatedObjectInfo(svc2), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}) if err != nil {