Merge pull request #1171 from robscott/validate-path-modifier

Adding validation to ensure prefix path match exists for path modifier
kubernetes-sigs · Jun 6, 2022 · 20683af · 20683af
2 parents c5bca72 + 65271b1
commit 20683af
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 7 deletions.
diff --git 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
@@ -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,