Skip to content

Commit ceaec64

Browse files
crmejiacrmejia
authored
Validating HTTPRouteFilter type consistency (#946)
* Validating HTTPRouteFilter type consistency * fixing Makefile regression * relaxing validation constraints to simplify switch statement * adding default type validation * refactoring validateHTTPRouteSpec * fixing paths * adding HTTPRouteFilter URLRewrite validation * validating route filters are not empty * simplying validation with validateHTTPRouteFilters * removing reflect.DeepEqual as is not necessary * validate route filters are not nil * small fixes * fixing empty filter test
1 parent 1deb7bc commit ceaec64

File tree

2 files changed

+227
-48
lines changed

2 files changed

+227
-48
lines changed

‎apis/v1alpha2/validation/httproute.go‎

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ func ValidateHTTPRoute(route *gatewayv1a2.HTTPRoute) field.ErrorList {
4747
// HTTPRoute specification.
4848
func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) field.ErrorList {
4949
var errs field.ErrorList
50-
51-
errs = append(errs, validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules"))...)
50+
for i, rule := range spec.Rules {
51+
errs = append(errs, validateHTTPRouteFilters(rule.Filters, path.Child("rules").Index(i))...)
52+
for j, backendRef := range rule.BackendRefs {
53+
errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...)
54+
}
55+
}
5256
errs = append(errs, validateHTTPRouteBackendServicePorts(spec.Rules, path.Child("rules"))...)
53-
5457
return errs
5558
}
5659

@@ -80,50 +83,24 @@ func validateHTTPRouteBackendServicePorts(rules []gatewayv1a2.HTTPRouteRule, pat
8083
return errs
8184
}
8285

83-
// validateHTTPRouteUniqueFilters validates whether each core and extended filter
84-
// is used at most once in each rule.
85-
func validateHTTPRouteUniqueFilters(rules []gatewayv1a2.HTTPRouteRule, path *field.Path) field.ErrorList {
86+
// validateHTTPRouteFilters validates that a list of core and extended filters
87+
// is used at most once and that the filter type matches its value
88+
func validateHTTPRouteFilters(filters []gatewayv1a2.HTTPRouteFilter, path *field.Path) field.ErrorList {
8689
var errs field.ErrorList
90+
counts := map[gatewayv1a2.HTTPRouteFilterType]int{}
8791

88-
for i, rule := range rules {
89-
counts := map[gatewayv1a2.HTTPRouteFilterType]int{}
90-
for _, filter := range rule.Filters {
91-
counts[filter.Type]++
92-
}
93-
// custom filters don't have any validation
94-
for _, key := range repeatableHTTPRouteFilters {
95-
delete(counts, key)
96-
}
97-
98-
for filterType, count := range counts {
99-
if count > 1 {
100-
errs = append(errs, field.Invalid(path.Index(i).Child("filters"), filterType, "cannot be used multiple times in the same rule"))
101-
}
102-
}
103-
104-
errs = append(errs, validateHTTPBackendUniqueFilters(rule.BackendRefs, path, i)...)
92+
for i, filter := range filters {
93+
counts[filter.Type]++
94+
validateHTTPRouteFilterTypeMatchesValue(filter, path.Index(i))
95+
}
96+
// custom filters don't have any validation
97+
for _, key := range repeatableHTTPRouteFilters {
98+
delete(counts, key)
10599
}
106100

107-
return errs
108-
}
109-
110-
func validateHTTPBackendUniqueFilters(ref []gatewayv1a2.HTTPBackendRef, path *field.Path, i int) field.ErrorList {
111-
var errs field.ErrorList
112-
113-
for _, bkr := range ref {
114-
counts := map[gatewayv1a2.HTTPRouteFilterType]int{}
115-
for _, filter := range bkr.Filters {
116-
counts[filter.Type]++
117-
}
118-
119-
for _, key := range repeatableHTTPRouteFilters {
120-
delete(counts, key)
121-
}
122-
123-
for filterType, count := range counts {
124-
if count > 1 {
125-
errs = append(errs, field.Invalid(path.Index(i).Child("BackendRefs"), filterType, "cannot be used multiple times in the same backend"))
126-
}
101+
for filterType, count := range counts {
102+
if count > 1 {
103+
errs = append(errs, field.Invalid(path.Child("filters"), filterType, "cannot be used multiple times in the same rule"))
127104
}
128105
}
129106
return errs
@@ -166,3 +143,40 @@ func validateHTTPPathMatch(path *gatewayv1a2.HTTPPathMatch, fldPath *field.Path)
166143
}
167144
return allErrs
168145
}
146+
147+
// validateHTTPRouteFilterTypeMatchesValue validates that only the expected fields are
148+
//// set for the specified filter type.
149+
func validateHTTPRouteFilterTypeMatchesValue(filter gatewayv1a2.HTTPRouteFilter, path *field.Path) field.ErrorList {
150+
var errs field.ErrorList
151+
if filter.ExtensionRef != nil && filter.Type != gatewayv1a2.HTTPRouteFilterExtensionRef {
152+
errs = append(errs, field.Invalid(path, filter.ExtensionRef, "must be nil if the HTTPRouteFilter.Type is not ExtensionRef"))
153+
}
154+
if filter.ExtensionRef == nil && filter.Type == gatewayv1a2.HTTPRouteFilterExtensionRef {
155+
errs = append(errs, field.Required(path, "filter.ExtensionRef must be specified for ExtensionRef HTTPRouteFilter.Type"))
156+
}
157+
if filter.RequestHeaderModifier != nil && filter.Type != gatewayv1a2.HTTPRouteFilterRequestHeaderModifier {
158+
errs = append(errs, field.Invalid(path, filter.RequestHeaderModifier, "must be nil if the HTTPRouteFilter.Type is not RequestHeaderModifier"))
159+
}
160+
if filter.RequestHeaderModifier == nil && filter.Type == gatewayv1a2.HTTPRouteFilterRequestHeaderModifier {
161+
errs = append(errs, field.Required(path, "filter.RequestHeaderModifier must be specified for RequestHeaderModifier HTTPRouteFilter.Type"))
162+
}
163+
if filter.RequestMirror != nil && filter.Type != gatewayv1a2.HTTPRouteFilterRequestMirror {
164+
errs = append(errs, field.Invalid(path, filter.RequestMirror, "must be nil if the HTTPRouteFilter.Type is not RequestMirror"))
165+
}
166+
if filter.RequestMirror == nil && filter.Type == gatewayv1a2.HTTPRouteFilterRequestMirror {
167+
errs = append(errs, field.Required(path, "filter.RequestMirror must be specified for RequestMirror HTTPRouteFilter.Type"))
168+
}
169+
if filter.RequestRedirect != nil && filter.Type != gatewayv1a2.HTTPRouteFilterRequestRedirect {
170+
errs = append(errs, field.Invalid(path, filter.RequestRedirect, "must be nil if the HTTPRouteFilter.Type is not RequestRedirect"))
171+
}
172+
if filter.RequestRedirect == nil && filter.Type == gatewayv1a2.HTTPRouteFilterRequestRedirect {
173+
errs = append(errs, field.Required(path, "filter.RequestRedirect must be specified for RequestRedirect HTTPRouteFilter.Type"))
174+
}
175+
if filter.URLRewrite != nil && filter.Type != gatewayv1a2.HTTPRouteFilterURLRewrite {
176+
errs = append(errs, field.Invalid(path, filter.URLRewrite, "must be nil if the HTTPRouteFilter.Type is not URLRewrite"))
177+
}
178+
if filter.URLRewrite == nil && filter.Type == gatewayv1a2.HTTPRouteFilterURLRewrite {
179+
errs = append(errs, field.Required(path, "filter.URLRewrite must be specified for URLRewrite HTTPRouteFilter.Type"))
180+
}
181+
return errs
182+
}

‎apis/v1alpha2/validation/httproute_test.go‎

Lines changed: 170 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,10 @@ func TestValidateHTTPRoute(t *testing.T) {
291291
}
292292
for _, tc := range tests {
293293
t.Run(tc.name, func(t *testing.T) {
294-
errs := validateHTTPRouteUniqueFilters(tc.rules, field.NewPath("spec").Child("rules"))
294+
var errs field.ErrorList
295+
for _, rules := range tc.rules {
296+
errs = validateHTTPRouteFilters(rules.Filters, field.NewPath("spec").Child("rules"))
297+
}
295298
if len(errs) != tc.errCount {
296299
t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tc.errCount)
297300
}
@@ -381,10 +384,12 @@ func TestValidateHTTPBackendUniqueFilters(t *testing.T) {
381384

382385
for _, tc := range tests {
383386
t.Run(tc.name, func(t *testing.T) {
384-
for index, rule := range tc.hRoute.Spec.Rules {
385-
errs := validateHTTPBackendUniqueFilters(rule.BackendRefs, field.NewPath("spec").Child("rules"), index)
386-
if len(errs) != tc.errCount {
387-
t.Errorf("ValidateHTTPRoute() got %d errors, want %d errors", len(errs), tc.errCount)
387+
for _, rule := range tc.hRoute.Spec.Rules {
388+
for _, backendRef := range rule.BackendRefs {
389+
errs := validateHTTPRouteFilters(backendRef.Filters, field.NewPath("spec").Child("rules"))
390+
if len(errs) != tc.errCount {
391+
t.Errorf("ValidateHTTPRoute() got %d errors, want %d errors", len(errs), tc.errCount)
392+
}
388393
}
389394
}
390395
})
@@ -553,3 +558,163 @@ func TestValidateServicePort(t *testing.T) {
553558
})
554559
}
555560
}
561+
562+
func TestValidateHTTPRouteTypeMatchesField(t *testing.T) {
563+
tests := []struct {
564+
name string
565+
routeFilter gatewayv1a2.HTTPRouteFilter
566+
errCount int
567+
}{
568+
{
569+
name: "valid HTTPRouteFilterRequestHeaderModifier route filter",
570+
routeFilter: gatewayv1a2.HTTPRouteFilter{
571+
Type: gatewayv1a2.HTTPRouteFilterRequestHeaderModifier,
572+
RequestHeaderModifier: &gatewayv1a2.HTTPRequestHeaderFilter{
573+
Set: []gatewayv1a2.HTTPHeader{{Name: "name"}},
574+
Add: []gatewayv1a2.HTTPHeader{{Name: "add"}},
575+
Remove: []string{"remove"},
576+
},
577+
},
578+
errCount: 0,
579+
},
580+
{
581+
name: "invalid HTTPRouteFilterRequestHeaderModifier type filter with non-matching field",
582+
routeFilter: gatewayv1a2.HTTPRouteFilter{
583+
Type: gatewayv1a2.HTTPRouteFilterRequestHeaderModifier,
584+
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{},
585+
},
586+
errCount: 2,
587+
},
588+
{
589+
name: "invalid HTTPRouteFilterRequestHeaderModifier type filter with empty value field",
590+
routeFilter: gatewayv1a2.HTTPRouteFilter{
591+
Type: gatewayv1a2.HTTPRouteFilterRequestHeaderModifier,
592+
},
593+
errCount: 1,
594+
},
595+
{
596+
name: "valid HTTPRouteFilterRequestMirror route filter",
597+
routeFilter: gatewayv1a2.HTTPRouteFilter{
598+
Type: gatewayv1a2.HTTPRouteFilterRequestMirror,
599+
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{BackendRef: gatewayv1a2.BackendObjectReference{
600+
Group: new(gatewayv1a2.Group),
601+
Kind: new(gatewayv1a2.Kind),
602+
Name: "name",
603+
Namespace: new(gatewayv1a2.Namespace),
604+
Port: pkgutils.PortNumberPtr(22),
605+
}},
606+
},
607+
errCount: 0,
608+
},
609+
{
610+
name: "invalid HTTPRouteFilterRequestMirror type filter with non-matching field",
611+
routeFilter: gatewayv1a2.HTTPRouteFilter{
612+
Type: gatewayv1a2.HTTPRouteFilterRequestMirror,
613+
RequestHeaderModifier: &gatewayv1a2.HTTPRequestHeaderFilter{},
614+
},
615+
errCount: 2,
616+
},
617+
{
618+
name: "invalid HTTPRouteFilterRequestMirror type filter with empty value field",
619+
routeFilter: gatewayv1a2.HTTPRouteFilter{
620+
Type: gatewayv1a2.HTTPRouteFilterRequestMirror,
621+
},
622+
errCount: 1,
623+
},
624+
{
625+
name: "valid HTTPRouteFilterRequestRedirect route filter",
626+
routeFilter: gatewayv1a2.HTTPRouteFilter{
627+
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
628+
RequestRedirect: &gatewayv1a2.HTTPRequestRedirectFilter{
629+
Scheme: new(string),
630+
Hostname: new(gatewayv1a2.Hostname),
631+
Path: &gatewayv1a2.HTTPPathModifier{},
632+
Port: new(gatewayv1a2.PortNumber),
633+
StatusCode: new(int),
634+
},
635+
},
636+
errCount: 0,
637+
},
638+
{
639+
name: "invalid HTTPRouteFilterRequestRedirect type filter with non-matching field",
640+
routeFilter: gatewayv1a2.HTTPRouteFilter{
641+
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
642+
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{},
643+
},
644+
errCount: 2,
645+
},
646+
{
647+
name: "invalid HTTPRouteFilterRequestRedirect type filter with empty value field",
648+
routeFilter: gatewayv1a2.HTTPRouteFilter{
649+
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
650+
},
651+
errCount: 1,
652+
},
653+
{
654+
name: "valid HTTPRouteFilterExtensionRef filter",
655+
routeFilter: gatewayv1a2.HTTPRouteFilter{
656+
Type: gatewayv1a2.HTTPRouteFilterExtensionRef,
657+
ExtensionRef: &gatewayv1a2.LocalObjectReference{
658+
Group: "group",
659+
Kind: "kind",
660+
Name: "name",
661+
},
662+
},
663+
errCount: 0,
664+
},
665+
{
666+
name: "invalid HTTPRouteFilterExtensionRef type filter with non-matching field",
667+
routeFilter: gatewayv1a2.HTTPRouteFilter{
668+
Type: gatewayv1a2.HTTPRouteFilterExtensionRef,
669+
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{},
670+
},
671+
errCount: 2,
672+
},
673+
{
674+
name: "invalid HTTPRouteFilterExtensionRef type filter with empty value field",
675+
routeFilter: gatewayv1a2.HTTPRouteFilter{
676+
Type: gatewayv1a2.HTTPRouteFilterExtensionRef,
677+
},
678+
errCount: 1,
679+
},
680+
{
681+
name: "valid HTTPRouteFilterURLRewrite route filter",
682+
routeFilter: gatewayv1a2.HTTPRouteFilter{
683+
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
684+
URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{
685+
Hostname: new(gatewayv1a2.Hostname),
686+
Path: &gatewayv1a2.HTTPPathModifier{},
687+
},
688+
},
689+
errCount: 0,
690+
},
691+
{
692+
name: "invalid HTTPRouteFilterURLRewrite type filter with non-matching field",
693+
routeFilter: gatewayv1a2.HTTPRouteFilter{
694+
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
695+
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{},
696+
},
697+
errCount: 2,
698+
},
699+
{
700+
name: "invalid HTTPRouteFilterURLRewrite type filter with empty value field",
701+
routeFilter: gatewayv1a2.HTTPRouteFilter{
702+
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
703+
},
704+
errCount: 1,
705+
},
706+
{
707+
name: "empty type filter is valid(caught by CRD validation)",
708+
routeFilter: gatewayv1a2.HTTPRouteFilter{},
709+
errCount: 0,
710+
},
711+
}
712+
for _, tc := range tests {
713+
t.Run(tc.name, func(t *testing.T) {
714+
errs := validateHTTPRouteFilterTypeMatchesValue(tc.routeFilter, field.NewPath("spec").Child("rules").Index(0).Child("filters").Index(0))
715+
if len(errs) != tc.errCount {
716+
t.Errorf("got %v errors, want %v errors: %s", len(errs), tc.errCount, errs)
717+
}
718+
})
719+
}
720+
}

0 commit comments

Comments
 (0)