From 977527f52cd3dc8b7a2d97fe37e68a9ac0e1dc78 Mon Sep 17 00:00:00 2001 From: Harry Bagdi Date: Fri, 16 Apr 2021 07:17:21 -0700 Subject: [PATCH] httproute: add duplicate filter validation A variation of this patch first appeared in #506. This patch is derived from #506 and has been reworked to include it in the validation package. Co-authored-by: Christopher M. Luciano --- apis/v1alpha1/validation/validation.go | 47 ++++ apis/v1alpha1/validation/validation_test.go | 264 ++++++++++++++++++++ go.mod | 1 + go.sum | 3 +- 4 files changed, 314 insertions(+), 1 deletion(-) diff --git a/apis/v1alpha1/validation/validation.go b/apis/v1alpha1/validation/validation.go index 994e118f6f..30ec53f0f8 100644 --- a/apis/v1alpha1/validation/validation.go +++ b/apis/v1alpha1/validation/validation.go @@ -26,6 +26,14 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) +var ( + // repeatableHTTPRouteFilters are filter types that can are allowed to be + // repeated multiple times in a rule. + repeatableHTTPRouteFilters = []gatewayv1a1.HTTPRouteFilterType{ + gatewayv1a1.HTTPRouteFilterExtensionRef, + } +) + // ValidateGateway validates gw according to the Gateway API specification. // For additional details of the Gateway spec, refer to: // https://gateway-api.sigs.k8s.io/spec/#networking.x-k8s.io/v1alpha1.Gateway @@ -75,3 +83,42 @@ func validateListenerHostname(listeners []gatewayv1a1.Listener, path *field.Path } return errs } + +// ValidateHTTPRoute validates HTTPRoute according to the Gateway API specification. +// For additional details of the HTTPRoute spec, refer to: +// https://gateway-api.sigs.k8s.io/spec/#networking.x-k8s.io/v1alpha1.HTTPRoute +func ValidateHTTPRoute(route *gatewayv1a1.HTTPRoute) field.ErrorList { + return validateHTTPRouteSpec(&route.Spec, field.NewPath("spec")) +} + +// validateHTTPRouteSpec validates that required fields of spec are set according to the +// HTTPRoute specification. +func validateHTTPRouteSpec(spec *gatewayv1a1.HTTPRouteSpec, path *field.Path) field.ErrorList { + return validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules")) +} + +// validateHTTPRouteUniqueFilters validates whether each core and extended filter +// is used at most once in each rule. +func validateHTTPRouteUniqueFilters(rules []gatewayv1a1.HTTPRouteRule, path *field.Path) field.ErrorList { + var errs field.ErrorList + + for i, rule := range rules { + counts := map[gatewayv1a1.HTTPRouteFilterType]int{} + for _, filter := range rule.Filters { + counts[filter.Type]++ + } + // custom filters don't have any validation + for _, key := range repeatableHTTPRouteFilters { + counts[key] = 0 + } + + for filterType, count := range counts { + if count > 1 { + errs = append(errs, field.Invalid(path.Index(i).Child("filters"), filterType, "cannot be used multiple times in the same rule")) + } + } + + } + + return errs +} diff --git a/apis/v1alpha1/validation/validation_test.go b/apis/v1alpha1/validation/validation_test.go index c5d99d1ae4..f9e2475bfc 100644 --- a/apis/v1alpha1/validation/validation_test.go +++ b/apis/v1alpha1/validation/validation_test.go @@ -22,6 +22,7 @@ import ( gatewayv1a1 "sigs.k8s.io/gateway-api/apis/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilpointer "k8s.io/utils/pointer" ) func TestValidateGateway(t *testing.T) { @@ -160,3 +161,266 @@ func TestValidateGateway(t *testing.T) { }) } } + +func TestValidateHTTPRoute(t *testing.T) { + testService := "test-service" + specialService := "special-service" + tests := []struct { + name string + hRoute gatewayv1a1.HTTPRoute + errCount int + }{ + { + name: "valid httpRoute with no filters", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + ForwardTo: []gatewayv1a1.HTTPRouteForwardTo{ + { + ServiceName: &testService, + Port: portNumberPtr(8080), + Weight: utilpointer.Int32(100), + }, + }, + }, + }, + }, + }, + errCount: 0, + }, + { + name: "valid httpRoute with 1 filter", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + Filters: []gatewayv1a1.HTTPRouteFilter{ + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8081), + }, + }, + }, + }, + }, + }, + }, + errCount: 0, + }, + { + name: "invalid httpRoute with 2 extended filters", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + Filters: []gatewayv1a1.HTTPRouteFilter{ + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8080), + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &specialService, + Port: portNumberPtr(8080), + }, + }, + }, + }, + }, + }, + }, + errCount: 1, + }, + { + name: "invalid httpRoute with mix of filters and one duplicate", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + Filters: []gatewayv1a1.HTTPRouteFilter{ + { + Type: gatewayv1a1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1a1.HTTPRequestHeaderFilter{ + Set: map[string]string{"special-header": "foo"}, + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8080), + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1a1.HTTPRequestHeaderFilter{ + Add: map[string]string{"my-header": "bar"}, + }, + }, + }, + }, + }, + }, + }, + errCount: 1, + }, + { + name: "invalid httpRoute with multiple duplicate filters", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + Filters: []gatewayv1a1.HTTPRouteFilter{ + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8080), + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1a1.HTTPRequestHeaderFilter{ + Set: map[string]string{"special-header": "foo"}, + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8080), + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1a1.HTTPRequestHeaderFilter{ + Add: map[string]string{"my-header": "bar"}, + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &specialService, + Port: portNumberPtr(8080), + }, + }, + }, + }, + }, + }, + }, + errCount: 2, + }, + { + name: "valid httpRoute with duplicate ExtensionRef filters", + hRoute: gatewayv1a1.HTTPRoute{ + Spec: gatewayv1a1.HTTPRouteSpec{ + Rules: []gatewayv1a1.HTTPRouteRule{ + { + Matches: []gatewayv1a1.HTTPRouteMatch{ + { + Path: &gatewayv1a1.HTTPPathMatch{ + Type: pathMatchTypePtr("Prefix"), + Value: utilpointer.String("/"), + }, + }, + }, + Filters: []gatewayv1a1.HTTPRouteFilter{ + { + Type: gatewayv1a1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1a1.HTTPRequestHeaderFilter{ + Set: map[string]string{"special-header": "foo"}, + }, + }, + { + Type: gatewayv1a1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewayv1a1.HTTPRequestMirrorFilter{ + ServiceName: &testService, + Port: portNumberPtr(8080), + }, + }, + { + Type: "ExtensionRef", + }, + { + Type: "ExtensionRef", + }, + { + Type: "ExtensionRef", + }, + }, + }, + }, + }, + }, + errCount: 0, + }, + } + for _, tt := range tests { + // copy variable to avoid scope problems with ranges + tt := tt + t.Run(tt.name, func(t *testing.T) { + errs := ValidateHTTPRoute(&tt.hRoute) + if len(errs) != tt.errCount { + t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tt.errCount) + } + }) + } +} + +func pathMatchTypePtr(s string) *gatewayv1a1.PathMatchType { + result := gatewayv1a1.PathMatchType(s) + return &result +} + +func portNumberPtr(p int) *gatewayv1a1.PortNumber { + result := gatewayv1a1.PortNumber(p) + return &result +} diff --git a/go.mod b/go.mod index 7c1c74d72b..dcea3e0b7b 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( k8s.io/apimachinery v0.21.0 k8s.io/client-go v0.21.0 k8s.io/code-generator v0.21.0 + k8s.io/utils v0.0.0-20210305010621-2afb4311ab10 sigs.k8s.io/controller-runtime v0.8.3 sigs.k8s.io/controller-tools v0.5.0 ) diff --git a/go.sum b/go.sum index d6724ea83a..0b42985fd8 100644 --- a/go.sum +++ b/go.sum @@ -721,8 +721,9 @@ k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAG k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7 h1:vEx13qjvaZ4yfObSSXW7BrMc/KQBBT/Jyee8XtLf4x0= k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7/go.mod h1:wXW5VT87nVfh/iLV8FpR2uDvrFyomxbtb1KivDbvPTE= k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= -k8s.io/utils v0.0.0-20210111153108-fddb29f9d009 h1:0T5IaWHO3sJTEmCP6mUlBvMukxPKUQWqiI/YuiBNMiQ= k8s.io/utils v0.0.0-20210111153108-fddb29f9d009/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +k8s.io/utils v0.0.0-20210305010621-2afb4311ab10 h1:u5rPykqiCpL+LBfjRkXvnK71gOgIdmq3eHUEkPrbeTI= +k8s.io/utils v0.0.0-20210305010621-2afb4311ab10/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=