[feat gw-api]modify route status update per parentRef

Author: shuqz

controllers/gateway/route_reconciler.go

@@ -156,35 +156,34 @@ func (d *routeReconcilerImpl) updateRouteStatus(route client.Object, routeData r
 		originalRouteStatus = r.Status.Parents
 		ParentRefs = r.Spec.ParentRefs
 	}
+	routeNamespace := route.GetNamespace()
 
 	// set conditions
 	var newRouteStatus []gwv1.RouteParentStatus
-	originalRouteStatusMap := createOriginalRouteStatusMap(originalRouteStatus)
-
+	originalRouteStatusMap := createOriginalRouteStatusMap(originalRouteStatus, routeNamespace)
 	for _, parentRef := range ParentRefs {
 		newRouteParentStatus := gwv1.RouteParentStatus{
 			ParentRef:      parentRef,
 			ControllerName: gwv1.GatewayController(controllerName),
 			Conditions:     []metav1.Condition{},
 		}
 		// if status related to parentRef exists, keep the condition first
-		if status, exists := originalRouteStatusMap[getParentStatusKey(parentRef)]; exists {
+		if status, exists := originalRouteStatusMap[getParentStatusKey(parentRef, routeNamespace)]; exists {
 			newRouteParentStatus.Conditions = status.Conditions
 		}
-		// Namespace is the namespace of the referent. When unspecified, this refers to the local namespace of the Route.
-		namespace := route.GetNamespace()
-		if parentRef.Namespace != nil {
-			namespace = string(*parentRef.Namespace)
-		}
+
+		// Generate key for routeData's parentRef
+		routeDataParentRefKey := getParentRefKeyFromRouteData(routeData.ParentRefGateway)
 
 		// do not allow backward generation update, Accepted and ResolvedRef always have same generation based on our implementation
 		if (len(newRouteParentStatus.Conditions) != 0 && newRouteParentStatus.Conditions[0].ObservedGeneration <= routeData.RouteMetadata.RouteGeneration) || len(newRouteParentStatus.Conditions) == 0 {
 			// for a given parentRef, if it has a statusInfo, this means condition is updated, override route condition based on route status info
-			if namespace == routeData.ParentRefGateway.Namespace && string(parentRef.Name) == routeData.ParentRefGateway.Name {
+			parentRefKey := getParentStatusKey(parentRef, routeNamespace)
+			if parentRefKey == routeDataParentRefKey {
 				d.setConditionsWithRouteStatusInfo(route, &newRouteParentStatus, routeData.RouteStatusInfo)
 			}
 
-			// resolve ref Gateway, if parentRef does not have namespace, getting it from Route
+			// handle parentRefNotExist: resolve ref Gateway, if parentRef does not have namespace, getting it from Route
 			if _, err := d.resolveRefGateway(parentRef, route.GetNamespace()); err != nil {
 				// set conditions if resolvedRef = false
 				d.setConditionsBasedOnResolveRefGateway(route, &newRouteParentStatus, err)
@@ -283,9 +282,9 @@ func (d *routeReconcilerImpl) setConditionsBasedOnResolveRefGateway(route client
 		},
 		{
 			Type:               string(gwv1.RouteConditionResolvedRefs),
-			Status:             metav1.ConditionFalse,
-			Reason:             routeutils.RouteStatusInfoRejectedParentRefNotExist,
-			Message:            resolveErr.Error(),
+			Status:             metav1.ConditionTrue,
+			Reason:             string(gwv1.RouteConditionResolvedRefs),
+			Message:            "",
 			LastTransitionTime: timeNow,
 			ObservedGeneration: route.GetGeneration(),
 		},
@@ -310,12 +309,12 @@ func (d *routeReconcilerImpl) isRouteStatusIdentical(routeOld client.Object, rou
 
 	// build maps, key is a string which is combined from parentRef fields
 	for _, status := range routeOldStatus {
-		key := getParentStatusKey(status.ParentRef)
+		key := getParentStatusKey(status.ParentRef, routeOld.GetNamespace())
 		oldStatusMap[key] = status
 	}
 
 	for _, status := range routeNewStatus {
-		key := getParentStatusKey(status.ParentRef)
+		key := getParentStatusKey(status.ParentRef, route.GetNamespace())
 		newStatusMap[key] = status
 	}
 
@@ -359,11 +358,37 @@ func getRouteStatus(route client.Object) []gwv1.RouteParentStatus {
 	return routeStatus
 }
 
+// Helper function to generate key from RouteData's ParentRefGateway, use same format as getParentStatusKey
+func getParentRefKeyFromRouteData(gatewayRef routeutils.ParentRefGateway) string {
+
+	namespace := gatewayRef.Namespace
+
+	sectionName := ""
+	if gatewayRef.SectionName != nil {
+		sectionName = string(*gatewayRef.SectionName)
+	}
+
+	port := ""
+	if gatewayRef.Port != nil {
+		port = strconv.Itoa(int(*gatewayRef.Port))
+	}
+
+	key := fmt.Sprintf("%s/%s/%s/%s",
+		namespace,
+		gatewayRef.Name,
+		sectionName,
+		port)
+
+	return key
+}
+
 // Helper function to generate a unique key for a RouteParentStatus
-func getParentStatusKey(ref gwv1.ParentReference) string {
+func getParentStatusKey(ref gwv1.ParentReference, routeNamespace string) string {
 	namespace := ""
 	if ref.Namespace != nil {
 		namespace = string(*ref.Namespace)
+	} else {
+		namespace = routeNamespace
 	}
 
 	sectionName := ""
@@ -376,19 +401,7 @@ func getParentStatusKey(ref gwv1.ParentReference) string {
 		port = strconv.Itoa(int(*ref.Port))
 	}
 
-	group := ""
-	if ref.Group != nil {
-		group = string(*ref.Group)
-	}
-
-	kind := ""
-	if ref.Kind != nil {
-		kind = string(*ref.Kind)
-	}
-
-	key := fmt.Sprintf("%s/%s/%s/%s/%s/%s",
-		group,
-		kind,
+	key := fmt.Sprintf("%s/%s/%s/%s",
 		namespace,
 		string(ref.Name),
 		sectionName,
@@ -427,10 +440,10 @@ func areConditionsEqual(oldConditions, newConditions []metav1.Condition) bool {
 	return true
 }
 
-func createOriginalRouteStatusMap(originalRouteStatus []gwv1.RouteParentStatus) map[string]gwv1.RouteParentStatus {
+func createOriginalRouteStatusMap(originalRouteStatus []gwv1.RouteParentStatus, routeNamespace string) map[string]gwv1.RouteParentStatus {
 	originalStatusMap := make(map[string]gwv1.RouteParentStatus)
 	for i := range originalRouteStatus {
-		key := getParentStatusKey(originalRouteStatus[i].ParentRef)
+		key := getParentStatusKey(originalRouteStatus[i].ParentRef, routeNamespace)
 		originalStatusMap[key] = originalRouteStatus[i]
 	}
 	return originalStatusMap

controllers/gateway/route_reconciler_test.go

@@ -289,6 +289,7 @@ func Test_getParentStatusKey(t *testing.T) {
 	portPtr := func(p gwv1.PortNumber) *gwv1.PortNumber {
 		return &p
 	}
+	routeNamespace := "route-namespace"
 
 	tests := []struct {
 		name   string
@@ -307,7 +308,20 @@ func Test_getParentStatusKey(t *testing.T) {
 					Port:        portPtr(80),
 				},
 			},
-			want: "networking.k8s.io/Gateway/test-namespace/test-gateway/test-section/80",
+			want: "test-namespace/test-gateway/test-section/80",
+		},
+		{
+			name: "no namespace should use route namespace",
+			status: gwv1.RouteParentStatus{
+				ParentRef: gwv1.ParentReference{
+					Group:       (*gwv1.Group)(ptr.To("networking.k8s.io")),
+					Kind:        (*gwv1.Kind)(ptr.To("Gateway")),
+					Name:        "test-gateway",
+					SectionName: (*gwv1.SectionName)(ptr.To("test-section")),
+					Port:        portPtr(80),
+				},
+			},
+			want: "route-namespace/test-gateway/test-section/80",
 		},
 		{
 			name: "no section or port",
@@ -319,13 +333,13 @@ func Test_getParentStatusKey(t *testing.T) {
 					Name:      "test-gateway",
 				},
 			},
-			want: "networking.k8s.io/Gateway/test-namespace/test-gateway//",
+			want: "test-namespace/test-gateway//",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := getParentStatusKey(tt.status.ParentRef)
+			got := getParentStatusKey(tt.status.ParentRef, routeNamespace)
 			if got != tt.want {
 				t.Errorf("getParentStatusKey() = %v, want %v", got, tt.want)
 			}
@@ -718,3 +732,55 @@ func findCondition(conditions []metav1.Condition, conditionType string) *metav1.
 	}
 	return nil
 }
+
+func Test_getParentRefKeyFromRouteData(t *testing.T) {
+	tests := []struct {
+		name       string
+		gatewayRef routeutils.ParentRefGateway
+		want       string
+	}{
+		{
+			name: "all fields provided",
+			gatewayRef: routeutils.ParentRefGateway{
+				Namespace:   "test-namespace",
+				Name:        "test-gateway",
+				SectionName: ptr.To(gwv1.SectionName("test-section")),
+				Port:        ptr.To(gwv1.PortNumber(80)),
+			},
+			want: "test-namespace/test-gateway/test-section/80",
+		},
+		{
+			name: "no section or port",
+			gatewayRef: routeutils.ParentRefGateway{
+				Namespace: "test-namespace",
+				Name:      "test-gateway",
+			},
+			want: "test-namespace/test-gateway//",
+		},
+		{
+			name: "with port only",
+			gatewayRef: routeutils.ParentRefGateway{
+				Namespace: "test-namespace",
+				Name:      "test-gateway",
+				Port:      ptr.To(gwv1.PortNumber(443)),
+			},
+			want: "test-namespace/test-gateway//443",
+		},
+		{
+			name: "with section only",
+			gatewayRef: routeutils.ParentRefGateway{
+				Namespace:   "test-namespace",
+				Name:        "test-gateway",
+				SectionName: ptr.To(gwv1.SectionName("https")),
+			},
+			want: "test-namespace/test-gateway/https/",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := getParentRefKeyFromRouteData(tt.gatewayRef)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

pkg/gateway/routeutils/descriptor.go

@@ -15,6 +15,7 @@ import (
 type routeMetadataDescriptor interface {
 	GetRouteNamespacedName() types.NamespacedName
 	GetRouteKind() RouteKind
+	GetRouteIdentifier() string
 	GetHostnames() []gwv1.Hostname
 	GetParentRefs() []gwv1.ParentReference
 	GetRawRoute() interface{}

pkg/gateway/routeutils/grpc.go

@@ -98,6 +98,10 @@ func (grpcRoute *grpcRouteDescription) GetRouteNamespacedName() types.Namespaced
 	return k8s.NamespacedName(grpcRoute.route)
 }
 
+func (grpcRoute *grpcRouteDescription) GetRouteIdentifier() string {
+	return string(grpcRoute.GetRouteKind()) + "-" + grpcRoute.GetRouteNamespacedName().String()
+}
+
 func convertGRPCRoute(r gwv1.GRPCRoute) *grpcRouteDescription {
 	return &grpcRouteDescription{route: &r, ruleAccumulator: defaultGRPCRuleAccumulator}
 }

pkg/gateway/routeutils/http.go

@@ -103,6 +103,10 @@ func (httpRoute *httpRouteDescription) GetRouteNamespacedName() types.Namespaced
 	return k8s.NamespacedName(httpRoute.route)
 }
 
+func (httpRoute *httpRouteDescription) GetRouteIdentifier() string {
+	return string(httpRoute.GetRouteKind()) + "-" + httpRoute.GetRouteNamespacedName().String()
+}
+
 func (httpRoute *httpRouteDescription) GetBackendRefs() []gwv1.BackendRef {
 	backendRefs := make([]gwv1.BackendRef, 0)
 	if httpRoute.route.Spec.Rules != nil {

pkg/gateway/routeutils/listener_attachment_helper.go

@@ -15,7 +15,7 @@ import (
 // listenerAttachmentHelper is an internal utility interface that can be used to determine if a listener will allow
 // a route to attach to it.
 type listenerAttachmentHelper interface {
-	listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error)
+	listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, matchedParentRef *gwv1.ParentReference, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error)
 }
 
 var _ listenerAttachmentHelper = &listenerAttachmentHelperImpl{}
@@ -36,21 +36,23 @@ func newListenerAttachmentHelper(k8sClient client.Client, logger logr.Logger) li
 // listenerAllowsAttachment utility method to determine if a listener will allow a route to connect using
 // Gateway API rules to determine compatibility between lister and route.
 // Returns: (compatibleHostnames, allowed, failedRouteData, error)
-func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error) {
+func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, matchedParentRef *gwv1.ParentReference, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error) {
+	port := matchedParentRef.Port
+	sectionName := matchedParentRef.SectionName
 	// check namespace
 	namespaceOK, err := attachmentHelper.namespaceCheck(ctx, gw, listener, route)
 	if err != nil {
 		return nil, false, nil, err
 	}
 	if !namespaceOK {
-		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
 		return nil, false, &rd, nil
 	}
 
 	// check kind
 	kindOK := attachmentHelper.kindCheck(listener, route)
 	if !kindOK {
-		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
 		return nil, false, &rd, nil
 	}
 
@@ -63,7 +65,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
 			return nil, false, nil, err
 		}
 		if !hostnameOK {
-			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
 			return nil, false, &rd, nil
 		}
 	}
@@ -73,7 +75,7 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
 		hostnameUniquenessOK, conflictRoute := attachmentHelper.crossServingHostnameUniquenessCheck(route, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes)
 		if !hostnameUniquenessOK {
 			message := fmt.Sprintf("HTTPRoute and GRPCRoute have overlap hostname, attachment is rejected. Conflict route: %s", conflictRoute)
-			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw, port, sectionName)
 			return nil, false, &rd, nil
 		}
 	}

pkg/gateway/routeutils/listener_attachment_helper_test.go

@@ -23,6 +23,12 @@ func (mnss *mockNamespaceSelector) getNamespacesFromSelector(_ context.Context,
 }
 
 func Test_listenerAllowsAttachment(t *testing.T) {
+	kind := gwv1.Kind("HTTPRoute")
+	port := gwv1.PortNumber(80)
+	matchedParentRef := gwv1.ParentReference{
+		Kind: &kind,
+		Port: &port,
+	}
 
 	type expectedRouteStatus struct {
 		reason  string
@@ -90,7 +96,7 @@ func Test_listenerAllowsAttachment(t *testing.T) {
 			hostnameFromGrpcRoute := map[types.NamespacedName][]gwv1.Hostname{}
 			_, result, statusUpdate, err := attachmentHelper.listenerAllowsAttachment(context.Background(), gw, gwv1.Listener{
 				Protocol: tc.listenerProtocol,
-			}, route, hostnameFromHttpRoute, hostnameFromGrpcRoute)
+			}, route, &matchedParentRef, hostnameFromHttpRoute, hostnameFromGrpcRoute)
 			assert.NoError(t, err)
 			assert.Equal(t, tc.expected, result)
 			if tc.expectedStatusUpdate == nil {