fix: HTTPRoute status only shows one parent when targeting multiple G…

…ateways from different GatewayClasses (#4587) * fix route status Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comment Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update unit test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
envoyproxy · Nov 4, 2024 · 04ac7b4 · 04ac7b4
1 parent 656ce52
commit 04ac7b4
Show file tree

Hide file tree

Showing 4 changed files with 359 additions and 11 deletions.
diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go
@@ -86,6 +86,7 @@ var (
 	QueryParamMatchTypeDerefOr = ptr.Deref[gwapiv1.QueryParamMatchType]
 )
 
+// Deprecated: use k8s.io/utils/ptr ptr.Deref instead
 func NamespaceDerefOr(namespace *gwapiv1.Namespace, defaultNamespace string) string {
 	if namespace != nil && *namespace != "" {
 		return string(*namespace)

diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go
@@ -8,6 +8,7 @@ package kubernetes
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -18,6 +19,7 @@ import (
 	gwapiv1a3 "sigs.k8s.io/gateway-api/apis/v1alpha3"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
+	"github.com/envoyproxy/gateway/internal/gatewayapi/resource"
 	"github.com/envoyproxy/gateway/internal/gatewayapi/status"
 	"github.com/envoyproxy/gateway/internal/message"
 	"github.com/envoyproxy/gateway/internal/utils"
@@ -74,7 +76,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 							panic(err)
 						}
 						hCopy := h.DeepCopy()
-						hCopy.Status.Parents = val.Parents
+						hCopy.Status.Parents = mergeRouteParentStatus(h.Namespace, h.Status.Parents, val.Parents)
 						return hCopy
 					}),
 				})
@@ -97,15 +99,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 					NamespacedName: key,
 					Resource:       new(gwapiv1.GRPCRoute),
 					Mutator: MutatorFunc(func(obj client.Object) client.Object {
-						h, ok := obj.(*gwapiv1.GRPCRoute)
+						g, ok := obj.(*gwapiv1.GRPCRoute)
 						if !ok {
 							err := fmt.Errorf("unsupported object type %T", obj)
 							errChan <- err
 							panic(err)
 						}
-						hCopy := h.DeepCopy()
-						hCopy.Status.Parents = val.Parents
-						return hCopy
+						gCopy := g.DeepCopy()
+						gCopy.Status.Parents = mergeRouteParentStatus(g.Namespace, g.Status.Parents, val.Parents)
+						return gCopy
 					}),
 				})
 			},
@@ -136,7 +138,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 							panic(err)
 						}
 						tCopy := t.DeepCopy()
-						tCopy.Status.Parents = val.Parents
+						tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents)
 						return tCopy
 					}),
 				})
@@ -168,7 +170,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 							panic(err)
 						}
 						tCopy := t.DeepCopy()
-						tCopy.Status.Parents = val.Parents
+						tCopy.Status.Parents = mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents)
 						return tCopy
 					}),
 				})
@@ -193,15 +195,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 					NamespacedName: key,
 					Resource:       new(gwapiv1a2.UDPRoute),
 					Mutator: MutatorFunc(func(obj client.Object) client.Object {
-						t, ok := obj.(*gwapiv1a2.UDPRoute)
+						u, ok := obj.(*gwapiv1a2.UDPRoute)
 						if !ok {
 							err := fmt.Errorf("unsupported object type %T", obj)
 							errChan <- err
 							panic(err)
 						}
-						tCopy := t.DeepCopy()
-						tCopy.Status.Parents = val.Parents
-						return tCopy
+						uCopy := u.DeepCopy()
+						uCopy.Status.Parents = mergeRouteParentStatus(u.Namespace, u.Status.Parents, val.Parents)
+						return uCopy
 					}),
 				})
 			},
@@ -469,6 +471,56 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext
 	}
 }
 
+// mergeRouteParentStatus merges the old and new RouteParentStatus.
+// This is needed because the RouteParentStatus doesn't support strategic merge patch yet.
+func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus {
+	merged := make([]gwapiv1.RouteParentStatus, len(old))
+	_ = copy(merged, old)
+	for _, parent := range new {
+		found := -1
+		for i, existing := range old {
+			if isParentRefEqual(parent.ParentRef, existing.ParentRef, ns) {
+				found = i
+				break
+			}
+		}
+		if found >= 0 {
+			merged[found] = parent
+		} else {
+			merged = append(merged, parent)
+		}
+	}
+	return merged
+}
+
+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)
+}
+
 func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) {
 	// nil check for unit tests.
 	if r.statusUpdater == nil {