Skip to content

Commit 77a86a5

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91034 from liggitt/ingress-validation
Ingress validation
2 parents 592a79c + 1758d17 commit 77a86a5

File tree

3 files changed

+20
-229
lines changed

3 files changed

+20
-229
lines changed

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

Lines changed: 2 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package validation
1919
import (
2020
"fmt"
2121
"net"
22-
"regexp"
2322
"strings"
2423

2524
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
@@ -198,8 +197,6 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain
198197

199198
// IngressValidationOptions cover beta to GA transitions for HTTP PathType
200199
type IngressValidationOptions struct {
201-
requireRegexPath bool
202-
allowResourceBackend bool
203200
}
204201

205202
// ValidateIngress validates Ingresses on create and update.
@@ -213,12 +210,7 @@ func validateIngress(ingress *networking.Ingress, opts IngressValidationOptions,
213210
func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList {
214211
allErrs := field.ErrorList{}
215212
var opts IngressValidationOptions
216-
opts = IngressValidationOptions{
217-
// TODO(robscott): Remove regex validation for 1.19.
218-
requireRegexPath: true,
219-
// TODO(cmluciano): Allow resource backend for 1.19.
220-
allowResourceBackend: false,
221-
}
213+
opts = IngressValidationOptions{}
222214
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
223215
annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass]
224216
if annotationIsSet && ingress.Spec.IngressClassName != nil {
@@ -232,13 +224,7 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
232224
func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList {
233225
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
234226
var opts IngressValidationOptions
235-
opts = IngressValidationOptions{
236-
// TODO(robscott): Remove regex validation for 1.19.
237-
// Only require regex path validation for this Ingress if the previous
238-
// version of the Ingress also passed that validation.
239-
requireRegexPath: allPathsPassRegexValidation(oldIngress),
240-
allowResourceBackend: resourceBackendPresent(oldIngress),
241-
}
227+
opts = IngressValidationOptions{}
242228

243229
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
244230
return allErrs
@@ -377,14 +363,6 @@ func validateHTTPIngressPath(path *networking.HTTPIngressPath, fldPath *field.Pa
377363
allErrs = append(allErrs, field.NotSupported(fldPath.Child("pathType"), *path.PathType, supportedPathTypes.List()))
378364
}
379365

380-
// TODO(robscott): Remove regex validation for 1.19.
381-
if opts.requireRegexPath {
382-
_, err := regexp.CompilePOSIX(path.Path)
383-
if err != nil {
384-
allErrs = append(allErrs, field.Invalid(fldPath.Child("path"), path.Path, "must be a valid regex"))
385-
}
386-
}
387-
388366
allErrs = append(allErrs, validateIngressBackend(&path.Backend, fldPath.Child("backend"), opts)...)
389367
return allErrs
390368
}
@@ -399,8 +377,6 @@ func validateIngressBackend(backend *networking.IngressBackend, fldPath *field.P
399377
switch {
400378
case hasResourceBackend && hasServiceBackend:
401379
return append(allErrs, field.Invalid(fldPath, "", "cannot set both resource and service backends"))
402-
case hasResourceBackend && !opts.allowResourceBackend:
403-
return append(allErrs, field.Forbidden(fldPath.Child("resource"), "not supported; only service backends are supported in this version"))
404380
case hasResourceBackend:
405381
allErrs = append(allErrs, validateIngressTypedLocalObjectReference(backend.Resource, fldPath.Child("resource"))...)
406382
default:
@@ -484,39 +460,3 @@ func validateIngressTypedLocalObjectReference(params *api.TypedLocalObjectRefere
484460

485461
return allErrs
486462
}
487-
488-
// allPathsPassRegexValidation returns true if the Ingress has paths that all
489-
// match the Ingress path validation with requireRegexPath enabled.
490-
func allPathsPassRegexValidation(ingress *networking.Ingress) bool {
491-
for _, rule := range ingress.Spec.Rules {
492-
if rule.HTTP == nil {
493-
continue
494-
}
495-
for _, path := range rule.HTTP.Paths {
496-
if len(path.Path) == 0 {
497-
continue
498-
}
499-
if _, err := regexp.CompilePOSIX(path.Path); err != nil {
500-
return false
501-
}
502-
}
503-
}
504-
return true
505-
}
506-
507-
func resourceBackendPresent(ingress *networking.Ingress) bool {
508-
if ingress.Spec.Backend != nil && ingress.Spec.Backend.Resource != nil {
509-
return true
510-
}
511-
for _, rule := range ingress.Spec.Rules {
512-
if rule.HTTP == nil {
513-
continue
514-
}
515-
for _, path := range rule.HTTP.Paths {
516-
if path.Backend.Resource != nil {
517-
return true
518-
}
519-
}
520-
}
521-
return false
522-
}

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

Lines changed: 16 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ func TestValidateIngress(t *testing.T) {
11221122
if gv == nil {
11231123
gv = &networkingv1.SchemeGroupVersion
11241124
}
1125-
errs := validateIngress(ingress, IngressValidationOptions{requireRegexPath: true}, *gv)
1125+
errs := validateIngress(ingress, IngressValidationOptions{}, *gv)
11261126
if len(testCase.expectErrsOnFields) != len(errs) {
11271127
t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectErrsOnFields), len(errs), errs)
11281128
}
@@ -1138,10 +1138,9 @@ func TestValidateIngress(t *testing.T) {
11381138
func TestValidateIngressRuleValue(t *testing.T) {
11391139
fldPath := field.NewPath("testing.http.paths[0].path")
11401140
testCases := map[string]struct {
1141-
pathType networking.PathType
1142-
path string
1143-
requireRegexPath bool
1144-
expectedErrs field.ErrorList
1141+
pathType networking.PathType
1142+
path string
1143+
expectedErrs field.ErrorList
11451144
}{
11461145
"implementation specific: no leading slash": {
11471146
pathType: networking.PathTypeImplementationSpecific,
@@ -1242,7 +1241,7 @@ func TestValidateIngressRuleValue(t *testing.T) {
12421241
},
12431242
}
12441243

1245-
errs := validateIngressRuleValue(irv, field.NewPath("testing"), IngressValidationOptions{requireRegexPath: true})
1244+
errs := validateIngressRuleValue(irv, field.NewPath("testing"), IngressValidationOptions{})
12461245

12471246
if len(errs) != len(testCase.expectedErrs) {
12481247
t.Fatalf("Expected %d errors, got %d (%+v)", len(testCase.expectedErrs), len(errs), errs)
@@ -1335,16 +1334,16 @@ func TestValidateIngressCreate(t *testing.T) {
13351334
},
13361335
}}
13371336
},
1338-
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec.rules[0].http.paths[0].path"), "/([a-z0-9]*)[", "must be a valid regex")},
1337+
expectedErrs: field.ErrorList{},
13391338
},
1340-
"Spec.Backend.Resource field not allowed on create": {
1339+
"Spec.Backend.Resource field allowed on create": {
13411340
tweakIngress: func(ingress *networking.Ingress) {
13421341
ingress.Spec.Backend = &networking.IngressBackend{
13431342
Resource: resourceBackend}
13441343
},
1345-
expectedErrs: field.ErrorList{field.Forbidden(field.NewPath("spec.backend.resource"), "not supported; only service backends are supported in this version")},
1344+
expectedErrs: field.ErrorList{},
13461345
},
1347-
"Paths.Backend.Resource field not allowed on create": {
1346+
"Paths.Backend.Resource field allowed on create": {
13481347
tweakIngress: func(ingress *networking.Ingress) {
13491348
ingress.Spec.Rules = []networking.IngressRule{{
13501349
Host: "foo.bar.com",
@@ -1360,7 +1359,7 @@ func TestValidateIngressCreate(t *testing.T) {
13601359
},
13611360
}}
13621361
},
1363-
expectedErrs: field.ErrorList{field.Forbidden(field.NewPath("spec.rules[0].http.paths[0].backend.resource"), "not supported; only service backends are supported in this version")},
1362+
expectedErrs: field.ErrorList{},
13641363
},
13651364
}
13661365

@@ -1484,7 +1483,7 @@ func TestValidateIngressUpdate(t *testing.T) {
14841483
},
14851484
}}
14861485
},
1487-
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec.rules[0].http.paths[0].path"), "/bar[", "must be a valid regex")},
1486+
expectedErrs: field.ErrorList{},
14881487
},
14891488
"invalid regex path -> valid regex path": {
14901489
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
@@ -1544,13 +1543,13 @@ func TestValidateIngressUpdate(t *testing.T) {
15441543
},
15451544
expectedErrs: field.ErrorList{},
15461545
},
1547-
"new Backend.Resource not allowed on update": {
1546+
"new Backend.Resource allowed on update": {
15481547
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
15491548
oldIngress.Spec.Backend = &defaultBackend
15501549
newIngress.Spec.Backend = &networking.IngressBackend{
15511550
Resource: resourceBackend}
15521551
},
1553-
expectedErrs: field.ErrorList{field.Forbidden(field.NewPath("spec.backend.resource"), "not supported; only service backends are supported in this version")},
1552+
expectedErrs: field.ErrorList{},
15541553
},
15551554
"old Backend.Resource allowed on update": {
15561555
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
@@ -1687,7 +1686,7 @@ func TestValidateIngressUpdate(t *testing.T) {
16871686
},
16881687
}}
16891688
},
1690-
expectedErrs: field.ErrorList{field.Forbidden(field.NewPath("spec.rules[0].http.paths[0].backend.resource"), "not supported; only service backends are supported in this version")},
1689+
expectedErrs: field.ErrorList{},
16911690
},
16921691
}
16931692

@@ -1956,7 +1955,7 @@ func TestValidateIngressTLS(t *testing.T) {
19561955
errorCases[badWildcardTLSErr] = badWildcardTLS
19571956

19581957
for k, v := range errorCases {
1959-
errs := validateIngress(&v, IngressValidationOptions{requireRegexPath: true}, networkingv1beta1.SchemeGroupVersion)
1958+
errs := validateIngress(&v, IngressValidationOptions{}, networkingv1beta1.SchemeGroupVersion)
19601959
if len(errs) == 0 {
19611960
t.Errorf("expected failure for %q", k)
19621961
} else {
@@ -1980,7 +1979,7 @@ func TestValidateIngressTLS(t *testing.T) {
19801979
}
19811980
validCases[fmt.Sprintf("spec.tls[0].hosts: Valid value: '%v'", wildHost)] = goodWildcardTLS
19821981
for k, v := range validCases {
1983-
errs := validateIngress(&v, IngressValidationOptions{requireRegexPath: true}, networkingv1beta1.SchemeGroupVersion)
1982+
errs := validateIngress(&v, IngressValidationOptions{}, networkingv1beta1.SchemeGroupVersion)
19841983
if len(errs) != 0 {
19851984
t.Errorf("expected success for %q", k)
19861985
}
@@ -2078,151 +2077,3 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
20782077
}
20792078
}
20802079
}
2081-
2082-
func TestValidateResourceBackendPresent(t *testing.T) {
2083-
implementationPathType := networking.PathTypeImplementationSpecific
2084-
defaultBackend := networking.IngressBackend{
2085-
ServiceName: "default-backend",
2086-
ServicePort: intstr.FromInt(80),
2087-
}
2088-
resourceBackend := &api.TypedLocalObjectReference{
2089-
APIGroup: utilpointer.StringPtr("example.com"),
2090-
Kind: "foo",
2091-
Name: "bar",
2092-
}
2093-
baseIngress := networking.Ingress{
2094-
ObjectMeta: metav1.ObjectMeta{
2095-
Name: "foo",
2096-
Namespace: metav1.NamespaceDefault,
2097-
},
2098-
Spec: networking.IngressSpec{
2099-
Backend: &networking.IngressBackend{
2100-
ServiceName: "default-backend",
2101-
ServicePort: intstr.FromInt(80),
2102-
},
2103-
Rules: []networking.IngressRule{
2104-
{
2105-
Host: "foo.bar.com",
2106-
IngressRuleValue: networking.IngressRuleValue{
2107-
HTTP: &networking.HTTPIngressRuleValue{
2108-
Paths: []networking.HTTPIngressPath{
2109-
{
2110-
Path: "/foo",
2111-
PathType: &implementationPathType,
2112-
Backend: defaultBackend,
2113-
},
2114-
},
2115-
},
2116-
},
2117-
},
2118-
},
2119-
},
2120-
Status: networking.IngressStatus{
2121-
LoadBalancer: api.LoadBalancerStatus{
2122-
Ingress: []api.LoadBalancerIngress{
2123-
{IP: "127.0.0.1"},
2124-
},
2125-
},
2126-
},
2127-
}
2128-
testCases := map[string]struct {
2129-
groupVersion *schema.GroupVersion
2130-
tweakIngress func(ing *networking.Ingress)
2131-
expectResourceBackend bool
2132-
}{
2133-
"nil spec.Backend and no paths": {
2134-
tweakIngress: func(ing *networking.Ingress) {
2135-
ing.Spec.Backend = nil
2136-
ing.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].Path = ""
2137-
},
2138-
expectResourceBackend: false,
2139-
},
2140-
"nil spec.Backend.Resource and no paths": {
2141-
tweakIngress: func(ing *networking.Ingress) {
2142-
ing.Spec.Backend = nil
2143-
ing.Spec.Rules[0].IngressRuleValue.HTTP.Paths = []networking.HTTPIngressPath{}
2144-
},
2145-
expectResourceBackend: false,
2146-
},
2147-
"non-nil spec.Backend.Resource and no paths": {
2148-
tweakIngress: func(ing *networking.Ingress) {
2149-
ing.Spec.Backend = &networking.IngressBackend{
2150-
Resource: resourceBackend,
2151-
}
2152-
ing.Spec.Rules[0].IngressRuleValue.HTTP.Paths[0].Path = ""
2153-
},
2154-
expectResourceBackend: true,
2155-
},
2156-
"nil spec.Backend, one rule with nil HTTP ": {
2157-
tweakIngress: func(ing *networking.Ingress) {
2158-
ing.Spec.Backend = nil
2159-
ing.Spec.Rules[0].IngressRuleValue.HTTP = nil
2160-
},
2161-
expectResourceBackend: false,
2162-
},
2163-
"nil spec.Backend, one rule with non-nil HTTP, no paths": {
2164-
tweakIngress: func(ing *networking.Ingress) {
2165-
ing.Spec.Backend = nil
2166-
ing.Spec.Rules[0].IngressRuleValue = networking.IngressRuleValue{
2167-
HTTP: &networking.HTTPIngressRuleValue{
2168-
Paths: []networking.HTTPIngressPath{},
2169-
},
2170-
}
2171-
},
2172-
expectResourceBackend: false,
2173-
},
2174-
"nil spec.Backend, one rule with non-nil HTTP, one path with nil Backend.Resource": {
2175-
tweakIngress: func(ing *networking.Ingress) {
2176-
ing.Spec.Backend = nil
2177-
ing.Spec.Rules[0].IngressRuleValue = networking.IngressRuleValue{
2178-
HTTP: &networking.HTTPIngressRuleValue{
2179-
Paths: []networking.HTTPIngressPath{
2180-
{
2181-
Path: "/foo",
2182-
PathType: &implementationPathType,
2183-
Backend: networking.IngressBackend{
2184-
Resource: nil,
2185-
},
2186-
},
2187-
},
2188-
},
2189-
}
2190-
},
2191-
expectResourceBackend: false,
2192-
},
2193-
"nil spec.Backend, one rule with non-nil HTTP, one path with non-nil Backend.Resource": {
2194-
tweakIngress: func(ing *networking.Ingress) {
2195-
ing.Spec.Backend = nil
2196-
ing.Spec.Rules[0].IngressRuleValue = networking.IngressRuleValue{
2197-
HTTP: &networking.HTTPIngressRuleValue{
2198-
Paths: []networking.HTTPIngressPath{
2199-
{
2200-
Path: "/foo",
2201-
PathType: &implementationPathType,
2202-
Backend: networking.IngressBackend{
2203-
Resource: resourceBackend,
2204-
},
2205-
},
2206-
},
2207-
},
2208-
}
2209-
},
2210-
expectResourceBackend: true,
2211-
},
2212-
}
2213-
2214-
for name, testCase := range testCases {
2215-
t.Run(name, func(t *testing.T) {
2216-
ingress := baseIngress.DeepCopy()
2217-
testCase.tweakIngress(ingress)
2218-
gv := testCase.groupVersion
2219-
if gv == nil {
2220-
gv = &networkingv1.SchemeGroupVersion
2221-
}
2222-
isBackendAllowed := resourceBackendPresent(ingress)
2223-
if isBackendAllowed != testCase.expectResourceBackend {
2224-
t.Errorf("Expected resourceBackendPresent to return: %v, got: %v", testCase.expectResourceBackend, isBackendAllowed)
2225-
}
2226-
})
2227-
}
2228-
}

‎pkg/registry/networking/ingress/storage/storage_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func TestCreate(t *testing.T) {
137137
noBackendAndRules.Spec.Rules = []networking.IngressRule{}
138138
badPath := validIngress()
139139
badPath.Spec.Rules = toIngressRules(map[string]IngressRuleValues{
140-
"foo.bar.com": {"/invalid[": "svc"}})
140+
"foo.bar.com": {"invalid-no-leading-slash": "svc"}})
141141
test.TestCreate(
142142
// valid
143143
ingress,
@@ -176,7 +176,7 @@ func TestUpdate(t *testing.T) {
176176
func(obj runtime.Object) runtime.Object {
177177
object := obj.(*networking.Ingress)
178178
object.Spec.Rules = toIngressRules(map[string]IngressRuleValues{
179-
"foo.bar.com": {"/invalid[": "svc"}})
179+
"foo.bar.com": {"invalid-no-leading-slash": "svc"}})
180180
return object
181181
},
182182
)

0 commit comments

Comments
 (0)