fix(gateway-api): verify TLS/TCPRoute is accepted before pushing to Gateway

CHANGELOG.md

@@ -135,9 +135,10 @@ Adding a new version? You'll need three changes:
 - CRDs' validations improvements: `UDPIngressRule.Port`, `IngressRule.Port` and `IngressBackend.ServiceName`
   instead of being validated in the Parser, are validated by the Kubernetes API now.
   [#3136](https://github.com/Kong/kubernetes-ingress-controller/pull/3136)
-- Gateway API: Implement port matching for routes as defined in
+- Gateway API: Implement port matching for HTTPRoute, TCPRoute and TLSRoute as defined in
   [GEP-957](https://gateway-api.sigs.k8s.io/geps/gep-957/)
   [#3129](https://github.com/Kong/kubernetes-ingress-controller/pull/3129)
+  [#3226](https://github.com/Kong/kubernetes-ingress-controller/pull/3226)
 - Warning events are recorded when one of `netv1.Ingress` related issues occurs
   (e.g. backing Kubernetes service couldn't be found, matching Kubernetes service port couldn't be found).
   [#3138](https://github.com/Kong/kubernetes-ingress-controller/pull/3138)

internal/controllers/gateway/gateway_controller.go

@@ -262,8 +262,8 @@ func referenceGrantHasGatewayFrom(obj client.Object) bool {
 // Gateway Controller - Reconciliation
 // -----------------------------------------------------------------------------
 
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/status,verbs=get;update
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/status,verbs=get;update
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -492,6 +492,9 @@ var (
 	// supportedKinds indicates which gateway kinds are supported by this implementation.
 	supportedKinds = []Kind{
 		Kind("HTTPRoute"),
+		Kind("TCPRoute"),
+		Kind("UDPRoute"),
+		Kind("TLSRoute"),
 	}
 
 	// supportedRouteGroupKinds indicates the full kinds with GVK that are supported by this implementation.

internal/controllers/gateway/gateway_utils.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/go-logr/logr"
+	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,6 +21,7 @@ import (
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/util/builder"
 )
 
 // -----------------------------------------------------------------------------
@@ -297,7 +299,7 @@ func (r *GatewayReconciler) getListenerStatus(
 		status := ListenerStatus{
 			Name:           listener.Name,
 			Conditions:     []metav1.Condition{},
-			SupportedKinds: supportedRouteGroupKinds,
+			SupportedKinds: getListenerSupportedRouteKinds(listener),
 			// this has been populated by initializeListenerMaps()
 			AttachedRoutes: listenerToAttached[listener.Name],
 		}
@@ -632,3 +634,39 @@ func isGatewayClassEventInClass(log logr.Logger, watchEvent interface{}) bool {
 
 	return false
 }
+
+// getListenerSupportedRouteKinds determines what RouteGroupKinds are supported by the Listener.
+// If no AllowedRoutes.Kinds are specified for the Listener, the supported RouteGroupKind is derived directly
+// from the Listener's Protocol.
+// Otherwise, user specified AllowedRoutes.Kinds are used, filtered by the global Gateway supported kinds.
+//
+// TODO: Make ListenerStatus.SupportedKinds compliant with GW API specification
+// https://github.com/Kong/kubernetes-ingress-controller/issues/3228
+func getListenerSupportedRouteKinds(l gatewayv1beta1.Listener) []gatewayv1beta1.RouteGroupKind {
+	if l.AllowedRoutes == nil || len(l.AllowedRoutes.Kinds) == 0 {
+		switch string(l.Protocol) {
+		case string(gatewayv1beta1.HTTPProtocolType), string(gatewayv1beta1.HTTPSProtocolType):
+			return builder.NewRouteGroupKind().HTTPRoute().IntoSlice()
+		case string(gatewayv1beta1.TCPProtocolType):
+			return builder.NewRouteGroupKind().TCPRoute().IntoSlice()
+		case string(gatewayv1beta1.UDPProtocolType):
+			return builder.NewRouteGroupKind().UDPRoute().IntoSlice()
+		case string(gatewayv1beta1.TLSProtocolType):
+			return builder.NewRouteGroupKind().TLSRoute().IntoSlice()
+		}
+	}
+
+	var supportedRGK []gatewayv1beta1.RouteGroupKind
+	for _, gk := range l.AllowedRoutes.Kinds {
+		if gk.Group != nil && *gk.Group == gatewayv1beta1.GroupName {
+			_, ok := lo.Find(supportedKinds, func(k Kind) bool {
+				return gk.Kind == k
+			})
+			if ok {
+				supportedRGK = append(supportedRGK, gk)
+			}
+		}
+	}
+
+	return supportedRGK
+}

internal/controllers/gateway/gateway_utils_test.go

@@ -0,0 +1,85 @@
+package gateway
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/util/builder"
+)
+
+func TestGetListenerSupportedRouteKinds(t *testing.T) {
+	testCases := []struct {
+		name                   string
+		listener               Listener
+		expectedSupportedKinds []gatewayv1beta1.RouteGroupKind
+	}{
+		{
+			name: "only HTTP protocol specified",
+			listener: Listener{
+				Protocol: HTTPProtocolType,
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().HTTPRoute().IntoSlice(),
+		},
+		{
+			name: "only HTTPS protocol specified",
+			listener: Listener{
+				Protocol: HTTPSProtocolType,
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().HTTPRoute().IntoSlice(),
+		},
+		{
+			name: "only TCP protocol specified",
+			listener: Listener{
+				Protocol: TCPProtocolType,
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().TCPRoute().IntoSlice(),
+		},
+		{
+			name: "only UDP protocol specified",
+			listener: Listener{
+				Protocol: UDPProtocolType,
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().UDPRoute().IntoSlice(),
+		},
+		{
+			name: "only TLS protocol specified",
+			listener: Listener{
+				Protocol: TLSProtocolType,
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().TLSRoute().IntoSlice(),
+		},
+		{
+			name: "Kind not included in global gets discarded",
+			listener: Listener{
+				Protocol: HTTPProtocolType,
+				AllowedRoutes: &gatewayv1beta1.AllowedRoutes{
+					Kinds: []gatewayv1beta1.RouteGroupKind{{
+						Group: addressOf(gatewayv1beta1.Group("unknown.group.com")),
+						Kind:  Kind("UnknownKind"),
+					}},
+				},
+			},
+			expectedSupportedKinds: nil,
+		},
+		{
+			name: "Kind included in global gets passed",
+			listener: Listener{
+				Protocol: HTTPProtocolType,
+				AllowedRoutes: &gatewayv1beta1.AllowedRoutes{
+					Kinds: builder.NewRouteGroupKind().HTTPRoute().IntoSlice(),
+				},
+			},
+			expectedSupportedKinds: builder.NewRouteGroupKind().HTTPRoute().IntoSlice(),
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := getListenerSupportedRouteKinds(tc.listener)
+			require.Equal(t, tc.expectedSupportedKinds, got)
+		})
+	}
+}

internal/controllers/gateway/tcproute_controller.go

@@ -228,8 +228,8 @@ func (r *TCPRouteReconciler) listTCPRoutesForGateway(obj client.Object) []reconc
 // TCPRoute Controller - Reconciliation
 // -----------------------------------------------------------------------------
 
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tcproutes,verbs=get;list;watch
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tcproutes/status,verbs=get;update
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tcproutes,verbs=get;list;watch
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tcproutes/status,verbs=get;update
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -310,19 +310,27 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 	}
 
-	// if the gateways are ready, and the TCPRoute is destined for them, ensure that
-	// the object is pushed to the dataplane.
-	if err := r.DataplaneClient.UpdateObject(tcproute); err != nil {
-		debug(log, tcproute, "failed to update object in data-plane, requeueing")
-		return ctrl.Result{}, err
-	}
-	if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
-		// if the dataplane client has reporting enabled (this is the default and is
-		// tied in with status updates being enabled in the controller manager) then
-		// we will wait until the object is reported as successfully configured before
-		// moving on to status updates.
-		if !r.DataplaneClient.KubernetesObjectIsConfigured(tcproute) {
-			return ctrl.Result{Requeue: true}, nil
+	if isRouteAccepted(gateways) {
+		// if the gateways are ready, and the TCPRoute is destined for them, ensure that
+		// the object is pushed to the dataplane.
+		if err := r.DataplaneClient.UpdateObject(tcproute); err != nil {
+			debug(log, tcproute, "failed to update object in data-plane, requeueing")
+			return ctrl.Result{}, err
+		}
+		if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
+			// if the dataplane client has reporting enabled (this is the default and is
+			// tied in with status updates being enabled in the controller manager) then
+			// we will wait until the object is reported as successfully configured before
+			// moving on to status updates.
+			if !r.DataplaneClient.KubernetesObjectIsConfigured(tcproute) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+		}
+	} else {
+		// route is not accepted, remove it from kong store
+		if err := r.DataplaneClient.DeleteObject(tcproute); err != nil {
+			debug(log, tcproute, "failed to delete object in data-plane, requeueing")
+			return ctrl.Result{}, err
 		}
 	}
 

internal/controllers/gateway/tlsroute_controller.go

@@ -228,8 +228,8 @@ func (r *TLSRouteReconciler) listTLSRoutesForGateway(obj client.Object) []reconc
 // TLSRoute Controller - Reconciliation
 // -----------------------------------------------------------------------------
 
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tlsroutes,verbs=get;list;watch
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tlsroutes/status,verbs=get;update
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tlsroutes,verbs=get;list;watch
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=tlsroutes/status,verbs=get;update
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -310,19 +310,27 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 	}
 
-	// if the gateways are ready, and the TLSRoute is destined for them, ensure that
-	// the object is pushed to the dataplane.
-	if err := r.DataplaneClient.UpdateObject(tlsroute); err != nil {
-		debug(log, tlsroute, "failed to update object in data-plane, requeueing")
-		return ctrl.Result{}, err
-	}
-	if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
-		// if the dataplane client has reporting enabled (this is the default and is
-		// tied in with status updates being enabled in the controller manager) then
-		// we will wait until the object is reported as successfully configured before
-		// moving on to status updates.
-		if !r.DataplaneClient.KubernetesObjectIsConfigured(tlsroute) {
-			return ctrl.Result{Requeue: true}, nil
+	if isRouteAccepted(gateways) {
+		// if the gateways are ready, and the TLSRoute is destined for them, ensure that
+		// the object is pushed to the dataplane.
+		if err := r.DataplaneClient.UpdateObject(tlsroute); err != nil {
+			debug(log, tlsroute, "failed to update object in data-plane, requeueing")
+			return ctrl.Result{}, err
+		}
+		if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
+			// if the dataplane client has reporting enabled (this is the default and is
+			// tied in with status updates being enabled in the controller manager) then
+			// we will wait until the object is reported as successfully configured before
+			// moving on to status updates.
+			if !r.DataplaneClient.KubernetesObjectIsConfigured(tlsroute) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+		}
+	} else {
+		// route is not accepted, remove it from kong store
+		if err := r.DataplaneClient.DeleteObject(tlsroute); err != nil {
+			debug(log, tlsroute, "failed to delete object in data-plane, requeueing")
+			return ctrl.Result{}, err
 		}
 	}
 

internal/controllers/gateway/udproute_controller.go

@@ -228,8 +228,8 @@ func (r *UDPRouteReconciler) listUDPRoutesForGateway(obj client.Object) []reconc
 // UDPRoute Controller - Reconciliation
 // -----------------------------------------------------------------------------
 
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=udproutes,verbs=get;list;watch
-//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=udproutes/status,verbs=get;update
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=udproutes,verbs=get;list;watch
+// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=udproutes/status,verbs=get;update
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -310,6 +310,9 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 	}
 
+	// TODO: UDPRoute should be 'Accepted' before proceeding further. Get here back once
+	// https://github.com/Kong/kubernetes-ingress-controller/issues/2544 is implemented.
+
 	// if the gateways are ready, and the UDPRoute is destined for them, ensure that
 	// the object is pushed to the dataplane.
 	if err := r.DataplaneClient.UpdateObject(udproute); err != nil {

test/integration/tcproute_test.go

@@ -376,6 +376,45 @@ func TestTCPRouteEssentials(t *testing.T) {
 		responded, err := test.TCPEchoResponds(fmt.Sprintf("%s:%d", proxyURL.Hostname(), ktfkong.DefaultTCPServicePort), testUUID1)
 		return responded == false && (errors.Is(err, io.EOF) || errors.Is(err, syscall.ECONNRESET))
 	}, ingressWait, waitTick)
+
+	t.Log("testing port matching")
+	t.Log("putting the Gateway back")
+	_, err = DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayv1beta1.Gateway) {
+		gw.Name = gatewayName
+		gw.Spec.Listeners = []gatewayv1beta1.Listener{{
+			Name:     "tcp",
+			Protocol: gatewayv1beta1.TCPProtocolType,
+			Port:     gatewayv1beta1.PortNumber(ktfkong.DefaultTCPServicePort),
+		}}
+	})
+	require.NoError(t, err)
+	t.Log("putting the GatewayClass back")
+	_, err = DeployGatewayClass(ctx, gatewayClient, gatewayClassName)
+	require.NoError(t, err)
+
+	t.Log("verifying that the TCPRoute responds before specifying a port not existent in Gateway")
+	require.Eventually(t, func() bool {
+		responded, err := test.TCPEchoResponds(fmt.Sprintf("%s:%d", proxyURL.Hostname(), ktfkong.DefaultTCPServicePort), testUUID1)
+		return err == nil && responded == true
+	}, ingressWait, waitTick)
+
+	t.Log("setting the port in ParentRef which does not have a matching listener in Gateway")
+	require.Eventually(t, func() bool {
+		tcpRoute, err = gatewayClient.GatewayV1alpha2().TCPRoutes(ns.Name).Get(ctx, tcpRoute.Name, metav1.GetOptions{})
+		if err != nil {
+			return false
+		}
+		notExistingPort := gatewayv1alpha2.PortNumber(81)
+		tcpRoute.Spec.ParentRefs[0].Port = &notExistingPort
+		tcpRoute, err = gatewayClient.GatewayV1alpha2().TCPRoutes(ns.Name).Update(ctx, tcpRoute, metav1.UpdateOptions{})
+		return err == nil
+	}, time.Minute, time.Second)
+
+	t.Log("verifying that the TCPRoute does not respond after specifying a port not existent in Gateway")
+	require.Eventually(t, func() bool {
+		responded, err := test.TCPEchoResponds(fmt.Sprintf("%s:%d", proxyURL.Hostname(), ktfkong.DefaultTCPServicePort), testUUID1)
+		return responded == false && (errors.Is(err, io.EOF) || errors.Is(err, syscall.ECONNRESET))
+	}, ingressWait, waitTick)
 }
 
 func TestTCPRouteReferenceGrant(t *testing.T) {