fix: remove stale parent statuses from HTTPRoute

CHANGELOG.md

@@ -198,6 +198,9 @@ Adding a new version? You'll need three changes:
   if the config was being updated while the HTTP endpoint was being hit at the same
   time.
   [#5474](https://github.com/Kong/kubernetes-ingress-controller/pull/5474)
+- Stale `HTTPRoute`'s parent statuses are now removed when the `HTTPRoute` no longer
+  defines a parent `Gateway` in its `spec.parentRefs`.
+  [#5477](https://github.com/Kong/kubernetes-ingress-controller/pull/5477)
 
 ### Changed
 

internal/controllers/gateway/httproute_controller.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/samber/lo"
+	"golang.org/x/exp/slices"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -417,6 +418,14 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, err
 	}
 
+	// Ensure we have no status for no-longer defined parentRefs.
+	if wasAnyStatusRemoved := ensureNoStaleParentStatus(httproute); wasAnyStatusRemoved {
+		if err := r.Status().Update(ctx, httproute); err != nil {
+			return ctrl.Result{}, err
+		}
+		return ctrl.Result{}, nil
+	}
+
 	// the referenced gateway object(s) for the HTTPRoute needs to be ready
 	// before we'll attempt any configurations of it. If it's not we'll
 	// requeue the object and wait until all supported gateways are ready.
@@ -752,3 +761,39 @@ func (r *HTTPRouteReconciler) getHTTPRouteRuleReason(ctx context.Context, httpRo
 func (r *HTTPRouteReconciler) SetLogger(l logr.Logger) {
 	r.Log = l
 }
+
+// ensureNoStaleParentStatus removes any status references to Gateways that are no longer in the HTTPRoute's parentRefs
+// and returns true if any status was removed.
+func ensureNoStaleParentStatus(httproute *gatewayapi.HTTPRoute) (wasAnyStatusRemoved bool) {
+	// Create a map of currently defined parentRefs for fast lookup.
+	currentlyDefinedParentRefs := make(map[string]struct{})
+	for _, parentRef := range httproute.Spec.ParentRefs {
+		currentlyDefinedParentRefs[parentReferenceKey(httproute.Namespace, parentRef)] = struct{}{}
+	}
+
+	for parentIdx, parentStatus := range httproute.Status.Parents {
+		// Don't touch statuses from other controllers.
+		if parentStatus.ControllerName != GetControllerName() {
+			continue
+		}
+		// Remove the status if the parentRef is no longer defined.
+		if _, ok := currentlyDefinedParentRefs[parentReferenceKey(httproute.Namespace, parentStatus.ParentRef)]; !ok {
+			httproute.Status.Parents = slices.Delete(httproute.Status.Parents, parentIdx, parentIdx+1)
+			wasAnyStatusRemoved = true
+		}
+	}
+	return wasAnyStatusRemoved
+}
+
+// parentReferenceKey returns a string key for a parentRef of a route. It can be used for indexing a map.
+func parentReferenceKey(routeNamespace string, parentRef gatewayapi.ParentReference) string {
+	namespace := routeNamespace
+	if parentRef.Namespace != nil {
+		namespace = string(*parentRef.Namespace)
+	}
+	sectionName := ""
+	if parentRef.SectionName != nil {
+		sectionName = string(*parentRef.SectionName)
+	}
+	return fmt.Sprintf("%s/%s/%s", namespace, parentRef.Name, sectionName)
+}

internal/controllers/gateway/httproute_controller_test.go

@@ -0,0 +1,160 @@
+package gateway
+
+import (
+	"testing"
+
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
+)
+
+func TestEnsureNoStaleParentStatus(t *testing.T) {
+	testCases := []struct {
+		name                     string
+		httproute                *gatewayapi.HTTPRoute
+		expectedAnyStatusRemoved bool
+		expectedStatusesForRefs  []gatewayapi.ParentReference
+	}{
+		{
+			name: "no stale status",
+			httproute: &gatewayapi.HTTPRoute{
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{Name: "defined-in-spec"},
+						},
+					},
+				},
+			},
+			expectedAnyStatusRemoved: false,
+			expectedStatusesForRefs:  nil, // There was no status for `defined-in-spec` created yet.
+		},
+		{
+			name: "no stale status with existing status for spec parent ref",
+			httproute: &gatewayapi.HTTPRoute{
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{Name: "defined-in-spec"},
+						},
+					},
+				},
+				Status: gatewayapi.HTTPRouteStatus{
+					RouteStatus: gatewayapi.RouteStatus{
+						Parents: []gatewayapi.RouteParentStatus{
+							{
+								ControllerName: GetControllerName(),
+								ParentRef:      gatewayapi.ParentReference{Name: "defined-in-spec"},
+							},
+						},
+					},
+				},
+			},
+			expectedStatusesForRefs: []gatewayapi.ParentReference{
+				{Name: "defined-in-spec"},
+			},
+			expectedAnyStatusRemoved: false,
+		},
+		{
+			name: "stale status",
+			httproute: &gatewayapi.HTTPRoute{
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{Name: "defined-in-spec"},
+						},
+					},
+				},
+				Status: gatewayapi.HTTPRouteStatus{
+					RouteStatus: gatewayapi.RouteStatus{
+						Parents: []gatewayapi.RouteParentStatus{
+							{
+								ControllerName: GetControllerName(),
+								ParentRef:      gatewayapi.ParentReference{Name: "not-defined-in-spec"},
+							},
+						},
+					},
+				},
+			},
+			expectedStatusesForRefs:  nil, // There was no status for `defined-in-spec` created yet.
+			expectedAnyStatusRemoved: true,
+		},
+		{
+			name: "stale status with existing status for spec parent ref",
+			httproute: &gatewayapi.HTTPRoute{
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{Name: "defined-in-spec"},
+						},
+					},
+				},
+				Status: gatewayapi.HTTPRouteStatus{
+					RouteStatus: gatewayapi.RouteStatus{
+						Parents: []gatewayapi.RouteParentStatus{
+							{
+								ControllerName: GetControllerName(),
+								ParentRef:      gatewayapi.ParentReference{Name: "not-defined-in-spec"},
+							},
+							{
+								ControllerName: GetControllerName(),
+								ParentRef:      gatewayapi.ParentReference{Name: "defined-in-spec"},
+							},
+						},
+					},
+				},
+			},
+			expectedStatusesForRefs: []gatewayapi.ParentReference{
+				{Name: "defined-in-spec"},
+			},
+			expectedAnyStatusRemoved: true,
+		},
+		{
+			name: "do not remove status for other controllers",
+			httproute: &gatewayapi.HTTPRoute{
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{Name: "defined-in-spec"},
+						},
+					},
+				},
+				Status: gatewayapi.HTTPRouteStatus{
+					RouteStatus: gatewayapi.RouteStatus{
+						Parents: []gatewayapi.RouteParentStatus{
+							{
+								ControllerName: gatewayapi.GatewayController("another-controller"),
+								ParentRef:      gatewayapi.ParentReference{Name: "not-defined-in-spec"},
+							},
+							{
+								ControllerName: GetControllerName(),
+								ParentRef:      gatewayapi.ParentReference{Name: "defined-in-spec"},
+							},
+						},
+					},
+				},
+			},
+			expectedStatusesForRefs: []gatewayapi.ParentReference{
+				{Name: "not-defined-in-spec"},
+				{Name: "defined-in-spec"},
+			},
+			expectedAnyStatusRemoved: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			wasAnyStatusRemoved := ensureNoStaleParentStatus(tc.httproute)
+			assert.Equal(t, tc.expectedAnyStatusRemoved, wasAnyStatusRemoved)
+
+			actualStatuses := lo.Map(tc.httproute.Status.Parents, func(status gatewayapi.RouteParentStatus, _ int) string {
+				return parentReferenceKey(tc.httproute.Namespace, status.ParentRef)
+			})
+			expectedStatuses := lo.Map(tc.expectedStatusesForRefs, func(ref gatewayapi.ParentReference, _ int) string {
+				return parentReferenceKey(tc.httproute.Namespace, ref)
+			})
+			assert.ElementsMatch(t, expectedStatuses, actualStatuses)
+		})
+	}
+}

test/envtest/httproute_controller_test.go

@@ -27,14 +27,14 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/test/mocks"
 )
 
+const (
+	waitDuration = 5 * time.Second
+	tickDuration = 100 * time.Millisecond
+)
+
 func TestHTTPRouteReconcilerProperlyReactsToReferenceGrant(t *testing.T) {
 	t.Parallel()
 
-	const (
-		waitDuration = 5 * time.Second
-		tickDuration = 100 * time.Millisecond
-	)
-
 	scheme := Scheme(t, WithGatewayAPI)
 	cfg := Setup(t, scheme)
 	client := NewControllerClient(t, scheme, cfg)
@@ -265,6 +265,120 @@ func TestHTTPRouteReconcilerProperlyReactsToReferenceGrant(t *testing.T) {
 	}
 }
 
+func TestHTTPRouteReconciler_RemovesOutdatedParentStatuses(t *testing.T) {
+	t.Parallel()
+
+	scheme := Scheme(t, WithGatewayAPI)
+	cfg := Setup(t, scheme)
+	client := NewControllerClient(t, scheme, cfg)
+
+	reconciler := &gateway.HTTPRouteReconciler{
+		Client:          client,
+		DataplaneClient: mocks.Dataplane{},
+	}
+
+	// We use a deferred cancel to stop the manager and not wait for its timeout.
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	ns := CreateNamespace(ctx, t, client)
+	nsRoute := CreateNamespace(ctx, t, client)
+
+	StartReconcilers(ctx, t, client.Scheme(), cfg, reconciler)
+
+	gwc := gatewayapi.GatewayClass{
+		Spec: gatewayapi.GatewayClassSpec{
+			ControllerName: gateway.GetControllerName(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: uuid.NewString(),
+			Annotations: map[string]string{
+				"konghq.com/gatewayclass-unmanaged": "placeholder",
+			},
+		},
+	}
+	require.NoError(t, client.Create(ctx, &gwc))
+	t.Cleanup(func() { _ = client.Delete(ctx, &gwc) })
+
+	gw := gatewayapi.Gateway{
+		Spec: gatewayapi.GatewaySpec{
+			GatewayClassName: gatewayapi.ObjectName(gwc.Name),
+			Listeners: []gatewayapi.Listener{
+				{
+					Name:     gatewayapi.SectionName("http"),
+					Port:     gatewayapi.PortNumber(80),
+					Protocol: gatewayapi.HTTPProtocolType,
+					AllowedRoutes: &gatewayapi.AllowedRoutes{
+						Namespaces: &gatewayapi.RouteNamespaces{
+							From: lo.ToPtr(gatewayapi.NamespacesFromAll),
+						},
+					},
+				},
+			},
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: ns.Name,
+			Name:      uuid.NewString(),
+		},
+	}
+	require.NoError(t, client.Create(ctx, &gw))
+
+	route := gatewayapi.HTTPRoute{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "HTTPRoute",
+			APIVersion: "v1beta1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: nsRoute.Name,
+			Name:      uuid.NewString(),
+		},
+		Spec: gatewayapi.HTTPRouteSpec{
+			CommonRouteSpec: gatewayapi.CommonRouteSpec{
+				ParentRefs: []gatewayapi.ParentReference{{
+					Name:      gatewayapi.ObjectName(gw.Name),
+					Namespace: lo.ToPtr(gatewayapi.Namespace(ns.Name)),
+				}},
+			},
+			Rules: []gatewayapi.HTTPRouteRule{{
+				BackendRefs: builder.NewHTTPBackendRef("backend-1").WithNamespace(ns.Name).ToSlice(),
+			}},
+		},
+	}
+	require.NoError(t, client.Create(ctx, &route))
+
+	// Status has to be updated separately.
+	route.Status = gatewayapi.HTTPRouteStatus{
+		RouteStatus: gatewayapi.RouteStatus{
+			Parents: []gatewayapi.RouteParentStatus{
+				{
+					ParentRef: gatewayapi.ParentReference{
+						Name: "other-gw-name",
+					},
+					ControllerName: gateway.GetControllerName(),
+				},
+			},
+		},
+	}
+	require.NoError(t, client.Status().Update(ctx, &route))
+
+	require.Eventually(t, func() bool {
+		err := client.Get(ctx, k8stypes.NamespacedName{Name: route.Name, Namespace: route.Namespace}, &route)
+		if err != nil {
+			t.Logf("failed to get HTTPRoute %s/%s: %v", route.Namespace, route.Name, err)
+			return false
+		}
+
+		if staleStatusFound := lo.ContainsBy(route.Status.Parents, func(ps gatewayapi.RouteParentStatus) bool {
+			return ps.ParentRef.Name == "other-gw-name"
+		}); staleStatusFound {
+			t.Log("found stale status for parent other-gw-name")
+			return false
+		}
+
+		return true
+	}, waitDuration, tickDuration, "expected stale status to be removed from HTTPRoute")
+}
+
 func printHTTPRoutesConditions(ctx context.Context, client ctrlclient.Client, nn k8stypes.NamespacedName) string {
 	var route gatewayapi.HTTPRoute
 	err := client.Get(ctx, ctrlclient.ObjectKey{Namespace: nn.Namespace, Name: nn.Name}, &route)