fix: add unsupported status condition for filters within BackendRef (e…

…nvoyproxy#2620) * add unsupported status condition for filters within BackendRef Signed-off-by: Karol Szwaj <[email protected]> * continue on unsupported backendref Signed-off-by: Karol Szwaj <[email protected]> * continue on unsupported backendref Signed-off-by: Karol Szwaj <[email protected]> * add BackendRefContext Signed-off-by: Karol Szwaj <[email protected]> * fix gosec lint Signed-off-by: Karol Szwaj <[email protected]> * clean up old funcs Signed-off-by: Karol Szwaj <[email protected]> * dedup code Signed-off-by: Karol Szwaj <[email protected]> * update old code Signed-off-by: Karol Szwaj <[email protected]> --------- Signed-off-by: Karol Szwaj <[email protected]> Signed-off-by: zirain <[email protected]>
zirain · Feb 28, 2024 · ac72796 · ac72796
1 parent 1c3c36f
commit ac72796
Show file tree

Hide file tree

Showing 6 changed files with 231 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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() {