From 4859d899833ee6f7258c5681f61b95c603ba2080 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 15 Feb 2024 14:56:36 +0100 Subject: [PATCH 1/8] add unsupported status condition for filters within BackendRef Signed-off-by: Karol Szwaj --- internal/gatewayapi/route.go | 17 +++ ...lid-backend-ref-unsupported-filter.in.yaml | 39 +++++ ...id-backend-ref-unsupported-filter.out.yaml | 133 ++++++++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.in.yaml create mode 100755 internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index c0821a94343..6081e3ea814 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -173,6 +173,15 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe dstAddrTypeMap := make(map[ir.DestinationAddressType]int) for _, backendRef := range rule.BackendRefs { + if len(backendRef.Filters) > 0 { + parentRef.SetCondition(httpRoute, + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + "UnsupportedRefValue", + "The filters field within BackendRef is not supported", + ) + } + ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, httpRoute, resources) if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil { dstAddrTypeMap[*ds.AddressType]++ @@ -468,6 +477,14 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } for _, backendRef := range rule.BackendRefs { + if len(backendRef.Filters) > 0 { + parentRef.SetCondition(grpcRoute, + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + "UnsupportedRefValue", + "The filters field within BackendRef is not supported", + ) + } ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources) for _, route := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.in.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.in.yaml new file mode 100644 index 00000000000..236a10736d2 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.in.yaml @@ -0,0 +1,39 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + rules: + - matches: + - path: + type: Exact + value: "/exact" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + set: + - name: "set-header-1" + value: "some-value" diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml new file mode 100755 index 00000000000..4427ad6c0c6 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -0,0 +1,133 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + rules: + - backendRefs: + - filters: + - responseHeaderModifier: + set: + - name: set-header-1 + value: some-value + type: ResponseHeaderModifier + name: service-1 + port: 8080 + matches: + - path: + type: Exact + value: /exact + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: The filters field within BackendRef is not supported + reason: UnsupportedRefValue + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: '*' + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + exact: /exact + name: "" From f126666775babab538135a2b5434152d90492535 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 15 Feb 2024 16:32:26 +0100 Subject: [PATCH 2/8] continue on unsupported backendref Signed-off-by: Karol Szwaj --- internal/gatewayapi/route.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 6081e3ea814..cb23650959d 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -180,6 +180,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe "UnsupportedRefValue", "The filters field within BackendRef is not supported", ) + continue } ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, httpRoute, resources) @@ -484,6 +485,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe "UnsupportedRefValue", "The filters field within BackendRef is not supported", ) + continue } ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources) for _, route := range ruleRoutes { From 9f2da9ec85b0ad3898cd3f2bce076461b251859d Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 15 Feb 2024 16:37:04 +0100 Subject: [PATCH 3/8] continue on unsupported backendref Signed-off-by: Karol Szwaj --- ...-with-invalid-backend-ref-unsupported-filter.out.yaml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml index 4427ad6c0c6..8f4cd5d718e 100755 --- a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -116,15 +116,6 @@ xdsIR: - backendWeights: invalid: 0 valid: 0 - destination: - name: httproute/default/httproute-1/rule/0 - settings: - - addressType: IP - endpoints: - - host: 7.7.7.7 - port: 8080 - protocol: HTTP - weight: 1 hostname: '*' name: httproute/default/httproute-1/rule/0/match/0/* pathMatch: From 62817b5478f8a3af19fb7217bf7745ab610e26c2 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 23 Feb 2024 20:34:02 +0100 Subject: [PATCH 4/8] add BackendRefContext Signed-off-by: Karol Szwaj --- internal/gatewayapi/contexts.go | 38 ++++++++++++++ internal/gatewayapi/filters.go | 9 +++- internal/gatewayapi/route.go | 49 +++++++++---------- ...id-backend-ref-unsupported-filter.out.yaml | 4 +- internal/gatewayapi/validate.go | 31 +++++++++++- 5 files changed, 101 insertions(+), 30 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 83eb153a94d..1edf13a72fa 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -417,3 +417,41 @@ func (r *RouteParentContext) HasCondition(route RouteContext, condType gwapiv1.R } return false } + +// BackendRefContext represents a generic BackendRef object that can reference Gateway objects. +type BackendRefContext struct { + HTTPBackendRef *gwapiv1.HTTPBackendRef + GRPCBackendRef *v1alpha2.GRPCBackendRef + BackendRef *gwapiv1.BackendRef +} + +// GetBackendRef returns the BackendRef object from BackendRefContext matching route Kind. +func (b *BackendRefContext) GetBackendRef(routeKind gwapiv1.Kind) *gwapiv1.BackendRef { + switch routeKind { + case KindHTTPRoute: + if b.HTTPBackendRef != nil { + return &b.HTTPBackendRef.BackendRef + } + case KindGRPCRoute: + if b.GRPCBackendRef != nil { + return &b.GRPCBackendRef.BackendRef + } + default: + return b.BackendRef + } + return nil +} + +func (b *BackendRefContext) GetHTTPFilters() []gwapiv1.HTTPRouteFilter { + if b.HTTPBackendRef != nil { + return b.HTTPBackendRef.Filters + } + return nil +} + +func (b *BackendRefContext) GetGRPCFilters() []v1alpha2.GRPCRouteFilter { + if b.GRPCBackendRef != nil { + return b.GRPCBackendRef.Filters + } + return nil +} diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 73dcf54ca7f..8958bb78af3 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -709,15 +709,20 @@ func (t *Translator) processRequestMirrorFilter( Weight: &weight, } + backendRefContext := BackendRefContext{ + HTTPBackendRef: &gwapiv1.HTTPBackendRef{ + BackendRef: mirrorBackendRef, + }, + } // This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter? filterNs := filterContext.Route.GetNamespace() serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs) - if !t.validateBackendRef(&mirrorBackendRef, filterContext.ParentRef, filterContext.Route, + if !t.validateBackendRef(backendRefContext, filterContext.ParentRef, filterContext.Route, resources, serviceNamespace, KindHTTPRoute) { return } - ds, _ := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources) + ds, _ := t.processDestination(backendRefContext, filterContext.ParentRef, filterContext.Route, resources) newMirror := &ir.RouteDestination{ Name: fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx), diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index cb23650959d..3bddc3d017d 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -173,17 +173,11 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe dstAddrTypeMap := make(map[ir.DestinationAddressType]int) for _, backendRef := range rule.BackendRefs { - if len(backendRef.Filters) > 0 { - parentRef.SetCondition(httpRoute, - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - "UnsupportedRefValue", - "The filters field within BackendRef is not supported", - ) - continue + backendRefContext := BackendRefContext{ + HTTPBackendRef: &backendRef, } - ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, httpRoute, resources) + ds, backendWeight := t.processDestination(backendRefContext, parentRef, httpRoute, resources) if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil { dstAddrTypeMap[*ds.AddressType]++ } @@ -478,16 +472,11 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } for _, backendRef := range rule.BackendRefs { - if len(backendRef.Filters) > 0 { - parentRef.SetCondition(grpcRoute, - gwapiv1.RouteConditionResolvedRefs, - metav1.ConditionFalse, - "UnsupportedRefValue", - "The filters field within BackendRef is not supported", - ) - continue + backendRefContext := BackendRefContext{ + GRPCBackendRef: &backendRef, } - ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources) + + ds, backendWeight := t.processDestination(backendRefContext, parentRef, grpcRoute, resources) for _, route := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip // processing any destinations for this route. @@ -741,7 +730,10 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour for _, rule := range tlsRoute.Spec.Rules { for _, backendRef := range rule.BackendRefs { backendRef := backendRef - ds, _ := t.processDestination(backendRef, parentRef, tlsRoute, resources) + backendRefContext := BackendRefContext{ + BackendRef: &backendRef, + } + ds, _ := t.processDestination(backendRefContext, parentRef, tlsRoute, resources) if ds != nil { destSettings = append(destSettings, ds) } @@ -879,7 +871,10 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour } backendRef := udpRoute.Spec.Rules[0].BackendRefs[0] - ds, _ := t.processDestination(backendRef, parentRef, udpRoute, resources) + backendRefContext := BackendRefContext{ + BackendRef: &backendRef, + } + ds, _ := t.processDestination(backendRefContext, parentRef, udpRoute, resources) // Skip further processing if route destination is not valid if ds == nil || len(ds.Endpoints) == 0 { continue @@ -1011,7 +1006,10 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour } backendRef := tcpRoute.Spec.Rules[0].BackendRefs[0] - ds, _ := t.processDestination(backendRef, parentRef, tcpRoute, resources) + backendRefContext := BackendRefContext{ + BackendRef: &backendRef, + } + ds, _ := t.processDestination(backendRefContext, parentRef, tcpRoute, resources) // Skip further processing if route destination is not valid if ds == nil || len(ds.Endpoints) == 0 { continue @@ -1087,20 +1085,19 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour // processDestination takes a backendRef and translates it into destination setting or sets error statuses and // returns the weight for the backend so that 500 error responses can be returned for invalid backends in // the same proportion as the backend would have otherwise received -func (t *Translator) processDestination(backendRef gwapiv1.BackendRef, +func (t *Translator) processDestination(backendRefContext BackendRefContext, parentRef *RouteParentContext, route RouteContext, resources *Resources) (ds *ir.DestinationSetting, backendWeight uint32) { - + routeType := GetRouteType(route) weight := uint32(1) + backendRef := backendRefContext.GetBackendRef(routeType) if backendRef.Weight != nil { weight = uint32(*backendRef.Weight) } backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace()) - - routeType := GetRouteType(route) - if !t.validateBackendRef(&backendRef, parentRef, route, resources, backendNamespace, routeType) { + if !t.validateBackendRef(backendRefContext, parentRef, route, resources, backendNamespace, routeType) { return nil, weight } diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml index 8f4cd5d718e..ca8a4298fae 100755 --- a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -114,8 +114,10 @@ xdsIR: port: 10080 routes: - backendWeights: - invalid: 0 + invalid: 1 valid: 0 + directResponse: + statusCode: 500 hostname: '*' name: httproute/default/httproute-1/rule/0/match/0/* pathMatch: diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 5c5c50098e1..dc76ceeb68b 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -22,8 +22,13 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) -func (t *Translator) validateBackendRef(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext, +func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, parentRef *RouteParentContext, route RouteContext, resources *Resources, backendNamespace string, routeKind gwapiv1.Kind) bool { + if !t.validateBackendRefFilters(backendRefContext, parentRef, route, routeKind) { + return false + } + backendRef := backendRefContext.GetBackendRef(routeKind) + if !t.validateBackendRefGroup(backendRef, parentRef, route) { return false } @@ -80,6 +85,30 @@ func (t *Translator) validateBackendRefKind(backendRef *gwapiv1a2.BackendRef, pa return true } +func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, parentRef *RouteParentContext, route RouteContext, routeKind gwapiv1.Kind) bool { + var filtersLen int + switch routeKind { + case KindHTTPRoute: + filtersLen = len(backendRef.GetHTTPFilters()) + case KindGRPCRoute: + filtersLen = len(backendRef.GetGRPCFilters()) + default: + return true + } + + if filtersLen > 0 { + parentRef.SetCondition(route, + gwapiv1.RouteConditionResolvedRefs, + metav1.ConditionFalse, + "UnsupportedRefValue", + "The filters field within BackendRef is not supported", + ) + return false + } + + return true +} + func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext, resources *Resources, routeKind gwapiv1.Kind) bool { if backendRef.Namespace != nil && string(*backendRef.Namespace) != "" && string(*backendRef.Namespace) != route.GetNamespace() { From 28e361775f8b6d3ad68a3babf13d6b06523fd9d7 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 23 Feb 2024 23:30:17 +0100 Subject: [PATCH 5/8] fix gosec lint Signed-off-by: Karol Szwaj --- internal/gatewayapi/route.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 3bddc3d017d..55401ea268e 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -173,6 +173,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe dstAddrTypeMap := make(map[ir.DestinationAddressType]int) for _, backendRef := range rule.BackendRefs { + backendRef := backendRef backendRefContext := BackendRefContext{ HTTPBackendRef: &backendRef, } @@ -472,6 +473,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } for _, backendRef := range rule.BackendRefs { + backendRef := backendRef backendRefContext := BackendRefContext{ GRPCBackendRef: &backendRef, } From 411513a37c9dccc4c0996f0dc70e4acb407ded55 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 23 Feb 2024 23:33:53 +0100 Subject: [PATCH 6/8] clean up old funcs Signed-off-by: Karol Szwaj --- internal/gatewayapi/contexts.go | 14 -------------- internal/gatewayapi/validate.go | 4 ++-- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 1edf13a72fa..e837a941645 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -441,17 +441,3 @@ func (b *BackendRefContext) GetBackendRef(routeKind gwapiv1.Kind) *gwapiv1.Backe } return nil } - -func (b *BackendRefContext) GetHTTPFilters() []gwapiv1.HTTPRouteFilter { - if b.HTTPBackendRef != nil { - return b.HTTPBackendRef.Filters - } - return nil -} - -func (b *BackendRefContext) GetGRPCFilters() []v1alpha2.GRPCRouteFilter { - if b.GRPCBackendRef != nil { - return b.GRPCBackendRef.Filters - } - return nil -} diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index dc76ceeb68b..b98a9126527 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -89,9 +89,9 @@ func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, par var filtersLen int switch routeKind { case KindHTTPRoute: - filtersLen = len(backendRef.GetHTTPFilters()) + filtersLen = len(backendRef.HTTPBackendRef.Filters) case KindGRPCRoute: - filtersLen = len(backendRef.GetGRPCFilters()) + filtersLen = len(backendRef.GRPCBackendRef.Filters) default: return true } From b6c2a60ab907e148f907cc11854d92b0bdc1b4b4 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Mon, 26 Feb 2024 10:46:11 +0100 Subject: [PATCH 7/8] dedup code Signed-off-by: Karol Szwaj --- internal/gatewayapi/contexts.go | 35 ++++++++----------- internal/gatewayapi/filters.go | 17 ++++----- internal/gatewayapi/route.go | 29 ++++----------- ...id-backend-ref-unsupported-filter.out.yaml | 1 + internal/gatewayapi/validate.go | 8 +++-- 5 files changed, 34 insertions(+), 56 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index e837a941645..04bfe6abf2e 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -418,26 +418,21 @@ func (r *RouteParentContext) HasCondition(route RouteContext, condType gwapiv1.R return false } -// BackendRefContext represents a generic BackendRef object that can reference Gateway objects. -type BackendRefContext struct { - HTTPBackendRef *gwapiv1.HTTPBackendRef - GRPCBackendRef *v1alpha2.GRPCBackendRef - BackendRef *gwapiv1.BackendRef -} +// BackendRefContext represents a generic BackendRef object that can reference different BackendRef objects. +type BackendRefContext any + +func GetBackendRef(b BackendRefContext) *gwapiv1.BackendRef { + rv := reflect.ValueOf(b) + br := rv.FieldByName("BackendRef") + if br.IsValid() { + backendRef := br.Interface().(gwapiv1.BackendRef) + return &backendRef -// GetBackendRef returns the BackendRef object from BackendRefContext matching route Kind. -func (b *BackendRefContext) GetBackendRef(routeKind gwapiv1.Kind) *gwapiv1.BackendRef { - switch routeKind { - case KindHTTPRoute: - if b.HTTPBackendRef != nil { - return &b.HTTPBackendRef.BackendRef - } - case KindGRPCRoute: - if b.GRPCBackendRef != nil { - return &b.GRPCBackendRef.BackendRef - } - default: - return b.BackendRef } - return nil + backendRef := b.(gwapiv1.BackendRef) + return &backendRef +} + +func GetFilters(b BackendRefContext) any { + return reflect.ValueOf(b).FieldByName("Filters") } diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 8958bb78af3..c728e1e863b 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -704,25 +704,22 @@ func (t *Translator) processRequestMirrorFilter( // Wrap the filter's BackendObjectReference into a BackendRef so we can use existing tooling to check it weight := int32(1) - mirrorBackendRef := gwapiv1.BackendRef{ - BackendObjectReference: mirrorBackend, - Weight: &weight, - } - - backendRefContext := BackendRefContext{ - HTTPBackendRef: &gwapiv1.HTTPBackendRef{ - BackendRef: mirrorBackendRef, + mirrorBackendRef := gwapiv1.HTTPBackendRef{ + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: mirrorBackend, + Weight: &weight, }, } + // This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter? filterNs := filterContext.Route.GetNamespace() serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs) - if !t.validateBackendRef(backendRefContext, filterContext.ParentRef, filterContext.Route, + if !t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources, serviceNamespace, KindHTTPRoute) { return } - ds, _ := t.processDestination(backendRefContext, filterContext.ParentRef, filterContext.Route, resources) + ds, _ := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources) newMirror := &ir.RouteDestination{ Name: fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx), diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 55401ea268e..b09e08ba7be 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -174,11 +174,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe for _, backendRef := range rule.BackendRefs { backendRef := backendRef - backendRefContext := BackendRefContext{ - HTTPBackendRef: &backendRef, - } - - ds, backendWeight := t.processDestination(backendRefContext, parentRef, httpRoute, resources) + ds, backendWeight := t.processDestination(backendRef, parentRef, httpRoute, resources) if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil { dstAddrTypeMap[*ds.AddressType]++ } @@ -474,11 +470,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe for _, backendRef := range rule.BackendRefs { backendRef := backendRef - backendRefContext := BackendRefContext{ - GRPCBackendRef: &backendRef, - } - - ds, backendWeight := t.processDestination(backendRefContext, parentRef, grpcRoute, resources) + ds, backendWeight := t.processDestination(backendRef, parentRef, grpcRoute, resources) for _, route := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip // processing any destinations for this route. @@ -732,10 +724,7 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour for _, rule := range tlsRoute.Spec.Rules { for _, backendRef := range rule.BackendRefs { backendRef := backendRef - backendRefContext := BackendRefContext{ - BackendRef: &backendRef, - } - ds, _ := t.processDestination(backendRefContext, parentRef, tlsRoute, resources) + ds, _ := t.processDestination(backendRef, parentRef, tlsRoute, resources) if ds != nil { destSettings = append(destSettings, ds) } @@ -873,10 +862,7 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour } backendRef := udpRoute.Spec.Rules[0].BackendRefs[0] - backendRefContext := BackendRefContext{ - BackendRef: &backendRef, - } - ds, _ := t.processDestination(backendRefContext, parentRef, udpRoute, resources) + ds, _ := t.processDestination(backendRef, parentRef, udpRoute, resources) // Skip further processing if route destination is not valid if ds == nil || len(ds.Endpoints) == 0 { continue @@ -1008,10 +994,7 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour } backendRef := tcpRoute.Spec.Rules[0].BackendRefs[0] - backendRefContext := BackendRefContext{ - BackendRef: &backendRef, - } - ds, _ := t.processDestination(backendRefContext, parentRef, tcpRoute, resources) + ds, _ := t.processDestination(backendRef, parentRef, tcpRoute, resources) // Skip further processing if route destination is not valid if ds == nil || len(ds.Endpoints) == 0 { continue @@ -1093,7 +1076,7 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext, resources *Resources) (ds *ir.DestinationSetting, backendWeight uint32) { routeType := GetRouteType(route) weight := uint32(1) - backendRef := backendRefContext.GetBackendRef(routeType) + backendRef := GetBackendRef(backendRefContext) if backendRef.Weight != nil { weight = uint32(*backendRef.Weight) } diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml index ca8a4298fae..f916d303053 100755 --- a/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -119,6 +119,7 @@ xdsIR: directResponse: statusCode: 500 hostname: '*' + isHTTP2: false name: httproute/default/httproute-1/rule/0/match/0/* pathMatch: distinct: false diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index b98a9126527..0bb443c09e5 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -27,7 +27,7 @@ func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, par if !t.validateBackendRefFilters(backendRefContext, parentRef, route, routeKind) { return false } - backendRef := backendRefContext.GetBackendRef(routeKind) + backendRef := GetBackendRef(backendRefContext) if !t.validateBackendRefGroup(backendRef, parentRef, route) { return false @@ -89,9 +89,11 @@ func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, par var filtersLen int switch routeKind { case KindHTTPRoute: - filtersLen = len(backendRef.HTTPBackendRef.Filters) + br := backendRef.(gwapiv1.HTTPBackendRef) + filtersLen = len(br.Filters) case KindGRPCRoute: - filtersLen = len(backendRef.GRPCBackendRef.Filters) + br := backendRef.(gwapiv1a2.GRPCBackendRef) + filtersLen = len(br.Filters) default: return true } From d59e52e5591e6682af9248e3d3d92c7cb50ce9fd Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Tue, 27 Feb 2024 00:39:43 +0100 Subject: [PATCH 8/8] update old code Signed-off-by: Karol Szwaj --- internal/gatewayapi/contexts.go | 4 ++-- internal/gatewayapi/validate.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go index 04bfe6abf2e..1ee327302b1 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -418,7 +418,7 @@ func (r *RouteParentContext) HasCondition(route RouteContext, condType gwapiv1.R return false } -// BackendRefContext represents a generic BackendRef object that can reference different BackendRef objects. +// BackendRefContext represents a generic BackendRef object (HTTPBackendRef, GRPCBackendRef or BackendRef itself) type BackendRefContext any func GetBackendRef(b BackendRefContext) *gwapiv1.BackendRef { @@ -434,5 +434,5 @@ func GetBackendRef(b BackendRefContext) *gwapiv1.BackendRef { } func GetFilters(b BackendRefContext) any { - return reflect.ValueOf(b).FieldByName("Filters") + return reflect.ValueOf(b).FieldByName("Filters").Interface() } diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 0bb443c09e5..e7ddb427a15 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -89,11 +89,11 @@ func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, par var filtersLen int switch routeKind { case KindHTTPRoute: - br := backendRef.(gwapiv1.HTTPBackendRef) - filtersLen = len(br.Filters) + filters := GetFilters(backendRef).([]gwapiv1.HTTPRouteFilter) + filtersLen = len(filters) case KindGRPCRoute: - br := backendRef.(gwapiv1a2.GRPCBackendRef) - filtersLen = len(br.Filters) + filters := GetFilters(backendRef).([]gwapiv1a2.GRPCRouteFilter) + filtersLen = len(filters) default: return true }