Skip to content

Commit

Permalink
Refactoring path modifier to use union discriminator
Browse files Browse the repository at this point in the history
  • Loading branch information
robscott committed Apr 21, 2022
1 parent c875841 commit a58e11a
Show file tree
Hide file tree
Showing 7 changed files with 229 additions and 57 deletions.
29 changes: 18 additions & 11 deletions apis/v1alpha2/httproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,13 +684,13 @@ type HTTPRequestHeaderFilter struct {
Remove []string `json:"remove,omitempty"`
}

// HTTPPathModifierType defines the type of path redirect.
// HTTPPathModifierType defines the type of path redirect or rewrite.
type HTTPPathModifierType string

const (
// This type of modifier indicates that the complete path will be replaced
// by the path redirect value.
AbsoluteHTTPPathModifier HTTPPathModifierType = "Absolute"
// This type of modifier indicates that the full path will be replaced
// by the specified value.
FullPathHTTPPathModifier HTTPPathModifierType = "ReplaceFullPath"

// This type of modifier indicates that any prefix path matches will be
// replaced by the substitution value. For example, a path with a prefix
Expand All @@ -705,17 +705,24 @@ type HTTPPathModifier struct {
// Type defines the type of path modifier.
//
// <gateway:experimental>
// +kubebuilder:validation:Enum=Absolute;ReplacePrefixMatch
// +kubebuilder:validation:Enum=ReplaceFullPath;ReplacePrefixMatch
Type HTTPPathModifierType `json:"type"`

// Substitution defines the HTTP path value to substitute. An empty value
// ("") indicates that the portion of the path to be changed should be
// removed from the resulting path. For example, a request to "/foo/bar"
// with a prefix match of "/foo" would be modified to "/bar".
// ReplaceFullPath specifies the value with which to replace the full path
// of a request during a rewrite or redirect.
//
// <gateway:experimental>
// +kubebuilder:validation:MaxLength=1024
Substitution string `json:"substitution"`
// +optional
ReplaceFullPath *string `json:"replaceFullPath,omitempty"`

// ReplacePrefixMatch specifies the value with which to replace the prefix
// match of a request during a rewrite or redirect.
//
// <gateway:experimental>
// +kubebuilder:validation:MaxLength=1024
// +optional
ReplacePrefixMatch *string `json:"replacePrefixMatch,omitempty"`
}

// HTTPRequestRedirect defines a filter that redirects a request. This filter
Expand Down Expand Up @@ -783,7 +790,7 @@ type HTTPURLRewriteFilter struct {
//
// <gateway:experimental>
// +optional
Hostname *Hostname `json:"hostname,omitempty"`
Hostname *PreciseHostname `json:"hostname,omitempty"`

// Path defines a path rewrite.
//
Expand Down
29 changes: 29 additions & 0 deletions apis/v1alpha2/validation/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,23 @@ func validateHTTPRouteFilters(filters []gatewayv1a2.HTTPRouteFilter, path *field

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"))...)
}
if filter.URLRewrite != nil && filter.URLRewrite.Path != nil {
errs = append(errs, validateHTTPPathModifier(*filter.URLRewrite.Path, path.Index(i).Child("urlRewrite", "path"))...)
}
errs = append(errs, validateHTTPRouteFilterTypeMatchesValue(filter, path.Index(i))...)
}
// custom filters don't have any validation
for _, key := range repeatableHTTPRouteFilters {
delete(counts, key)
}

if counts[gatewayv1a2.HTTPRouteFilterRequestRedirect] > 0 && counts[gatewayv1a2.HTTPRouteFilterURLRewrite] > 0 {
errs = append(errs, field.Invalid(path.Child("filters"), gatewayv1a2.HTTPRouteFilterRequestRedirect, "Redirect and Rewrite filters cannot be defined in the same list of filters"))
}

for filterType, count := range counts {
if count > 1 {
errs = append(errs, field.Invalid(path.Child("filters"), filterType, "cannot be used multiple times in the same rule"))
Expand Down Expand Up @@ -185,3 +195,22 @@ func validateHTTPRouteFilterTypeMatchesValue(filter gatewayv1a2.HTTPRouteFilter,
}
return errs
}

// validateHTTPPathModifier validates that only the expected fields are set in a
// path modifier.
func validateHTTPPathModifier(modifier gatewayv1a2.HTTPPathModifier, 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"))
}
if modifier.ReplaceFullPath == nil && modifier.Type == gatewayv1a2.FullPathHTTPPathModifier {
errs = append(errs, field.Invalid(path, modifier.ReplaceFullPath, "must not be nil if the HTTPRouteFilter.Type is ReplaceFullPath"))
}
if modifier.ReplacePrefixMatch != nil && modifier.Type != gatewayv1a2.PrefixMatchHTTPPathModifier {
errs = append(errs, field.Invalid(path, modifier.ReplaceFullPath, "must be nil if the HTTPRouteFilter.Type is not ReplacePrefixMatch"))
}
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"))
}
return errs
}
84 changes: 83 additions & 1 deletion apis/v1alpha2/validation/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,88 @@ func TestValidateHTTPRoute(t *testing.T) {
},
},
},
}, {
name: "valid redirect path modifier",
errCount: 0,
rules: []gatewayv1a2.HTTPRouteRule{
{
Filters: []gatewayv1a2.HTTPRouteFilter{
{
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
RequestRedirect: &gatewayv1a2.HTTPRequestRedirectFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.FullPathHTTPPathModifier,
ReplaceFullPath: utilpointer.String("foo"),
},
},
},
},
},
},
}, {
name: "redirect path modifier with type mismatch",
errCount: 2,
rules: []gatewayv1a2.HTTPRouteRule{{
Filters: []gatewayv1a2.HTTPRouteFilter{{
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
RequestRedirect: &gatewayv1a2.HTTPRequestRedirectFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.PrefixMatchHTTPPathModifier,
ReplaceFullPath: utilpointer.String("foo"),
},
},
}},
}},
}, {
name: "valid rewrite path modifier",
errCount: 0,
rules: []gatewayv1a2.HTTPRouteRule{{
Filters: []gatewayv1a2.HTTPRouteFilter{{
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.PrefixMatchHTTPPathModifier,
ReplacePrefixMatch: utilpointer.String("foo"),
},
},
}},
}},
}, {
name: "redirect path modifier with type mismatch",
errCount: 2,
rules: []gatewayv1a2.HTTPRouteRule{{
Filters: []gatewayv1a2.HTTPRouteFilter{{
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.FullPathHTTPPathModifier,
ReplacePrefixMatch: utilpointer.String("foo"),
},
},
}},
}},
}, {
name: "rewrite and redirect filters combined (invalid)",
errCount: 1,
rules: []gatewayv1a2.HTTPRouteRule{{
Filters: []gatewayv1a2.HTTPRouteFilter{{
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.PrefixMatchHTTPPathModifier,
ReplacePrefixMatch: utilpointer.String("foo"),
},
},
}, {
Type: gatewayv1a2.HTTPRouteFilterRequestRedirect,
RequestRedirect: &gatewayv1a2.HTTPRequestRedirectFilter{
Path: &gatewayv1a2.HTTPPathModifier{
Type: gatewayv1a2.PrefixMatchHTTPPathModifier,
ReplacePrefixMatch: utilpointer.String("foo"),
},
},
}},
}},
}}

for _, tc := range tests {
Expand Down Expand Up @@ -654,7 +736,7 @@ func TestValidateHTTPRouteTypeMatchesField(t *testing.T) {
routeFilter: gatewayv1a2.HTTPRouteFilter{
Type: gatewayv1a2.HTTPRouteFilterURLRewrite,
URLRewrite: &gatewayv1a2.HTTPURLRewriteFilter{
Hostname: new(gatewayv1a2.Hostname),
Hostname: new(gatewayv1a2.PreciseHostname),
Path: &gatewayv1a2.HTTPPathModifier{},
},
},
Expand Down
16 changes: 13 additions & 3 deletions apis/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a58e11a

Please sign in to comment.