[release/v1.4] cherry-pick: do not return 500 for all requests when part of BackendRefs are invalid

internal/gatewayapi/route.go

@@ -231,11 +231,10 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe
 					err.Reason(),
 				))
 				failedProcessDestination = true
-				continue
 			}
 
-			// ds can be nil if the backendRef weight is 0
-			if ds == nil {
+			// skip backendRefs with weight 0 as they do not affect the traffic distribution
+			if ds.Weight != nil && *ds.Weight == 0 {
 				continue
 			}
 			allDs = append(allDs, ds)
@@ -258,7 +257,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe
 			case irRoute.DirectResponse != nil || irRoute.Redirect != nil:
 			// return 500 if any destination setting is invalid
 			// the error is already added to the error list when processing the destination
-			case failedProcessDestination:
+			case failedProcessDestination && destination.ToBackendWeights().Valid == 0:
 				irRoute.DirectResponse = &ir.CustomResponse{
 					StatusCode: ptr.To(uint32(500)),
 				}
@@ -662,10 +661,10 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe
 					err.Reason(),
 				))
 				failedProcessDestination = true
-				continue
 			}
 
-			if ds == nil {
+			// skip backendRefs with weight 0 as they do not affect the traffic distribution
+			if ds.Weight != nil && *ds.Weight == 0 {
 				continue
 			}
 			allDs = append(allDs, ds)
@@ -684,7 +683,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe
 			case irRoute.DirectResponse != nil || irRoute.Redirect != nil:
 			// return 500 if any destination setting is invalid
 			// the error is already added to the error list when processing the destination
-			case failedProcessDestination:
+			case failedProcessDestination && destination.ToBackendWeights().Valid == 0:
 				irRoute.DirectResponse = &ir.CustomResponse{
 					StatusCode: ptr.To(uint32(500)),
 				}
@@ -1100,7 +1099,7 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour
 			}
 
 			// Skip nil destination settings
-			if ds != nil {
+			if ds.Weight != nil && *ds.Weight > 0 {
 				destSettings = append(destSettings, ds)
 			}
 		}
@@ -1259,7 +1258,7 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
 				continue
 			}
 			// Skip nil destination settings
-			if ds != nil {
+			if ds.Weight != nil && *ds.Weight > 0 {
 				destSettings = append(destSettings, ds)
 			}
 		}
@@ -1362,25 +1361,31 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
 func (t *Translator) processDestination(name string, backendRefContext BackendRefContext,
 	parentRef *RouteParentContext, route RouteContext, resources *resource.Resources,
 ) (ds *ir.DestinationSetting, err status.Error) {
-	routeType := GetRouteType(route)
-	weight := uint32(1)
-	backendRef := GetBackendRef(backendRefContext)
-	if backendRef.Weight != nil {
-		weight = uint32(*backendRef.Weight)
+	var (
+		routeType  = GetRouteType(route)
+		backendRef = GetBackendRef(backendRefContext)
+		weight     = (uint32(ptr.Deref(backendRef.Weight, int32(1))))
+	)
+
+	// Create an empty DS without endpoints
+	// This represents an invalid DS.
+	emptyDS := &ir.DestinationSetting{
+		Name:   name,
+		Weight: &weight,
 	}
 
 	backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace())
 	err = t.validateBackendRef(backendRefContext, route, resources, backendNamespace, routeType)
 	{
 		// return with empty endpoint means the backend is invalid and an error to fail the associated route.
 		if err != nil {
-			return nil, err
+			return emptyDS, err
 		}
 	}
 
 	// Skip processing backends with 0 weight
 	if weight == 0 {
-		return nil, nil
+		return emptyDS, nil
 	}
 
 	var envoyProxy *egv1a1.EnvoyProxy
@@ -1422,17 +1427,17 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe
 		ds.IsDynamicResolver,
 	)
 	if tlsErr != nil {
-		return nil, status.NewRouteStatusError(tlsErr, status.RouteReasonInvalidBackendTLS)
+		return emptyDS, status.NewRouteStatusError(tlsErr, status.RouteReasonInvalidBackendTLS)
 	}
 
 	var filtersErr error
 	ds.Filters, filtersErr = t.processDestinationFilters(routeType, backendRefContext, parentRef, route, resources)
 	if filtersErr != nil {
-		return nil, status.NewRouteStatusError(filtersErr, status.RouteReasonInvalidBackendFilters)
+		return emptyDS, status.NewRouteStatusError(filtersErr, status.RouteReasonInvalidBackendFilters)
 	}
 
 	if err := validateDestinationSettings(ds, t.IsEnvoyServiceRouting(envoyProxy), backendRef.Kind); err != nil {
-		return nil, err
+		return emptyDS, err
 	}
 
 	ds.Weight = &weight

internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.in.yaml

@@ -0,0 +1,45 @@
+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
+grpcRoutes:
+- apiVersion: gateway.networking.k8s.io/v1alpha2
+  kind: GRPCRoute
+  metadata:
+    namespace: default
+    name: grpcroute-1
+  spec:
+    parentRefs:
+    - namespace: envoy-gateway
+      name: gateway-1
+      sectionName: http
+    rules:
+    - matches:
+      - method:
+          method: ExampleExact
+          type: Exact
+      backendRefs:
+      - name: service-1
+        port: 8080
+      - name: service-not-exist
+        port: 8080
+services:
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: service-1
+  spec:
+    clusterIP: 7.7.7.7
+    ports:
+    - port: 8080

internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.out.yaml

@@ -0,0 +1,148 @@
+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
+grpcRoutes:
+- apiVersion: gateway.networking.k8s.io/v1alpha2
+  kind: GRPCRoute
+  metadata:
+    creationTimestamp: null
+    name: grpcroute-1
+    namespace: default
+  spec:
+    parentRefs:
+    - name: gateway-1
+      namespace: envoy-gateway
+      sectionName: http
+    rules:
+    - backendRefs:
+      - name: service-1
+        port: 8080
+      - name: service-not-exist
+        port: 8080
+      matches:
+      - method:
+          method: ExampleExact
+          type: Exact
+  status:
+    parents:
+    - conditions:
+      - lastTransitionTime: null
+        message: Route is accepted
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: 'Failed to process route rule 0 backendRef 1: service default/service-not-exist
+          not found.'
+        reason: BackendNotFound
+        status: "False"
+        type: ResolvedRefs
+      controllerName: gateway.envoyproxy.io/gatewayclass-controller
+      parentRef:
+        name: gateway-1
+        namespace: envoy-gateway
+        sectionName: http
+infraIR:
+  envoy-gateway/gateway-1:
+    proxy:
+      listeners:
+      - address: null
+        name: envoy-gateway/gateway-1/http
+        ports:
+        - containerPort: 10080
+          name: http-80
+          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
+      namespace: envoy-gateway-system
+xdsIR:
+  envoy-gateway/gateway-1:
+    accessLog:
+      json:
+      - path: /dev/stdout
+    http:
+    - address: 0.0.0.0
+      hostnames:
+      - '*'
+      isHTTP2: true
+      metadata:
+        kind: Gateway
+        name: gateway-1
+        namespace: envoy-gateway
+        sectionName: http
+      name: envoy-gateway/gateway-1/http
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 10080
+      routes:
+      - destination:
+          name: grpcroute/default/grpcroute-1/rule/0
+          settings:
+          - addressType: IP
+            endpoints:
+            - host: 7.7.7.7
+              port: 8080
+            name: grpcroute/default/grpcroute-1/rule/0/backend/0
+            protocol: GRPC
+            weight: 1
+          - name: grpcroute/default/grpcroute-1/rule/0/backend/1
+            weight: 1
+        headerMatches:
+        - distinct: false
+          name: :path
+          suffix: /ExampleExact
+        hostname: '*'
+        isHTTP2: true
+        metadata:
+          kind: GRPCRoute
+          name: grpcroute-1
+          namespace: default
+        name: grpcroute/default/grpcroute-1/rule/0/match/0/*
+    readyListener:
+      address: 0.0.0.0
+      ipFamily: IPv4
+      path: /ready
+      port: 19003

internal/gatewayapi/testdata/httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml

@@ -298,8 +298,25 @@ xdsIR:
         mergeSlashes: true
       port: 10080
       routes:
-      - directResponse:
-          statusCode: 500
+      - destination:
+          name: httproute/default/httproute-1/rule/0
+          settings:
+          - name: httproute/default/httproute-1/rule/0/backend/0
+            weight: 1
+          - addressType: IP
+            endpoints:
+            - host: 1.1.1.1
+              port: 3001
+            name: httproute/default/httproute-1/rule/0/backend/1
+            protocol: HTTP
+            weight: 1
+          - addressType: FQDN
+            endpoints:
+            - host: primary.foo.com
+              port: 3000
+            name: httproute/default/httproute-1/rule/0/backend/2
+            protocol: HTTP
+            weight: 1
         hostname: '*'
         isHTTP2: false
         metadata:
@@ -311,8 +328,18 @@ xdsIR:
           distinct: false
           name: ""
           prefix: /1
-      - directResponse:
-          statusCode: 500
+      - destination:
+          name: httproute/default/httproute-2/rule/0
+          settings:
+          - name: httproute/default/httproute-2/rule/0/backend/0
+            weight: 1
+          - addressType: IP
+            endpoints:
+            - host: 1.1.1.1
+              port: 3001
+            name: httproute/default/httproute-2/rule/0/backend/1
+            protocol: HTTP
+            weight: 1
         hostname: '*'
         isHTTP2: false
         metadata:
@@ -352,8 +379,18 @@ xdsIR:
           distinct: false
           name: ""
           prefix: /3
-      - directResponse:
-          statusCode: 500
+      - destination:
+          name: httproute/default/httproute-3/rule/0
+          settings:
+          - name: httproute/default/httproute-3/rule/0/backend/0
+            weight: 1
+          - addressType: FQDN
+            endpoints:
+            - host: primary.foo.com
+              port: 3000
+            name: httproute/default/httproute-3/rule/0/backend/1
+            protocol: HTTP
+            weight: 1
         hostname: '*'
         isHTTP2: false
         metadata: