From a64c94bf8b8a179f4a753d83d9cdaca1a5e65def Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Tue, 27 Feb 2024 18:11:40 +0100 Subject: [PATCH] fix: add unsupported status condition for filters within BackendRef (#2620) * add unsupported status condition for filters within BackendRef Signed-off-by: Karol Szwaj * continue on unsupported backendref Signed-off-by: Karol Szwaj * continue on unsupported backendref Signed-off-by: Karol Szwaj * add BackendRefContext Signed-off-by: Karol Szwaj * fix gosec lint Signed-off-by: Karol Szwaj * clean up old funcs Signed-off-by: Karol Szwaj * dedup code Signed-off-by: Karol Szwaj * update old code Signed-off-by: Karol Szwaj --------- Signed-off-by: Karol Szwaj --- internal/gatewayapi/contexts.go | 19 +++ internal/gatewayapi/filters.go | 10 +- internal/gatewayapi/route.go | 15 ++- ...lid-backend-ref-unsupported-filter.in.yaml | 39 ++++++ ...id-backend-ref-unsupported-filter.out.yaml | 127 ++++++++++++++++++ internal/gatewayapi/validate.go | 33 ++++- 6 files changed, 231 insertions(+), 12 deletions(-) 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/contexts.go b/internal/gatewayapi/contexts.go index 83eb153a94d..1ee327302b1 100644 --- a/internal/gatewayapi/contexts.go +++ b/internal/gatewayapi/contexts.go @@ -417,3 +417,22 @@ func (r *RouteParentContext) HasCondition(route RouteContext, condType gwapiv1.R } return false } + +// BackendRefContext represents a generic BackendRef object (HTTPBackendRef, GRPCBackendRef or BackendRef itself) +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 + + } + backendRef := b.(gwapiv1.BackendRef) + return &backendRef +} + +func GetFilters(b BackendRefContext) any { + return reflect.ValueOf(b).FieldByName("Filters").Interface() +} diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 73dcf54ca7f..c728e1e863b 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -704,15 +704,17 @@ 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, + 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(&mirrorBackendRef, filterContext.ParentRef, filterContext.Route, + if !t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources, serviceNamespace, KindHTTPRoute) { return } diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index c0821a94343..b09e08ba7be 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -173,7 +173,8 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe dstAddrTypeMap := make(map[ir.DestinationAddressType]int) for _, backendRef := range rule.BackendRefs { - ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, httpRoute, resources) + backendRef := backendRef + ds, backendWeight := t.processDestination(backendRef, parentRef, httpRoute, resources) if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil { dstAddrTypeMap[*ds.AddressType]++ } @@ -468,7 +469,8 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } for _, backendRef := range rule.BackendRefs { - ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources) + backendRef := backendRef + 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. @@ -1068,20 +1070,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 := GetBackendRef(backendRefContext) 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.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..f916d303053 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-unsupported-filter.out.yaml @@ -0,0 +1,127 @@ +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: 1 + valid: 0 + directResponse: + statusCode: 500 + hostname: '*' + isHTTP2: false + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + exact: /exact + name: "" diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 5c5c50098e1..e7ddb427a15 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 := GetBackendRef(backendRefContext) + if !t.validateBackendRefGroup(backendRef, parentRef, route) { return false } @@ -80,6 +85,32 @@ 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: + filters := GetFilters(backendRef).([]gwapiv1.HTTPRouteFilter) + filtersLen = len(filters) + case KindGRPCRoute: + filters := GetFilters(backendRef).([]gwapiv1a2.GRPCRouteFilter) + filtersLen = len(filters) + 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() {