From 2e3a07ede3acd17ea65221dee49572aa0c56b0ef Mon Sep 17 00:00:00 2001 From: Ciara Stacke <18287516+ciarams87@users.noreply.github.com> Date: Thu, 16 May 2024 10:43:24 +0100 Subject: [PATCH] Add request header filter support for gRPC (#1909) * Add request header filter support for gRPC & Add base gRPC headers --- internal/mode/static/nginx/config/servers.go | 27 +- .../mode/static/nginx/config/servers_test.go | 105 ++-- internal/mode/static/state/graph/grpcroute.go | 69 ++- .../mode/static/state/graph/grpcroute_test.go | 162 +++++- internal/mode/static/state/graph/httproute.go | 170 +----- .../mode/static/state/graph/httproute_test.go | 519 +++--------------- .../mode/static/state/graph/route_common.go | 159 ++++++ .../static/state/graph/route_common_test.go | 323 +++++++++++ .../overview/gateway-api-compatibility.md | 5 +- 9 files changed, 876 insertions(+), 663 deletions(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 9e3fe06177..b67b98af76 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -20,8 +20,8 @@ const ( rootPath = "/" ) -// baseHeaders contains the constant headers set in each server block -var baseHeaders = []http.Header{ +// httpBaseHeaders contains the constant headers set in each HTTP server block +var httpBaseHeaders = []http.Header{ { Name: "Host", Value: "$gw_api_compliant_host", @@ -40,6 +40,22 @@ var baseHeaders = []http.Header{ }, } +// grpcBaseHeaders contains the constant headers set in each gRPC server block +var grpcBaseHeaders = []http.Header{ + { + Name: "Host", + Value: "$gw_api_compliant_host", + }, + { + Name: "X-Forwarded-For", + Value: "$proxy_add_x_forwarded_for", + }, + { + Name: "Authority", + Value: "$gw_api_compliant_host", + }, +} + func executeServers(conf dataplane.Configuration) []executeResult { servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) @@ -558,8 +574,11 @@ func createMatchLocation(path string) http.Location { func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.Header { var headers []http.Header if !grpc { - headers = make([]http.Header, len(baseHeaders)) - copy(headers, baseHeaders) + headers = make([]http.Header, len(httpBaseHeaders)) + copy(headers, httpBaseHeaders) + } else { + headers = make([]http.Header, len(grpcBaseHeaders)) + copy(headers, grpcBaseHeaders) } if filters != nil && filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 5f039a7a86..04fd0298ca 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -649,19 +649,19 @@ func TestCreateServers(t *testing.T) { { Path: "@rule0-route0", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "@rule0-route1", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "@rule0-route2", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { @@ -671,7 +671,7 @@ func TestCreateServers(t *testing.T) { { Path: "@rule1-route0", ProxyPass: "http://$test__route1_rule1$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { @@ -681,19 +681,19 @@ func TestCreateServers(t *testing.T) { { Path: "/path-only/", ProxyPass: "http://invalid-backend-ref$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "= /path-only", ProxyPass: "http://invalid-backend-ref$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "/backend-tls-policy/", ProxyPass: "https://test_btp_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ProxySSLVerify: &http.ProxySSLVerify{ Name: "test-btp.example.com", TrustedCertificate: "/etc/nginx/secrets/test-btp.crt", @@ -702,7 +702,7 @@ func TestCreateServers(t *testing.T) { { Path: "= /backend-tls-policy", ProxyPass: "https://test_btp_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ProxySSLVerify: &http.ProxySSLVerify{ Name: "test-btp.example.com", TrustedCertificate: "/etc/nginx/secrets/test-btp.crt", @@ -809,13 +809,13 @@ func TestCreateServers(t *testing.T) { { Path: "= /exact", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "@rule12-route0", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { @@ -898,7 +898,7 @@ func TestCreateServers(t *testing.T) { Path: "= /grpc/method", ProxyPass: "grpc://test_foo_80", GRPC: true, - ProxySetHeaders: nil, + ProxySetHeaders: grpcBaseHeaders, }, { Path: "= /grpc-with-backend-tls-policy/method", @@ -908,7 +908,7 @@ func TestCreateServers(t *testing.T) { TrustedCertificate: "/etc/nginx/secrets/test-btp.crt", }, GRPC: true, - ProxySetHeaders: nil, + ProxySetHeaders: grpcBaseHeaders, }, } } @@ -1028,13 +1028,13 @@ func TestCreateServersConflicts(t *testing.T) { { Path: "/coffee/", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "= /coffee", ProxyPass: "http://test_bar_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, createDefaultRootLocation(), @@ -1068,13 +1068,13 @@ func TestCreateServersConflicts(t *testing.T) { { Path: "= /coffee", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "/coffee/", ProxyPass: "http://test_bar_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, createDefaultRootLocation(), @@ -1118,13 +1118,13 @@ func TestCreateServersConflicts(t *testing.T) { { Path: "/coffee/", ProxyPass: "http://test_bar_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "= /coffee", ProxyPass: "http://test_baz_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, createDefaultRootLocation(), @@ -1243,13 +1243,13 @@ func TestCreateLocationsRootPath(t *testing.T) { { Path: "/path-1", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "/path-2", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { @@ -1268,17 +1268,18 @@ func TestCreateLocationsRootPath(t *testing.T) { { Path: "/path-1", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, }, { Path: "/path-2", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, }, { - Path: "/grpc", - ProxyPass: "grpc://test_foo_80", - GRPC: true, + Path: "/grpc", + ProxyPass: "grpc://test_foo_80", + GRPC: true, + ProxySetHeaders: grpcBaseHeaders, }, { Path: "/", @@ -1295,19 +1296,19 @@ func TestCreateLocationsRootPath(t *testing.T) { { Path: "/path-1", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "/path-2", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, { Path: "/", ProxyPass: "http://test_foo_80$request_uri", - ProxySetHeaders: baseHeaders, + ProxySetHeaders: httpBaseHeaders, ResponseHeaders: http.ResponseHeaders{}, }, }, @@ -2027,9 +2028,51 @@ func TestGenerateProxySetHeaders(t *testing.T) { }, }, { - msg: "grpc", - expectedHeaders: nil, - GRPC: true, + msg: "header filter with gRPC", + GRPC: true, + filters: &dataplane.HTTPFilters{ + RequestHeaderModifiers: &dataplane.HTTPHeaderFilter{ + Add: []dataplane.HTTPHeader{ + { + Name: "Authorization", + Value: "my-auth", + }, + }, + Set: []dataplane.HTTPHeader{ + { + Name: "Accept-Encoding", + Value: "gzip", + }, + }, + Remove: []string{"my-header"}, + }, + }, + expectedHeaders: []http.Header{ + { + Name: "Authorization", + Value: "${authorization_header_var}my-auth", + }, + { + Name: "Accept-Encoding", + Value: "gzip", + }, + { + Name: "my-header", + Value: "", + }, + { + Name: "Host", + Value: "$gw_api_compliant_host", + }, + { + Name: "X-Forwarded-For", + Value: "$proxy_add_x_forwarded_for", + }, + { + Name: "Authority", + Value: "$gw_api_compliant_host", + }, + }, }, } diff --git a/internal/mode/static/state/graph/grpcroute.go b/internal/mode/static/state/graph/grpcroute.go index bffb929a9b..d50001422f 100644 --- a/internal/mode/static/state/graph/grpcroute.go +++ b/internal/mode/static/state/graph/grpcroute.go @@ -73,26 +73,22 @@ func processGRPCRouteRules( validator validation.HTTPFieldsValidator, ) (rules []RouteRule, atLeastOneValid bool, allRulesErrs field.ErrorList) { rules = make([]RouteRule, len(specRules)) - validFilters := true for i, rule := range specRules { rulePath := field.NewPath("spec").Child("rules").Index(i) var allErrs field.ErrorList - var matchesErrs field.ErrorList + var filtersErrs field.ErrorList + for j, match := range rule.Matches { matchPath := rulePath.Child("matches").Index(j) matchesErrs = append(matchesErrs, validateGRPCMatch(validator, match, matchPath)...) } - if len(rule.Filters) > 0 { - filterPath := rulePath.Child("filters") - allErrs = append( - allErrs, - field.NotSupported(filterPath, rule.Filters, []string{"gRPC filters are not yet supported"}), - ) - validFilters = false + for j, filter := range rule.Filters { + filterPath := rulePath.Child("filters").Index(j) + filtersErrs = append(filtersErrs, validateGRPCFilter(validator, filter, filterPath)...) } backendRefs := make([]RouteBackendRef, 0, len(rule.BackendRefs)) @@ -114,17 +110,25 @@ func processGRPCRouteRules( } allErrs = append(allErrs, matchesErrs...) + allErrs = append(allErrs, filtersErrs...) allRulesErrs = append(allRulesErrs, allErrs...) if len(allErrs) == 0 { atLeastOneValid = true } + 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: nil, + Filters: convertedFilters, RouteBackendRefs: backendRefs, } } @@ -234,3 +238,48 @@ func validateGRPCMethodMatch( } return allErrs } + +func validateGRPCFilter( + validator validation.HTTPFieldsValidator, + filter v1alpha2.GRPCRouteFilter, + filterPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + switch filter.Type { + case v1alpha2.GRPCRouteFilterRequestHeaderModifier: + return validateFilterHeaderModifier(validator, filter.RequestHeaderModifier, filterPath.Child(string(filter.Type))) + default: + valErr := field.NotSupported( + filterPath.Child("type"), + filter.Type, + []string{ + string(v1alpha2.GRPCRouteFilterRequestHeaderModifier), + }, + ) + allErrs = append(allErrs, valErr) + return allErrs + } +} + +// 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)) + for _, filter := range filters { + switch filter.Type { + case v1alpha2.GRPCRouteFilterRequestHeaderModifier: + httpRequestHeaderFilter := v1.HTTPRouteFilter{ + Type: v1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: filter.RequestHeaderModifier, + } + httpFilters = append(httpFilters, httpRequestHeaderFilter) + default: + continue + } + } + return httpFilters +} diff --git a/internal/mode/static/state/graph/grpcroute_test.go b/internal/mode/static/state/graph/grpcroute_test.go index 574cbc6e60..2983127d67 100644 --- a/internal/mode/static/state/graph/grpcroute_test.go +++ b/internal/mode/static/state/graph/grpcroute_test.go @@ -223,7 +223,7 @@ func TestBuildGRPCRoute(t *testing.T) { grInvalidFilterRule.Filters = []v1alpha2.GRPCRouteFilter{ { - Type: "RequestHeaderModifier", + Type: "RequestMirror", }, } @@ -234,6 +234,33 @@ func TestBuildGRPCRoute(t *testing.T) { []v1alpha2.GRPCRouteRule{grInvalidFilterRule}, ) + grValidFilterRule := createGRPCMethodMatch("myService", "myMethod", "Exact") + + grValidFilterRule.Filters = []v1alpha2.GRPCRouteFilter{ + { + Type: "RequestHeaderModifier", + RequestHeaderModifier: &v1.HTTPHeaderFilter{ + Remove: []string{"header"}, + }, + }, + } + + grValidFilter := createGRPCRoute( + "gr", + gatewayNsName.Name, + "example.com", + []v1alpha2.GRPCRouteRule{grValidFilterRule}, + ) + + convertedFilters := []v1.HTTPRouteFilter{ + { + Type: v1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &v1.HTTPHeaderFilter{ + Remove: []string{"header"}, + }, + }, + } + createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { v := &validationfakes.FakeHTTPFieldsValidator{} v.ValidateMethodInMatchReturns(true, nil) @@ -310,6 +337,36 @@ func TestBuildGRPCRoute(t *testing.T) { }, name: "valid rule with empty match", }, + { + validator: createAllValidValidator(), + gr: grValidFilter, + expected: &L7Route{ + RouteType: RouteTypeGRPC, + Source: grValidFilter, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: gatewayNsName, + SectionName: grValidFilter.Spec.ParentRefs[0].SectionName, + }, + }, + Valid: true, + Attachable: true, + Spec: L7RouteSpec{ + Hostnames: grValidFilter.Spec.Hostnames, + Rules: []RouteRule{ + { + ValidMatches: true, + ValidFilters: true, + Matches: convertGRPCMatches(grValidFilter.Spec.Rules[0].Matches), + RouteBackendRefs: []RouteBackendRef{}, + Filters: convertedFilters, + }, + }, + }, + }, + name: "valid rule with filter", + }, { validator: createAllValidValidator(), gr: grInvalidMatchesEmptyMethodFields, @@ -522,10 +579,8 @@ func TestBuildGRPCRoute(t *testing.T) { }, Conditions: []conditions.Condition{ staticConds.NewRouteUnsupportedValue( - `All rules are invalid: spec.rules[0].filters: Unsupported value: []v1alpha2.GRPCRouteFilter{v1alpha2.` + - `GRPCRouteFilter{Type:"RequestHeaderModifier", RequestHeaderModifier:(*v1.HTTPHeaderFilter)(nil), ` + - `ResponseHeaderModifier:(*v1.HTTPHeaderFilter)(nil), RequestMirror:(*v1.HTTPRequestMirrorFilter)(nil), ` + - `ExtensionRef:(*v1.LocalObjectReference)(nil)}}: supported values: "gRPC filters are not yet supported"`, + `All rules are invalid: spec.rules[0].filters[0].type: ` + + `Unsupported value: "RequestMirror": supported values: "RequestHeaderModifier"`, ), }, Spec: L7RouteSpec{ @@ -584,3 +639,100 @@ func TestBuildGRPCRoute(t *testing.T) { }) } } + +func TestConvertGRPCMatches(t *testing.T) { + methodMatch := createGRPCMethodMatch("myService", "myMethod", "Exact").Matches + + headersMatch := createGRPCHeadersMatch("Exact", "MyHeader", "SomeValue").Matches + + expectedHTTPMatches := []v1.HTTPRouteMatch{ + { + Path: &v1.HTTPPathMatch{ + Type: helpers.GetPointer(v1.PathMatchExact), + Value: helpers.GetPointer("/myService/myMethod"), + }, + Headers: []v1.HTTPHeaderMatch{}, + }, + } + + expectedHeadersMatches := []v1.HTTPRouteMatch{ + { + Path: &v1.HTTPPathMatch{ + Type: helpers.GetPointer(v1.PathMatchPathPrefix), + Value: helpers.GetPointer("/"), + }, + Headers: []v1.HTTPHeaderMatch{ + { + Value: "SomeValue", + Name: v1.HTTPHeaderName("MyHeader"), + }, + }, + }, + } + + expectedEmptyMatches := []v1.HTTPRouteMatch{ + { + Path: &v1.HTTPPathMatch{ + Type: helpers.GetPointer(v1.PathMatchPathPrefix), + Value: helpers.GetPointer("/"), + }, + }, + } + + tests := []struct { + name string + methodMatches []v1alpha2.GRPCRouteMatch + expected []v1.HTTPRouteMatch + }{ + { + name: "exact match", + methodMatches: methodMatch, + expected: expectedHTTPMatches, + }, + { + name: "headers matches", + methodMatches: headersMatch, + expected: expectedHeadersMatches, + }, + { + name: "empty matches", + methodMatches: []v1alpha2.GRPCRouteMatch{}, + expected: expectedEmptyMatches, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + httpMatches := convertGRPCMatches(test.methodMatches) + g.Expect(helpers.Diff(test.expected, httpMatches)).To(BeEmpty()) + }) + } +} + +func TestConvertGRPCFilters(t *testing.T) { + grFilters := []v1alpha2.GRPCRouteFilter{ + { + Type: "RequestHeaderModifier", + RequestHeaderModifier: &v1.HTTPHeaderFilter{ + Remove: []string{"header"}, + }, + }, + { + Type: "RequestMirror", + }, + } + + expectedHTTPFilters := []v1.HTTPRouteFilter{ + { + Type: v1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: grFilters[0].RequestHeaderModifier, + }, + } + + g := NewWithT(t) + + httpFilters := convertGRPCFilters(grFilters) + g.Expect(helpers.Diff(expectedHTTPFilters, httpFilters)).To(BeEmpty()) +} diff --git a/internal/mode/static/state/graph/httproute.go b/internal/mode/static/state/graph/httproute.go index bd5aaefdce..cac85228c6 100644 --- a/internal/mode/static/state/graph/httproute.go +++ b/internal/mode/static/state/graph/httproute.go @@ -2,7 +2,6 @@ package graph import ( "fmt" - "strings" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" @@ -249,9 +248,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.Child(string(filter.Type))) case v1.HTTPRouteFilterResponseHeaderModifier: @@ -276,12 +275,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 { @@ -326,12 +324,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 { @@ -366,162 +363,3 @@ func validateFilterRewrite( return allErrs } - -func validateFilterHeaderModifier( - validator validation.HTTPFieldsValidator, - headerModifier *v1.HTTPHeaderFilter, - filterPath *field.Path, -) field.ErrorList { - if headerModifier == nil { - return field.ErrorList{field.Required(filterPath, "cannot be nil")} - } - - return validateFilterHeaderModifierFields(validator, headerModifier, filterPath) -} - -func validateFilterHeaderModifierFields( - validator validation.HTTPFieldsValidator, - headerModifier *v1.HTTPHeaderFilter, - headerModifierPath *field.Path, -) field.ErrorList { - var allErrs field.ErrorList - - // Ensure that the header names are case-insensitive unique - allErrs = append(allErrs, validateRequestHeadersCaseInsensitiveUnique( - headerModifier.Add, - headerModifierPath.Child(add))..., - ) - allErrs = append(allErrs, validateRequestHeadersCaseInsensitiveUnique( - headerModifier.Set, - headerModifierPath.Child(set))..., - ) - allErrs = append(allErrs, validateRequestHeaderStringCaseInsensitiveUnique( - headerModifier.Remove, - headerModifierPath.Child(remove))..., - ) - - for _, h := range headerModifier.Add { - if err := validator.ValidateFilterHeaderName(string(h.Name)); err != nil { - valErr := field.Invalid(headerModifierPath.Child(add), h, err.Error()) - allErrs = append(allErrs, valErr) - } - if err := validator.ValidateFilterHeaderValue(h.Value); err != nil { - valErr := field.Invalid(headerModifierPath.Child(add), h, err.Error()) - allErrs = append(allErrs, valErr) - } - } - for _, h := range headerModifier.Set { - if err := validator.ValidateFilterHeaderName(string(h.Name)); err != nil { - valErr := field.Invalid(headerModifierPath.Child(set), h, err.Error()) - allErrs = append(allErrs, valErr) - } - if err := validator.ValidateFilterHeaderValue(h.Value); err != nil { - valErr := field.Invalid(headerModifierPath.Child(set), h, err.Error()) - allErrs = append(allErrs, valErr) - } - } - for _, h := range headerModifier.Remove { - if err := validator.ValidateFilterHeaderName(h); err != nil { - valErr := field.Invalid(headerModifierPath.Child(remove), h, err.Error()) - allErrs = append(allErrs, valErr) - } - } - - return allErrs -} - -func validateFilterResponseHeaderModifier( - validator validation.HTTPFieldsValidator, - responseHeaderModifier *v1.HTTPHeaderFilter, - filterPath *field.Path, -) field.ErrorList { - if errList := validateFilterHeaderModifier(validator, responseHeaderModifier, filterPath); errList != nil { - return errList - } - var allErrs field.ErrorList - - allErrs = append(allErrs, validateResponseHeaders( - responseHeaderModifier.Add, - filterPath.Child(add))..., - ) - - allErrs = append(allErrs, validateResponseHeaders( - responseHeaderModifier.Set, - filterPath.Child(set))..., - ) - - var removeHeaders []v1.HTTPHeader - for _, h := range responseHeaderModifier.Remove { - removeHeaders = append(removeHeaders, v1.HTTPHeader{Name: v1.HTTPHeaderName(h)}) - } - - allErrs = append(allErrs, validateResponseHeaders( - removeHeaders, - filterPath.Child(remove))..., - ) - - return allErrs -} - -func validateResponseHeaders( - headers []v1.HTTPHeader, - path *field.Path, -) field.ErrorList { - var allErrs field.ErrorList - disallowedResponseHeaderSet := map[string]struct{}{ - "server": {}, - "date": {}, - "x-pad": {}, - "content-type": {}, - "content-length": {}, - "connection": {}, - } - invalidPrefix := "x-accel" - - for _, h := range headers { - valErr := field.Invalid(path, h, "header name is not allowed") - name := strings.ToLower(string(h.Name)) - if _, exists := disallowedResponseHeaderSet[name]; exists || strings.HasPrefix(name, strings.ToLower(invalidPrefix)) { - allErrs = append(allErrs, valErr) - } - } - - return allErrs -} - -func validateRequestHeadersCaseInsensitiveUnique( - headers []v1.HTTPHeader, - path *field.Path, -) field.ErrorList { - var allErrs field.ErrorList - - seen := make(map[string]struct{}) - - for _, h := range headers { - name := strings.ToLower(string(h.Name)) - if _, exists := seen[name]; exists { - valErr := field.Invalid(path, h, "header name is not unique") - allErrs = append(allErrs, valErr) - } - seen[name] = struct{}{} - } - - return allErrs -} - -func validateRequestHeaderStringCaseInsensitiveUnique(headers []string, path *field.Path) field.ErrorList { - var allErrs field.ErrorList - - seen := make(map[string]struct{}) - - for _, h := range headers { - name := strings.ToLower(h) - if _, exists := seen[name]; exists { - valErr := field.Invalid(path, h, "header name is not unique") - allErrs = append(allErrs, valErr) - } - seen[name] = struct{}{} - } - - return allErrs -} diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go index ede2abcbd7..60af11914a 100644 --- a/internal/mode/static/state/graph/httproute_test.go +++ b/internal/mode/static/state/graph/httproute_test.go @@ -904,42 +904,33 @@ func TestValidateFilterRedirect(t *testing.T) { } tests := []struct { - filter gatewayv1.HTTPRouteFilter - validator *validationfakes.FakeHTTPFieldsValidator - name string - expectErrCount int + requestRedirect *gatewayv1.HTTPRequestRedirectFilter + validator *validationfakes.FakeHTTPFieldsValidator + name string + expectErrCount int }{ { - validator: &validationfakes.FakeHTTPFieldsValidator{}, - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: nil, - }, - name: "nil filter", - expectErrCount: 1, + validator: &validationfakes.FakeHTTPFieldsValidator{}, + requestRedirect: nil, + name: "nil filter", + expectErrCount: 1, }, { validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("http"), - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]("example.com"), - Port: helpers.GetPointer[gatewayv1.PortNumber](80), - StatusCode: helpers.GetPointer(301), - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]("example.com"), + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + StatusCode: helpers.GetPointer(301), }, expectErrCount: 0, name: "valid redirect filter", }, { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{}, - }, - expectErrCount: 0, - name: "valid redirect filter with no fields set", + validator: createAllValidValidator(), + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{}, + expectErrCount: 0, + name: "valid redirect filter with no fields set", }, { validator: func() *validationfakes.FakeHTTPFieldsValidator { @@ -947,11 +938,8 @@ func TestValidateFilterRedirect(t *testing.T) { validator.ValidateRedirectSchemeReturns(false, []string{"valid-scheme"}) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("http"), // any value is invalid by the validator - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), // any value is invalid by the validator }, expectErrCount: 1, name: "redirect filter with invalid scheme", @@ -962,13 +950,10 @@ func TestValidateFilterRedirect(t *testing.T) { validator.ValidateHostnameReturns(errors.New("invalid hostname")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( - "example.com", - ), // any value is invalid by the validator - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( + "example.com", + ), // any value is invalid by the validator }, expectErrCount: 1, name: "redirect filter with invalid hostname", @@ -979,22 +964,16 @@ func TestValidateFilterRedirect(t *testing.T) { validator.ValidateRedirectPortReturns(errors.New("invalid port")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Port: helpers.GetPointer[gatewayv1.PortNumber](80), // any value is invalid by the validator - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Port: helpers.GetPointer[gatewayv1.PortNumber](80), // any value is invalid by the validator }, expectErrCount: 1, name: "redirect filter with invalid port", }, { validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Path: &gatewayv1.HTTPPathModifier{}, - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Path: &gatewayv1.HTTPPathModifier{}, }, expectErrCount: 1, name: "redirect filter with unsupported path modifier", @@ -1005,11 +984,8 @@ func TestValidateFilterRedirect(t *testing.T) { validator.ValidateRedirectStatusCodeReturns(false, []string{"200"}) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - StatusCode: helpers.GetPointer(301), // any value is invalid by the validator - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + StatusCode: helpers.GetPointer(301), // any value is invalid by the validator }, expectErrCount: 1, name: "redirect filter with invalid status code", @@ -1021,16 +997,13 @@ func TestValidateFilterRedirect(t *testing.T) { validator.ValidateRedirectPortReturns(errors.New("invalid port")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestRedirect, - RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( - "example.com", - ), // any value is invalid by the validator - Port: helpers.GetPointer[gatewayv1.PortNumber]( - 80, - ), // any value is invalid by the validator - }, + requestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( + "example.com", + ), // any value is invalid by the validator + Port: helpers.GetPointer[gatewayv1.PortNumber]( + 80, + ), // any value is invalid by the validator }, expectErrCount: 2, name: "redirect filter with multiple errors", @@ -1043,7 +1016,7 @@ func TestValidateFilterRedirect(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - allErrs := validateFilterRedirect(test.validator, test.filter, filterPath) + allErrs := validateFilterRedirect(test.validator, test.requestRedirect, filterPath) g.Expect(allErrs).To(HaveLen(test.expectErrCount)) }) } @@ -1051,41 +1024,32 @@ func TestValidateFilterRedirect(t *testing.T) { func TestValidateFilterRewrite(t *testing.T) { tests := []struct { - filter gatewayv1.HTTPRouteFilter + urlRewrite *gatewayv1.HTTPURLRewriteFilter validator *validationfakes.FakeHTTPFieldsValidator name string expectErrCount int }{ { - validator: &validationfakes.FakeHTTPFieldsValidator{}, - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: nil, - }, + validator: &validationfakes.FakeHTTPFieldsValidator{}, + urlRewrite: nil, name: "nil filter", expectErrCount: 1, }, { validator: &validationfakes.FakeHTTPFieldsValidator{}, - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]("example.com"), - Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.FullPathHTTPPathModifier, - ReplaceFullPath: helpers.GetPointer("/path"), - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]("example.com"), + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.FullPathHTTPPathModifier, + ReplaceFullPath: helpers.GetPointer("/path"), }, }, expectErrCount: 0, name: "valid rewrite filter", }, { - validator: &validationfakes.FakeHTTPFieldsValidator{}, - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{}, - }, + validator: &validationfakes.FakeHTTPFieldsValidator{}, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{}, expectErrCount: 0, name: "valid rewrite filter with no fields set", }, @@ -1095,25 +1059,19 @@ func TestValidateFilterRewrite(t *testing.T) { validator.ValidateHostnameReturns(errors.New("invalid hostname")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( - "example.com", - ), // any value is invalid by the validator - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( + "example.com", + ), // any value is invalid by the validator }, expectErrCount: 1, name: "rewrite filter with invalid hostname", }, { validator: &validationfakes.FakeHTTPFieldsValidator{}, - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Path: &gatewayv1.HTTPPathModifier{ - Type: "bad-type", - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Path: &gatewayv1.HTTPPathModifier{ + Type: "bad-type", }, }, expectErrCount: 1, @@ -1125,14 +1083,11 @@ func TestValidateFilterRewrite(t *testing.T) { validator.ValidateRewritePathReturns(errors.New("invalid path value")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.FullPathHTTPPathModifier, - ReplaceFullPath: helpers.GetPointer("/path"), - }, // any value is invalid by the validator - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.FullPathHTTPPathModifier, + ReplaceFullPath: helpers.GetPointer("/path"), + }, // any value is invalid by the validator }, expectErrCount: 1, name: "rewrite filter with invalid full path", @@ -1143,14 +1098,11 @@ func TestValidateFilterRewrite(t *testing.T) { validator.ValidateRewritePathReturns(errors.New("invalid path")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.PrefixMatchHTTPPathModifier, - ReplacePrefixMatch: helpers.GetPointer("/path"), - }, // any value is invalid by the validator - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: helpers.GetPointer("/path"), + }, // any value is invalid by the validator }, expectErrCount: 1, name: "rewrite filter with invalid prefix path", @@ -1162,17 +1114,14 @@ func TestValidateFilterRewrite(t *testing.T) { validator.ValidateRewritePathReturns(errors.New("invalid path")) return validator }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( - "example.com", - ), // any value is invalid by the validator - Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.PrefixMatchHTTPPathModifier, - ReplacePrefixMatch: helpers.GetPointer("/path"), - }, // any value is invalid by the validator - }, + urlRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]( + "example.com", + ), // any value is invalid by the validator + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: helpers.GetPointer("/path"), + }, // any value is invalid by the validator }, expectErrCount: 2, name: "rewrite filter with multiple errors", @@ -1184,329 +1133,7 @@ func TestValidateFilterRewrite(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - allErrs := validateFilterRewrite(test.validator, test.filter, filterPath) - g.Expect(allErrs).To(HaveLen(test.expectErrCount)) - }) - } -} - -func TestValidateFilterRequestHeaderModifier(t *testing.T) { - createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { - v := &validationfakes.FakeHTTPFieldsValidator{} - return v - } - - tests := []struct { - filter gatewayv1.HTTPRouteFilter - validator *validationfakes.FakeHTTPFieldsValidator - name string - expectErrCount int - }{ - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "MyBespokeHeader", Value: "my-value"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "gzip"}, - }, - Remove: []string{"Cache-Control"}, - }, - }, - expectErrCount: 0, - name: "valid request header modifier filter", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: nil, - }, - expectErrCount: 1, - name: "nil request header modifier filter", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Add: []gatewayv1.HTTPHeader{ - {Name: "$var_name", Value: "gzip"}, - }, - }, - }, - expectErrCount: 1, - name: "request header modifier filter with invalid add", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Remove: []string{"$var-name"}, - }, - }, - expectErrCount: 1, - name: "request header modifier filter with invalid remove", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "yhu$"}, - }, - }, - }, - expectErrCount: 1, - name: "request header modifier filter with invalid header value", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "Host", Value: "my_host"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "}90yh&$", Value: "gzip$"}, - {Name: "}67yh&$", Value: "compress$"}, - }, - Remove: []string{"Cache-Control$}"}, - }, - }, - expectErrCount: 7, - name: "request header modifier filter all fields invalid", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, - RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "MyBespokeHeader", Value: "my-value"}, - {Name: "mYbespokeHEader", Value: "duplicate"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "gzip"}, - {Name: "accept-encodING", Value: "gzip"}, - }, - Remove: []string{"Cache-Control", "cache-control"}, - }, - }, - expectErrCount: 3, - name: "request header modifier filter not unique names", - }, - } - - filterPath := field.NewPath("test") - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - allErrs := validateFilterHeaderModifier( - test.validator, test.filter.RequestHeaderModifier, filterPath, - ) - g.Expect(allErrs).To(HaveLen(test.expectErrCount)) - }) - } -} - -func TestValidateFilterResponseHeaderModifier(t *testing.T) { - createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { - v := &validationfakes.FakeHTTPFieldsValidator{} - return v - } - - tests := []struct { - filter gatewayv1.HTTPRouteFilter - validator *validationfakes.FakeHTTPFieldsValidator - name string - expectErrCount int - }{ - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "MyBespokeHeader", Value: "my-value"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "gzip"}, - }, - Remove: []string{"Cache-Control"}, - }, - }, - expectErrCount: 0, - name: "valid response header modifier filter", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: nil, - }, - expectErrCount: 1, - name: "nil response header modifier filter", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Add: []gatewayv1.HTTPHeader{ - {Name: "$var_name", Value: "gzip"}, - }, - }, - }, - expectErrCount: 1, - name: "response header modifier filter with invalid add", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Remove: []string{"$var-name"}, - }, - }, - expectErrCount: 1, - name: "response header modifier filter with invalid remove", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "yhu$"}, - }, - }, - }, - expectErrCount: 1, - name: "response header modifier filter with invalid header value", - }, - { - validator: func() *validationfakes.FakeHTTPFieldsValidator { - v := createAllValidValidator() - v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) - v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) - return v - }(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "Host", Value: "my_host"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "}90yh&$", Value: "gzip$"}, - {Name: "}67yh&$", Value: "compress$"}, - }, - Remove: []string{"Cache-Control$}"}, - }, - }, - expectErrCount: 7, - name: "response header modifier filter all fields invalid", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "MyBespokeHeader", Value: "my-value"}, - {Name: "mYbespokeHEader", Value: "duplicate"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "Accept-Encoding", Value: "gzip"}, - {Name: "accept-encodING", Value: "gzip"}, - }, - Remove: []string{"Cache-Control", "cache-control"}, - }, - }, - expectErrCount: 3, - name: "response header modifier filter not unique names", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "Content-Length", Value: "163"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "Content-Type", Value: "text/plain"}, - }, - Remove: []string{"X-Pad"}, - }, - }, - expectErrCount: 3, - name: "response header modifier filter with disallowed header name", - }, - { - validator: createAllValidValidator(), - filter: gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, - ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ - Set: []gatewayv1.HTTPHeader{ - {Name: "X-Accel-Redirect", Value: "/protected/iso.img"}, - }, - Add: []gatewayv1.HTTPHeader{ - {Name: "X-Accel-Limit-Rate", Value: "1024"}, - }, - Remove: []string{"X-Accel-Charset"}, - }, - }, - expectErrCount: 3, - name: "response header modifier filter with disallowed header name prefix", - }, - } - - filterPath := field.NewPath("test") - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - allErrs := validateFilterResponseHeaderModifier( - test.validator, test.filter.ResponseHeaderModifier, filterPath, - ) + allErrs := validateFilterRewrite(test.validator, test.urlRewrite, filterPath) g.Expect(allErrs).To(HaveLen(test.expectErrCount)) }) } diff --git a/internal/mode/static/state/graph/route_common.go b/internal/mode/static/state/graph/route_common.go index dbfe8a541e..914cbdfeb7 100644 --- a/internal/mode/static/state/graph/route_common.go +++ b/internal/mode/static/state/graph/route_common.go @@ -573,3 +573,162 @@ func validateHeaderMatch( return allErrs } + +func validateFilterHeaderModifier( + validator validation.HTTPFieldsValidator, + headerModifier *v1.HTTPHeaderFilter, + filterPath *field.Path, +) field.ErrorList { + if headerModifier == nil { + return field.ErrorList{field.Required(filterPath, "cannot be nil")} + } + + return validateFilterHeaderModifierFields(validator, headerModifier, filterPath) +} + +func validateFilterHeaderModifierFields( + validator validation.HTTPFieldsValidator, + headerModifier *v1.HTTPHeaderFilter, + headerModifierPath *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + // Ensure that the header names are case-insensitive unique + allErrs = append(allErrs, validateRequestHeadersCaseInsensitiveUnique( + headerModifier.Add, + headerModifierPath.Child(add))..., + ) + allErrs = append(allErrs, validateRequestHeadersCaseInsensitiveUnique( + headerModifier.Set, + headerModifierPath.Child(set))..., + ) + allErrs = append(allErrs, validateRequestHeaderStringCaseInsensitiveUnique( + headerModifier.Remove, + headerModifierPath.Child(remove))..., + ) + + for _, h := range headerModifier.Add { + if err := validator.ValidateFilterHeaderName(string(h.Name)); err != nil { + valErr := field.Invalid(headerModifierPath.Child(add), h, err.Error()) + allErrs = append(allErrs, valErr) + } + if err := validator.ValidateFilterHeaderValue(h.Value); err != nil { + valErr := field.Invalid(headerModifierPath.Child(add), h, err.Error()) + allErrs = append(allErrs, valErr) + } + } + for _, h := range headerModifier.Set { + if err := validator.ValidateFilterHeaderName(string(h.Name)); err != nil { + valErr := field.Invalid(headerModifierPath.Child(set), h, err.Error()) + allErrs = append(allErrs, valErr) + } + if err := validator.ValidateFilterHeaderValue(h.Value); err != nil { + valErr := field.Invalid(headerModifierPath.Child(set), h, err.Error()) + allErrs = append(allErrs, valErr) + } + } + for _, h := range headerModifier.Remove { + if err := validator.ValidateFilterHeaderName(h); err != nil { + valErr := field.Invalid(headerModifierPath.Child(remove), h, err.Error()) + allErrs = append(allErrs, valErr) + } + } + + return allErrs +} + +func validateFilterResponseHeaderModifier( + validator validation.HTTPFieldsValidator, + responseHeaderModifier *v1.HTTPHeaderFilter, + filterPath *field.Path, +) field.ErrorList { + if errList := validateFilterHeaderModifier(validator, responseHeaderModifier, filterPath); errList != nil { + return errList + } + var allErrs field.ErrorList + + allErrs = append(allErrs, validateResponseHeaders( + responseHeaderModifier.Add, + filterPath.Child(add))..., + ) + + allErrs = append(allErrs, validateResponseHeaders( + responseHeaderModifier.Set, + filterPath.Child(set))..., + ) + + var removeHeaders []v1.HTTPHeader + for _, h := range responseHeaderModifier.Remove { + removeHeaders = append(removeHeaders, v1.HTTPHeader{Name: v1.HTTPHeaderName(h)}) + } + + allErrs = append(allErrs, validateResponseHeaders( + removeHeaders, + filterPath.Child(remove))..., + ) + + return allErrs +} + +func validateResponseHeaders( + headers []v1.HTTPHeader, + path *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + disallowedResponseHeaderSet := map[string]struct{}{ + "server": {}, + "date": {}, + "x-pad": {}, + "content-type": {}, + "content-length": {}, + "connection": {}, + } + invalidPrefix := "x-accel" + + for _, h := range headers { + valErr := field.Invalid(path, h, "header name is not allowed") + name := strings.ToLower(string(h.Name)) + if _, exists := disallowedResponseHeaderSet[name]; exists || strings.HasPrefix(name, strings.ToLower(invalidPrefix)) { + allErrs = append(allErrs, valErr) + } + } + + return allErrs +} + +func validateRequestHeadersCaseInsensitiveUnique( + headers []v1.HTTPHeader, + path *field.Path, +) field.ErrorList { + var allErrs field.ErrorList + + seen := make(map[string]struct{}) + + for _, h := range headers { + name := strings.ToLower(string(h.Name)) + if _, exists := seen[name]; exists { + valErr := field.Invalid(path, h, "header name is not unique") + allErrs = append(allErrs, valErr) + } + seen[name] = struct{}{} + } + + return allErrs +} + +func validateRequestHeaderStringCaseInsensitiveUnique(headers []string, path *field.Path) field.ErrorList { + var allErrs field.ErrorList + + seen := make(map[string]struct{}) + + for _, h := range headers { + name := strings.ToLower(h) + if _, exists := seen[name]; exists { + valErr := field.Invalid(path, h, "header name is not unique") + allErrs = append(allErrs, valErr) + } + seen[name] = struct{}{} + } + + return allErrs +} diff --git a/internal/mode/static/state/graph/route_common_test.go b/internal/mode/static/state/graph/route_common_test.go index 36a7031783..85d5802b2e 100644 --- a/internal/mode/static/state/graph/route_common_test.go +++ b/internal/mode/static/state/graph/route_common_test.go @@ -16,6 +16,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation/validationfakes" ) func TestBuildSectionNameRefs(t *testing.T) { @@ -1242,3 +1243,325 @@ func TestValidateHostnames(t *testing.T) { }) } } + +func TestValidateFilterRequestHeaderModifier(t *testing.T) { + createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { + v := &validationfakes.FakeHTTPFieldsValidator{} + return v + } + + tests := []struct { + filter gatewayv1.HTTPRouteFilter + validator *validationfakes.FakeHTTPFieldsValidator + name string + expectErrCount int + }{ + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "MyBespokeHeader", Value: "my-value"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "gzip"}, + }, + Remove: []string{"Cache-Control"}, + }, + }, + expectErrCount: 0, + name: "valid request header modifier filter", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: nil, + }, + expectErrCount: 1, + name: "nil request header modifier filter", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Add: []gatewayv1.HTTPHeader{ + {Name: "$var_name", Value: "gzip"}, + }, + }, + }, + expectErrCount: 1, + name: "request header modifier filter with invalid add", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Remove: []string{"$var-name"}, + }, + }, + expectErrCount: 1, + name: "request header modifier filter with invalid remove", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "yhu$"}, + }, + }, + }, + expectErrCount: 1, + name: "request header modifier filter with invalid header value", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "Host", Value: "my_host"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "}90yh&$", Value: "gzip$"}, + {Name: "}67yh&$", Value: "compress$"}, + }, + Remove: []string{"Cache-Control$}"}, + }, + }, + expectErrCount: 7, + name: "request header modifier filter all fields invalid", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "MyBespokeHeader", Value: "my-value"}, + {Name: "mYbespokeHEader", Value: "duplicate"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "gzip"}, + {Name: "accept-encodING", Value: "gzip"}, + }, + Remove: []string{"Cache-Control", "cache-control"}, + }, + }, + expectErrCount: 3, + name: "request header modifier filter not unique names", + }, + } + + filterPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + allErrs := validateFilterHeaderModifier( + test.validator, test.filter.RequestHeaderModifier, filterPath, + ) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} + +func TestValidateFilterResponseHeaderModifier(t *testing.T) { + createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator { + v := &validationfakes.FakeHTTPFieldsValidator{} + return v + } + + tests := []struct { + filter gatewayv1.HTTPRouteFilter + validator *validationfakes.FakeHTTPFieldsValidator + name string + expectErrCount int + }{ + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "MyBespokeHeader", Value: "my-value"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "gzip"}, + }, + Remove: []string{"Cache-Control"}, + }, + }, + expectErrCount: 0, + name: "valid response header modifier filter", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: nil, + }, + expectErrCount: 1, + name: "nil response header modifier filter", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Add: []gatewayv1.HTTPHeader{ + {Name: "$var_name", Value: "gzip"}, + }, + }, + }, + expectErrCount: 1, + name: "response header modifier filter with invalid add", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Remove: []string{"$var-name"}, + }, + }, + expectErrCount: 1, + name: "response header modifier filter with invalid remove", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "yhu$"}, + }, + }, + }, + expectErrCount: 1, + name: "response header modifier filter with invalid header value", + }, + { + validator: func() *validationfakes.FakeHTTPFieldsValidator { + v := createAllValidValidator() + v.ValidateFilterHeaderValueReturns(errors.New("Invalid header value")) + v.ValidateFilterHeaderNameReturns(errors.New("Invalid header")) + return v + }(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "Host", Value: "my_host"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "}90yh&$", Value: "gzip$"}, + {Name: "}67yh&$", Value: "compress$"}, + }, + Remove: []string{"Cache-Control$}"}, + }, + }, + expectErrCount: 7, + name: "response header modifier filter all fields invalid", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "MyBespokeHeader", Value: "my-value"}, + {Name: "mYbespokeHEader", Value: "duplicate"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "Accept-Encoding", Value: "gzip"}, + {Name: "accept-encodING", Value: "gzip"}, + }, + Remove: []string{"Cache-Control", "cache-control"}, + }, + }, + expectErrCount: 3, + name: "response header modifier filter not unique names", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "Content-Length", Value: "163"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "Content-Type", Value: "text/plain"}, + }, + Remove: []string{"X-Pad"}, + }, + }, + expectErrCount: 3, + name: "response header modifier filter with disallowed header name", + }, + { + validator: createAllValidValidator(), + filter: gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterResponseHeaderModifier, + ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{ + Set: []gatewayv1.HTTPHeader{ + {Name: "X-Accel-Redirect", Value: "/protected/iso.img"}, + }, + Add: []gatewayv1.HTTPHeader{ + {Name: "X-Accel-Limit-Rate", Value: "1024"}, + }, + Remove: []string{"X-Accel-Charset"}, + }, + }, + expectErrCount: 3, + name: "response header modifier filter with disallowed header name prefix", + }, + } + + filterPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + allErrs := validateFilterResponseHeaderModifier( + test.validator, test.filter.ResponseHeaderModifier, filterPath, + ) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index cdb8896d18..dff2e713b9 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -199,7 +199,10 @@ See the [static-mode]({{< relref "/reference/cli-help.md#static-mode">}}) comman - `matches` - `method`: Partially supported. Only `Exact` type with both `method.service` and `method.method` specified. - `headers`: Partially supported. Only `Exact` type. - - `filters`: Not supported + - `filters` + - `type`: Supported. + - `requestHeaderModifier`: Supported. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. + - `responseHeaderModifier`, `requestMirror`, `extensionRef`: Not supported. - `backendRefs`: Partially supported. Backend ref `filters` are not supported. - `status` - `parents`