From 1134220e011ab63ec6eb1d6baaefebfd0cecb21a Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:49:36 +0200 Subject: [PATCH 1/7] Add RelaxedServiceNameValidation feature gate --- pkg/features/kube_features.go | 10 ++++++++++ .../reference/versioned_feature_list.yaml | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 4fd912345c2..87c7a5542a2 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -636,6 +636,12 @@ const ( // Allow almost all printable ASCII characters in environment variables RelaxedEnvironmentVariableValidation featuregate.Feature = "RelaxedEnvironmentVariableValidation" + // owner: @adrianmoisey + // kep: https://kep.k8s.io/5311 + // + // Relaxed DNS search string validation. + RelaxedServiceNameValidation featuregate.Feature = "RelaxedServiceNameValidation" + // owner: @zhangweikop // // Enable kubelet tls server to update certificate if the specified certificate files are changed. @@ -1641,6 +1647,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.34"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.37 }, + RelaxedServiceNameValidation: { + {Version: version.MustParse("1.34"), Default: false, PreRelease: featuregate.Alpha}, + }, + ReloadKubeletServerCertificateFile: { {Version: version.MustParse("1.31"), Default: true, PreRelease: featuregate.Beta}, }, diff --git a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml index bbc0044cbff..61f8fec52b6 100644 --- a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml +++ b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml @@ -1185,6 +1185,12 @@ lockToDefault: true preRelease: GA version: "1.34" +- name: RelaxedServiceNameValidation + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.34" - name: ReloadKubeletServerCertificateFile versionedSpecs: - default: true From 487eb8a9e47187bc60c2f086dd076ddbe627fff2 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:51:57 +0200 Subject: [PATCH 2/7] Allow Service names to be validated with apimachineryvalidation.NameIsDNSLabel Only validate when feature gate RelaxedServiceNameValidation is enabled. Also remove name validation on Service updates, as the name is immutable. Move ValidateObjectMeta out of ValidateService Put it into ValidateServiceCreate(), making the code path as such: ``` pkg/registry/core/service/strategy.go Validate -> validation.ValidateServiceCreate -> ValidateObjectMeta -> ValidateService ValidateUpdate -> validation.ValidateServiceUpdate -> ValidateObjectMetaUpdate -> ValidateService ``` Other resources I checked pass the update objects through ValidateObjectMeta and ValidateObjectMetaUpdate, so this breaks the pattern, but it seems to be how the ValidateObjectMeta/ValidateObjectMetaUpdate functions are designed to operate. --- pkg/apis/core/validation/validation.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 770fed10b14..86814104b0f 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5924,9 +5924,12 @@ var supportedServiceIPFamilyPolicy = sets.New( core.IPFamilyPolicyRequireDualStack) // ValidateService tests if required fields/annotations of a Service are valid. -func ValidateService(service, oldService *core.Service) field.ErrorList { +func validateService(service, oldService *core.Service) field.ErrorList { metaPath := field.NewPath("metadata") - allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, metaPath) + + // Don't validate ObjectMeta here - that is handled in the ValidateServiceCreate/ValidateServiceUpdate + // functions which call ValidateObjectMeta and ValidateObjectMetaUpdate respectively. + var allErrs field.ErrorList topologyHintsVal, topologyHintsSet := service.Annotations[core.DeprecatedAnnotationTopologyAwareHints] topologyModeVal, topologyModeSet := service.Annotations[core.AnnotationTopologyMode] @@ -6276,7 +6279,17 @@ func validateServiceTrafficDistribution(service *core.Service) field.ErrorList { // ValidateServiceCreate validates Services as they are created. func ValidateServiceCreate(service *core.Service) field.ErrorList { - return ValidateService(service, nil) + metaPath := field.NewPath("metadata") + + // KEP-5311 Relaxed validation for Services names + validateServiceNameFunc := ValidateServiceName + if utilfeature.DefaultFeatureGate.Enabled(features.RelaxedServiceNameValidation) { + validateServiceNameFunc = apimachineryvalidation.NameIsDNSLabel + } + + allErrs := ValidateObjectMeta(&service.ObjectMeta, true, validateServiceNameFunc, metaPath) + + return append(allErrs, validateService(service, nil)...) } // ValidateServiceUpdate tests if required fields in the service are set during an update @@ -6299,7 +6312,7 @@ func ValidateServiceUpdate(service, oldService *core.Service) field.ErrorList { allErrs = append(allErrs, validateServiceExternalTrafficFieldsUpdate(oldService, service)...) - return append(allErrs, ValidateService(service, oldService)...) + return append(allErrs, validateService(service, oldService)...) } // ValidateServiceStatusUpdate tests if required fields in the Service are set when updating status. From b430159c86adca58c9e84bfa5fa325fe47e4b885 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:55:28 +0200 Subject: [PATCH 3/7] Allow Ingress service refs to be validated with apimachineryvalidation.NameIsDNSLabel Only validate when feature gate RelaxedServiceNameValidation is enabled or when the Ingess resource contains a service ref that already validates with apimachineryvalidation.NameIsDNSLabel --- pkg/apis/networking/validation/validation.go | 54 ++++++++++++++++++-- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/pkg/apis/networking/validation/validation.go b/pkg/apis/networking/validation/validation.go index d7edbc121ae..25d0d45dcd8 100644 --- a/pkg/apis/networking/validation/validation.go +++ b/pkg/apis/networking/validation/validation.go @@ -27,9 +27,11 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/networking" + "k8s.io/kubernetes/pkg/features" netutils "k8s.io/utils/net" "k8s.io/utils/ptr" ) @@ -283,6 +285,9 @@ type IngressValidationOptions struct { // AllowInvalidWildcardHostRule indicates whether invalid rule values are allowed in rules with wildcard hostnames AllowInvalidWildcardHostRule bool + + // AllowRelaxedServiceNameValidation indicates if the backend service name can be validated with apimachineryvalidation.NameIsDNSLabel + AllowRelaxedServiceNameValidation bool } // ValidateIngress validates Ingresses on create and update. @@ -296,8 +301,9 @@ func validateIngress(ingress *networking.Ingress, opts IngressValidationOptions) func ValidateIngressCreate(ingress *networking.Ingress) field.ErrorList { allErrs := field.ErrorList{} opts := IngressValidationOptions{ - AllowInvalidSecretName: false, - AllowInvalidWildcardHostRule: false, + AllowInvalidSecretName: false, + AllowInvalidWildcardHostRule: false, + AllowRelaxedServiceNameValidation: allowRelaxedServiceNameValidation(nil), } allErrs = append(allErrs, validateIngress(ingress, opts)...) annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass] @@ -312,8 +318,9 @@ func ValidateIngressCreate(ingress *networking.Ingress) field.ErrorList { func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress) field.ErrorList { allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata")) opts := IngressValidationOptions{ - AllowInvalidSecretName: allowInvalidSecretName(oldIngress), - AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(oldIngress), + AllowInvalidSecretName: allowInvalidSecretName(oldIngress), + AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(oldIngress), + AllowRelaxedServiceNameValidation: allowRelaxedServiceNameValidation(oldIngress), } allErrs = append(allErrs, validateIngress(ingress, opts)...) @@ -513,7 +520,13 @@ func validateIngressBackend(backend *networking.IngressBackend, fldPath *field.P if len(backend.Service.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("service", "name"), "")) } else { - for _, msg := range apivalidation.ValidateServiceName(backend.Service.Name, false) { + + validationFunc := apivalidation.ValidateServiceName + if opts.AllowRelaxedServiceNameValidation { + validationFunc = apimachineryvalidation.NameIsDNSLabel + } + + for _, msg := range validationFunc(backend.Service.Name, false) { allErrs = append(allErrs, field.Invalid(fldPath.Child("service", "name"), backend.Service.Name, msg)) } } @@ -685,6 +698,37 @@ func allowInvalidWildcardHostRule(oldIngress *networking.Ingress) bool { return false } +func allowRelaxedServiceNameValidation(oldIngress *networking.Ingress) bool { + // Early exit if the feature gate is enabled, as it allows relaxed validation + if utilfeature.DefaultFeatureGate.Enabled(features.RelaxedServiceNameValidation) { + return true + } + // Early exit if no old Ingress is provided + if oldIngress == nil { + return false + } + // If feature gate is disabled, check if any service names in the old Ingresss + for _, rule := range oldIngress.Spec.Rules { + if rule.HTTP == nil { + continue + } + for _, path := range rule.HTTP.Paths { + if path.Backend.Service == nil { + continue + } + serviceName := path.Backend.Service.Name + // If a name doesn't validate with apimachineryvalidation.NameIsDNS1035Label, but does validate with apimachineryvalidation.NameIsDNSLabel, + // then we allow it to be used as a Service name in an Ingress. + dnsLabelValidationErrors := apimachineryvalidation.NameIsDNSLabel(serviceName, false) + dns1035LabelValidationErrors := apimachineryvalidation.NameIsDNS1035Label(serviceName, false) + if len(dnsLabelValidationErrors) == 0 && len(dns1035LabelValidationErrors) > 0 { + return true + } + } + } + return false +} + // ValidateIPAddressName validates that the name is the decimal representation of an IP address. // IPAddress does not support generating names, prefix is not considered. func ValidateIPAddressName(name string, prefix bool) []string { From 37a90b7c244e0f27db525c0c3402479049a2164e Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:57:09 +0200 Subject: [PATCH 4/7] Add unit test for Service relaxed validation Test the behaviour of feature gate RelaxedServiceNameValidation. --- pkg/apis/core/validation/validation_test.go | 44 +++++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index fbdb066ef08..dd644165fe2 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -15474,11 +15474,12 @@ func TestValidateServiceCreate(t *testing.T) { preferDualStack := core.IPFamilyPolicyPreferDualStack testCases := []struct { - name string - tweakSvc func(svc *core.Service) // given a basic valid service, each test case can customize it - numErrs int - legacyIPs bool - newTrafficDist bool + name string + tweakSvc func(svc *core.Service) // given a basic valid service, each test case can customize it + numErrs int + legacyIPs bool + newTrafficDist bool + relaxedServiceNames bool }{{ name: "default", tweakSvc: func(s *core.Service) {}, @@ -16764,12 +16765,28 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.TrafficDistribution = ptr.To("PreferSameNode") }, numErrs: 1, + }, { + + name: "valid: service name begins with a digit feature gate enabled", + relaxedServiceNames: true, + tweakSvc: func(s *core.Service) { + s.Name = "1-test-service" + }, + numErrs: 0, + }, { + name: "invalid: service name begins with a digit feature gate disabled", + relaxedServiceNames: false, + tweakSvc: func(s *core.Service) { + s.Name = "1-test-service" + }, + numErrs: 1, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PreferSameTrafficDistribution, tc.newTrafficDist) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, tc.relaxedServiceNames) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !tc.legacyIPs) svc := makeValidService() tc.tweakSvc(&svc) @@ -18214,9 +18231,10 @@ func TestValidateServiceUpdate(t *testing.T) { preferDualStack := core.IPFamilyPolicyPreferDualStack singleStack := core.IPFamilyPolicySingleStack testCases := []struct { - name string - tweakSvc func(oldSvc, newSvc *core.Service) // given basic valid services, each test case can customize them - numErrs int + name string + tweakSvc func(oldSvc, newSvc *core.Service) // given basic valid services, each test case can customize them + numErrs int + relaxedServiceNames bool }{{ name: "no change", tweakSvc: func(oldSvc, newSvc *core.Service) { @@ -19477,12 +19495,22 @@ func TestValidateServiceUpdate(t *testing.T) { newSvc.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "010.0.0.0/8, 1.2.3.0/24" }, numErrs: 1, + }, { + name: "can modify a pre-existing relaxed service name without error", + tweakSvc: func(oldSvc, newSvc *core.Service) { + oldSvc.Name = "1-test-service" + newSvc.Name = "1-test-service" + newSvc.Labels["foo"] = "bar" + }, + relaxedServiceNames: false, + numErrs: 0, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, tc.relaxedServiceNames) oldSvc := makeValidService() newSvc := makeValidService() From 19e7e38af29c20b274e691104d98f3c307e74370 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:58:31 +0200 Subject: [PATCH 5/7] Add unit test for Ingress service ref relaxed validation Test the behaviour of feature gate RelaxedServiceNameValidation. --- .../networking/validation/validation_test.go | 298 +++++++++++++++++- 1 file changed, 294 insertions(+), 4 deletions(-) diff --git a/pkg/apis/networking/validation/validation_test.go b/pkg/apis/networking/validation/validation_test.go index c2b807995cb..c1427db436a 100644 --- a/pkg/apis/networking/validation/validation_test.go +++ b/pkg/apis/networking/validation/validation_test.go @@ -1016,8 +1016,9 @@ func TestValidateIngressCreate(t *testing.T) { } testCases := map[string]struct { - tweakIngress func(ingress *networking.Ingress) - expectedErrs field.ErrorList + tweakIngress func(ingress *networking.Ingress) + expectedErrs field.ErrorList + relaxedServiceName bool }{ "class field set": { tweakIngress: func(ingress *networking.Ingress) { @@ -1150,10 +1151,76 @@ func TestValidateIngressCreate(t *testing.T) { }, expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("rules").Index(0).Child("http").Child("paths").Index(0).Child("path"), "foo", `must be an absolute path`)}, }, + "create service name with RelaxedServiceNameValidation feature gate enabled": { + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.Rules = []networking.IngressRule{{ + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &exactPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + relaxedServiceName: true, + }, + "create default service name with RelaxedServiceNameValidation feature gate enabled": { + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.DefaultBackend = &networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + } + }, + relaxedServiceName: true, + }, + "create service name with RelaxedServiceNameValidation feature gate disabled": { + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.Rules = []networking.IngressRule{{ + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &exactPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("rules").Index(0).Child("http").Child("paths").Index(0).Child("backend").Child("service").Child("name"), "1-test-service", `a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')`)}, + }, + "create default service name with RelaxedServiceNameValidation feature gate disabled": { + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.DefaultBackend = &networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-default-backend", + Port: networking.ServiceBackendPort{Number: 80}, + }, + } + }, + expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("defaultBackend").Child("service").Child("name"), "1-test-default-backend", `a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')`)}, + }, } for name, testCase := range testCases { t.Run(name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, testCase.relaxedServiceName) + newIngress := baseIngress.DeepCopy() testCase.tweakIngress(newIngress) errs := ValidateIngressCreate(newIngress) @@ -1199,8 +1266,9 @@ func TestValidateIngressUpdate(t *testing.T) { } testCases := map[string]struct { - tweakIngresses func(newIngress, oldIngress *networking.Ingress) - expectedErrs field.ErrorList + tweakIngresses func(newIngress, oldIngress *networking.Ingress) + expectedErrs field.ErrorList + relaxedServiceName bool }{ "class field set": { tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { @@ -1556,6 +1624,160 @@ func TestValidateIngressUpdate(t *testing.T) { }} }, }, + "update service to conform to relaxed service name - RelaxedServiceNameValidation disabled": { + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + newIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("rules").Index(0).Child("http").Child("paths").Index(0).Child("backend").Child("service").Child("name"), "1-test-service", `a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')`)}, + }, + "update service to conform to relaxed service name - RelaxedServiceNameValidation enabled": { + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + newIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/foo", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + relaxedServiceName: true, + }, + "updating an already existing relaxed validation service name with RelaxedServiceNameValidation disabled": { + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + newIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "2-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + }, + "updating an already existing relaxed validation service name to a non-relaxed name with RelaxedServiceNameValidation disabled": { + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "1-test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + newIngress.Spec.Rules = []networking.IngressRule{{ + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Path: "/", + PathType: &implementationPathType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "test-service", + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + }} + }, + }, } for name, testCase := range testCases { @@ -1563,6 +1785,7 @@ func TestValidateIngressUpdate(t *testing.T) { newIngress := baseIngress.DeepCopy() oldIngress := baseIngress.DeepCopy() testCase.tweakIngresses(newIngress, oldIngress) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, testCase.relaxedServiceName) errs := ValidateIngressUpdate(newIngress, oldIngress) @@ -2647,3 +2870,70 @@ func TestValidateServiceCIDRUpdate(t *testing.T) { }) } } + +func TestAllowRelaxedServiceNameValidation(t *testing.T) { + basicIngress := func(serviceNames ...string) *networking.Ingress { + if len(serviceNames) == 0 { + return &networking.Ingress{Spec: networking.IngressSpec{Rules: nil}} + } + rules := make([]networking.IngressRule, len(serviceNames)) + for i, name := range serviceNames { + rules[i] = networking.IngressRule{ + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{{ + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: name, + Port: networking.ServiceBackendPort{Number: 80}, + }, + }, + }}, + }, + }, + } + } + return &networking.Ingress{Spec: networking.IngressSpec{Rules: rules}} + } + + tests := []struct { + name string + ingress *networking.Ingress + expect bool + }{ + { + name: "nil ingress", + ingress: nil, + expect: false, + }, + { + name: "no rules", + ingress: basicIngress(), + expect: false, + }, + { + name: "service name is valid DNS1035 and DNS1123", + ingress: basicIngress("validname"), + expect: false, + }, + { + name: "service name is valid DNS1123 but not DNS1035 (contains dash, starts with digit)", + ingress: basicIngress("1abc-def"), + expect: true, + }, + { + name: "multiple rules, one triggers relaxed validation", + ingress: basicIngress("validname", "1abc-def"), + expect: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := allowRelaxedServiceNameValidation(tc.ingress) + if got != tc.expect { + t.Errorf("allowRelaxedServiceNameValidation() = %v, want %v", got, tc.expect) + } + }) + } +} From 97c1974e9c27cf0a88328ec16eb7be0b632891af Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 20:59:07 +0200 Subject: [PATCH 6/7] Add integration test for RelaxedServiceNameValidation. To test enabling and disabling of RelaxedServiceNameValidation feature gate --- test/integration/service/service_test.go | 92 ++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/test/integration/service/service_test.go b/test/integration/service/service_test.go index e7ec010a852..04b11f9f7ee 100644 --- a/test/integration/service/service_test.go +++ b/test/integration/service/service_test.go @@ -1215,3 +1215,95 @@ func Test_ServiceWatchUntil(t *testing.T) { } t.Logf("Service %s deleted", testSvcName) } + +func Test_ServiceValidation_FeatureGateEnableDisable(t *testing.T) { + + //////////////////////////////////////////////////////////////////////////// + // Start kube-apiserver with RelaxedServiceNameValidation feature-gate + // enabled. + //////////////////////////////////////////////////////////////////////////// + + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, true) + + sharedEtcd := framework.SharedEtcd() + server1 := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), sharedEtcd) + + client1, err := clientset.NewForConfig(server1.ClientConfig) + if err != nil { + t.Fatalf("Error creating clientset: %v", err) + } + + //////////////////////////////////////////////////////////////////////////// + // Create services with names that start with a digit and a letter. + // + // Assert that the services are created successfully with the feature gate enabled + //////////////////////////////////////////////////////////////////////////// + + ns := framework.CreateNamespaceOrDie(client1, "test-service-traffic-distribution", t) + makeService := func(serviceName string) *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceName, + Namespace: ns.GetName(), + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 443}}, + }, + } + } + + // Expected to pass, as the feature gate is enabled + _, err = client1.CoreV1().Services(ns.Name).Create(t.Context(), makeService("test-service-1"), metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create test service: %v", err) + } + + // Expected to pass, as the feature gate is enabled + _, err = client1.CoreV1().Services(ns.Name).Create(t.Context(), makeService("9-test-service-1"), metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Successfully created service, but shouldn't have: %v", err) + } + + //////////////////////////////////////////////////////////////////////////// + // Restart the kube-apiserver with RelaxedServiceNameValidation feature-gate + // disabled. + // + // Assert that the services are created using previous validation only + //////////////////////////////////////////////////////////////////////////// + + server1.TearDownFn() + featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.34")) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RelaxedServiceNameValidation, false) + + server2 := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), sharedEtcd) + client2, err := clientset.NewForConfig(server2.ClientConfig) + if err != nil { + t.Fatalf("Error creating clientset: %v", err) + } + + // Expected to pass, as the feature gate is disabled + _, err = client2.CoreV1().Services(ns.Name).Create(t.Context(), makeService("test-service-2"), metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create test service: %v", err) + } + + // Expected to fail, as the feature gate is disabled and this name requires relaxed validation + _, err = client2.CoreV1().Services(ns.Name).Create(t.Context(), makeService("9-test-service-2"), metav1.CreateOptions{}) + if err == nil { + t.Fatalf("Successfully created service, but shouldn't have: %v", err) + } + + //////////////////////////////////////////////////////////////////////////// + // Assert that the services created prior to the feature gate being disabled + // can still be patched successfully even though it requires relaxed validation. + //////////////////////////////////////////////////////////////////////////// + + // Expected to pass as the service was created before the feature gate was disabled + patch := []byte(`{"spec":{"selector":{"foo":"baz"}}}`) + _, err = client2.CoreV1().Services(ns.Name).Patch(t.Context(), "9-test-service-1", types.StrategicMergePatchType, patch, metav1.PatchOptions{}) + if err != nil { + t.Fatalf("Failed to patch selector of service '9-test-service-1': %v", err) + } + + server2.TearDownFn() +} From 22138ef552b9e380079739daf59bf99fee75f3aa Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Sat, 14 Jun 2025 21:00:06 +0200 Subject: [PATCH 7/7] Add DNS e2e test of NameIsDNSLabel validated Service names --- test/e2e/network/dns.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/e2e/network/dns.go b/test/e2e/network/dns.go index c080c9febab..9a87ba4c065 100644 --- a/test/e2e/network/dns.go +++ b/test/e2e/network/dns.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/e2e/framework" e2enode "k8s.io/kubernetes/test/e2e/framework/node" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" @@ -654,6 +655,35 @@ var _ = common.SIGDescribe("DNS", func() { } validateDNSResults(ctx, f, pod, append(agnhostFileNames, jessieFileNames...)) }) + + framework.It("should work with a service name that starts with a digit", framework.WithFeatureGate(features.RelaxedServiceNameValidation), func(ctx context.Context) { + svcName := "1kubernetes" + svc := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{Port: 443}}, + }, + } + + createServiceReportErr(ctx, f.ClientSet, f.Namespace.Name, &svc) + namesToResolve := []string{ + fmt.Sprintf("%s.%s.svc.%s", svcName, f.Namespace.Name, framework.TestContext.ClusterDNSDomain), + } + + agnhostProbeCmd, agnhostFileNames := createProbeCommand(namesToResolve, nil, "", "agnhost", f.Namespace.Name, framework.TestContext.ClusterDNSDomain, framework.TestContext.ClusterIsIPv6()) + agnhostProber := dnsQuerier{name: "agnhost", image: imageutils.Agnhost, cmd: agnhostProbeCmd} + jessieProbeCmd, jessieFileNames := createProbeCommand(namesToResolve, nil, "", "jessie", f.Namespace.Name, framework.TestContext.ClusterDNSDomain, framework.TestContext.ClusterIsIPv6()) + jessieProber := dnsQuerier{name: "jessie", image: imageutils.JessieDnsutils, cmd: jessieProbeCmd} + ginkgo.By("Running these commands on agnhost: " + agnhostProbeCmd + "\n") + ginkgo.By("Running these commands on jessie: " + jessieProbeCmd + "\n") + + // Run a pod which probes DNS and exposes the results by HTTP. + ginkgo.By("creating a pod to probe DNS") + pod := createDNSPod(f.Namespace.Name, []dnsQuerier{agnhostProber, jessieProber}, dnsTestPodHostName, dnsTestServiceName) + validateDNSResults(ctx, f, pod, append(agnhostFileNames, jessieFileNames...)) + }) }) var _ = common.SIGDescribe("DNS HostNetwork", func() {