From 65271b1221f23966376242f8e2cac6a6328540a1 Mon Sep 17 00:00:00 2001 From: Rob Scott Date: Mon, 23 May 2022 21:52:53 +0000 Subject: [PATCH] Adding validation to ensure prefix path match exists for path modifier --- apis/v1alpha2/validation/httproute.go | 30 ++++++++++--- apis/v1alpha2/validation/httproute_test.go | 49 +++++++++++++++++++++- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/apis/v1alpha2/validation/httproute.go b/apis/v1alpha2/validation/httproute.go index f3a15a782d..c892fbba7c 100644 --- a/apis/v1alpha2/validation/httproute.go +++ b/apis/v1alpha2/validation/httproute.go @@ -48,9 +48,9 @@ func ValidateHTTPRoute(route *gatewayv1a2.HTTPRoute) field.ErrorList { func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) field.ErrorList { var errs field.ErrorList for i, rule := range spec.Rules { - errs = append(errs, validateHTTPRouteFilters(rule.Filters, path.Child("rules").Index(i))...) + errs = append(errs, validateHTTPRouteFilters(rule.Filters, rule.Matches, path.Child("rules").Index(i))...) for j, backendRef := range rule.BackendRefs { - errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) + errs = append(errs, validateHTTPRouteFilters(backendRef.Filters, rule.Matches, path.Child("rules").Index(i).Child("backendsrefs").Index(j))...) } for j, m := range rule.Matches { if m.Path != nil { @@ -90,17 +90,17 @@ func validateHTTPRouteBackendServicePorts(rules []gatewayv1a2.HTTPRouteRule, pat // validateHTTPRouteFilters validates that a list of core and extended filters // is used at most once and that the filter type matches its value -func validateHTTPRouteFilters(filters []gatewayv1a2.HTTPRouteFilter, path *field.Path) field.ErrorList { +func validateHTTPRouteFilters(filters []gatewayv1a2.HTTPRouteFilter, matches []gatewayv1a2.HTTPRouteMatch, path *field.Path) field.ErrorList { var errs field.ErrorList counts := map[gatewayv1a2.HTTPRouteFilterType]int{} for i, filter := range filters { counts[filter.Type]++ if filter.RequestRedirect != nil && filter.RequestRedirect.Path != nil { - errs = append(errs, validateHTTPPathModifier(*filter.RequestRedirect.Path, path.Index(i).Child("requestRedirect", "path"))...) + errs = append(errs, validateHTTPPathModifier(*filter.RequestRedirect.Path, matches, path.Index(i).Child("requestRedirect", "path"))...) } if filter.URLRewrite != nil && filter.URLRewrite.Path != nil { - errs = append(errs, validateHTTPPathModifier(*filter.URLRewrite.Path, path.Index(i).Child("urlRewrite", "path"))...) + errs = append(errs, validateHTTPPathModifier(*filter.URLRewrite.Path, matches, path.Index(i).Child("urlRewrite", "path"))...) } errs = append(errs, validateHTTPRouteFilterTypeMatchesValue(filter, path.Index(i))...) } @@ -198,7 +198,7 @@ func validateHTTPRouteFilterTypeMatchesValue(filter gatewayv1a2.HTTPRouteFilter, // validateHTTPPathModifier validates that only the expected fields are set in a // path modifier. -func validateHTTPPathModifier(modifier gatewayv1a2.HTTPPathModifier, path *field.Path) field.ErrorList { +func validateHTTPPathModifier(modifier gatewayv1a2.HTTPPathModifier, matches []gatewayv1a2.HTTPRouteMatch, path *field.Path) field.ErrorList { var errs field.ErrorList if modifier.ReplaceFullPath != nil && modifier.Type != gatewayv1a2.FullPathHTTPPathModifier { errs = append(errs, field.Invalid(path, modifier.ReplaceFullPath, "must be nil if the HTTPRouteFilter.Type is not ReplaceFullPath")) @@ -212,5 +212,23 @@ func validateHTTPPathModifier(modifier gatewayv1a2.HTTPPathModifier, path *field if modifier.ReplacePrefixMatch == nil && modifier.Type == gatewayv1a2.PrefixMatchHTTPPathModifier { errs = append(errs, field.Invalid(path, modifier.ReplacePrefixMatch, "must not be nil if the HTTPRouteFilter.Type is ReplacePrefixMatch")) } + + if modifier.Type == gatewayv1a2.PrefixMatchHTTPPathModifier && modifier.ReplacePrefixMatch != nil { + if !hasExactlyOnePrefixMatch(matches) { + errs = append(errs, field.Invalid(path, modifier.ReplacePrefixMatch, "exactly one PathPrefix match must be specified to use this path modifier")) + } + } return errs } + +func hasExactlyOnePrefixMatch(matches []gatewayv1a2.HTTPRouteMatch) bool { + if len(matches) != 1 || matches[0].Path == nil { + return false + } + pathMatchType := matches[0].Path.Type + if *pathMatchType != gatewayv1a2.PathMatchPathPrefix { + return false + } + + return true +} diff --git a/apis/v1alpha2/validation/httproute_test.go b/apis/v1alpha2/validation/httproute_test.go index abf4c35335..ee2262325b 100644 --- a/apis/v1alpha2/validation/httproute_test.go +++ b/apis/v1alpha2/validation/httproute_test.go @@ -30,6 +30,8 @@ import ( func TestValidateHTTPRoute(t *testing.T) { testService := gatewayv1a2.ObjectName("test-service") specialService := gatewayv1a2.ObjectName("special-service") + pathPrefixMatchType := gatewayv1a2.PathMatchPathPrefix + tests := []struct { name string rules []gatewayv1a2.HTTPRouteRule @@ -329,6 +331,51 @@ func TestValidateHTTPRoute(t *testing.T) { name: "valid rewrite path modifier", errCount: 0, rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + Path: &gatewayv1a2.HTTPPathMatch{ + Type: &pathPrefixMatchType, + Value: utilpointer.String("/bar"), + }, + }}, + Filters: []gatewayv1a2.HTTPRouteFilter{{ + Type: gatewayv1a2.HTTPRouteFilterURLRewrite, + URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{ + Path: &gatewayv1a2.HTTPPathModifier{ + Type: gatewayv1a2.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: utilpointer.String("foo"), + }, + }, + }}, + }}, + }, { + name: "rewrite path modifier missing path match", + errCount: 1, + rules: []gatewayv1a2.HTTPRouteRule{{ + Filters: []gatewayv1a2.HTTPRouteFilter{{ + Type: gatewayv1a2.HTTPRouteFilterURLRewrite, + URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{ + Path: &gatewayv1a2.HTTPPathModifier{ + Type: gatewayv1a2.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: utilpointer.String("foo"), + }, + }, + }}, + }}, + }, { + name: "rewrite path too many matches", + errCount: 1, + rules: []gatewayv1a2.HTTPRouteRule{{ + Matches: []gatewayv1a2.HTTPRouteMatch{{ + Path: &gatewayv1a2.HTTPPathMatch{ + Type: &pathPrefixMatchType, + Value: utilpointer.String("/foo"), + }, + }, { + Path: &gatewayv1a2.HTTPPathMatch{ + Type: &pathPrefixMatchType, + Value: utilpointer.String("/bar"), + }, + }}, Filters: []gatewayv1a2.HTTPRouteFilter{{ Type: gatewayv1a2.HTTPRouteFilterURLRewrite, URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{ @@ -355,7 +402,7 @@ func TestValidateHTTPRoute(t *testing.T) { }}, }, { name: "rewrite and redirect filters combined (invalid)", - errCount: 1, + errCount: 3, rules: []gatewayv1a2.HTTPRouteRule{{ Filters: []gatewayv1a2.HTTPRouteFilter{{ Type: gatewayv1a2.HTTPRouteFilterURLRewrite,