fix: Route with multiple parents has incorrect namespace in parentRef…

… status (#4592)

fix route status wrong ns

Signed-off-by: Huabing Zhao <[email protected]>

internal/gatewayapi/contexts.go

@@ -238,21 +238,26 @@ func GetRouteStatus(route RouteContext) *gwapiv1.RouteStatus {
 	return &rs
 }
 
-// GetRouteParentContext returns RouteParentContext by using the Route
-// objects' ParentReference.
+// GetRouteParentContext returns RouteParentContext by using the Route objects' ParentReference.
+// It creates a new RouteParentContext and add a new RouteParentStatus to the Route's Status if the ParentReference is not found.
 func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentReference) *RouteParentContext {
 	rv := reflect.ValueOf(route).Elem()
 	pr := rv.FieldByName("ParentRefs")
+
+	// If the ParentRefs field is nil, initialize it.
 	if pr.IsNil() {
 		mm := reflect.MakeMap(reflect.TypeOf(map[gwapiv1.ParentReference]*RouteParentContext{}))
 		pr.Set(mm)
 	}
 
+	// If the RouteParentContext is already in the RouteContext, return it.
 	if p := pr.MapIndex(reflect.ValueOf(forParentRef)); p.IsValid() && !p.IsZero() {
 		ctx := p.Interface().(*RouteParentContext)
 		return ctx
 	}
 
+	// Verify that the ParentReference is present in the Route.Spec.ParentRefs.
+	// This is just a sanity check, the parentRef should always be present, otherwise it's a programming error.
 	var parentRef *gwapiv1.ParentReference
 	specParentRefs := rv.FieldByName("Spec").FieldByName("ParentRefs")
 	for i := 0; i < specParentRefs.Len(); i++ {
@@ -266,25 +271,19 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 		panic("parentRef not found")
 	}
 
+	// Find the parent in the Route's Status.
 	routeParentStatusIdx := -1
-	defaultNamespace := gwapiv1.Namespace(metav1.NamespaceDefault)
 	statusParents := rv.FieldByName("Status").FieldByName("Parents")
+
 	for i := 0; i < statusParents.Len(); i++ {
 		p := statusParents.Index(i).FieldByName("ParentRef").Interface().(gwapiv1.ParentReference)
-		// For those non-v1 routes, their underlying type of `ParentReference` is v1 as well.
-		// So we can skip upgrading these routes for simplicity.
-		if forParentRef.Namespace == nil {
-			forParentRef.Namespace = &defaultNamespace
-		}
-		if p.Namespace == nil {
-			p.Namespace = &defaultNamespace
-		}
-		if reflect.DeepEqual(p, forParentRef) {
+		if isParentRefEqual(p, *parentRef, route.GetNamespace()) {
 			routeParentStatusIdx = i
 			break
 		}
 	}
 
+	// If the parent is not found in the Route's Status, create a new RouteParentStatus and add it to the Route's Status.
 	if routeParentStatusIdx == -1 {
 		rParentStatus := gwapiv1a2.RouteParentStatus{
 			ControllerName: gwapiv1a2.GatewayController(rv.FieldByName("GatewayControllerName").String()),
@@ -294,6 +293,7 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 		routeParentStatusIdx = statusParents.Len() - 1
 	}
 
+	// Also add the RouteParentContext to the RouteContext.
 	ctx := &RouteParentContext{
 		ParentReference:      parentRef,
 		routeParentStatusIdx: routeParentStatusIdx,
@@ -304,6 +304,34 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 	return ctx
 }
 
+func isParentRefEqual(ref1, ref2 gwapiv1.ParentReference, routeNS string) bool {
+	defaultGroup := (*gwapiv1.Group)(&gwapiv1.GroupVersion.Group)
+	if ref1.Group == nil {
+		ref1.Group = defaultGroup
+	}
+	if ref2.Group == nil {
+		ref2.Group = defaultGroup
+	}
+
+	defaultKind := gwapiv1.Kind(resource.KindGateway)
+	if ref1.Kind == nil {
+		ref1.Kind = &defaultKind
+	}
+	if ref2.Kind == nil {
+		ref2.Kind = &defaultKind
+	}
+
+	// If the parent's namespace is not set, default to the namespace of the Route.
+	defaultNS := gwapiv1.Namespace(routeNS)
+	if ref1.Namespace == nil {
+		ref1.Namespace = &defaultNS
+	}
+	if ref2.Namespace == nil {
+		ref2.Namespace = &defaultNS
+	}
+	return reflect.DeepEqual(ref1, ref2)
+}
+
 // RouteParentContext wraps a ParentReference and provides helper methods for
 // setting conditions and other status information on the associated
 // HTTPRoute, TLSRoute etc.

internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.in.yaml

@@ -0,0 +1,55 @@
+gateways:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      name: gateway-a
+      namespace: default
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: default
+          port: 80
+          protocol: HTTP
+          hostname: '*.a.example.com'
+          allowedRoutes:
+            namespaces:
+              from: All
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: Gateway
+    metadata:
+      name: gateway-b
+      namespace: envoy-gateway
+    spec:
+      gatewayClassName: envoy-gateway-class
+      listeners:
+        - name: default
+          port: 80
+          protocol: HTTP
+          hostname: '*.b.example.com'
+          allowedRoutes:
+            namespaces:
+              from: All
+httpRoutes:
+  - apiVersion: gateway.networking.k8s.io/v1
+    kind: HTTPRoute
+    metadata:
+      name: targeted-route
+      namespace: envoy-gateway
+    spec:
+      hostnames:
+        - targeted.a.example.com
+        - targeted.b.example.com
+      parentRefs:
+        - group: gateway.networking.k8s.io
+          kind: Gateway
+          name: gateway-a
+          namespace: default
+        - group: gateway.networking.k8s.io
+          kind: Gateway
+          name: gateway-b
+      rules:
+        - matches:
+            - method: GET
+              path:
+                type: PathPrefix
+                value: /toy