fix: fix refChecker when namespace is specified and is the same as route's namespace

CHANGELOG.md

@@ -137,7 +137,6 @@ Adding a new version? You'll need three changes:
   [#5354](https://github.com/Kong/kubernetes-ingress-controller/pull/5354)
   [#5384](https://github.com/Kong/kubernetes-ingress-controller/pull/5384)
 
-
 ### Fixed
 
 - Validators of `KongPlugin` and `KongClusterPlugin` will not return `500` on
@@ -161,6 +160,9 @@ Adding a new version? You'll need three changes:
   [#5401](https://github.com/Kong/kubernetes-ingress-controller/pull/5401)
 - Support properly ConsumerGroups when fallback to the last valid configuration.
   [#5438](https://github.com/Kong/kubernetes-ingress-controller/pull/5438)
+- When specifying Gateway API Routes' `backendRef`s with namespace specified, those
+  refs are checked for existence and allowed if they exist.
+  [#5392](https://github.com/Kong/kubernetes-ingress-controller/pull/5392)
 
 ### Changed
 

internal/dataplane/translator/backendref.go

@@ -1,19 +1,30 @@
 package translator
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/go-logr/logr"
-	k8stypes "k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 )
 
+// backendRefsToKongStateBackends takes a list of BackendRefs and returns a list of ServiceBackends.
+// The backendRefs are checked for the following conditions. If any of these conditions are met, the BackendRef is
+// not included in the returned list:
+// - If a BackendRef is not permitted by the provided ReferenceGrantTo set,
+// - If a BackendRef is not found,
+// - If a BackendRef Group & Kind pair is not supported (currently only Service is supported),
+// - If a BackendRef is missing a port.
+// The provided client is used to retrieve the Backend referenced by the BackendRef
+// to check if it exists.
 func backendRefsToKongStateBackends(
 	logger logr.Logger,
+	storer store.Storer,
 	route client.Object,
 	backendRefs []gatewayapi.BackendRef,
 	allowed map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo,
@@ -23,42 +34,65 @@ func backendRefsToKongStateBackends(
 	for _, backendRef := range backendRefs {
 		logger := loggerForBackendRef(logger, route, backendRef)
 
-		if util.IsBackendRefGroupKindSupported(
-			backendRef.Group,
-			backendRef.Kind,
-		) && newRefChecker(backendRef).IsRefAllowedByGrant(allowed) {
-			port := int32(-1)
-			if backendRef.Port != nil {
-				port = int32(*backendRef.Port)
-			}
-			namespace := route.GetNamespace()
-			if backendRef.Namespace != nil {
-				namespace = string(*backendRef.Namespace)
-			}
-			backend, err := kongstate.NewServiceBackendForService(
-				k8stypes.NamespacedName{
-					Namespace: namespace,
-					Name:      string(backendRef.Name),
-				},
-				kongstate.PortDef{
-					Mode:   kongstate.PortModeByNumber,
-					Number: port,
-				},
-			)
-			if err != nil {
-				logger.Error(err, "failed to create ServiceBackend for backendRef")
-			}
-			if backendRef.Weight != nil {
-				backend.SetWeight(*backendRef.Weight)
+		nn := client.ObjectKey{
+			Name:      string(backendRef.Name),
+			Namespace: route.GetNamespace(),
+		}
+		if backendRef.Namespace != nil {
+			nn.Namespace = string(*backendRef.Namespace)
+		}
+
+		if backendRef.Kind == nil {
+			// This should never happen as the default value defined by Gateway API is 'Service'. Checking just in case.
+			logger.Error(nil, "Object requested backendRef to target, but no Kind was specified, skipping...")
+			continue
+		}
+
+		var err error
+		switch *backendRef.Kind {
+		case "Service":
+			_, err = storer.GetService(nn.Namespace, nn.Name)
+		default:
+			err = fmt.Errorf("unsupported kind %q, only 'Service' is supported", *backendRef.Kind)
+		}
+		if err != nil {
+			if errors.As(err, &store.NotFoundError{}) {
+				logger.Error(err, "Object requested backendRef to target, but it does not exist, skipping...")
+			} else {
+				logger.Error(err, "Object requested backendRef to target, but an error occurred, skipping...")
 			}
-			backends = append(backends, backend)
-		} else {
+			continue
+		}
+
+		if !util.IsBackendRefGroupKindSupported(backendRef.Group, backendRef.Kind) ||
+			!newRefCheckerForRoute(route, backendRef).IsRefAllowedByGrant(allowed) {
 			// we log impermissible refs rather than failing the entire rule. while we cannot actually route to
 			// these, we do not want a single impermissible ref to take the entire rule offline. in the case of edits,
 			// failing the entire rule could potentially delete routes that were previously online and in use, and
 			// that remain viable (because they still have some permissible backendRefs)
 			logger.Error(nil, "Object requested backendRef to target, but no ReferenceGrant permits it, skipping...")
+			continue
+		}
+
+		port := int32(-1)
+		if backendRef.Port != nil {
+			port = int32(*backendRef.Port)
+		}
+		backend, err := kongstate.NewServiceBackendForService(
+			nn,
+			kongstate.PortDef{
+				Mode:   kongstate.PortModeByNumber,
+				Number: port,
+			},
+		)
+		if err != nil {
+			logger.Error(err, "failed to create ServiceBackend for backendRef")
+			continue
+		}
+		if backendRef.Weight != nil {
+			backend.SetWeight(*backendRef.Weight)
 		}
+		backends = append(backends, backend)
 	}
 
 	return backends

internal/dataplane/translator/backendref_test.go

@@ -0,0 +1,167 @@
+package translator
+
+import (
+	"testing"
+
+	"github.com/go-logr/logr"
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8stypes "k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
+	"github.com/kong/kubernetes-ingress-controller/v3/internal/util/builder"
+)
+
+func TestBackendRefsToKongStateBackends(t *testing.T) {
+	testcases := []struct {
+		name        string
+		route       client.Object
+		backendRefs []gatewayapi.BackendRef
+		allowed     map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo
+		objects     store.FakeObjects
+		expected    kongstate.ServiceBackends
+	}{
+		{
+			name: "correct ReferenceGrant and an existing Service as backendRef returns a KongStateBackend with a Service",
+			route: &gatewayapi.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic-httproute",
+					Namespace: corev1.NamespaceDefault,
+				},
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
+					Hostnames: []gatewayapi.Hostname{
+						"konghq.com",
+						"www.konghq.com",
+					},
+					Rules: []gatewayapi.HTTPRouteRule{{
+						// BackendRefs are taken from the backendRefs argument.
+						// This is done this way because the backendRefs are
+						// extracted from the route's spec before.
+						// Potentially this could be refactored to be extracted
+						// in backendRefsToKongStateBackends using a type switch.
+					}},
+				},
+			},
+			backendRefs: []gatewayapi.BackendRef{
+				builder.NewBackendRef("fake-service").WithPort(80).Build(),
+			},
+			allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
+				gatewayapi.Namespace(corev1.NamespaceDefault): {
+					{
+						Group: "",
+						Kind:  "Service",
+						Name:  lo.ToPtr(gatewayapi.ObjectName("fake-service")),
+					},
+				},
+			},
+			objects: store.FakeObjects{
+				Services: []*corev1.Service{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "fake-service",
+							Namespace: corev1.NamespaceDefault,
+						},
+						Spec: corev1.ServiceSpec{
+							Ports: []corev1.ServicePort{
+								builder.NewServicePort().WithPort(80).Build(),
+							},
+						},
+					},
+				},
+			},
+			expected: func() kongstate.ServiceBackends {
+				svcBackend, err := kongstate.NewServiceBackend(
+					kongstate.ServiceBackendTypeKubernetesService,
+					k8stypes.NamespacedName{Namespace: corev1.NamespaceDefault, Name: "fake-service"},
+					kongstate.PortDef{
+						Mode:   kongstate.PortModeByNumber,
+						Number: 80,
+					},
+				)
+				require.NoError(t, err)
+				return kongstate.ServiceBackends{svcBackend}
+			}(),
+		},
+		{
+			name: "no ReferenceGrant and an existing Service as backendRef doesn't return a KongStateBackend with the Service",
+			route: &gatewayapi.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic-httproute",
+					Namespace: corev1.NamespaceDefault,
+				},
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
+					Hostnames: []gatewayapi.Hostname{
+						"konghq.com",
+						"www.konghq.com",
+					},
+				},
+			},
+			allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
+				gatewayapi.Namespace(corev1.NamespaceDefault): {
+					{
+						Group: "",
+						Kind:  "ImaginaryKind",
+						Name:  lo.ToPtr(gatewayapi.ObjectName("fake-service")),
+					},
+				},
+			},
+			objects: store.FakeObjects{
+				Services: []*corev1.Service{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "fake-service",
+							Namespace: corev1.NamespaceDefault,
+						},
+					},
+				},
+			},
+			expected: kongstate.ServiceBackends{},
+		},
+		{
+			name: "ReferenceGrant and a non existing Service as backendRef doesn't return a KongStateBackend with the Service",
+			route: &gatewayapi.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic-httproute",
+					Namespace: corev1.NamespaceDefault,
+				},
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
+					Hostnames: []gatewayapi.Hostname{
+						"konghq.com",
+						"www.konghq.com",
+					},
+				},
+			},
+			backendRefs: []gatewayapi.BackendRef{
+				builder.NewBackendRef("fake-service").WithPort(80).Build(),
+			},
+			allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
+				gatewayapi.Namespace(corev1.NamespaceDefault): {
+					{
+						Group: "",
+						Kind:  "Service",
+						Name:  lo.ToPtr(gatewayapi.ObjectName("fake-service")),
+					},
+				},
+			},
+			expected: kongstate.ServiceBackends{},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			fakestore, err := store.NewFakeStore(tc.objects)
+			require.NoError(t, err)
+			logger := logr.Discard()
+			ret := backendRefsToKongStateBackends(logger, fakestore, tc.route, tc.backendRefs, tc.allowed)
+			require.Equal(t, tc.expected, ret)
+		})
+	}
+}