Merge pull request #93954 from Miciah/fix-validation-of-ingress-rules-with-wildcard-host

Fix validation of ingress rules with wildcard host

pkg/apis/networking/validation/validation.go

@@ -201,6 +201,9 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain
 type IngressValidationOptions struct {
 	// AllowInvalidSecretName indicates whether spec.tls[*].secretName values that are not valid Secret names should be allowed
 	AllowInvalidSecretName bool
+
+	// AllowInvalidWildcardHostRule indicates whether invalid rule values are allowed in rules with wildcard hostnames
+	AllowInvalidWildcardHostRule bool
 }
 
 // ValidateIngress validates Ingresses on create and update.
@@ -216,6 +219,7 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
 	var opts IngressValidationOptions
 	opts = IngressValidationOptions{
 		AllowInvalidSecretName:       allowInvalidSecretName(requestGV, nil),
+		AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(requestGV, nil),
 	}
 	allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
 	annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass]
@@ -232,6 +236,7 @@ func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV sc
 	var opts IngressValidationOptions
 	opts = IngressValidationOptions{
 		AllowInvalidSecretName:       allowInvalidSecretName(requestGV, oldIngress),
+		AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(requestGV, oldIngress),
 	}
 
 	allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
@@ -313,6 +318,7 @@ func validateIngressRules(ingressRules []networking.IngressRule, fldPath *field.
 		return append(allErrs, field.Required(fldPath, ""))
 	}
 	for i, ih := range ingressRules {
+		wildcardHost := false
 		if len(ih.Host) > 0 {
 			if isIP := (net.ParseIP(ih.Host) != nil); isIP {
 				allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, "must be a DNS name, not an IP address"))
@@ -323,13 +329,17 @@ func validateIngressRules(ingressRules []networking.IngressRule, fldPath *field.
 				for _, msg := range validation.IsWildcardDNS1123Subdomain(ih.Host) {
 					allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
 				}
-				continue
+				wildcardHost = true
-			}
+			} else {
 				for _, msg := range validation.IsDNS1123Subdomain(ih.Host) {
 					allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
 				}
 			}
-		allErrs = append(allErrs, validateIngressRuleValue(&ih.IngressRuleValue, fldPath.Index(0), opts, requestGV)...)
+		}
+
+		if !wildcardHost || !opts.AllowInvalidWildcardHostRule {
+			allErrs = append(allErrs, validateIngressRuleValue(&ih.IngressRuleValue, fldPath.Index(i), opts, requestGV)...)
+		}
 	}
 	return allErrs
 }
@@ -558,3 +568,19 @@ func validateTLSSecretName(name string) []string {
 	}
 	return apivalidation.ValidateSecretName(name, false)
 }
+
+func allowInvalidWildcardHostRule(gv schema.GroupVersion, oldIngress *networking.Ingress) bool {
+	if gv == networkingv1beta1.SchemeGroupVersion || gv == extensionsv1beta1.SchemeGroupVersion {
+		// backwards compatibility with released API versions that allowed invalid rules
+		return true
+	}
+	if oldIngress != nil {
+		for _, rule := range oldIngress.Spec.Rules {
+			if strings.Contains(rule.Host, "*") && len(validateIngressRuleValue(&rule.IngressRuleValue, nil, IngressValidationOptions{}, gv)) > 0 {
+				// backwards compatibility with existing invalid data
+				return true
+			}
+		}
+	}
+	return false
+}

pkg/apis/networking/validation/validation_test.go

@@ -1270,6 +1270,7 @@ func TestValidateIngressRuleValue(t *testing.T) {
 
 func TestValidateIngressCreate(t *testing.T) {
 	implementationPathType := networking.PathTypeImplementationSpecific
+	exactPathType := networking.PathTypeExact
 	serviceBackend := &networking.IngressServiceBackend{
 		Name: "defaultbackend",
 		Port: networking.ServiceBackendPort{
@@ -1404,6 +1405,79 @@ func TestValidateIngressCreate(t *testing.T) {
 				ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}}
 			},
 		},
+		"v1: valid rules with wildcard host": {
+			groupVersion: &networkingv1.SchemeGroupVersion,
+			tweakIngress: func(ingress *networking.Ingress) {
+				ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				ingress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "/foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
+		"v1: invalid rules with wildcard host": {
+			groupVersion: &networkingv1.SchemeGroupVersion,
+			tweakIngress: func(ingress *networking.Ingress) {
+				ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				ingress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+			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`)},
+		},
+		"v1beta1: valid rules with wildcard host": {
+			groupVersion: &networkingv1beta1.SchemeGroupVersion,
+			tweakIngress: func(ingress *networking.Ingress) {
+				ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				ingress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "/foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
+		"v1beta1: invalid rules with wildcard host": {
+			groupVersion: &networkingv1beta1.SchemeGroupVersion,
+			tweakIngress: func(ingress *networking.Ingress) {
+				ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				ingress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
 	}
 
 	for name, testCase := range testCases {
@@ -1430,6 +1504,7 @@ func TestValidateIngressCreate(t *testing.T) {
 
 func TestValidateIngressUpdate(t *testing.T) {
 	implementationPathType := networking.PathTypeImplementationSpecific
+	exactPathType := networking.PathTypeExact
 	serviceBackend := &networking.IngressServiceBackend{
 		Name: "defaultbackend",
 		Port: networking.ServiceBackendPort{
@@ -1769,6 +1844,131 @@ func TestValidateIngressUpdate(t *testing.T) {
 				newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}}
 			},
 		},
+		"v1: change valid rules with wildcard host -> invalid rules": {
+			gv: networkingv1.SchemeGroupVersion,
+			tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
+				oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				oldIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "/foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+				newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				newIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+			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`)},
+		},
+		"v1: change invalid rules with wildcard host -> invalid rules": {
+			gv: networkingv1.SchemeGroupVersion,
+			tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
+				oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				oldIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+				newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				newIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "bar",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
+		"v1beta1: change valid rules with wildcard host -> invalid rules": {
+			gv: networkingv1beta1.SchemeGroupVersion,
+			tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
+				oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				oldIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "/foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+				newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				newIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
+		"v1beta1: change invalid rules with wildcard host -> invalid rules": {
+			gv: networkingv1beta1.SchemeGroupVersion,
+			tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
+				oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				oldIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "foo",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+				newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
+				newIngress.Spec.Rules = []networking.IngressRule{{
+					Host: "*.foo.com",
+					IngressRuleValue: networking.IngressRuleValue{
+						HTTP: &networking.HTTPIngressRuleValue{
+							Paths: []networking.HTTPIngressPath{{
+								Path:     "bar",
+								PathType: &exactPathType,
+								Backend:  defaultBackend,
+							}},
+						},
+					},
+				}}
+			},
+		},
 	}
 
 	for name, testCase := range testCases {
@@ -1984,6 +2184,7 @@ func TestValidateIngressClassUpdate(t *testing.T) {
 }
 
 func TestValidateIngressTLS(t *testing.T) {
+	pathTypeImplementationSpecific := networking.PathTypeImplementationSpecific
 	serviceBackend := &networking.IngressServiceBackend{
 		Name: "defaultbackend",
 		Port: networking.ServiceBackendPort{
@@ -2009,6 +2210,7 @@ func TestValidateIngressTLS(t *testing.T) {
 								Paths: []networking.HTTPIngressPath{
 									{
 										Path:     "/foo",
+										PathType: &pathTypeImplementationSpecific,
 										Backend:  defaultBackend,
 									},
 								},