Fix invalid backend attachment

Author: sjberman

internal/mode/static/state/graph/route_common.go

@@ -42,8 +42,9 @@ type ParentRefAttachmentStatus struct {
 	// AcceptedHostnames is an intersection between the hostnames supported by an attached Listener
 	// and the hostnames from this Route. Key is <gatewayNamespacedName/listenerName>, value is list of hostnames.
 	AcceptedHostnames map[string][]string
-	// FailedConditions are the conditions that describes why the ParentRef is not attached to the Gateway. They are
-	// set when Attached is false.
+	// FailedConditions are the conditions that describe why the ParentRef is not attached to the Gateway, or other
+	// failures that may lead to partial attachments. For example, a backendRef could be invalid, but the route can
+	// still attach. The backendRef condition would be displayed here.
 	FailedConditions []conditions.Condition
 	// ListenerPort is the port on the Listener that the Route is attached to.
 	ListenerPort v1.PortNumber
@@ -549,14 +550,14 @@ func bindL4RouteToListeners(
 
 		attachment, attachableListeners := validateParentRef(ref, gw)
 
-		if cond, ok := route.Spec.BackendRef.InvalidForGateways[gwNsName]; ok {
-			attachment.FailedConditions = append(attachment.FailedConditions, cond)
-		}
-
 		if len(attachment.FailedConditions) > 0 {
 			continue
 		}
 
+		if cond, ok := route.Spec.BackendRef.InvalidForGateways[gwNsName]; ok {
+			attachment.FailedConditions = append(attachment.FailedConditions, cond)
+		}
+
 		// Try to attach Route to all matching listeners
 
 		cond, attached := tryToAttachL4RouteToListeners(
@@ -721,6 +722,10 @@ func bindL7RouteToListeners(
 			)
 		}
 
+		if len(attachment.FailedConditions) > 0 {
+			continue
+		}
+
 		for _, rule := range route.Spec.Rules {
 			for _, backendRef := range rule.BackendRefs {
 				if cond, ok := backendRef.InvalidForGateways[gwNsName]; ok {
@@ -729,10 +734,6 @@ func bindL7RouteToListeners(
 			}
 		}
 
-		if len(attachment.FailedConditions) > 0 {
-			continue
-		}
-
 		// Try to attach Route to all matching listeners
 
 		cond, attached := tryToAttachL7RouteToListeners(

internal/mode/static/state/graph/route_common_test.go

@@ -465,6 +465,19 @@ func TestBindRouteToListeners(t *testing.T) {
 		l.Source.Hostname = helpers.GetPointer[gatewayv1.Hostname]("bar.example.com")
 	})
 
+	routeWithInvalidBackendRefs := createNormalHTTPRoute(gw)
+	routeWithInvalidBackendRefs.Spec.Rules = []RouteRule{
+		{
+			BackendRefs: []BackendRef{
+				{
+					InvalidForGateways: map[types.NamespacedName]conditions.Condition{
+						client.ObjectKeyFromObject(gw): {Message: "invalid backend"},
+					},
+				},
+			},
+		},
+	}
+
 	createGRPCRouteWithSectionNameAndPort := func(
 		sectionName *gatewayv1.SectionName,
 		port *gatewayv1.PortNumber,
@@ -1290,6 +1303,43 @@ func TestBindRouteToListeners(t *testing.T) {
 			},
 			name: "http route allowed when listener kind is HTTPRoute",
 		},
+		{
+			route: routeWithInvalidBackendRefs,
+			gateway: &Gateway{
+				Source: gw,
+				Valid:  true,
+				Listeners: []*Listener{
+					createListener("listener-80-1"),
+				},
+			},
+			expectedSectionNameRefs: []ParentRef{
+				{
+					Idx:         0,
+					Gateway:     &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
+					SectionName: hr.Spec.ParentRefs[0].SectionName,
+					Attachment: &ParentRefAttachmentStatus{
+						Attached: true,
+						FailedConditions: []conditions.Condition{
+							{Message: "invalid backend"},
+						},
+						AcceptedHostnames: map[string][]string{
+							CreateGatewayListenerKey(
+								client.ObjectKeyFromObject(gw),
+								"listener-80-1",
+							): {"foo.example.com"},
+						},
+					},
+				},
+			},
+			expectedGatewayListeners: []*Listener{
+				createModifiedListener("listener-80-1", func(l *Listener) {
+					l.Routes = map[RouteKey]*L7Route{
+						CreateRouteKey(hr): routeWithInvalidBackendRefs,
+					}
+				}),
+			},
+			name: "route still allowed if backendRef failure conditions exist",
+		},
 	}
 
 	namespaces := map[types.NamespacedName]*v1.Namespace{
@@ -1706,6 +1756,13 @@ func TestBindL4RouteToListeners(t *testing.T) {
 		Attachable: true,
 	}
 
+	routeWithInvalidBackendRefs := createNormalRoute(gw)
+	routeWithInvalidBackendRefs.Spec.BackendRef = BackendRef{
+		InvalidForGateways: map[types.NamespacedName]conditions.Condition{
+			client.ObjectKeyFromObject(gw): {Message: "invalid backend"},
+		},
+	}
+
 	tests := []struct {
 		route                    *L4Route
 		gateway                  *Gateway
@@ -2178,6 +2235,47 @@ func TestBindL4RouteToListeners(t *testing.T) {
 			},
 			name: "route kind not allowed",
 		},
+		{
+			route: routeWithInvalidBackendRefs,
+			gateway: &Gateway{
+				Source: gw,
+				Valid:  true,
+				DeploymentName: types.NamespacedName{
+					Namespace: "test",
+					Name:      "gateway",
+				},
+				Listeners: []*Listener{
+					createListener("listener-443"),
+				},
+			},
+			expectedSectionNameRefs: []ParentRef{
+				{
+					Idx:         0,
+					Gateway:     &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
+					SectionName: tr.Spec.ParentRefs[0].SectionName,
+					Attachment: &ParentRefAttachmentStatus{
+						Attached: true,
+						FailedConditions: []conditions.Condition{
+							{Message: "invalid backend"},
+						},
+						AcceptedHostnames: map[string][]string{
+							CreateGatewayListenerKey(
+								client.ObjectKeyFromObject(gw),
+								"listener-443",
+							): {"foo.example.com"},
+						},
+					},
+				},
+			},
+			expectedGatewayListeners: []*Listener{
+				createModifiedListener("listener-443", func(l *Listener) {
+					l.L4Routes = map[L4RouteKey]*L4Route{
+						CreateRouteKeyL4(tr): routeWithInvalidBackendRefs,
+					}
+				}),
+			},
+			name: "route still allowed if backendRef failure conditions exist",
+		},
 	}
 
 	namespaces := map[types.NamespacedName]*v1.Namespace{

internal/mode/static/status/prepare_requests.go

@@ -111,16 +111,16 @@ func prepareRouteStatus(
 
 	for _, ref := range parentRefs {
 		failedAttachmentCondCount := 0
-		if ref.Attachment != nil && !ref.Attachment.Attached {
-			failedAttachmentCondCount = 1
+		if ref.Attachment != nil {
+			failedAttachmentCondCount = len(ref.Attachment.FailedConditions)
 		}
 		allConds := make([]conditions.Condition, 0, len(conds)+len(defaultConds)+failedAttachmentCondCount)
 
 		// We add defaultConds first, so that any additional conditions will override them, which is
 		// ensured by DeduplicateConditions.
 		allConds = append(allConds, defaultConds...)
 		allConds = append(allConds, conds...)
-		if failedAttachmentCondCount == 1 {
+		if failedAttachmentCondCount > 0 {
 			allConds = append(allConds, ref.Attachment.FailedConditions...)
 		}
 

internal/mode/static/status/prepare_requests_test.go

@@ -71,6 +71,9 @@ var (
 			{
 				SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"),
 			},
+			{
+				SectionName: helpers.GetPointer[v1.SectionName]("listener-80-3"),
+			},
 		},
 	}
 
@@ -100,6 +103,15 @@ var (
 				FailedConditions: []conditions.Condition{invalidAttachmentCondition},
 			},
 		},
+		{
+			Idx:         2,
+			Gateway:     &graph.ParentRefGateway{NamespacedName: gwNsName},
+			SectionName: commonRouteSpecValid.ParentRefs[2].SectionName,
+			Attachment: &graph.ParentRefAttachmentStatus{
+				Attached:         true,
+				FailedConditions: []conditions.Condition{invalidAttachmentCondition},
+			},
+		},
 	}
 
 	parentRefsInvalid = []graph.ParentRef{
@@ -171,6 +183,38 @@ var (
 					},
 				},
 			},
+			{
+				ParentRef: v1.ParentReference{
+					Namespace:   helpers.GetPointer(v1.Namespace(gwNsName.Namespace)),
+					Name:        v1.ObjectName(gwNsName.Name),
+					SectionName: helpers.GetPointer[v1.SectionName]("listener-80-3"),
+				},
+				ControllerName: gatewayCtlrName,
+				Conditions: []metav1.Condition{
+					{
+						Type:               string(v1.RouteConditionAccepted),
+						Status:             metav1.ConditionTrue,
+						ObservedGeneration: 3,
+						LastTransitionTime: transitionTime,
+						Reason:             string(v1.RouteReasonAccepted),
+						Message:            "The route is accepted",
+					},
+					{
+						Type:               string(v1.RouteConditionResolvedRefs),
+						Status:             metav1.ConditionTrue,
+						ObservedGeneration: 3,
+						LastTransitionTime: transitionTime,
+						Reason:             string(v1.RouteReasonResolvedRefs),
+						Message:            "All references are resolved",
+					},
+					{
+						Type:               invalidAttachmentCondition.Type,
+						Status:             metav1.ConditionTrue,
+						ObservedGeneration: 3,
+						LastTransitionTime: transitionTime,
+					},
+				},
+			},
 		},
 	}