Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed May 1, 2024
1 parent 44058a5 commit 3e47a3e
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 135 deletions.
14 changes: 10 additions & 4 deletions internal/mode/static/state/graph/grpcroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,16 @@ func processGRPCRouteRules(

validFilters := len(filtersErrs) == 0

var convertedFilters []v1.HTTPRouteFilter
if validFilters {
convertedFilters = convertGRPCFilters(rule.Filters)
}

rules[i] = RouteRule{
ValidMatches: len(matchesErrs) == 0,
ValidFilters: validFilters,
Matches: convertGRPCMatches(rule.Matches),
Filters: convertGRPCFilters(rule.Filters, validFilters),
Filters: convertedFilters,
RouteBackendRefs: backendRefs,
}
}
Expand Down Expand Up @@ -259,9 +264,10 @@ func validateGRPCFilter(
}
}

func convertGRPCFilters(filters []v1alpha2.GRPCRouteFilter, validFilters bool) []v1.HTTPRouteFilter {
// validation has already been done, don't process the filters if they are invalid
if !validFilters || len(filters) == 0 {
// convertGRPCFilters converts GRPCRouteFilters (a subset of HTTPRouteFilter) to HTTPRouteFilters
// so we can reuse the logic from HTTPRoute filter validation and processing
func convertGRPCFilters(filters []v1alpha2.GRPCRouteFilter) []v1.HTTPRouteFilter {
if len(filters) == 0 {
return nil
}
httpFilters := make([]v1.HTTPRouteFilter, 0, len(filters))
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/state/graph/grpcroute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func TestBuildGRPCRoute(t *testing.T) {
ValidFilters: true,
Matches: convertGRPCMatches(grValidFilter.Spec.Rules[0].Matches),
RouteBackendRefs: []RouteBackendRef{},
Filters: convertGRPCFilters(grValidFilter.Spec.Rules[0].Filters, true),
Filters: convertGRPCFilters(grValidFilter.Spec.Rules[0].Filters),
},
},
},
Expand Down
10 changes: 4 additions & 6 deletions internal/mode/static/state/graph/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ func validateFilter(

switch filter.Type {
case v1.HTTPRouteFilterRequestRedirect:
return validateFilterRedirect(validator, filter, filterPath)
return validateFilterRedirect(validator, filter.RequestRedirect, filterPath)
case v1.HTTPRouteFilterURLRewrite:
return validateFilterRewrite(validator, filter, filterPath)
return validateFilterRewrite(validator, filter.URLRewrite, filterPath)
case v1.HTTPRouteFilterRequestHeaderModifier:
return validateFilterHeaderModifier(validator, filter.RequestHeaderModifier, filterPath)
default:
Expand All @@ -264,12 +264,11 @@ func validateFilter(

func validateFilterRedirect(
validator validation.HTTPFieldsValidator,
filter v1.HTTPRouteFilter,
redirect *v1.HTTPRequestRedirectFilter,
filterPath *field.Path,
) field.ErrorList {
var allErrs field.ErrorList

redirect := filter.RequestRedirect
redirectPath := filterPath.Child("requestRedirect")

if redirect == nil {
Expand Down Expand Up @@ -314,12 +313,11 @@ func validateFilterRedirect(

func validateFilterRewrite(
validator validation.HTTPFieldsValidator,
filter v1.HTTPRouteFilter,
rewrite *v1.HTTPURLRewriteFilter,
filterPath *field.Path,
) field.ErrorList {
var allErrs field.ErrorList

rewrite := filter.URLRewrite
rewritePath := filterPath.Child("urlRewrite")

if rewrite == nil {
Expand Down
Loading

0 comments on commit 3e47a3e

Please sign in to comment.