Fix validation of ingress rules with wildcard host

Fix ingress validation so that it validates the rules of an ingress that
specifies a wildcard host.  Commit 60f4fbf4f2
added an inopportune continue statement that caused this validation to be
skipped.  For backwards compatibility, this change restores validation for
v1 of the api but still skips it on v1beta1.

* pkg/apis/networking/validation/validation.go (IngressValidationOptions):
Add AllowInvalidWildcardHostRule field to indicate that validation of rules
should be skipped for ingresses that specify wildcard hosts.
(ValidateIngressCreate): Set AllowInvalidWildcardHostRule to true if the
request is using the v1beta1 API version.
(ValidateIngressUpdate): Set AllowInvalidWildcardHostRule to true if the
request or old ingress is using the v1beta1 API version.
(validateIngressRules): Don't skip validation of the ingress rules unless
the ingress has a wildcard host and AllowInvalidWildcardHostRule is true.
(allowInvalidWildcardHostRule): New helper for ValidateIngressCreate and
ValidateIngressUpdate.
* pkg/apis/networking/validation/validation_test.go
(TestValidateIngressCreate, TestValidateIngressUpdate): Add test cases to
ensure that validation is performed on v1 objects and skipped on v1beta
objects for backwards compatibility.
(TestValidateIngressTLS): Specify PathType so that the test passes.

Co-authored-by: jordan@liggitt.net
This commit is contained in:
Miciah Masters 2020-08-12 20:44:38 -04:00 committed by Miciah Dashiel Butler Masters
parent 68168541ea
commit e648deca3b
2 changed files with 237 additions and 9 deletions

View File

@ -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.
@ -215,7 +218,8 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
allErrs := field.ErrorList{}
var opts IngressValidationOptions
opts = IngressValidationOptions{
AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil),
AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil),
AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(requestGV, nil),
}
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass]
@ -231,7 +235,8 @@ func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV sc
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
var opts IngressValidationOptions
opts = IngressValidationOptions{
AllowInvalidSecretName: allowInvalidSecretName(requestGV, oldIngress),
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
}
for _, msg := range validation.IsDNS1123Subdomain(ih.Host) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
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
}

View File

@ -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{
@ -2008,8 +2209,9 @@ func TestValidateIngressTLS(t *testing.T) {
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/foo",
Backend: defaultBackend,
Path: "/foo",
PathType: &pathTypeImplementationSpecific,
Backend: defaultBackend,
},
},
},