From 2e6be16da15538320dc90f9ed2e826e9d11e3525 Mon Sep 17 00:00:00 2001 From: bitliu Date: Thu, 8 Dec 2022 15:00:27 +0800 Subject: [PATCH 1/2] feat: add support for ResponseHeaderModifier Filter Signed-off-by: bitliu --- docs/latest/user/http-request-headers.md | 44 +-- docs/latest/user/http-response-headers.md | 264 ++++++++++++++++++ ...e-with-response-header-filter-adds.in.yaml | 57 ++++ ...-with-response-header-filter-adds.out.yaml | 132 +++++++++ ...ter-duplicate-add-multiple-filters.in.yaml | 52 ++++ ...er-duplicate-add-multiple-filters.out.yaml | 120 ++++++++ ...ponse-header-filter-duplicate-adds.in.yaml | 61 ++++ ...onse-header-filter-duplicate-adds.out.yaml | 136 +++++++++ ...-duplicate-remove-multiple-filters.in.yaml | 48 ++++ ...duplicate-remove-multiple-filters.out.yaml | 110 ++++++++ ...se-header-filter-duplicate-removes.in.yaml | 42 +++ ...e-header-filter-duplicate-removes.out.yaml | 103 +++++++ ...-header-filter-empty-header-values.in.yaml | 46 +++ ...header-filter-empty-header-values.out.yaml | 111 ++++++++ ...sponse-header-filter-empty-headers.in.yaml | 48 ++++ ...ponse-header-filter-empty-headers.out.yaml | 110 ++++++++ ...onse-header-filter-invalid-headers.in.yaml | 48 ++++ ...nse-header-filter-invalid-headers.out.yaml | 110 ++++++++ ...-response-header-filter-no-headers.in.yaml | 42 +++ ...response-header-filter-no-headers.out.yaml | 101 +++++++ ...nse-header-filter-no-valid-headers.in.yaml | 42 +++ ...se-header-filter-no-valid-headers.out.yaml | 101 +++++++ ...with-response-header-filter-remove.in.yaml | 43 +++ ...ith-response-header-filter-remove.out.yaml | 106 +++++++ internal/gatewayapi/translator.go | 181 +++++++++++- internal/ir/xds.go | 31 +- internal/ir/xds_test.go | 114 +++++++- internal/ir/zz_generated.deepcopy.go | 10 + .../provider/kubernetes/kubernetes_test.go | 123 +++++++- internal/xds/translator/route.go | 28 ++ .../http-route-response-add-headers.yaml | 28 ++ ...ttp-route-response-add-remove-headers.yaml | 31 ++ .../http-route-response-remove-headers.yaml | 15 + ...p-route-response-add-headers.clusters.yaml | 18 ++ ...-route-response-add-headers.listeners.yaml | 40 +++ ...ttp-route-response-add-headers.routes.yaml | 31 ++ ...-response-add-remove-headers.clusters.yaml | 18 ++ ...response-add-remove-headers.listeners.yaml | 40 +++ ...te-response-add-remove-headers.routes.yaml | 34 +++ ...oute-response-remove-headers.clusters.yaml | 18 ++ ...ute-response-remove-headers.listeners.yaml | 40 +++ ...-route-response-remove-headers.routes.yaml | 13 + internal/xds/translator/translator_test.go | 9 + test/conformance/conformance_test.go | 6 +- 44 files changed, 2836 insertions(+), 69 deletions(-) create mode 100644 docs/latest/user/http-response-headers.md create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-adds.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-adds.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-add-multiple-filters.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-add-multiple-filters.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-adds.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-adds.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-remove-multiple-filters.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-remove-multiple-filters.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-removes.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-duplicate-removes.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-empty-header-values.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-empty-header-values.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-empty-headers.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-empty-headers.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-no-headers.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-no-headers.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-no-valid-headers.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-no-valid-headers.out.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-remove.in.yaml create mode 100644 internal/gatewayapi/testdata/httproute-with-response-header-filter-remove.out.yaml create mode 100644 internal/xds/translator/testdata/in/xds-ir/http-route-response-add-headers.yaml create mode 100644 internal/xds/translator/testdata/in/xds-ir/http-route-response-add-remove-headers.yaml create mode 100644 internal/xds/translator/testdata/in/xds-ir/http-route-response-remove-headers.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.routes.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.routes.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.routes.yaml diff --git a/docs/latest/user/http-request-headers.md b/docs/latest/user/http-request-headers.md index 98b750db39..0fe3b627d5 100644 --- a/docs/latest/user/http-request-headers.md +++ b/docs/latest/user/http-request-headers.md @@ -221,49 +221,7 @@ $ curl -vvv --header "Host: headers.example" "http://${GATEWAY_HOST}:8080/get" - ## Combining Filters -Headers can be added/set/removed in separate filters on the same HTTPRoute and they will all perform as expected - -```shell -cat < GET /get HTTP/1.1 +> Host: headers.example +> User-Agent: curl/7.81.0 +> Accept: */* +> X-Echo-Set-Header: X-Foo: value1 +> +* Mark bundle as not supporting multiuse +< HTTP/1.1 200 OK +< content-type: application/json +< x-content-type-options: nosniff +< content-length: 474 +< x-envoy-upstream-service-time: 0 +< server: envoy +< x-foo: value1 +< add-header: foo +< +... + "headers": { + "Accept": [ + "*/*" + ], + "X-Echo-Set-Header": [ + "X-Foo: value1" + ] +... +``` + +## Setting Response Headers + +Setting headers is similar to adding headers. If the response does not have the header configured by the filter, then it will be added, but unlike [adding response headers](#adding-response-headers) which will append the value of the header if the response already contains it, setting a header will cause the value to be replaced by the value configured in the filter. + +```shell +cat < GET /get HTTP/1.1 +> Host: headers.example +> User-Agent: curl/7.81.0 +> Accept: */* +> X-Echo-Set-Header: set-header: value1 +> +* Mark bundle as not supporting multiuse +< HTTP/1.1 200 OK +< content-type: application/json +< x-content-type-options: nosniff +< content-length: 474 +< x-envoy-upstream-service-time: 0 +< server: envoy +< set-header: foo +< + "headers": { + "Accept": [ + "*/*" + ], + "X-Echo-Set-Header": [ + "set-header": value1" + ] +... +``` + +## Removing Response Headers + +Headers can be removed from a response by simply supplying a list of header names. + +Setting headers is similar to adding headers. If the response does not have the header configured by the filter, then it will be added, but unlike [adding response headers](#adding-response-headers) which will append the value of the header if the response already contains it, setting a header will cause the value to be replaced by the value configured in the filter. + +```shell +cat < GET /get HTTP/1.1 +> Host: headers.example +> User-Agent: curl/7.81.0 +> Accept: */* +> X-Echo-Set-Header: remove-header: value1 +> +* Mark bundle as not supporting multiuse +< HTTP/1.1 200 OK +< content-type: application/json +< x-content-type-options: nosniff +< content-length: 474 +< x-envoy-upstream-service-time: 0 +< server: envoy +< + + "headers": { + "Accept": [ + "*/*" + ], + "X-Echo-Set-Header": [ + "remove-header": value1" + ] +... +``` + +## Combining Filters + +Headers can be added/set/removed in a single filter on the same HTTPRoute and they will all perform as expected + +```shell +cat < 0 { + emptyFilterConfig = false + } + for _, addHeader := range headersToAdd { + emptyFilterConfig = false + if addHeader.Name == "" { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + "ResponseHeaderModifier Filter cannot add a header with an empty name", + ) + // try to process the rest of the headers and produce a valid config. + continue + } + // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names + if strings.Contains(string(addHeader.Name), "/") || strings.Contains(string(addHeader.Name), ":") { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)), + ) + continue + } + // Check if the header is a duplicate + headerKey := string(addHeader.Name) + canAddHeader := true + for _, h := range addResponseHeaders { + if strings.EqualFold(h.Name, headerKey) { + canAddHeader = false + break + } + } + + if !canAddHeader { + continue + } + + newHeader := ir.AddHeader{ + Name: headerKey, + Append: true, + Value: addHeader.Value, + } + + addResponseHeaders = append(addResponseHeaders, newHeader) + } + } + + // Set headers + if headersToSet := headerModifier.Set; headersToSet != nil { + if len(headersToSet) > 0 { + emptyFilterConfig = false + } + for _, setHeader := range headersToSet { + + if setHeader.Name == "" { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + "ResponseHeaderModifier Filter cannot set a header with an empty name", + ) + continue + } + // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names + if strings.Contains(string(setHeader.Name), "/") || strings.Contains(string(setHeader.Name), ":") { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)), + ) + continue + } + + // Check if the header to be set has already been configured + headerKey := string(setHeader.Name) + canAddHeader := true + for _, h := range addResponseHeaders { + if strings.EqualFold(h.Name, headerKey) { + canAddHeader = false + break + } + } + if !canAddHeader { + continue + } + newHeader := ir.AddHeader{ + Name: string(setHeader.Name), + Append: false, + Value: setHeader.Value, + } + + addResponseHeaders = append(addResponseHeaders, newHeader) + } + } + + // Remove response headers + // As far as Envoy is concerned, it is ok to configure a header to be added/set and also in the list of + // headers to remove. It will remove the original header if present and then add/set the header after. + if headersToRemove := headerModifier.Remove; headersToRemove != nil { + if len(headersToRemove) > 0 { + emptyFilterConfig = false + } + for _, removedHeader := range headersToRemove { + if removedHeader == "" { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + "ResponseHeaderModifier Filter cannot remove a header with an empty name", + ) + continue + } + + canRemHeader := true + for _, h := range removeResponseHeaders { + if strings.EqualFold(h, removedHeader) { + canRemHeader = false + break + } + } + if !canRemHeader { + continue + } + + removeResponseHeaders = append(removeResponseHeaders, removedHeader) + + } + } + + // Update the status if the filter failed to configure any valid headers to add/remove + if len(addResponseHeaders) == 0 && len(removeResponseHeaders) == 0 && !emptyFilterConfig { + parentRef.SetCondition(httpRoute, + v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + v1beta1.RouteReasonUnsupportedValue, + "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", + ) + } case v1beta1.HTTPRouteFilterExtensionRef: // "If a reference to a custom filter type cannot be resolved, the filter MUST NOT be skipped. // Instead, requests that would have been processed by that filter MUST receive a HTTP error response." @@ -1111,6 +1266,12 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*v1beta1.HTTPRoute, gateways if len(removeRequestHeaders) > 0 { irRoute.RemoveRequestHeaders = removeRequestHeaders } + if len(addResponseHeaders) > 0 { + irRoute.AddResponseHeaders = addResponseHeaders + } + if len(removeResponseHeaders) > 0 { + irRoute.RemoveResponseHeaders = removeResponseHeaders + } ruleRoutes = append(ruleRoutes, irRoute) } @@ -1179,15 +1340,17 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*v1beta1.HTTPRoute, gateways for _, routeRoute := range routeRoutes { hostRoute := &ir.HTTPRoute{ - Name: fmt.Sprintf("%s-%s", routeRoute.Name, host), - PathMatch: routeRoute.PathMatch, - HeaderMatches: append(headerMatches, routeRoute.HeaderMatches...), - QueryParamMatches: routeRoute.QueryParamMatches, - AddRequestHeaders: routeRoute.AddRequestHeaders, - RemoveRequestHeaders: routeRoute.RemoveRequestHeaders, - Destinations: routeRoute.Destinations, - Redirect: routeRoute.Redirect, - DirectResponse: routeRoute.DirectResponse, + Name: fmt.Sprintf("%s-%s", routeRoute.Name, host), + PathMatch: routeRoute.PathMatch, + HeaderMatches: append(headerMatches, routeRoute.HeaderMatches...), + QueryParamMatches: routeRoute.QueryParamMatches, + AddRequestHeaders: routeRoute.AddRequestHeaders, + RemoveRequestHeaders: routeRoute.RemoveRequestHeaders, + AddResponseHeaders: routeRoute.AddResponseHeaders, + RemoveResponseHeaders: routeRoute.RemoveResponseHeaders, + Destinations: routeRoute.Destinations, + Redirect: routeRoute.Redirect, + DirectResponse: routeRoute.DirectResponse, } // Don't bother copying over the weights unless the route has invalid backends. if routeRoute.BackendWeights.Invalid > 0 { diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 7acdeeb22d..03d13de7d0 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -199,6 +199,10 @@ type HTTPRoute struct { AddRequestHeaders []AddHeader // RemoveRequestHeaders defines a list of headers to be removed from requests. RemoveRequestHeaders []string + // AddResponseHeaders defines header/value sets to be added to the headers of response. + AddResponseHeaders []AddHeader + // RemoveResponseHeaders defines a list of headers to be removed from response. + RemoveResponseHeaders []string // Direct responses to be returned for this route. Takes precedence over Destinations and Redirect. DirectResponse *DirectResponse // Redirections to be returned for this route. Takes precedence over Destinations. @@ -271,6 +275,31 @@ func (h HTTPRoute) Validate() error { } } } + if len(h.AddResponseHeaders) > 0 { + occurred := map[string]bool{} + for _, header := range h.AddResponseHeaders { + if err := header.Validate(); err != nil { + errs = multierror.Append(errs, err) + } + if !occurred[header.Name] { + occurred[header.Name] = true + } else { + errs = multierror.Append(errs, ErrAddHeaderDuplicate) + break + } + } + } + if len(h.RemoveResponseHeaders) > 0 { + occurred := map[string]bool{} + for _, header := range h.RemoveResponseHeaders { + if !occurred[header] { + occurred[header] = true + } else { + errs = multierror.Append(errs, ErrRemoveHeaderDuplicate) + break + } + } + } return errs } @@ -298,7 +327,7 @@ func (r RouteDestination) Validate() error { return errs } -// Add header configures a headder to be added to a request. +// Add header configures a headder to be added to a request or response. // +k8s:deepcopy-gen=true type AddHeader struct { Name string diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go index cf06dab00d..16201df8bb 100644 --- a/internal/ir/xds_test.go +++ b/internal/ir/xds_test.go @@ -214,7 +214,7 @@ var ( }, } - addHeaderHTTPRoute = HTTPRoute{ + addRequestHeaderHTTPRoute = HTTPRoute{ Name: "addheader", PathMatch: &StringMatch{ Exact: ptrTo("addheader"), @@ -238,7 +238,7 @@ var ( }, } - removeHeaderHTTPRoute = HTTPRoute{ + removeRequestHeaderHTTPRoute = HTTPRoute{ Name: "remheader", PathMatch: &StringMatch{ Exact: ptrTo("remheader"), @@ -250,7 +250,7 @@ var ( }, } - addRemoveHeadersDupeHTTPRoute = HTTPRoute{ + addAndRemoveRequestHeadersDupeHTTPRoute = HTTPRoute{ Name: "duplicateheader", PathMatch: &StringMatch{ Exact: ptrTo("duplicateheader"), @@ -274,7 +274,7 @@ var ( }, } - addHeaderEmptyHTTPRoute = HTTPRoute{ + addRequestHeaderEmptyHTTPRoute = HTTPRoute{ Name: "addemptyheader", PathMatch: &StringMatch{ Exact: ptrTo("addemptyheader"), @@ -288,6 +288,80 @@ var ( }, } + addResponseHeaderHTTPRoute = HTTPRoute{ + Name: "addheader", + PathMatch: &StringMatch{ + Exact: ptrTo("addheader"), + }, + AddResponseHeaders: []AddHeader{ + { + Name: "example-header", + Value: "example-value", + Append: true, + }, + { + Name: "example-header-2", + Value: "example-value-2", + Append: false, + }, + { + Name: "empty-header", + Value: "", + Append: false, + }, + }, + } + + removeResponseHeaderHTTPRoute = HTTPRoute{ + Name: "remheader", + PathMatch: &StringMatch{ + Exact: ptrTo("remheader"), + }, + RemoveResponseHeaders: []string{ + "x-request-header", + "example-header", + "another-header", + }, + } + + addAndRemoveResponseHeadersDupeHTTPRoute = HTTPRoute{ + Name: "duplicateheader", + PathMatch: &StringMatch{ + Exact: ptrTo("duplicateheader"), + }, + AddResponseHeaders: []AddHeader{ + { + Name: "example-header", + Value: "example-value", + Append: true, + }, + { + Name: "example-header", + Value: "example-value-2", + Append: false, + }, + }, + RemoveResponseHeaders: []string{ + "x-request-header", + "example-header", + "example-header", + }, + } + + addResponseHeaderEmptyHTTPRoute = HTTPRoute{ + Name: "addemptyheader", + PathMatch: &StringMatch{ + Exact: ptrTo("addemptyheader"), + }, + AddResponseHeaders: []AddHeader{ + { + Name: "", + Value: "example-value", + Append: true, + }, + }, + } + // RouteDestination happyRouteDestination = RouteDestination{ Host: "10.11.12.13", @@ -609,22 +683,42 @@ func TestValidateHTTPRoute(t *testing.T) { }, { name: "add-request-headers-httproute", - input: addHeaderHTTPRoute, + input: addRequestHeaderHTTPRoute, want: nil, }, { name: "remove-request-headers-httproute", - input: removeHeaderHTTPRoute, + input: removeRequestHeaderHTTPRoute, + want: nil, + }, + { + name: "add-remove-request-headers-duplicate", + input: addAndRemoveRequestHeadersDupeHTTPRoute, + want: []error{ErrAddHeaderDuplicate, ErrRemoveHeaderDuplicate}, + }, + { + name: "add-request-header-empty", + input: addRequestHeaderEmptyHTTPRoute, + want: []error{ErrAddHeaderEmptyName}, + }, + { + name: "add-response-headers-httproute", + input: addResponseHeaderHTTPRoute, + want: nil, + }, + { + name: "remove-response-headers-httproute", + input: removeResponseHeaderHTTPRoute, want: nil, }, { - name: "add-remove-headers-duplicate", - input: addRemoveHeadersDupeHTTPRoute, + name: "add-remove-response-headers-duplicate", + input: addAndRemoveResponseHeadersDupeHTTPRoute, want: []error{ErrAddHeaderDuplicate, ErrRemoveHeaderDuplicate}, }, { - name: "add-header-empty", - input: addHeaderEmptyHTTPRoute, + name: "add-response-header-empty", + input: addResponseHeaderEmptyHTTPRoute, want: []error{ErrAddHeaderEmptyName}, }, } diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index a64cfa5768..868eb71875 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -151,6 +151,16 @@ func (in *HTTPRoute) DeepCopyInto(out *HTTPRoute) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.AddResponseHeaders != nil { + in, out := &in.AddResponseHeaders, &out.AddResponseHeaders + *out = make([]AddHeader, len(*in)) + copy(*out, *in) + } + if in.RemoveResponseHeaders != nil { + in, out := &in.RemoveResponseHeaders, &out.RemoveResponseHeaders + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.DirectResponse != nil { in, out := &in.DirectResponse, &out.DirectResponse *out = new(DirectResponse) diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 631cf8d9c6..7ea75f1cd9 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -591,10 +591,10 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, }, { - name: "addheader-httproute", + name: "add-request-header-httproute", route: gwapiv1b1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ - Name: "httproute-addheader-test", + Name: "httproute-add-request-header-test", Namespace: ns.Name, }, Spec: gwapiv1b1.HTTPRouteSpec{ @@ -654,10 +654,10 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, }, { - name: "remheader-httproute", + name: "remove-request-header-httproute", route: gwapiv1b1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ - Name: "httproute-remheader-test", + Name: "httproute-remove-request-header-test", Namespace: ns.Name, }, Spec: gwapiv1b1.HTTPRouteSpec{ @@ -705,6 +705,121 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, }, }, + { + name: "add-response-header-httproute", + route: gwapiv1b1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute-add-response-header-test", + Namespace: ns.Name, + }, + Spec: gwapiv1b1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1b1.CommonRouteSpec{ + ParentRefs: []gwapiv1b1.ParentReference{ + { + Name: gwapiv1b1.ObjectName(gw.Name), + }, + }, + }, + Hostnames: []gwapiv1b1.Hostname{"test.hostname.local"}, + Rules: []gwapiv1b1.HTTPRouteRule{ + { + Matches: []gwapiv1b1.HTTPRouteMatch{ + { + Path: &gwapiv1b1.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gwapiv1b1.PathMatchPathPrefix), + Value: gatewayapi.StringPtr("/addheader/"), + }, + }, + }, + BackendRefs: []gwapiv1b1.HTTPBackendRef{ + { + BackendRef: gwapiv1b1.BackendRef{ + BackendObjectReference: gwapiv1b1.BackendObjectReference{ + Name: "test", + }, + }, + }, + }, + Filters: []gwapiv1b1.HTTPRouteFilter{ + { + Type: gwapiv1b1.HTTPRouteFilterType("ResponseHeaderModifier"), + ResponseHeaderModifier: &gwapiv1b1.HTTPHeaderFilter{ + Add: []gwapiv1b1.HTTPHeader{ + { + Name: gwapiv1b1.HTTPHeaderName("header-1"), + Value: "value-1", + }, + { + Name: gwapiv1b1.HTTPHeaderName("header-2"), + Value: "value-2", + }, + }, + Set: []gwapiv1b1.HTTPHeader{ + { + Name: gwapiv1b1.HTTPHeaderName("header-3"), + Value: "value-3", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "remove-response-header-httproute", + route: gwapiv1b1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute-remove-response-header-test", + Namespace: ns.Name, + }, + Spec: gwapiv1b1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1b1.CommonRouteSpec{ + ParentRefs: []gwapiv1b1.ParentReference{ + { + Name: gwapiv1b1.ObjectName(gw.Name), + }, + }, + }, + Hostnames: []gwapiv1b1.Hostname{"test.hostname.local"}, + Rules: []gwapiv1b1.HTTPRouteRule{ + { + Matches: []gwapiv1b1.HTTPRouteMatch{ + { + Path: &gwapiv1b1.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gwapiv1b1.PathMatchPathPrefix), + Value: gatewayapi.StringPtr("/remheader/"), + }, + }, + }, + BackendRefs: []gwapiv1b1.HTTPBackendRef{ + { + BackendRef: gwapiv1b1.BackendRef{ + BackendObjectReference: gwapiv1b1.BackendObjectReference{ + Name: "test", + }, + }, + }, + }, + Filters: []gwapiv1b1.HTTPRouteFilter{ + { + Type: gwapiv1b1.HTTPRouteFilterType("ResponseHeaderModifier"), + ResponseHeaderModifier: &gwapiv1b1.HTTPHeaderFilter{ + Remove: []string{ + "example-header-1", + "test-header", + "example", + }, + }, + }, + }, + }, + }, + }, + }, + }, } for _, testCase := range testCases { diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index 296a126b05..89c7fc2abe 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -26,6 +26,13 @@ func buildXdsRoute(httpRoute *ir.HTTPRoute) (*route.Route, error) { ret.RequestHeadersToRemove = httpRoute.RemoveRequestHeaders } + if len(httpRoute.AddResponseHeaders) > 0 { + ret.ResponseHeadersToAdd = buildXdsAddedResponseHeaders(httpRoute.AddResponseHeaders) + } + if len(httpRoute.RemoveResponseHeaders) > 0 { + ret.ResponseHeadersToRemove = httpRoute.RemoveResponseHeaders + } + switch { case httpRoute.DirectResponse != nil: ret.Action = &route.Route_DirectResponse{DirectResponse: buildXdsDirectResponseAction(httpRoute.DirectResponse)} @@ -241,3 +248,24 @@ func buildXdsAddedRequestHeaders(headersToAdd []ir.AddHeader) []*core.HeaderValu return ret } + +func buildXdsAddedResponseHeaders(headersToAdd []ir.AddHeader) []*core.HeaderValueOption { + ret := make([]*core.HeaderValueOption, len(headersToAdd)) + + for i, header := range headersToAdd { + ret[i] = &core.HeaderValueOption{ + Header: &core.HeaderValue{ + Key: header.Name, + Value: header.Value, + }, + Append: &wrapperspb.BoolValue{Value: header.Append}, + } + + // Allow empty headers to be set, but don't add the config to do so unless necessary + if header.Value == "" { + ret[i].KeepEmptyValue = true + } + } + + return ret +} diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-headers.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-headers.yaml new file mode 100644 index 0000000000..d4b7fba7c9 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-headers.yaml @@ -0,0 +1,28 @@ +name: "http-route" +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + routes: + - name: "response-header-route" + destinations: + - host: "1.2.3.4" + port: 50000 + addResponseHeaders: + - name: "some-header" + value: "some-value" + append: true + - name: "some-header-2" + value: "some-value" + append: true + - name: "some-header3" + value: "some-value" + append: false + - name: "some-header4" + value: "some-value" + append: false + - name: "empty-header" + value: "" + append: false diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-remove-headers.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-remove-headers.yaml new file mode 100644 index 0000000000..33629740a4 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/http-route-response-add-remove-headers.yaml @@ -0,0 +1,31 @@ +name: "http-route" +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + routes: + - name: "response-header-route" + destinations: + - host: "1.2.3.4" + port: 50000 + addResponseHeaders: + - name: "some-header" + value: "some-value" + append: true + - name: "some-header-2" + value: "some-value" + append: true + - name: "some-header3" + value: "some-value" + append: false + - name: "some-header4" + value: "some-value" + append: false + - name: "empty-header" + value: "" + append: false + removeResponseHeaders: + - "some-header5" + - "some-header6" diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-response-remove-headers.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-response-remove-headers.yaml new file mode 100644 index 0000000000..49fb464caa --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/http-route-response-remove-headers.yaml @@ -0,0 +1,15 @@ +name: "http-route" +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + routes: + - name: "response-header-route" + destinations: + - host: "1.2.3.4" + port: 50000 + removeResponseHeaders: + - "some-header5" + - "some-header6" diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.clusters.yaml new file mode 100644 index 0000000000..c2cde1d7b7 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.clusters.yaml @@ -0,0 +1,18 @@ +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 5s + dnsLookupFamily: V4_ONLY + loadAssignment: + clusterName: response-header-route + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + locality: {} + name: response-header-route + outlierDetection: {} + type: STATIC diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.listeners.yaml new file mode 100644 index 0000000000..6c55557397 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.listeners.yaml @@ -0,0 +1,40 @@ +- accessLog: + - filter: + responseFlagFilter: + flags: + - NR + name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + accessLog: + - name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + rds: + configSource: + apiConfigSource: + apiType: DELTA_GRPC + grpcServices: + - envoyGrpc: + clusterName: xds_cluster + setNodeOnFirstMessageOnly: true + transportApiVersion: V3 + resourceApiVersion: V3 + routeConfigName: first-listener + statPrefix: http + name: first-listener diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.routes.yaml new file mode 100644 index 0000000000..e70f9925b4 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-headers.routes.yaml @@ -0,0 +1,31 @@ +- name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener + routes: + - match: + prefix: / + responseHeadersToAdd: + - append: true + header: + key: some-header + value: some-value + - append: true + header: + key: some-header-2 + value: some-value + - append: false + header: + key: some-header3 + value: some-value + - append: false + header: + key: some-header4 + value: some-value + - append: false + header: + key: empty-header + keepEmptyValue: true + route: + cluster: response-header-route diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.clusters.yaml new file mode 100644 index 0000000000..c2cde1d7b7 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.clusters.yaml @@ -0,0 +1,18 @@ +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 5s + dnsLookupFamily: V4_ONLY + loadAssignment: + clusterName: response-header-route + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + locality: {} + name: response-header-route + outlierDetection: {} + type: STATIC diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.listeners.yaml new file mode 100644 index 0000000000..6c55557397 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.listeners.yaml @@ -0,0 +1,40 @@ +- accessLog: + - filter: + responseFlagFilter: + flags: + - NR + name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + accessLog: + - name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + rds: + configSource: + apiConfigSource: + apiType: DELTA_GRPC + grpcServices: + - envoyGrpc: + clusterName: xds_cluster + setNodeOnFirstMessageOnly: true + transportApiVersion: V3 + resourceApiVersion: V3 + routeConfigName: first-listener + statPrefix: http + name: first-listener diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.routes.yaml new file mode 100644 index 0000000000..4774d7fc23 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-add-remove-headers.routes.yaml @@ -0,0 +1,34 @@ +- name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener + routes: + - match: + prefix: / + responseHeadersToAdd: + - append: true + header: + key: some-header + value: some-value + - append: true + header: + key: some-header-2 + value: some-value + - append: false + header: + key: some-header3 + value: some-value + - append: false + header: + key: some-header4 + value: some-value + - append: false + header: + key: empty-header + keepEmptyValue: true + responseHeadersToRemove: + - some-header5 + - some-header6 + route: + cluster: response-header-route diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.clusters.yaml new file mode 100644 index 0000000000..c2cde1d7b7 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.clusters.yaml @@ -0,0 +1,18 @@ +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 5s + dnsLookupFamily: V4_ONLY + loadAssignment: + clusterName: response-header-route + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + locality: {} + name: response-header-route + outlierDetection: {} + type: STATIC diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.listeners.yaml new file mode 100644 index 0000000000..6c55557397 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.listeners.yaml @@ -0,0 +1,40 @@ +- accessLog: + - filter: + responseFlagFilter: + flags: + - NR + name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + accessLog: + - name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/stdout + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + rds: + configSource: + apiConfigSource: + apiType: DELTA_GRPC + grpcServices: + - envoyGrpc: + clusterName: xds_cluster + setNodeOnFirstMessageOnly: true + transportApiVersion: V3 + resourceApiVersion: V3 + routeConfigName: first-listener + statPrefix: http + name: first-listener diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.routes.yaml new file mode 100644 index 0000000000..ffab196216 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-response-remove-headers.routes.yaml @@ -0,0 +1,13 @@ +- name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener + routes: + - match: + prefix: / + responseHeadersToRemove: + - some-header5 + - some-header6 + route: + cluster: response-header-route diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index 5591363f81..b662283a16 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -48,6 +48,15 @@ func TestTranslate(t *testing.T) { { name: "http-route-request-headers", }, + { + name: "http-route-response-add-headers", + }, + { + name: "http-route-response-remove-headers", + }, + { + name: "http-route-response-add-remove-headers", + }, { name: "http-route-weighted-invalid-backend", }, diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index bc36b0170d..721cfb378c 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -54,14 +54,16 @@ func TestGatewayAPIConformance(t *testing.T) { CleanupBaseResources: *flags.CleanupBaseResources, ValidUniqueListenerPorts: validUniqueListenerPorts, SupportedFeatures: map[suite.SupportedFeature]bool{ - suite.SupportHTTPRouteQueryParamMatching: true, - suite.SupportReferenceGrant: true, + suite.SupportHTTPRouteQueryParamMatching: true, + suite.SupportReferenceGrant: true, + suite.SupportHTTPResponseHeaderModification: true, }, }) cSuite.Setup(t) egTests := []suite.ConformanceTest{ tests.HTTPRouteSimpleSameNamespace, tests.HTTPRouteRequestHeaderModifier, + tests.HTTPRouteResponseHeaderModifier, tests.HTTPRouteQueryParamMatching, tests.HTTPRouteInvalidCrossNamespaceParentRef, tests.HTTPExactPathMatching, From cc739078a4cbc97f0507a857105b58902e9feefb Mon Sep 17 00:00:00 2001 From: bitliu Date: Tue, 13 Dec 2022 22:21:30 +0800 Subject: [PATCH 2/2] merge add header Signed-off-by: bitliu --- internal/xds/translator/route.go | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index 89c7fc2abe..2c2d226834 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -20,14 +20,14 @@ func buildXdsRoute(httpRoute *ir.HTTPRoute) (*route.Route, error) { } if len(httpRoute.AddRequestHeaders) > 0 { - ret.RequestHeadersToAdd = buildXdsAddedRequestHeaders(httpRoute.AddRequestHeaders) + ret.RequestHeadersToAdd = buildXdsAddedHeaders(httpRoute.AddRequestHeaders) } if len(httpRoute.RemoveRequestHeaders) > 0 { ret.RequestHeadersToRemove = httpRoute.RemoveRequestHeaders } if len(httpRoute.AddResponseHeaders) > 0 { - ret.ResponseHeadersToAdd = buildXdsAddedResponseHeaders(httpRoute.AddResponseHeaders) + ret.ResponseHeadersToAdd = buildXdsAddedHeaders(httpRoute.AddResponseHeaders) } if len(httpRoute.RemoveResponseHeaders) > 0 { ret.ResponseHeadersToRemove = httpRoute.RemoveResponseHeaders @@ -228,28 +228,7 @@ func buildXdsDirectResponseAction(res *ir.DirectResponse) *route.DirectResponseA return ret } -func buildXdsAddedRequestHeaders(headersToAdd []ir.AddHeader) []*core.HeaderValueOption { - ret := make([]*core.HeaderValueOption, len(headersToAdd)) - - for i, header := range headersToAdd { - ret[i] = &core.HeaderValueOption{ - Header: &core.HeaderValue{ - Key: header.Name, - Value: header.Value, - }, - Append: &wrapperspb.BoolValue{Value: header.Append}, - } - - // Allow empty headers to be set, but don't add the config to do so unless necessary - if header.Value == "" { - ret[i].KeepEmptyValue = true - } - } - - return ret -} - -func buildXdsAddedResponseHeaders(headersToAdd []ir.AddHeader) []*core.HeaderValueOption { +func buildXdsAddedHeaders(headersToAdd []ir.AddHeader) []*core.HeaderValueOption { ret := make([]*core.HeaderValueOption, len(headersToAdd)) for i, header := range headersToAdd {