Remove ingress regex path requirement

This commit is contained in:
Jordan Liggitt 2020-05-12 13:58:35 -04:00
parent 0a6c826d3e
commit 8f7b8105a1
3 changed files with 11 additions and 47 deletions

View File

@ -19,7 +19,6 @@ package validation
import ( import (
"fmt" "fmt"
"net" "net"
"regexp"
"strings" "strings"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
@ -198,7 +197,6 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain
// IngressValidationOptions cover beta to GA transitions for HTTP PathType // IngressValidationOptions cover beta to GA transitions for HTTP PathType
type IngressValidationOptions struct { type IngressValidationOptions struct {
requireRegexPath bool
allowResourceBackend bool allowResourceBackend bool
} }
@ -214,8 +212,6 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
var opts IngressValidationOptions var opts IngressValidationOptions
opts = IngressValidationOptions{ opts = IngressValidationOptions{
// TODO(robscott): Remove regex validation for 1.19.
requireRegexPath: true,
// TODO(cmluciano): Allow resource backend for 1.19. // TODO(cmluciano): Allow resource backend for 1.19.
allowResourceBackend: false, allowResourceBackend: false,
} }
@ -233,10 +229,6 @@ func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV sc
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata")) allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
var opts IngressValidationOptions var opts IngressValidationOptions
opts = IngressValidationOptions{ opts = IngressValidationOptions{
// TODO(robscott): Remove regex validation for 1.19.
// Only require regex path validation for this Ingress if the previous
// version of the Ingress also passed that validation.
requireRegexPath: allPathsPassRegexValidation(oldIngress),
allowResourceBackend: resourceBackendPresent(oldIngress), allowResourceBackend: resourceBackendPresent(oldIngress),
} }
@ -377,14 +369,6 @@ func validateHTTPIngressPath(path *networking.HTTPIngressPath, fldPath *field.Pa
allErrs = append(allErrs, field.NotSupported(fldPath.Child("pathType"), *path.PathType, supportedPathTypes.List())) allErrs = append(allErrs, field.NotSupported(fldPath.Child("pathType"), *path.PathType, supportedPathTypes.List()))
} }
// TODO(robscott): Remove regex validation for 1.19.
if opts.requireRegexPath {
_, err := regexp.CompilePOSIX(path.Path)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("path"), path.Path, "must be a valid regex"))
}
}
allErrs = append(allErrs, validateIngressBackend(&path.Backend, fldPath.Child("backend"), opts)...) allErrs = append(allErrs, validateIngressBackend(&path.Backend, fldPath.Child("backend"), opts)...)
return allErrs return allErrs
} }
@ -485,25 +469,6 @@ func validateIngressTypedLocalObjectReference(params *api.TypedLocalObjectRefere
return allErrs return allErrs
} }
// allPathsPassRegexValidation returns true if the Ingress has paths that all
// match the Ingress path validation with requireRegexPath enabled.
func allPathsPassRegexValidation(ingress *networking.Ingress) bool {
for _, rule := range ingress.Spec.Rules {
if rule.HTTP == nil {
continue
}
for _, path := range rule.HTTP.Paths {
if len(path.Path) == 0 {
continue
}
if _, err := regexp.CompilePOSIX(path.Path); err != nil {
return false
}
}
}
return true
}
func resourceBackendPresent(ingress *networking.Ingress) bool { func resourceBackendPresent(ingress *networking.Ingress) bool {
if ingress.Spec.Backend != nil && ingress.Spec.Backend.Resource != nil { if ingress.Spec.Backend != nil && ingress.Spec.Backend.Resource != nil {
return true return true

View File

@ -1122,7 +1122,7 @@ func TestValidateIngress(t *testing.T) {
if gv == nil { if gv == nil {
gv = &networkingv1.SchemeGroupVersion gv = &networkingv1.SchemeGroupVersion
} }
errs := validateIngress(ingress, IngressValidationOptions{requireRegexPath: true}, *gv) errs := validateIngress(ingress, IngressValidationOptions{}, *gv)
if len(testCase.expectErrsOnFields) != len(errs) { if len(testCase.expectErrsOnFields) != len(errs) {
t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectErrsOnFields), len(errs), errs) t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectErrsOnFields), len(errs), errs)
} }
@ -1140,7 +1140,6 @@ func TestValidateIngressRuleValue(t *testing.T) {
testCases := map[string]struct { testCases := map[string]struct {
pathType networking.PathType pathType networking.PathType
path string path string
requireRegexPath bool
expectedErrs field.ErrorList expectedErrs field.ErrorList
}{ }{
"implementation specific: no leading slash": { "implementation specific: no leading slash": {
@ -1242,7 +1241,7 @@ func TestValidateIngressRuleValue(t *testing.T) {
}, },
} }
errs := validateIngressRuleValue(irv, field.NewPath("testing"), IngressValidationOptions{requireRegexPath: true}) errs := validateIngressRuleValue(irv, field.NewPath("testing"), IngressValidationOptions{})
if len(errs) != len(testCase.expectedErrs) { if len(errs) != len(testCase.expectedErrs) {
t.Fatalf("Expected %d errors, got %d (%+v)", len(testCase.expectedErrs), len(errs), errs) t.Fatalf("Expected %d errors, got %d (%+v)", len(testCase.expectedErrs), len(errs), errs)
@ -1335,7 +1334,7 @@ func TestValidateIngressCreate(t *testing.T) {
}, },
}} }}
}, },
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec.rules[0].http.paths[0].path"), "/([a-z0-9]*)[", "must be a valid regex")}, expectedErrs: field.ErrorList{},
}, },
"Spec.Backend.Resource field not allowed on create": { "Spec.Backend.Resource field not allowed on create": {
tweakIngress: func(ingress *networking.Ingress) { tweakIngress: func(ingress *networking.Ingress) {
@ -1484,7 +1483,7 @@ func TestValidateIngressUpdate(t *testing.T) {
}, },
}} }}
}, },
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec.rules[0].http.paths[0].path"), "/bar[", "must be a valid regex")}, expectedErrs: field.ErrorList{},
}, },
"invalid regex path -> valid regex path": { "invalid regex path -> valid regex path": {
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) { tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
@ -1956,7 +1955,7 @@ func TestValidateIngressTLS(t *testing.T) {
errorCases[badWildcardTLSErr] = badWildcardTLS errorCases[badWildcardTLSErr] = badWildcardTLS
for k, v := range errorCases { for k, v := range errorCases {
errs := validateIngress(&v, IngressValidationOptions{requireRegexPath: true}, networkingv1beta1.SchemeGroupVersion) errs := validateIngress(&v, IngressValidationOptions{}, networkingv1beta1.SchemeGroupVersion)
if len(errs) == 0 { if len(errs) == 0 {
t.Errorf("expected failure for %q", k) t.Errorf("expected failure for %q", k)
} else { } else {
@ -1980,7 +1979,7 @@ func TestValidateIngressTLS(t *testing.T) {
} }
validCases[fmt.Sprintf("spec.tls[0].hosts: Valid value: '%v'", wildHost)] = goodWildcardTLS validCases[fmt.Sprintf("spec.tls[0].hosts: Valid value: '%v'", wildHost)] = goodWildcardTLS
for k, v := range validCases { for k, v := range validCases {
errs := validateIngress(&v, IngressValidationOptions{requireRegexPath: true}, networkingv1beta1.SchemeGroupVersion) errs := validateIngress(&v, IngressValidationOptions{}, networkingv1beta1.SchemeGroupVersion)
if len(errs) != 0 { if len(errs) != 0 {
t.Errorf("expected success for %q", k) t.Errorf("expected success for %q", k)
} }

View File

@ -137,7 +137,7 @@ func TestCreate(t *testing.T) {
noBackendAndRules.Spec.Rules = []networking.IngressRule{} noBackendAndRules.Spec.Rules = []networking.IngressRule{}
badPath := validIngress() badPath := validIngress()
badPath.Spec.Rules = toIngressRules(map[string]IngressRuleValues{ badPath.Spec.Rules = toIngressRules(map[string]IngressRuleValues{
"foo.bar.com": {"/invalid[": "svc"}}) "foo.bar.com": {"invalid-no-leading-slash": "svc"}})
test.TestCreate( test.TestCreate(
// valid // valid
ingress, ingress,
@ -176,7 +176,7 @@ func TestUpdate(t *testing.T) {
func(obj runtime.Object) runtime.Object { func(obj runtime.Object) runtime.Object {
object := obj.(*networking.Ingress) object := obj.(*networking.Ingress)
object.Spec.Rules = toIngressRules(map[string]IngressRuleValues{ object.Spec.Rules = toIngressRules(map[string]IngressRuleValues{
"foo.bar.com": {"/invalid[": "svc"}}) "foo.bar.com": {"invalid-no-leading-slash": "svc"}})
return object return object
}, },
) )