fix: remove stale parent statuses from HTTPRoute (#5477)

CHANGELOG.md

@@ -201,6 +201,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

@@ -4,10 +4,12 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strconv"
 	"time"
 
 	"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 +419,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 +762,46 @@ 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)
+	}
+	portNumber := ""
+	if parentRef.Port != nil {
+		portNumber = strconv.Itoa(int(*parentRef.Port))
+	}
+
+	// We intentionally do not take into account Kind and Group here as we only support Gateways
+	// and that's the only kind we should be getting here thanks to the admission webhook validation.
+	return fmt.Sprintf("%s/%s/%s/%s", namespace, parentRef.Name, sectionName, portNumber)
+}

internal/controllers/gateway/httproute_controller_test.go

@@ -0,0 +1,227 @@
+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)
+		})
+	}
+}
+
+func TestParentReferenceKey(t *testing.T) {
+	const routeNamespace = "route-ns"
+	testCases := []struct {
+		name     string
+		ref      gatewayapi.ParentReference
+		expected string
+	}{
+		{
+			name: "only name",
+			ref: gatewayapi.ParentReference{
+				Name: "foo",
+			},
+			expected: "route-ns/foo//",
+		},
+		{
+			name: "name with namespace",
+			ref: gatewayapi.ParentReference{
+				Name:      "foo",
+				Namespace: lo.ToPtr(gatewayapi.Namespace("bar")),
+			},
+			expected: "bar/foo//",
+		},
+		{
+			name: "name with port number",
+			ref: gatewayapi.ParentReference{
+				Name: "foo",
+				Port: lo.ToPtr(gatewayapi.PortNumber(1234)),
+			},
+			expected: "route-ns/foo//1234",
+		},
+		{
+			name: "name with section name",
+			ref: gatewayapi.ParentReference{
+				Name:        "foo",
+				SectionName: lo.ToPtr(gatewayapi.SectionName("section")),
+			},
+			expected: "route-ns/foo/section/",
+		},
+		{
+			name: "all fields",
+			ref: gatewayapi.ParentReference{
+				Name:        "foo",
+				Namespace:   lo.ToPtr(gatewayapi.Namespace("bar")),
+				Port:        lo.ToPtr(gatewayapi.PortNumber(1234)),
+				SectionName: lo.ToPtr(gatewayapi.SectionName("section")),
+			},
+			expected: "bar/foo/section/1234",
+		},
+		{
+			name: "group and kind are ignored",
+			ref: gatewayapi.ParentReference{
+				Name:  "foo",
+				Group: lo.ToPtr(gatewayapi.Group("group")),
+				Kind:  lo.ToPtr(gatewayapi.Kind("kind")),
+			},
+			expected: "route-ns/foo//",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := parentReferenceKey(routeNamespace, tc.ref)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}