Skip to content

Commit e648dec

Browse files
MiciahMiciah
committed
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 60f4fbf 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: [email protected]
1 parent 6816854 commit e648dec

File tree

2 files changed

+237
-9
lines changed

2 files changed

+237
-9
lines changed

‎pkg/apis/networking/validation/validation.go‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,9 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain
201201
type IngressValidationOptions struct {
202202
// AllowInvalidSecretName indicates whether spec.tls[*].secretName values that are not valid Secret names should be allowed
203203
AllowInvalidSecretName bool
204+
205+
// AllowInvalidWildcardHostRule indicates whether invalid rule values are allowed in rules with wildcard hostnames
206+
AllowInvalidWildcardHostRule bool
204207
}
205208

206209
// ValidateIngress validates Ingresses on create and update.
@@ -215,7 +218,8 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
215218
allErrs := field.ErrorList{}
216219
var opts IngressValidationOptions
217220
opts = IngressValidationOptions{
218-
AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil),
221+
AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil),
222+
AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(requestGV, nil),
219223
}
220224
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
221225
annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass]
@@ -231,7 +235,8 @@ func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV sc
231235
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
232236
var opts IngressValidationOptions
233237
opts = IngressValidationOptions{
234-
AllowInvalidSecretName: allowInvalidSecretName(requestGV, oldIngress),
238+
AllowInvalidSecretName: allowInvalidSecretName(requestGV, oldIngress),
239+
AllowInvalidWildcardHostRule: allowInvalidWildcardHostRule(requestGV, oldIngress),
235240
}
236241

237242
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
@@ -313,6 +318,7 @@ func validateIngressRules(ingressRules []networking.IngressRule, fldPath *field.
313318
return append(allErrs, field.Required(fldPath, ""))
314319
}
315320
for i, ih := range ingressRules {
321+
wildcardHost := false
316322
if len(ih.Host) > 0 {
317323
if isIP := (net.ParseIP(ih.Host) != nil); isIP {
318324
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.
323329
for _, msg := range validation.IsWildcardDNS1123Subdomain(ih.Host) {
324330
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
325331
}
326-
continue
327-
}
328-
for _, msg := range validation.IsDNS1123Subdomain(ih.Host) {
329-
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
332+
wildcardHost = true
333+
} else {
334+
for _, msg := range validation.IsDNS1123Subdomain(ih.Host) {
335+
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
336+
}
330337
}
331338
}
332-
allErrs = append(allErrs, validateIngressRuleValue(&ih.IngressRuleValue, fldPath.Index(0), opts, requestGV)...)
339+
340+
if !wildcardHost || !opts.AllowInvalidWildcardHostRule {
341+
allErrs = append(allErrs, validateIngressRuleValue(&ih.IngressRuleValue, fldPath.Index(i), opts, requestGV)...)
342+
}
333343
}
334344
return allErrs
335345
}
@@ -558,3 +568,19 @@ func validateTLSSecretName(name string) []string {
558568
}
559569
return apivalidation.ValidateSecretName(name, false)
560570
}
571+
572+
func allowInvalidWildcardHostRule(gv schema.GroupVersion, oldIngress *networking.Ingress) bool {
573+
if gv == networkingv1beta1.SchemeGroupVersion || gv == extensionsv1beta1.SchemeGroupVersion {
574+
// backwards compatibility with released API versions that allowed invalid rules
575+
return true
576+
}
577+
if oldIngress != nil {
578+
for _, rule := range oldIngress.Spec.Rules {
579+
if strings.Contains(rule.Host, "*") && len(validateIngressRuleValue(&rule.IngressRuleValue, nil, IngressValidationOptions{}, gv)) > 0 {
580+
// backwards compatibility with existing invalid data
581+
return true
582+
}
583+
}
584+
}
585+
return false
586+
}

‎pkg/apis/networking/validation/validation_test.go‎

Lines changed: 204 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,7 @@ func TestValidateIngressRuleValue(t *testing.T) {
12701270

12711271
func TestValidateIngressCreate(t *testing.T) {
12721272
implementationPathType := networking.PathTypeImplementationSpecific
1273+
exactPathType := networking.PathTypeExact
12731274
serviceBackend := &networking.IngressServiceBackend{
12741275
Name: "defaultbackend",
12751276
Port: networking.ServiceBackendPort{
@@ -1404,6 +1405,79 @@ func TestValidateIngressCreate(t *testing.T) {
14041405
ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}}
14051406
},
14061407
},
1408+
"v1: valid rules with wildcard host": {
1409+
groupVersion: &networkingv1.SchemeGroupVersion,
1410+
tweakIngress: func(ingress *networking.Ingress) {
1411+
ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1412+
ingress.Spec.Rules = []networking.IngressRule{{
1413+
Host: "*.foo.com",
1414+
IngressRuleValue: networking.IngressRuleValue{
1415+
HTTP: &networking.HTTPIngressRuleValue{
1416+
Paths: []networking.HTTPIngressPath{{
1417+
Path: "/foo",
1418+
PathType: &exactPathType,
1419+
Backend: defaultBackend,
1420+
}},
1421+
},
1422+
},
1423+
}}
1424+
},
1425+
},
1426+
"v1: invalid rules with wildcard host": {
1427+
groupVersion: &networkingv1.SchemeGroupVersion,
1428+
tweakIngress: func(ingress *networking.Ingress) {
1429+
ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1430+
ingress.Spec.Rules = []networking.IngressRule{{
1431+
Host: "*.foo.com",
1432+
IngressRuleValue: networking.IngressRuleValue{
1433+
HTTP: &networking.HTTPIngressRuleValue{
1434+
Paths: []networking.HTTPIngressPath{{
1435+
Path: "foo",
1436+
PathType: &exactPathType,
1437+
Backend: defaultBackend,
1438+
}},
1439+
},
1440+
},
1441+
}}
1442+
},
1443+
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`)},
1444+
},
1445+
"v1beta1: valid rules with wildcard host": {
1446+
groupVersion: &networkingv1beta1.SchemeGroupVersion,
1447+
tweakIngress: func(ingress *networking.Ingress) {
1448+
ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1449+
ingress.Spec.Rules = []networking.IngressRule{{
1450+
Host: "*.foo.com",
1451+
IngressRuleValue: networking.IngressRuleValue{
1452+
HTTP: &networking.HTTPIngressRuleValue{
1453+
Paths: []networking.HTTPIngressPath{{
1454+
Path: "/foo",
1455+
PathType: &exactPathType,
1456+
Backend: defaultBackend,
1457+
}},
1458+
},
1459+
},
1460+
}}
1461+
},
1462+
},
1463+
"v1beta1: invalid rules with wildcard host": {
1464+
groupVersion: &networkingv1beta1.SchemeGroupVersion,
1465+
tweakIngress: func(ingress *networking.Ingress) {
1466+
ingress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1467+
ingress.Spec.Rules = []networking.IngressRule{{
1468+
Host: "*.foo.com",
1469+
IngressRuleValue: networking.IngressRuleValue{
1470+
HTTP: &networking.HTTPIngressRuleValue{
1471+
Paths: []networking.HTTPIngressPath{{
1472+
Path: "foo",
1473+
PathType: &exactPathType,
1474+
Backend: defaultBackend,
1475+
}},
1476+
},
1477+
},
1478+
}}
1479+
},
1480+
},
14071481
}
14081482

14091483
for name, testCase := range testCases {
@@ -1430,6 +1504,7 @@ func TestValidateIngressCreate(t *testing.T) {
14301504

14311505
func TestValidateIngressUpdate(t *testing.T) {
14321506
implementationPathType := networking.PathTypeImplementationSpecific
1507+
exactPathType := networking.PathTypeExact
14331508
serviceBackend := &networking.IngressServiceBackend{
14341509
Name: "defaultbackend",
14351510
Port: networking.ServiceBackendPort{
@@ -1769,6 +1844,131 @@ func TestValidateIngressUpdate(t *testing.T) {
17691844
newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}}
17701845
},
17711846
},
1847+
"v1: change valid rules with wildcard host -> invalid rules": {
1848+
gv: networkingv1.SchemeGroupVersion,
1849+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1850+
oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1851+
oldIngress.Spec.Rules = []networking.IngressRule{{
1852+
Host: "*.foo.com",
1853+
IngressRuleValue: networking.IngressRuleValue{
1854+
HTTP: &networking.HTTPIngressRuleValue{
1855+
Paths: []networking.HTTPIngressPath{{
1856+
Path: "/foo",
1857+
PathType: &exactPathType,
1858+
Backend: defaultBackend,
1859+
}},
1860+
},
1861+
},
1862+
}}
1863+
newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1864+
newIngress.Spec.Rules = []networking.IngressRule{{
1865+
Host: "*.foo.com",
1866+
IngressRuleValue: networking.IngressRuleValue{
1867+
HTTP: &networking.HTTPIngressRuleValue{
1868+
Paths: []networking.HTTPIngressPath{{
1869+
Path: "foo",
1870+
PathType: &exactPathType,
1871+
Backend: defaultBackend,
1872+
}},
1873+
},
1874+
},
1875+
}}
1876+
},
1877+
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`)},
1878+
},
1879+
"v1: change invalid rules with wildcard host -> invalid rules": {
1880+
gv: networkingv1.SchemeGroupVersion,
1881+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1882+
oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1883+
oldIngress.Spec.Rules = []networking.IngressRule{{
1884+
Host: "*.foo.com",
1885+
IngressRuleValue: networking.IngressRuleValue{
1886+
HTTP: &networking.HTTPIngressRuleValue{
1887+
Paths: []networking.HTTPIngressPath{{
1888+
Path: "foo",
1889+
PathType: &exactPathType,
1890+
Backend: defaultBackend,
1891+
}},
1892+
},
1893+
},
1894+
}}
1895+
newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1896+
newIngress.Spec.Rules = []networking.IngressRule{{
1897+
Host: "*.foo.com",
1898+
IngressRuleValue: networking.IngressRuleValue{
1899+
HTTP: &networking.HTTPIngressRuleValue{
1900+
Paths: []networking.HTTPIngressPath{{
1901+
Path: "bar",
1902+
PathType: &exactPathType,
1903+
Backend: defaultBackend,
1904+
}},
1905+
},
1906+
},
1907+
}}
1908+
},
1909+
},
1910+
"v1beta1: change valid rules with wildcard host -> invalid rules": {
1911+
gv: networkingv1beta1.SchemeGroupVersion,
1912+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1913+
oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1914+
oldIngress.Spec.Rules = []networking.IngressRule{{
1915+
Host: "*.foo.com",
1916+
IngressRuleValue: networking.IngressRuleValue{
1917+
HTTP: &networking.HTTPIngressRuleValue{
1918+
Paths: []networking.HTTPIngressPath{{
1919+
Path: "/foo",
1920+
PathType: &exactPathType,
1921+
Backend: defaultBackend,
1922+
}},
1923+
},
1924+
},
1925+
}}
1926+
newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1927+
newIngress.Spec.Rules = []networking.IngressRule{{
1928+
Host: "*.foo.com",
1929+
IngressRuleValue: networking.IngressRuleValue{
1930+
HTTP: &networking.HTTPIngressRuleValue{
1931+
Paths: []networking.HTTPIngressPath{{
1932+
Path: "foo",
1933+
PathType: &exactPathType,
1934+
Backend: defaultBackend,
1935+
}},
1936+
},
1937+
},
1938+
}}
1939+
},
1940+
},
1941+
"v1beta1: change invalid rules with wildcard host -> invalid rules": {
1942+
gv: networkingv1beta1.SchemeGroupVersion,
1943+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1944+
oldIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1945+
oldIngress.Spec.Rules = []networking.IngressRule{{
1946+
Host: "*.foo.com",
1947+
IngressRuleValue: networking.IngressRuleValue{
1948+
HTTP: &networking.HTTPIngressRuleValue{
1949+
Paths: []networking.HTTPIngressPath{{
1950+
Path: "foo",
1951+
PathType: &exactPathType,
1952+
Backend: defaultBackend,
1953+
}},
1954+
},
1955+
},
1956+
}}
1957+
newIngress.Spec.TLS = []networking.IngressTLS{{Hosts: []string{"*.bar.com"}}}
1958+
newIngress.Spec.Rules = []networking.IngressRule{{
1959+
Host: "*.foo.com",
1960+
IngressRuleValue: networking.IngressRuleValue{
1961+
HTTP: &networking.HTTPIngressRuleValue{
1962+
Paths: []networking.HTTPIngressPath{{
1963+
Path: "bar",
1964+
PathType: &exactPathType,
1965+
Backend: defaultBackend,
1966+
}},
1967+
},
1968+
},
1969+
}}
1970+
},
1971+
},
17721972
}
17731973

17741974
for name, testCase := range testCases {
@@ -1984,6 +2184,7 @@ func TestValidateIngressClassUpdate(t *testing.T) {
19842184
}
19852185

19862186
func TestValidateIngressTLS(t *testing.T) {
2187+
pathTypeImplementationSpecific := networking.PathTypeImplementationSpecific
19872188
serviceBackend := &networking.IngressServiceBackend{
19882189
Name: "defaultbackend",
19892190
Port: networking.ServiceBackendPort{
@@ -2008,8 +2209,9 @@ func TestValidateIngressTLS(t *testing.T) {
20082209
HTTP: &networking.HTTPIngressRuleValue{
20092210
Paths: []networking.HTTPIngressPath{
20102211
{
2011-
Path: "/foo",
2012-
Backend: defaultBackend,
2212+
Path: "/foo",
2213+
PathType: &pathTypeImplementationSpecific,
2214+
Backend: defaultBackend,
20132215
},
20142216
},
20152217
},

0 commit comments

Comments
 (0)