diff --git a/pkg/apis/networking/validation/validation.go b/pkg/apis/networking/validation/validation.go index 7508bd4851f..f3a5e82ad96 100644 --- a/pkg/apis/networking/validation/validation.go +++ b/pkg/apis/networking/validation/validation.go @@ -199,6 +199,8 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain // IngressValidationOptions cover beta to GA transitions for HTTP PathType type IngressValidationOptions struct { + // AllowInvalidSecretName indicates whether spec.tls[*].secretName values that are not valid Secret names should be allowed + AllowInvalidSecretName bool } // ValidateIngress validates Ingresses on create and update. @@ -212,7 +214,9 @@ func validateIngress(ingress *networking.Ingress, opts IngressValidationOptions, func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList { allErrs := field.ErrorList{} var opts IngressValidationOptions - opts = IngressValidationOptions{} + opts = IngressValidationOptions{ + AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil), + } allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...) annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass] if annotationIsSet && ingress.Spec.IngressClassName != nil { @@ -226,26 +230,34 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList { allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata")) var opts IngressValidationOptions - opts = IngressValidationOptions{} + opts = IngressValidationOptions{ + AllowInvalidSecretName: allowInvalidSecretName(requestGV, oldIngress), + } allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...) return allErrs } -func validateIngressTLS(spec *networking.IngressSpec, fldPath *field.Path) field.ErrorList { +func validateIngressTLS(spec *networking.IngressSpec, fldPath *field.Path, opts IngressValidationOptions) field.ErrorList { allErrs := field.ErrorList{} // TODO: Perform a more thorough validation of spec.TLS.Hosts that takes // the wildcard spec from RFC 6125 into account. - for _, itls := range spec.TLS { + for tlsIndex, itls := range spec.TLS { for i, host := range itls.Hosts { if strings.Contains(host, "*") { for _, msg := range validation.IsWildcardDNS1123Subdomain(host) { - allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("hosts"), host, msg)) + allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("hosts").Index(i), host, msg)) } continue } for _, msg := range validation.IsDNS1123Subdomain(host) { - allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("hosts"), host, msg)) + allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("hosts").Index(i), host, msg)) + } + } + + if !opts.AllowInvalidSecretName { + for _, msg := range validateTLSSecretName(itls.SecretName) { + allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("secretName"), itls.SecretName, msg)) } } } @@ -278,7 +290,7 @@ func ValidateIngressSpec(spec *networking.IngressSpec, fldPath *field.Path, opts allErrs = append(allErrs, validateIngressRules(spec.Rules, fldPath.Child("rules"), opts, requestGV)...) } if len(spec.TLS) > 0 { - allErrs = append(allErrs, validateIngressTLS(spec, fldPath.Child("tls"))...) + allErrs = append(allErrs, validateIngressTLS(spec, fldPath.Child("tls"), opts)...) } if spec.IngressClassName != nil { for _, msg := range ValidateIngressClassName(*spec.IngressClassName, false) { @@ -523,3 +535,26 @@ func validateIngressTypedLocalObjectReference(params *api.TypedLocalObjectRefere return allErrs } + +func allowInvalidSecretName(gv schema.GroupVersion, oldIngress *networking.Ingress) bool { + if gv == networkingv1beta1.SchemeGroupVersion || gv == extensionsv1beta1.SchemeGroupVersion { + // backwards compatibility with released API versions that allowed invalid names + return true + } + if oldIngress != nil { + for _, tls := range oldIngress.Spec.TLS { + if len(validateTLSSecretName(tls.SecretName)) > 0 { + // backwards compatibility with existing persisted object + return true + } + } + } + return false +} + +func validateTLSSecretName(name string) []string { + if len(name) == 0 { + return nil + } + return apivalidation.ValidateSecretName(name, false) +} diff --git a/pkg/apis/networking/validation/validation_test.go b/pkg/apis/networking/validation/validation_test.go index 104ed5ffa6f..afbb0ccd349 100644 --- a/pkg/apis/networking/validation/validation_test.go +++ b/pkg/apis/networking/validation/validation_test.go @@ -1379,6 +1379,31 @@ func TestValidateIngressCreate(t *testing.T) { }, expectedErrs: field.ErrorList{}, }, + "v1: valid secret": { + groupVersion: &networkingv1.SchemeGroupVersion, + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}} + }, + }, + "v1: invalid secret": { + groupVersion: &networkingv1.SchemeGroupVersion, + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}} + }, + expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("tls").Index(0).Child("secretName"), "invalid name", `a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`)}, + }, + "v1beta1: valid secret": { + groupVersion: &networkingv1beta1.SchemeGroupVersion, + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}} + }, + }, + "v1beta1: invalid secret": { + groupVersion: &networkingv1beta1.SchemeGroupVersion, + tweakIngress: func(ingress *networking.Ingress) { + ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}} + }, + }, } for name, testCase := range testCases { @@ -1431,6 +1456,7 @@ func TestValidateIngressUpdate(t *testing.T) { } testCases := map[string]struct { + gv schema.GroupVersion tweakIngresses func(newIngress, oldIngress *networking.Ingress) expectedErrs field.ErrorList }{ @@ -1714,6 +1740,35 @@ func TestValidateIngressUpdate(t *testing.T) { }, expectedErrs: field.ErrorList{}, }, + "v1: change valid secret -> invalid secret": { + gv: networkingv1.SchemeGroupVersion, + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}} + newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}} + }, + expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("tls").Index(0).Child("secretName"), "invalid name", `a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`)}, + }, + "v1: change invalid secret -> invalid secret": { + gv: networkingv1.SchemeGroupVersion, + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}} + newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}} + }, + }, + "v1beta1: change valid secret -> invalid secret": { + gv: networkingv1beta1.SchemeGroupVersion, + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}} + newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}} + }, + }, + "v1beta1: change invalid secret -> invalid secret": { + gv: networkingv1beta1.SchemeGroupVersion, + tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { + oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}} + newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}} + }, + }, } for name, testCase := range testCases { @@ -1722,7 +1777,11 @@ func TestValidateIngressUpdate(t *testing.T) { oldIngress := baseIngress.DeepCopy() testCase.tweakIngresses(newIngress, oldIngress) - errs := ValidateIngressUpdate(newIngress, oldIngress, networkingv1beta1.SchemeGroupVersion) + gv := testCase.gv + if gv.Empty() { + gv = networkingv1beta1.SchemeGroupVersion + } + errs := ValidateIngressUpdate(newIngress, oldIngress, gv) if len(errs) != len(testCase.expectedErrs) { t.Fatalf("Expected %d errors, got %d (%+v)", len(testCase.expectedErrs), len(errs), errs) @@ -1978,7 +2037,7 @@ func TestValidateIngressTLS(t *testing.T) { Hosts: []string{wildcardHost}, }, } - badWildcardTLSErr := fmt.Sprintf("spec.tls[0].hosts: Invalid value: '%v'", wildcardHost) + badWildcardTLSErr := fmt.Sprintf("spec.tls[0].hosts[0]: Invalid value: '%v'", wildcardHost) errorCases[badWildcardTLSErr] = badWildcardTLS for k, v := range errorCases { diff --git a/pkg/registry/networking/ingress/storage/storage_test.go b/pkg/registry/networking/ingress/storage/storage_test.go index dda9220ce01..000f3cad4a7 100644 --- a/pkg/registry/networking/ingress/storage/storage_test.go +++ b/pkg/registry/networking/ingress/storage/storage_test.go @@ -60,7 +60,7 @@ var ( defaultPathType = networking.PathTypeImplementationSpecific defaultPathMap = map[string]string{defaultPath: defaultBackendName} defaultTLS = []networking.IngressTLS{ - {Hosts: []string{"foo.bar.com", "*.bar.com"}, SecretName: "fooSecret"}, + {Hosts: []string{"foo.bar.com", "*.bar.com"}, SecretName: "foosecret"}, } serviceBackend = &networking.IngressServiceBackend{ Name: "defaultbackend", @@ -173,7 +173,7 @@ func TestUpdate(t *testing.T) { }) object.Spec.TLS = append(object.Spec.TLS, networking.IngressTLS{ Hosts: []string{"*.google.com"}, - SecretName: "googleSecret", + SecretName: "googlesecret", }) return object },