From 8aac49c8cfb892926e14d56953e219d14bf8b3d5 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 18 Mar 2025 05:04:50 +0000 Subject: [PATCH 1/2] host header is not allowed to be modified Signed-off-by: Huabing (Robin) Zhao --- internal/gatewayapi/filters.go | 173 ++++++++---------- ...with-header-filter-invalid-headers.in.yaml | 58 +++++- ...ith-header-filter-invalid-headers.out.yaml | 103 ++++++++++- ...onse-header-filter-invalid-headers.in.yaml | 58 +++++- ...nse-header-filter-invalid-headers.out.yaml | 103 ++++++++++- 5 files changed, 379 insertions(+), 116 deletions(-) diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 02c032d788..c4efbc9055 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -453,28 +453,19 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "RequestHeaderModifier 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), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)), + if !isModifiableHeader(string(addHeader.Name)) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf( + "RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + string(addHeader.Name)), ) continue } @@ -510,27 +501,19 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "RequestHeaderModifier 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), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)), + + if !isModifiableHeader(string(setHeader.Name)) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf( + "RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + string(setHeader.Name)), ) continue } @@ -566,18 +549,23 @@ func (t *Translator) processRequestHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "RequestHeaderModifier Filter cannot remove a header with an empty name", ) continue } + if !isModifiableHeader(removedHeader) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf( + "RequestHeaderModifier Filter cannot remove headers with a '/' or ':' character in them, or the host header. Header: %q", + removedHeader), + ) + continue + } + canRemHeader := true for _, h := range filterContext.RemoveRequestHeaders { if strings.EqualFold(h, removedHeader) { @@ -595,18 +583,31 @@ func (t *Translator) processRequestHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddRequestHeaders) == 0 && len(filterContext.RemoveRequestHeaders) == 0 && !emptyFilterConfig { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "RequestHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", ) } } +func updateRouteStatusForRHMFilter(filterContext *HTTPFiltersContext, message string) { + routeStatus := GetRouteStatus(filterContext.Route) + status.SetRouteStatusCondition(routeStatus, + filterContext.ParentRef.routeParentStatusIdx, + filterContext.Route.GetGeneration(), + gwapiv1.RouteConditionAccepted, + metav1.ConditionFalse, + gwapiv1.RouteReasonUnsupportedValue, + message, + ) +} + +func isModifiableHeader(headerName string) bool { + // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names + // And Envoy does not allow modification the pseudo headers and the host header + return !strings.Contains(headerName, "/") && !strings.Contains(headerName, ":") && !strings.EqualFold(headerName, "host") +} + func (t *Translator) processResponseHeaderModifierFilter( headerModifier *gwapiv1.HTTPHeaderFilter, filterContext *HTTPFiltersContext, @@ -625,28 +626,18 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "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), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)), + if !isModifiableHeader(string(addHeader.Name)) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + string(addHeader.Name)), ) continue } @@ -682,27 +673,18 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "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), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)), + + if !isModifiableHeader(string(setHeader.Name)) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + string(setHeader.Name)), ) continue } @@ -738,17 +720,21 @@ func (t *Translator) processResponseHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "ResponseHeaderModifier Filter cannot remove a header with an empty name", ) continue } + if !isModifiableHeader(removedHeader) { + updateRouteStatusForRHMFilter( + filterContext, + fmt.Sprintf( + "ResponseHeaderModifier Filter cannot remove headers with a '/' or ':' character in them, or the host header. Header: %q", + removedHeader), + ) + continue + } canRemHeader := true for _, h := range filterContext.RemoveResponseHeaders { @@ -768,13 +754,8 @@ func (t *Translator) processResponseHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddResponseHeaders) == 0 && len(filterContext.RemoveResponseHeaders) == 0 && !emptyFilterConfig { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, + updateRouteStatusForRHMFilter( + filterContext, "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", ) } diff --git a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml index aaf5bad87f..869f9122c2 100644 --- a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml +++ b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml @@ -30,7 +30,7 @@ httpRoutes: rules: - matches: - path: - value: "/" + value: "/foo" backendRefs: - name: service-1 port: 8080 @@ -42,7 +42,57 @@ httpRoutes: value: "some-value" - name: "good-header" value: "some-value" - add: - - name: "example/2" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: RequestHeaderModifier + requestHeaderModifier: + remove: + - "example/2" + set: + - name: "good-header" + value: "some-value" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/baz" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: RequestHeaderModifier + requestHeaderModifier: + set: + - name: "host" + value: "example.com" + - name: "good-header" value: "some-value" - diff --git a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml index 8e4e0fff0f..386b4f8437 100644 --- a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml @@ -17,7 +17,7 @@ gateways: protocol: HTTP status: listeners: - - attachedRoutes: 1 + - attachedRoutes: 3 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -60,9 +60,6 @@ httpRoutes: port: 8080 filters: - requestHeaderModifier: - add: - - name: example/2 - value: some-value set: - name: example:1 value: some-value @@ -71,13 +68,107 @@ httpRoutes: type: RequestHeaderModifier matches: - path: - value: / + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'RequestHeaderModifier Filter cannot set headers with a ''/'' or + '':'' character in them, or the host header. Header: "example:1"' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - requestHeaderModifier: + remove: + - example/2 + set: + - name: good-header + value: some-value + type: RequestHeaderModifier + matches: + - path: + value: /bar + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'RequestHeaderModifier Filter cannot remove headers with a ''/'' + or '':'' character in them, or the host header. Header: "example/2"' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - requestHeaderModifier: + set: + - name: host + value: example.com + - name: good-header + value: some-value + type: RequestHeaderModifier + matches: + - path: + value: /baz status: parents: - conditions: - lastTransitionTime: null message: 'RequestHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them. Header: ''example:1''' + '':'' character in them, or the host header. Header: "host"' reason: UnsupportedValue status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml index 6e1e57425b..95da714409 100644 --- a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml +++ b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml @@ -30,7 +30,7 @@ httpRoutes: rules: - matches: - path: - value: "/" + value: "/foo" backendRefs: - name: service-1 port: 8080 @@ -42,7 +42,57 @@ httpRoutes: value: "some-value" - name: "good-header" value: "some-value" - add: - - name: "example/2" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + remove: + - "example/2" + set: + - name: "good-header" + value: "some-value" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/baz" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + set: + - name: "host" + value: "example.com" + - name: "good-header" value: "some-value" - diff --git a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml index 649cbb8e7d..389d1d05bd 100644 --- a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml @@ -17,7 +17,7 @@ gateways: protocol: HTTP status: listeners: - - attachedRoutes: 1 + - attachedRoutes: 3 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -60,9 +60,6 @@ httpRoutes: port: 8080 filters: - responseHeaderModifier: - add: - - name: example/2 - value: some-value set: - name: example:1 value: some-value @@ -71,13 +68,107 @@ httpRoutes: type: ResponseHeaderModifier matches: - path: - value: / + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'ResponseHeaderModifier Filter cannot set headers with a ''/'' or + '':'' character in them, or the host header. Header: "example:1"' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - responseHeaderModifier: + remove: + - example/2 + set: + - name: good-header + value: some-value + type: ResponseHeaderModifier + matches: + - path: + value: /bar + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'ResponseHeaderModifier Filter cannot remove headers with a ''/'' + or '':'' character in them, or the host header. Header: "example/2"' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - responseHeaderModifier: + set: + - name: host + value: example.com + - name: good-header + value: some-value + type: ResponseHeaderModifier + matches: + - path: + value: /baz status: parents: - conditions: - lastTransitionTime: null message: 'ResponseHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them. Header: ''example:1''' + '':'' character in them, or the host header. Header: "host"' reason: UnsupportedValue status: "False" type: Accepted From 653e9a58d1a56d74de5059ebfe68a7dffeca75b5 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 21 Mar 2025 03:13:42 +0000 Subject: [PATCH 2/2] address comment Signed-off-by: Huabing (Robin) Zhao --- internal/gatewayapi/filters.go | 315 +++++------------- ...ith-header-filter-invalid-headers.out.yaml | 14 +- ...nse-header-filter-invalid-headers.out.yaml | 12 +- 3 files changed, 105 insertions(+), 236 deletions(-) diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index c4efbc9055..85fdfb4e3c 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -192,15 +192,9 @@ func (t *Translator) processURLRewriteFilter( if filterContext.URLRewrite != nil { if hasMultipleCoreRewrites(rewrite, filterContext.URLRewrite) || hasConflictingCoreAndExtensionRewrites(rewrite, filterContext.URLRewrite) { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule") return } } @@ -213,15 +207,7 @@ func (t *Translator) processURLRewriteFilter( if rewrite.Hostname != nil { if err := t.validateHostname(string(*rewrite.Hostname)); err != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - err.Error(), - ) + updateRouteStatusForFilter(filterContext, err.Error()) return } redirectHost := string(*rewrite.Hostname) @@ -234,29 +220,15 @@ func (t *Translator) processURLRewriteFilter( switch rewrite.Path.Type { case gwapiv1.FullPathHTTPPathModifier: if rewrite.Path.ReplacePrefixMatch != nil { - errMsg := "ReplacePrefixMatch cannot be set when rewrite path type is \"ReplaceFullPath\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplacePrefixMatch cannot be set when rewrite path type is \"ReplaceFullPath\"") return } if rewrite.Path.ReplaceFullPath == nil { - errMsg := "ReplaceFullPath must be set when rewrite path type is \"ReplaceFullPath\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceFullPath must be set when rewrite path type is \"ReplaceFullPath\"") return } if rewrite.Path.ReplaceFullPath != nil { @@ -268,29 +240,15 @@ func (t *Translator) processURLRewriteFilter( } case gwapiv1.PrefixMatchHTTPPathModifier: if rewrite.Path.ReplaceFullPath != nil { - errMsg := "ReplaceFullPath cannot be set when rewrite path type is \"ReplacePrefixMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceFullPath cannot be set when rewrite path type is \"ReplacePrefixMatch\"") return } if rewrite.Path.ReplacePrefixMatch == nil { - errMsg := "ReplacePrefixMatch must be set when rewrite path type is \"ReplacePrefixMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplacePrefixMatch must be set when rewrite path type is \"ReplacePrefixMatch\"") return } if rewrite.Path.ReplacePrefixMatch != nil { @@ -301,16 +259,11 @@ func (t *Translator) processURLRewriteFilter( } } default: - errMsg := fmt.Sprintf("Rewrite path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", rewrite.Path.Type) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Rewrite path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", + rewrite.Path.Type)) return } } @@ -324,15 +277,9 @@ func (t *Translator) processRedirectFilter( ) { // Can't have two redirects for the same route if filterContext.RedirectResponse != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple requestRedirect filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple requestRedirect filters for a single HTTPRouteRule") return } @@ -347,31 +294,16 @@ func (t *Translator) processRedirectFilter( if *redirect.Scheme == "http" || *redirect.Scheme == "https" { redir.Scheme = redirect.Scheme } else { - errMsg := fmt.Sprintf("Scheme: %s is unsupported, only 'https' and 'http' are supported", *redirect.Scheme) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf("Scheme: %s is unsupported, only 'https' and 'http' are supported", *redirect.Scheme)) return } } if redirect.Hostname != nil { if err := t.validateHostname(string(*redirect.Hostname)); err != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - err.Error(), - ) + updateRouteStatusForFilter(filterContext, err.Error()) } else { redirectHost := string(*redirect.Hostname) redir.Hostname = &redirectHost @@ -393,16 +325,11 @@ func (t *Translator) processRedirectFilter( } } default: - errMsg := fmt.Sprintf("Redirect path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", redirect.Path.Type) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Redirect path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", + redirect.Path.Type)) return } } @@ -414,15 +341,7 @@ func (t *Translator) processRedirectFilter( redir.StatusCode = &redirectCode } else { errMsg := fmt.Sprintf("Status code %d is invalid, only 302 and 301 are supported", redirectCode) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) return } } @@ -453,18 +372,18 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "RequestHeaderModifier Filter cannot add a header with an empty name", - ) + "RequestHeaderModifier Filter cannot add a header with an empty name") // try to process the rest of the headers and produce a valid config. continue } if !isModifiableHeader(string(addHeader.Name)) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, fmt.Sprintf( - "RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + "Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.", string(addHeader.Name)), ) continue @@ -501,18 +420,18 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "RequestHeaderModifier Filter cannot set a header with an empty name", - ) + "RequestHeaderModifier Filter cannot set a header with an empty name") continue } if !isModifiableHeader(string(setHeader.Name)) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, fmt.Sprintf( - "RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", + "Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.", string(setHeader.Name)), ) continue @@ -549,18 +468,18 @@ func (t *Translator) processRequestHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "RequestHeaderModifier Filter cannot remove a header with an empty name", - ) + "RequestHeaderModifier Filter cannot remove a header with an empty name") continue } if !isModifiableHeader(removedHeader) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, fmt.Sprintf( - "RequestHeaderModifier Filter cannot remove headers with a '/' or ':' character in them, or the host header. Header: %q", + "Header: %q. The RequestHeaderModifier filter cannot remove the Host header or headers with a '/' "+ + "or ':' character in them.", removedHeader), ) continue @@ -583,14 +502,13 @@ func (t *Translator) processRequestHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddRequestHeaders) == 0 && len(filterContext.RemoveRequestHeaders) == 0 && !emptyFilterConfig { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "RequestHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", - ) + "RequestHeaderModifier Filter did not provide valid configuration to add/set/remove any headers") } } -func updateRouteStatusForRHMFilter(filterContext *HTTPFiltersContext, message string) { +func updateRouteStatusForFilter(filterContext *HTTPFiltersContext, message string) { routeStatus := GetRouteStatus(filterContext.Route) status.SetRouteStatusCondition(routeStatus, filterContext.ParentRef.routeParentStatusIdx, @@ -626,19 +544,19 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "ResponseHeaderModifier Filter cannot add a header with an empty name", - ) + "ResponseHeaderModifier Filter cannot add a header with an empty name") // try to process the rest of the headers and produce a valid config. continue } if !isModifiableHeader(string(addHeader.Name)) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", - string(addHeader.Name)), - ) + fmt.Sprintf( + "Header: %q. The ResponseHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them.", + string(addHeader.Name))) continue } // Check if the header is a duplicate @@ -673,19 +591,19 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "ResponseHeaderModifier Filter cannot set a header with an empty name", - ) + "ResponseHeaderModifier Filter cannot set a header with an empty name") continue } if !isModifiableHeader(string(setHeader.Name)) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them, or the host header. Header: %q", - string(setHeader.Name)), - ) + fmt.Sprintf( + "Header: %q. The ResponseHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them.", + string(setHeader.Name))) continue } @@ -720,19 +638,18 @@ func (t *Translator) processResponseHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "ResponseHeaderModifier Filter cannot remove a header with an empty name", - ) + "ResponseHeaderModifier Filter cannot remove a header with an empty name") continue } if !isModifiableHeader(removedHeader) { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, fmt.Sprintf( - "ResponseHeaderModifier Filter cannot remove headers with a '/' or ':' character in them, or the host header. Header: %q", - removedHeader), - ) + "Header: %q. The ResponseHeaderModifier filter cannot remove the Host header or headers with a '/' "+ + "or ':' character in them.", + removedHeader)) continue } @@ -754,10 +671,9 @@ func (t *Translator) processResponseHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddResponseHeaders) == 0 && len(filterContext.RemoveResponseHeaders) == 0 && !emptyFilterConfig { - updateRouteStatusForRHMFilter( + updateRouteStatusForFilter( filterContext, - "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", - ) + "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers") } } @@ -779,15 +695,9 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec if filterContext.URLRewrite != nil { if hasMultipleExtensionRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) || hasConflictingExtensionAndCoreRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule") return } } @@ -796,30 +706,16 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec if hrf.Spec.URLRewrite.Path.Type == egv1a1.RegexHTTPPathModifier { if hrf.Spec.URLRewrite.Path.ReplaceRegexMatch == nil || hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern == "" { - errMsg := "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\"") return } else if _, err := regexp.Compile(hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern); err != nil { // Avoid envoy NACKs due to invalid regex. // Golang's regexp is almost identical to RE2: https://pkg.go.dev/regexp/syntax - errMsg := "ReplaceRegexMatch must be a valid RE2 regular expression" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceRegexMatch must be a valid RE2 regular expression") return } @@ -848,16 +744,9 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec var hm *ir.HTTPHostModifier if hrf.Spec.URLRewrite.Hostname.Type == egv1a1.HeaderHTTPHostnameModifier { if hrf.Spec.URLRewrite.Hostname.Header == nil { - errMsg := "Header must be set when rewrite path type is \"Header\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "Header must be set when rewrite path type is \"Header\"") return } hm = &ir.HTTPHostModifier{ @@ -1012,14 +901,7 @@ func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *H gwapiv1.RouteReasonBackendNotFound, errMsg, ) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } @@ -1027,31 +909,16 @@ func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *H func (t *Translator) processUnsupportedHTTPFilter(filterType string, filterContext *HTTPFiltersContext) { errMsg := fmt.Sprintf("Unsupported filter type: %s", filterType) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } } func (t *Translator) processInvalidHTTPFilter(filterType string, filterContext *HTTPFiltersContext, err error) { - errMsg := fmt.Sprintf("Invalid filter %s: %v", filterType, err) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf("Invalid filter %s: %v", filterType, err)) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } diff --git a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml index 386b4f8437..2c2f5cb821 100644 --- a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml @@ -73,8 +73,9 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'RequestHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them, or the host header. Header: "example:1"' + message: 'Header: "example:1". The RequestHeaderModifier filter cannot set + the Host header or headers with a ''/'' or '':'' character in them. To modify + the Host header use the URLRewrite or the HTTPRouteFilter filter.' reason: UnsupportedValue status: "False" type: Accepted @@ -120,8 +121,8 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'RequestHeaderModifier Filter cannot remove headers with a ''/'' - or '':'' character in them, or the host header. Header: "example/2"' + message: 'Header: "example/2". The RequestHeaderModifier filter cannot remove + the Host header or headers with a ''/'' or '':'' character in them.' reason: UnsupportedValue status: "False" type: Accepted @@ -167,8 +168,9 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'RequestHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them, or the host header. Header: "host"' + message: 'Header: "host". The RequestHeaderModifier filter cannot set the + Host header or headers with a ''/'' or '':'' character in them. To modify + the Host header use the URLRewrite or the HTTPRouteFilter filter.' reason: UnsupportedValue status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml index 389d1d05bd..9e476685c7 100644 --- a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml @@ -73,8 +73,8 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'ResponseHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them, or the host header. Header: "example:1"' + message: 'Header: "example:1". The ResponseHeaderModifier filter cannot set + the Host header or headers with a ''/'' or '':'' character in them.' reason: UnsupportedValue status: "False" type: Accepted @@ -120,8 +120,8 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'ResponseHeaderModifier Filter cannot remove headers with a ''/'' - or '':'' character in them, or the host header. Header: "example/2"' + message: 'Header: "example/2". The ResponseHeaderModifier filter cannot remove + the Host header or headers with a ''/'' or '':'' character in them.' reason: UnsupportedValue status: "False" type: Accepted @@ -167,8 +167,8 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: 'ResponseHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them, or the host header. Header: "host"' + message: 'Header: "host". The ResponseHeaderModifier filter cannot set the + Host header or headers with a ''/'' or '':'' character in them.' reason: UnsupportedValue status: "False" type: Accepted