handler_test.go

/*
SPDX-License-Identifier: Apache-2.0

Copyright Contributors to the Submariner project.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package ovn_test

import (
	"context"
	"net"
	"os"

	"github.com/kelseyhightower/envconfig"
	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"
	libovsdbclient "github.com/ovn-org/libovsdb/client"
	"github.com/ovn-org/libovsdb/model"
	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
	"github.com/submariner-io/admiral/pkg/syncer/test"
	assert "github.com/submariner-io/admiral/pkg/test"
	"github.com/submariner-io/admiral/pkg/watcher"
	submarinerv1 "github.com/submariner-io/submariner/pkg/apis/submariner.io/v1"
	"github.com/submariner-io/submariner/pkg/event"
	"github.com/submariner-io/submariner/pkg/event/testing"
	netlinkAPI "github.com/submariner-io/submariner/pkg/netlink"
	"github.com/submariner-io/submariner/pkg/packetfilter"
	fakePF "github.com/submariner-io/submariner/pkg/packetfilter/fake"
	"github.com/submariner-io/submariner/pkg/routeagent_driver/chains"
	"github.com/submariner-io/submariner/pkg/routeagent_driver/constants"
	"github.com/submariner-io/submariner/pkg/routeagent_driver/environment"
	"github.com/submariner-io/submariner/pkg/routeagent_driver/handlers/ovn"
	fakeovn "github.com/submariner-io/submariner/pkg/routeagent_driver/handlers/ovn/fake"
	"github.com/vishvananda/netlink"
	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	k8snet "k8s.io/utils/net"
)

const (
	ipv4ClusterCIDR      = "171.0.1.0/24"
	ipv4serviceCIDR      = "181.0.1.0/24"
	ipv6ClusterCIDR      = "c000:100::/64"
	ipv6serviceCIDR      = "d000:100::/64"
	ipv4OVNK8sMgmntIntGw = "100.1.1.1"
	ipv6OVNK8sMgmntIntGw = "b000:100::"
	ipv4MatchField       = "ip4.dst"
	ipv6MatchField       = "ip6.dst"
)

var (
	ipv4Subnets = []string{"192.0.1.0/24", "192.0.2.0/24", "192.0.3.0/24"}
	ipv6Subnets = []string{"fc00:100::/64", "fd00:100::/64", "fe00:100::/64"}
)

var _ = Describe("Handler", func() {
	t := newHandlerTestDriver()

	Context("IPv4", func() {
		t.testRemoteEndpoint(ipv4Subnets, ipv6Subnets)
		t.testGatewayTransitions(ipv4Subnets, ipv6Subnets)
		t.testGatewayRoute(ipv4Subnets, ipv6OVNK8sMgmntIntGw, ipv6Subnets)
		t.testNonGatewayRoutes(ipv4OVNK8sMgmntIntGw, ipv4Subnets, []string{"172.0.1.0/24"}, ipv6OVNK8sMgmntIntGw, ipv6Subnets)
	})

	Context("IPv6", func() {
		BeforeEach(func() {
			t.ipFamily = k8snet.IPv6
		})

		t.testRemoteEndpoint(ipv6Subnets, ipv4Subnets)
		t.testGatewayTransitions(ipv6Subnets, ipv4Subnets)
		t.testGatewayRoute(ipv6Subnets, ipv4OVNK8sMgmntIntGw, ipv4Subnets)
		t.testNonGatewayRoutes(ipv6OVNK8sMgmntIntGw, ipv6Subnets, []string{"ab00:100::/64"}, ipv4OVNK8sMgmntIntGw, ipv4Subnets)
	})

	When("the OVN management interface address changes", t.testOVNMgmtInterfaceAddressChange)

	Context("on Uninstall", t.testUninstall)

	When("Intra-cluster routing is disabled", t.testIntraClusterRoutingDisabled)
})

var _ = Describe("GetHandlers", func() {
	t := newTestDriver()

	var env environment.Specification

	BeforeEach(func() {
		fakePF.New(k8snet.IPv4)

		os.Setenv("SUBMARINER_CLUSTERCIDR", ipv4ClusterCIDR)
		os.Setenv("SUBMARINER_SERVICECIDR", ipv4serviceCIDR)
		os.Setenv("SUBMARINER_NAMESPACE", testing.Namespace)

		DeferCleanup(func() {
			os.Unsetenv("SUBMARINER_CLUSTERCIDR")
			os.Unsetenv("SUBMARINER_SERVICECIDR")
			os.Unsetenv("SUBMARINER_NAMESPACE")
		})
	})

	JustBeforeEach(func() {
		Expect(envconfig.Process("submariner", &env)).To(Succeed())
	})

	It("should return the correct Handlers", func() {
		handlers := ovn.GetHandlers(k8snet.IPv4, &env, t.submClient, t.k8sClient, t.dynClient, &watcher.Config{
			Client: t.dynClient,
		})

		Expect(handlers).To(HaveLen(3))

		ovnHandler, ok := handlers[0].(*ovn.Handler)
		Expect(ok).To(BeTrue())
		Expect(ovnHandler.Namespace).To(Equal(env.Namespace))
		Expect(ovnHandler.ClusterCIDR).To(Equal(env.ClusterCidr))
		Expect(ovnHandler.ServiceCIDR).To(Equal(env.ServiceCidr))
		Expect(ovnHandler.IntraRoutingDisabled).To(BeFalse())

		_, ok = handlers[1].(*ovn.GatewayRouteHandler)
		Expect(ok).To(BeTrue())

		_, ok = handlers[2].(*ovn.NonGatewayRouteHandler)
		Expect(ok).To(BeTrue())
	})

	When("Intra-cluster routing is disabled", func() {
		BeforeEach(func() {
			os.Setenv("SUBMARINER_INTRAROUTINGDISABLED", "true")

			DeferCleanup(func() {
				os.Unsetenv("SUBMARINER_INTRAROUTINGDISABLED")
			})
		})

		It("should not return the NonGatewayRouteHandler", func() {
			handlers := ovn.GetHandlers(k8snet.IPv4, &env, t.submClient, t.k8sClient, t.dynClient, &watcher.Config{
				Client: t.dynClient,
			})

			for _, h := range handlers {
				_, ok := h.(*ovn.NonGatewayRouteHandler)
				Expect(ok).To(BeFalse())
			}
		})
	})
})

type handlerTestDriver struct {
	*testDriver
	handler              event.Handler
	pFilter              *fakePF.PacketFilter
	ipFamily             k8snet.IPFamily
	clusterCIDR          string
	serviceCIDR          string
	OVNK8sMgmntIntGw     string
	ovsdbClient          *fakeovn.OVSDBClient
	intraRoutingDisabled bool
}

func newHandlerTestDriver() *handlerTestDriver {
	t := &handlerTestDriver{testDriver: newTestDriver()}

	BeforeEach(func() {
		t.ipFamily = k8snet.IPv4
		t.intraRoutingDisabled = false
	})

	JustBeforeEach(func(ctx context.Context) {
		t.ovsdbClient = fakeovn.NewOVSDBClient()

		_, _ = t.ovsdbClient.Create(&nbdb.LogicalRouter{
			Name: ovn.OVNClusterRouter,
		})

		t.netLink.SetupDefaultGateway(t.ipFamily, net.Interface{Name: "gw-intf"})

		t.pFilter = fakePF.New(t.ipFamily)

		if t.ipFamily == k8snet.IPv4 {
			t.clusterCIDR = ipv4ClusterCIDR
			t.serviceCIDR = ipv4serviceCIDR
			t.OVNK8sMgmntIntGw = ipv4OVNK8sMgmntIntGw
		} else {
			t.clusterCIDR = ipv6ClusterCIDR
			t.serviceCIDR = ipv6serviceCIDR
			t.OVNK8sMgmntIntGw = ipv6OVNK8sMgmntIntGw
		}

		_, err := t.k8sClient.CoreV1().Pods(testing.Namespace).Create(ctx, &corev1.Pod{
			ObjectMeta: metav1.ObjectMeta{
				Name:   "ovn-pod",
				Labels: map[string]string{"app": "ovnkube-node"},
			},
		}, metav1.CreateOptions{})
		Expect(err).To(Succeed())

		Expect(t.netLink.RouteAdd(&netlink.Route{
			LinkIndex: OVNK8sMgmntIntIndex,
			Family:    netlinkAPI.ToNetlinkFamily(t.ipFamily),
			Dst:       toIPNet(t.clusterCIDR),
			Gw:        net.ParseIP(t.OVNK8sMgmntIntGw),
		})).To(Succeed())

		restMapper := test.GetRESTMapperFor(&submarinerv1.GatewayRoute{}, &submarinerv1.NonGatewayRoute{})

		transitSwitchIP := ovn.NewTransitSwitchIP(t.ipFamily)

		t.handler = ovn.NewHandler(t.ipFamily, &ovn.HandlerConfig{
			Namespace:   testing.Namespace,
			ClusterCIDR: []string{t.clusterCIDR},
			ServiceCIDR: []string{t.serviceCIDR},
			SubmClient:  t.submClient,
			K8sClient:   t.k8sClient,
			DynClient:   t.dynClient,
			WatcherConfig: &watcher.Config{
				RestMapper: restMapper,
				Client:     t.dynClient,
			},
			NewOVSDBClient: func(_ model.ClientDBModel, _ ...libovsdbclient.Option) (libovsdbclient.Client, error) {
				return t.ovsdbClient, nil
			},
			TransitSwitchIP:      transitSwitchIP,
			IntraRoutingDisabled: t.intraRoutingDisabled,
		})

		t.Start(ctx, t.handler)

		Expect(t.ovsdbClient.Connected()).To(BeTrue())
	})

	return t
}

func (t *handlerTestDriver) Start(ctx context.Context, handler event.Handler) {
	t.ControllerSupport.Start(ctx, handler)
	t.CreateNode(ctx, t.node)
}

//nolint:gocognit // Ignore "cognitive complexity ... is high".
func (t *handlerTestDriver) testRemoteEndpoint(ipFamilySubnets, nonIPFamilySubnets []string) {
	var (
		newEndpointSubnet string
		endpointSubnets   []string
	)

	BeforeEach(func() {
		newEndpointSubnet = ipFamilySubnets[len(ipFamilySubnets)-1]
		endpointSubnets = ipFamilySubnets[:len(ipFamilySubnets)-1]
	})

	When("a remote Endpoint is created, updated, and deleted", func() {
		It("should correctly update the host network dataplane", func(ctx context.Context) {
			By("Creating remote Endpoint")

			endpoint := t.createEndpoint(ctx, append(endpointSubnets, nonIPFamilySubnets...)...)

			for _, s := range endpointSubnets {
				t.netLink.AwaitRule(constants.RouteAgentHostNetworkTableID, "", s)
				t.netLink.EnsureNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
				t.netLink.EnsureNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)
			}

			for _, s := range nonIPFamilySubnets {
				t.netLink.EnsureNoRule(constants.RouteAgentHostNetworkTableID, "", s)
			}

			t.netLink.AwaitGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)

			By("Updating remote Endpoint")

			oldSubnets := endpointSubnets
			endpointSubnets := make([]string, 0, 1+len(nonIPFamilySubnets))
			endpointSubnets = append(endpointSubnets, newEndpointSubnet)

			//nolint:gocritic // Ignore "append result not assigned to the same slice"
			endpoint.Spec.Subnets = append(endpointSubnets, nonIPFamilySubnets...)

			t.UpdateEndpoint(ctx, endpoint)

			for _, s := range oldSubnets {
				t.netLink.AwaitNoRule(constants.RouteAgentHostNetworkTableID, "", s)
			}

			for _, s := range endpointSubnets {
				t.netLink.AwaitRule(constants.RouteAgentHostNetworkTableID, "", s)
			}

			By("Deleting remote Endpoint")

			t.DeleteEndpoint(ctx, endpoint.Name)

			for _, s := range endpointSubnets {
				t.netLink.AwaitNoRule(constants.RouteAgentHostNetworkTableID, "", s)
			}
		})

		Context("on the gateway", func() {
			JustBeforeEach(func(ctx context.Context) {
				t.CreateLocalHostEndpoint(ctx)
			})

			It("should correctly update the gateway dataplane", func(ctx context.Context) {
				By("Creating remote Endpoint")

				endpoint := t.createEndpoint(ctx, append(endpointSubnets, nonIPFamilySubnets...)...)

				for _, s := range endpointSubnets {
					t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
					t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)

					t.pFilter.AwaitRule(packetfilter.TableTypeNAT, chains.SmPostRouting, ContainSubstring("\"SrcCIDR\":%q", s))
					t.pFilter.AwaitRule(packetfilter.TableTypeNAT, chains.SmPostRouting, ContainSubstring("\"DestCIDR\":%q", s))
				}

				t.awaitOVNKNodeAnnotationContaining(ctx, endpointSubnets...)

				By("Updating remote Endpoint")

				oldSubnets := endpointSubnets
				endpointSubnets := make([]string, 0, 2+len(nonIPFamilySubnets))
				endpointSubnets = append(endpointSubnets, oldSubnets[0], newEndpointSubnet)

				//nolint:gocritic // Ignore "append result not assigned to the same slice"
				endpoint.Spec.Subnets = append(endpointSubnets, nonIPFamilySubnets...)

				t.UpdateEndpoint(ctx, endpoint)

				for i := 1; i < len(oldSubnets); i++ {
					t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, oldSubnets[i], t.clusterCIDR)
					t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, oldSubnets[i], t.serviceCIDR)
				}

				for _, s := range endpointSubnets {
					t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
					t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)
				}

				By("Deleting remote Endpoint")

				t.DeleteEndpoint(ctx, endpoint.Name)

				for _, s := range endpointSubnets {
					t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
					t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)

					t.pFilter.AwaitNoRule(packetfilter.TableTypeNAT, chains.SmPostRouting, ContainSubstring("\"SrcCIDR\":%q", s))
					t.pFilter.AwaitNoRule(packetfilter.TableTypeNAT, chains.SmPostRouting, ContainSubstring("\"DestCIDR\":%q", s))
				}

				// Since we updated the subnets above, the original second one will remain b/c the annotation isn't currently
				// updated on an Endpoint update.
				t.awaitOVNKNodeAnnotationContaining(ctx, oldSubnets[1])
			})
		})
	})
}

func (t *handlerTestDriver) testGatewayTransitions(ipFamilySubnets, nonIPFamilySubnets []string) {
	Context("on gateway transitions", func() {
		It("should correctly update the gateway dataplane", func(ctx context.Context) {
			t.createEndpoint(ctx, append(ipFamilySubnets, nonIPFamilySubnets...)...)

			By("Creating local gateway Endpoint")

			localEP := t.CreateLocalHostEndpoint(ctx)

			for _, s := range ipFamilySubnets {
				t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
				t.netLink.AwaitRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)
				t.pFilter.AwaitRule(packetfilter.TableTypeFilter, chains.SmForward, ContainSubstring(s))
				t.pFilter.AwaitRule(packetfilter.TableTypeFilter, chains.SmForwardMSSClamp, ContainSubstring(s))
			}

			for _, s := range nonIPFamilySubnets {
				t.netLink.EnsureNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
				t.pFilter.EnsureNoRule(packetfilter.TableTypeFilter, chains.SmForward, ContainSubstring(s))
				t.pFilter.EnsureNoRule(packetfilter.TableTypeFilter, chains.SmForwardMSSClamp, ContainSubstring(s))
			}

			t.awaitOVNKNodeAnnotationContaining(ctx, ipFamilySubnets...)

			t.netLink.AwaitGwRoutes(0, constants.RouteAgentInterClusterNetworkTableID, t.OVNK8sMgmntIntGw)

			By("Deleting local gateway Endpoint")

			t.DeleteEndpoint(ctx, localEP.Name)

			for _, s := range ipFamilySubnets {
				t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.clusterCIDR)
				t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, s, t.serviceCIDR)
				t.pFilter.AwaitNoRule(packetfilter.TableTypeFilter, chains.SmForward, ContainSubstring(s))
				t.pFilter.AwaitNoRule(packetfilter.TableTypeFilter, chains.SmForwardMSSClamp, ContainSubstring(s))
			}

			t.awaitOVNKNodeAnnotationContaining(ctx)

			t.netLink.AwaitNoGwRoutes(0, constants.RouteAgentInterClusterNetworkTableID, t.OVNK8sMgmntIntGw)
		})
	})
}

func (t *handlerTestDriver) getIPMatchField() string {
	if t.ipFamily == k8snet.IPv6 {
		return ipv6MatchField
	}

	return ipv4MatchField
}

func (t *handlerTestDriver) testGatewayRoute(ipFamilySubnets []string, nonIPFamilyNextHop string, nonIPFamilySubnets []string) {
	When("a GatewayRoute is created and deleted", func() {
		It("should correctly reconcile OVN router policies", func(ctx context.Context) {
			client := t.dynClient.Resource(submarinerv1.SchemeGroupVersion.WithResource("gatewayroutes")).Namespace(testing.Namespace)
			ipMatchField := t.getIPMatchField()

			gwRoute := &submarinerv1.GatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-gateway-route",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{t.OVNK8sMgmntIntCIDR[t.ipFamily].IP.String()},
					RemoteCIDRs: ipFamilySubnets,
				},
			}

			test.CreateResource(ctx, client, gwRoute)

			for _, cidr := range gwRoute.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterPolicy{
					Match:    ipMatchField + " == " + cidr,
					Nexthops: gwRoute.RoutePolicySpec.NextHops,
				})

				t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterStaticRoute{
					IPPrefix: cidr,
				})
			}

			Expect(client.Delete(ctx, gwRoute.Name, metav1.DeleteOptions{})).To(Succeed())

			for _, cidr := range gwRoute.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitNoModel(&nbdb.LogicalRouterPolicy{
					Match:    ipMatchField + " == " + cidr,
					Nexthops: gwRoute.RoutePolicySpec.NextHops,
				})

				t.ovsdbClient.AwaitNoModel(&nbdb.LogicalRouterStaticRoute{
					IPPrefix: cidr,
				})
			}

			test.CreateResource(ctx, client, &submarinerv1.GatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-gateway-route",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{nonIPFamilyNextHop},
					RemoteCIDRs: nonIPFamilySubnets,
				},
			})
		})
	})

	When("a non-Submariner route exists with the same nexthop", func() {
		It("should not delete the non-Submariner route during reconciliation", func(ctx context.Context) {
			client := t.dynClient.Resource(submarinerv1.SchemeGroupVersion.WithResource("gatewayroutes")).Namespace(testing.Namespace)

			// Create a route that simulates an OVN-K managed route (no submariner external_id)
			// This uses the same nexthop as Submariner but has a different prefix (local cluster subnet)
			ovnkRoute := &nbdb.LogicalRouterStaticRoute{
				IPPrefix:    t.clusterCIDR,
				Nexthop:     t.OVNK8sMgmntIntCIDR[t.ipFamily].IP.String(),
				ExternalIDs: map[string]string{},
			}

			_, err := t.ovsdbClient.Create(ovnkRoute)
			Expect(err).To(Succeed())

			// Create a GatewayRoute which will trigger reconciliation
			gwRoute := &submarinerv1.GatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-gateway-route-preserve",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{ovnkRoute.Nexthop},
					RemoteCIDRs: ipFamilySubnets,
				},
			}

			test.CreateResource(ctx, client, gwRoute)

			// Wait for Submariner routes to be reconciled.
			for _, cidr := range gwRoute.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterStaticRoute{
					IPPrefix: cidr,
				})
			}

			// Verify the OVN-K route still exists and is untagged
			t.ovsdbClient.EnsureModel(&nbdb.LogicalRouterStaticRoute{
				IPPrefix: ovnkRoute.IPPrefix,
			})
			retrievedRoute := t.ovsdbClient.GetModel(&nbdb.LogicalRouterStaticRoute{
				IPPrefix: ovnkRoute.IPPrefix,
			}).(*nbdb.LogicalRouterStaticRoute)
			Expect(retrievedRoute.ExternalIDs).ToNot(HaveKey(ovn.SubmarinerExternalIDKey),
				"OVN-K route should not be tagged with submariner")

			Expect(client.Delete(ctx, gwRoute.Name, metav1.DeleteOptions{})).To(Succeed())

			// Check the OVN-K route still exists and remains untagged after removing the gateway
			t.ovsdbClient.EnsureModel(&nbdb.LogicalRouterStaticRoute{
				IPPrefix: ovnkRoute.IPPrefix,
			})
			retrievedRoute = t.ovsdbClient.GetModel(&nbdb.LogicalRouterStaticRoute{
				IPPrefix: ovnkRoute.IPPrefix,
			}).(*nbdb.LogicalRouterStaticRoute)
			Expect(retrievedRoute.ExternalIDs).ToNot(HaveKey(ovn.SubmarinerExternalIDKey),
				"OVN-K route should not be tagged with submariner after cleanup")
		})
	})

	When("a non-Submariner policy exists with the same priority", func() {
		It("should not delete the non-Submariner policy during reconciliation", func(ctx context.Context) {
			client := t.dynClient.Resource(submarinerv1.SchemeGroupVersion.WithResource("gatewayroutes")).Namespace(testing.Namespace)

			// Determine priority and match field based on IP family
			priority := 20000
			ipMatchField := t.getIPMatchField()

			if t.ipFamily == k8snet.IPv6 {
				priority = 20100
			}

			// Create a policy that simulates an OVN-K managed policy (no submariner external_id)
			// This uses the same priority as Submariner but has a different match
			ovnkPolicy := &nbdb.LogicalRouterPolicy{
				Priority:    priority,
				Match:       ipMatchField + " == " + t.clusterCIDR,
				Action:      "reroute",
				Nexthop:     new(t.OVNK8sMgmntIntCIDR[t.ipFamily].IP.String()),
				ExternalIDs: map[string]string{}, // No submariner tag
			}

			_, err := t.ovsdbClient.Create(ovnkPolicy)
			Expect(err).To(Succeed())

			// Create a GatewayRoute which will trigger reconciliation
			gwRoute := &submarinerv1.GatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-gateway-route-preserve-policy",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{*ovnkPolicy.Nexthop},
					RemoteCIDRs: ipFamilySubnets,
				},
			}

			test.CreateResource(ctx, client, gwRoute)

			// Wait for Submariner policies to be reconciled
			for _, cidr := range gwRoute.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterPolicy{
					Match:    ipMatchField + " == " + cidr,
					Nexthops: []string{*ovnkPolicy.Nexthop},
				})
			}

			// Verify the OVN-K policy still exists and is untagged
			t.ovsdbClient.EnsureModel(&nbdb.LogicalRouterPolicy{
				Match:   ovnkPolicy.Match,
				Nexthop: ovnkPolicy.Nexthop,
			})
			retrievedPolicy := t.ovsdbClient.GetModel(&nbdb.LogicalRouterPolicy{
				Match:   ovnkPolicy.Match,
				Nexthop: ovnkPolicy.Nexthop,
			}).(*nbdb.LogicalRouterPolicy)
			Expect(retrievedPolicy.ExternalIDs).ToNot(HaveKey(ovn.SubmarinerExternalIDKey),
				"OVN-K policy should not be tagged with submariner")

			Expect(client.Delete(ctx, gwRoute.Name, metav1.DeleteOptions{})).To(Succeed())

			// Check the OVN-K policy still exists and remains untagged after removing the gateway
			t.ovsdbClient.EnsureModel(&nbdb.LogicalRouterPolicy{
				Match:   ovnkPolicy.Match,
				Nexthop: ovnkPolicy.Nexthop,
			})
			retrievedPolicy = t.ovsdbClient.GetModel(&nbdb.LogicalRouterPolicy{
				Match:   ovnkPolicy.Match,
				Nexthop: ovnkPolicy.Nexthop,
			}).(*nbdb.LogicalRouterPolicy)
			Expect(retrievedPolicy.ExternalIDs).ToNot(HaveKey(ovn.SubmarinerExternalIDKey),
				"OVN-K policy should not be tagged with submariner after cleanup")
		})

		It("should migrate policies from deprecated Nexthop to Nexthops field", func(ctx context.Context) {
			client := t.dynClient.Resource(submarinerv1.SchemeGroupVersion.WithResource("gatewayroutes")).Namespace(testing.Namespace)

			// Create an old-style policy with deprecated Nexthop field (and Nexthops populated)
			// simulating what older Submariner versions created
			priority := 20000 // ovnRoutePoliciesPrioV4
			ipMatchField := t.getIPMatchField()

			if t.ipFamily == k8snet.IPv6 {
				priority = 20100 // ovnRoutePoliciesPrioV6
			}

			nextHop := t.OVNK8sMgmntIntCIDR[t.ipFamily].IP.String()
			testSubnet := ipFamilySubnets[0]
			legacyNexthop := new(nextHop) // Save the Nexthop pointer for verification

			oldPolicy := &nbdb.LogicalRouterPolicy{
				Priority:    priority,
				Match:       ipMatchField + " == " + testSubnet,
				Action:      "reroute",
				Nexthop:     legacyNexthop,     // Deprecated field
				Nexthops:    []string{nextHop}, // New field also populated
				ExternalIDs: map[string]string{ovn.SubmarinerExternalIDKey: "test"},
			}

			_, err := t.ovsdbClient.Create(oldPolicy)
			Expect(err).To(Succeed())

			// Create a GatewayRoute which will trigger reconciliation
			gwRoute := &submarinerv1.GatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-gateway-route-migration",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{nextHop},
					RemoteCIDRs: ipFamilySubnets,
				},
			}

			test.CreateResource(ctx, client, gwRoute)

			// Verify the legacy policy with Nexthop field is removed
			t.ovsdbClient.AwaitNoModel(&nbdb.LogicalRouterPolicy{
				Match:   ipMatchField + " == " + testSubnet,
				Nexthop: legacyNexthop,
			})

			// Verify new policy with only Nexthops field exists
			t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterPolicy{
				Match:    ipMatchField + " == " + testSubnet,
				Nexthops: []string{nextHop},
			})

			// Verify the new policy exists with Nexthops array
			retrievedPolicy := t.ovsdbClient.GetModel(&nbdb.LogicalRouterPolicy{
				Match:    ipMatchField + " == " + testSubnet,
				Nexthops: []string{nextHop},
			}).(*nbdb.LogicalRouterPolicy)

			// Note: Skipping Nexthop=nil assertion due to fake OVSDB client limitations
			Expect(retrievedPolicy.Nexthops).To(Equal([]string{nextHop}), "Migrated policy should have Nexthops array")
		})
	})
}

func (t *handlerTestDriver) testNonGatewayRoutes(ipFamilyNextHop string, ipFamilyCIDRs1, ipFamilyCIDRs2 []string, nonIPFamilyNextHop string,
	nonIPFamilyCIDRs []string,
) {
	When("NonGatewayRoutes are created, updated and deleted", func() {
		verifyLogicalRouterPolicies := func(ngr *submarinerv1.NonGatewayRoute, nextHop string) {
			ipMatchField := t.getIPMatchField()
			for _, cidr := range ngr.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitModel(&nbdb.LogicalRouterPolicy{
					Match:    ipMatchField + " == " + cidr,
					Nexthops: []string{nextHop},
				})
			}
		}

		verifyNoLogicalRouterPolicies := func(ngr *submarinerv1.NonGatewayRoute, nextHop string) {
			ipMatchField := t.getIPMatchField()
			for _, cidr := range ngr.RoutePolicySpec.RemoteCIDRs {
				t.ovsdbClient.AwaitNoModel(&nbdb.LogicalRouterPolicy{
					Match:    ipMatchField + " == " + cidr,
					Nexthops: []string{nextHop},
				})
			}
		}

		It("should correctly reconcile OVN router policies", func(ctx context.Context) {
			client := t.dynClient.Resource(submarinerv1.SchemeGroupVersion.WithResource("nongatewayroutes")).Namespace(testing.Namespace)

			By("Creating first NonGatewayRoute")

			nonGWRoute1 := &submarinerv1.NonGatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-nongateway-route1",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{ipFamilyNextHop},
					RemoteCIDRs: ipFamilyCIDRs1,
				},
			}

			test.CreateResource(ctx, client, nonGWRoute1)

			verifyLogicalRouterPolicies(nonGWRoute1, ipFamilyNextHop)

			By("Creating second NonGatewayRoute")

			nonGWRoute2 := &submarinerv1.NonGatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-nongateway-route2",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{ipFamilyNextHop},
					RemoteCIDRs: ipFamilyCIDRs2,
				},
			}

			test.CreateResource(ctx, client, nonGWRoute2)

			verifyLogicalRouterPolicies(nonGWRoute1, ipFamilyNextHop)
			verifyLogicalRouterPolicies(nonGWRoute2, ipFamilyNextHop)

			By("Updating NextHop for first NonGatewayRoute")

			prevNextHop := ipFamilyNextHop

			newIP := net.ParseIP(prevNextHop)
			newIP[len(newIP)-1]++
			ipFamilyNextHop = newIP.String()

			nonGWRoute1.RoutePolicySpec.NextHops[0] = ipFamilyNextHop

			test.UpdateResource(ctx, client, nonGWRoute1)

			verifyLogicalRouterPolicies(nonGWRoute1, ipFamilyNextHop)
			verifyNoLogicalRouterPolicies(nonGWRoute1, prevNextHop)
			verifyNoLogicalRouterPolicies(nonGWRoute2, prevNextHop)

			By("Updating NextHop for second NonGatewayRoute")

			nonGWRoute2.RoutePolicySpec.NextHops[0] = ipFamilyNextHop

			test.UpdateResource(ctx, client, nonGWRoute2)

			verifyLogicalRouterPolicies(nonGWRoute1, ipFamilyNextHop)
			verifyLogicalRouterPolicies(nonGWRoute2, ipFamilyNextHop)

			By("Deleting first NonGatewayRoute")

			Expect(client.Delete(ctx, nonGWRoute1.Name, metav1.DeleteOptions{})).To(Succeed())

			verifyNoLogicalRouterPolicies(nonGWRoute1, ipFamilyNextHop)

			By("Creating NonGatewayRoute for other IP family")

			test.CreateResource(ctx, client, &submarinerv1.NonGatewayRoute{
				ObjectMeta: metav1.ObjectMeta{
					Name: "test-nongateway-route-other",
				},
				RoutePolicySpec: submarinerv1.RoutePolicySpec{
					NextHops:    []string{nonIPFamilyNextHop},
					RemoteCIDRs: nonIPFamilyCIDRs,
				},
			})

			nonIPFamilyMatchField := "ip6.dst"
			if t.ipFamily == k8snet.IPv6 {
				nonIPFamilyMatchField = "ip4.dst"
			}

			for _, cidr := range nonIPFamilyCIDRs {
				t.ovsdbClient.EnsureNoModel(&nbdb.LogicalRouterPolicy{
					Match:    nonIPFamilyMatchField + " == " + cidr,
					Nexthops: []string{nonIPFamilyNextHop},
				})
			}
		})
	})
}

func (t *handlerTestDriver) testOVNMgmtInterfaceAddressChange() {
	JustBeforeEach(func(ctx context.Context) {
		t.CreateLocalHostEndpoint(ctx)
		t.netLink.AwaitGwRoutes(0, constants.RouteAgentInterClusterNetworkTableID, t.OVNK8sMgmntIntGw)

		t.createEndpoint(ctx, "192.0.1.0/24")
		t.netLink.AwaitGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)
	})

	It("should update the gateway and host network dataplanes", func() {
		Expect(t.netLink.FlushRouteTable(constants.RouteAgentInterClusterNetworkTableID)).To(Succeed())
		Expect(t.netLink.FlushRouteTable(constants.RouteAgentHostNetworkTableID)).To(Succeed())

		link, err := t.netLink.LinkByName(ovn.OVNK8sMgmntIntfName)
		Expect(err).To(Succeed())

		Expect(t.netLink.AddrDel(link, &netlink.Addr{
			IPNet: t.OVNK8sMgmntIntCIDR[k8snet.IPv4],
		})).To(Succeed())

		t.OVNK8sMgmntIntCIDR[k8snet.IPv4] = toIPNet("128.2.30.3/24")
		Expect(t.netLink.AddrAdd(link, &netlink.Addr{
			IPNet: t.OVNK8sMgmntIntCIDR[k8snet.IPv4],
		})).To(Succeed())

		t.netLink.AwaitGwRoutes(0, constants.RouteAgentInterClusterNetworkTableID, t.OVNK8sMgmntIntGw)
		t.netLink.AwaitGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)
	})

	It("should skip network address routes and use valid gateway when route order changes", func(ctx context.Context) {
		// This reproduces issue #4121 where after ROKS node reboot, route ordering changes
		// and Submariner picks the wrong route (network address instead of valid gateway)
		//
		// Scenario:
		// - Multiple routes exist for the same destination (cluster CIDR)
		// - One route has Gw=nil (would use Dst.IP = network address like 171.0.1.0)
		// - Another route has valid Gw=171.0.1.1
		// - After reboot, the bad route (Gw=nil) comes FIRST in iteration order
		// - Bug: Code returns first match (network address 171.0.1.0)
		// - Fix: Code skips network addresses and continues to find valid gateway
		link, err := t.netLink.LinkByName(ovn.OVNK8sMgmntIntfName)
		Expect(err).To(Succeed())

		// Remove existing routes
		routes, err := t.netLink.RouteList(link, t.ipFamily)
		Expect(err).To(Succeed())

		for i := range routes {
			if routes[i].Dst != nil && routes[i].Dst.String() == t.clusterCIDR {
				Expect(t.netLink.RouteDel(&routes[i])).To(Succeed())
			}
		}

		// Simulate "after reboot" scenario: Add routes in the order that causes the bug
		// Route 1 (comes FIRST): Dst=cluster CIDR, Gw=nil (will use network address)
		networkAddr := toIPNet(t.clusterCIDR)
		Expect(t.netLink.RouteAdd(&netlink.Route{
			LinkIndex: OVNK8sMgmntIntIndex,
			Family:    netlinkAPI.ToNetlinkFamily(t.ipFamily),
			Dst:       networkAddr,
			Gw:        nil, // No gateway - Dst.IP is network address (e.g., 171.0.1.0)
		})).To(Succeed())

		// Route 2 (comes SECOND): Dst=cluster CIDR, Gw=valid gateway IP
		validGateway := net.ParseIP(t.OVNK8sMgmntIntGw)
		Expect(t.netLink.RouteAdd(&netlink.Route{
			LinkIndex: OVNK8sMgmntIntIndex,
			Family:    netlinkAPI.ToNetlinkFamily(t.ipFamily),
			Dst:       networkAddr,
			Gw:        validGateway, // Valid gateway (e.g., 171.0.1.1)
		})).To(Succeed())

		// Create a remote endpoint to trigger updateHostNetworkDataplane
		endpoint := t.createEndpoint(ctx, ipv4Subnets[0])

		// With the bug: Would use the first route's network address (171.0.1.0)
		// With the fix: Should skip first route and use second route's valid gateway (171.0.1.1)
		t.netLink.AwaitGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)

		// Verify the correct gateway was chosen
		routes150, err := t.netLink.RouteList(nil, t.ipFamily)
		Expect(err).To(Succeed())

		foundValidRoute := false

		for i := range routes150 {
			if routes150[i].Table == constants.RouteAgentHostNetworkTableID && routes150[i].Gw != nil {
				foundValidRoute = true

				// Must use the valid gateway, not the network address
				Expect(routes150[i].Gw.String()).To(Equal(t.OVNK8sMgmntIntGw),
					"Should use valid gateway %s, not network address", t.OVNK8sMgmntIntGw)

				// Double-check: Gateway should NOT be a network address (last octet != 0)
				gwBytes := routes150[i].Gw.To4()
				if gwBytes != nil {
					Expect(gwBytes[3]).NotTo(Equal(byte(0)),
						"Gateway %s is a network address (ends in .0)", routes150[i].Gw.String())
				}
			}
		}

		Expect(foundValidRoute).To(BeTrue(), "Should have created route with valid gateway")

		t.DeleteEndpoint(ctx, endpoint.Name)
	})

	It("should accept host routes (/32 or /128) with Gw=nil", func(ctx context.Context) {
		// This tests that we don't reject valid host routes (/32 IPv4 or /128 IPv6)
		// when Gw=nil. A host route's Dst.IP is a valid host address even though
		// it equals Dst.IP.Mask(Dst.Mask) (because mask is all 1s).
		link, err := t.netLink.LinkByName(ovn.OVNK8sMgmntIntfName)
		Expect(err).To(Succeed())

		// Remove existing routes
		routes, err := t.netLink.RouteList(link, t.ipFamily)
		Expect(err).To(Succeed())

		for i := range routes {
			if routes[i].Dst != nil && routes[i].Dst.String() == t.clusterCIDR {
				Expect(t.netLink.RouteDel(&routes[i])).To(Succeed())
			}
		}

		// Add a network address route (should be skipped)
		networkAddr := toIPNet(t.clusterCIDR)
		Expect(t.netLink.RouteAdd(&netlink.Route{
			LinkIndex: OVNK8sMgmntIntIndex,
			Family:    netlinkAPI.ToNetlinkFamily(t.ipFamily),
			Dst:       networkAddr,
			Gw:        nil, // Network address - should be skipped
		})).To(Succeed())

		// Add a /32 host route with Gw=nil (should be accepted)
		hostIP := net.ParseIP(t.OVNK8sMgmntIntGw)
		hostRoute := &net.IPNet{
			IP:   hostIP,
			Mask: net.CIDRMask(32, 32), // /32 for IPv4 or /128 for IPv6
		}

		if t.ipFamily == k8snet.IPv6 {
			hostRoute.Mask = net.CIDRMask(128, 128)
		}

		Expect(t.netLink.RouteAdd(&netlink.Route{
			LinkIndex: OVNK8sMgmntIntIndex,
			Family:    netlinkAPI.ToNetlinkFamily(t.ipFamily),
			Dst:       hostRoute,
			Gw:        nil, // No gateway - will use Dst.IP which is a valid host IP
		})).To(Succeed())

		// Create a remote endpoint to trigger updateHostNetworkDataplane
		endpoint := t.createEndpoint(ctx, ipv4Subnets[0])

		// Should use the host route's Dst.IP, not the network address
		t.netLink.AwaitGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)

		// Verify the correct IP was chosen
		routes150, err := t.netLink.RouteList(nil, t.ipFamily)
		Expect(err).To(Succeed())

		foundValidRoute := false

		for i := range routes150 {
			if routes150[i].Table == constants.RouteAgentHostNetworkTableID && routes150[i].Gw != nil {
				foundValidRoute = true

				// Must use the host route's IP
				Expect(routes150[i].Gw.String()).To(Equal(t.OVNK8sMgmntIntGw),
					"Should use host route IP %s", t.OVNK8sMgmntIntGw)
			}
		}

		Expect(foundValidRoute).To(BeTrue(), "Should have created route with host route IP")

		t.DeleteEndpoint(ctx, endpoint.Name)
	})
}

func (t *handlerTestDriver) testUninstall() {
	It("should delete the table rules and chains", func(ctx context.Context) {
		Expect(t.pFilter.ChainExists(packetfilter.TableTypeFilter, chains.SmForward)).To(BeTrue())
		Expect(t.pFilter.ChainExists(packetfilter.TableTypeFilter, chains.SmForwardMSSClamp)).To(BeTrue())
		Expect(t.pFilter.ChainExists(packetfilter.TableTypeNAT, chains.SmPostRouting)).To(BeTrue())

		Expect(t.netLink.RuleAdd(&netlink.Rule{
			Table:  constants.RouteAgentHostNetworkTableID,
			Family: netlink.FAMILY_V4,
		})).To(Succeed())

		Expect(t.netLink.RuleAdd(&netlink.Rule{
			Table:  constants.RouteAgentInterClusterNetworkTableID,
			Family: netlink.FAMILY_V4,
		})).To(Succeed())

		Expect(t.pFilter.Append(packetfilter.TableTypeFilter, chains.SmForward, &packetfilter.Rule{
			DestCIDR: "1.2.3.4/16",
			Action:   packetfilter.RuleActionAccept,
		})).To(Succeed())

		Expect(t.pFilter.Append(packetfilter.TableTypeFilter, chains.SmForward, &packetfilter.Rule{
			DestCIDR:  "1.2.3.4/16",
			Action:    packetfilter.RuleActionMss,
			ClampType: packetfilter.ToValue,
			MssValue:  "5",
		})).To(Succeed())

		Expect(t.handler.Uninstall(ctx)).To(Succeed())

		t.netLink.AwaitNoRule(constants.RouteAgentHostNetworkTableID, "", "")
		t.netLink.AwaitNoRule(constants.RouteAgentInterClusterNetworkTableID, "", "")

		Expect(t.pFilter.ChainExists(packetfilter.TableTypeNAT, chains.SmPostRouting)).To(BeFalse())
		Expect(t.pFilter.ChainExists(packetfilter.TableTypeFilter, chains.SmForward)).To(BeFalse())
		Expect(t.pFilter.ChainExists(packetfilter.TableTypeFilter, chains.SmForwardMSSClamp)).To(BeFalse())
	})
}

func (t *handlerTestDriver) testIntraClusterRoutingDisabled() {
	BeforeEach(func() {
		t.intraRoutingDisabled = true
	})

	When("not on the gateway", func() {
		It("should not update the host network dataplane on remote Endpoint creation and deletion", func(ctx context.Context) {
			By("Creating remote Endpoint")

			endpoint := t.createEndpoint(ctx, ipv4Subnets[0])

			t.netLink.EnsureNoRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])
			t.netLink.EnsureNoGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)

			By("Deleting remote Endpoint")

			t.DeleteEndpoint(ctx, endpoint.Name)

			t.netLink.EnsureNoGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)
		})
	})

	When("on the gateway", func() {
		JustBeforeEach(func(ctx context.Context) {
			t.CreateLocalHostEndpoint(ctx)
		})

		It("should update the host network dataplane on remote Endpoint creation and deletion", func(ctx context.Context) {
			By("Creating remote Endpoint")

			endpoint := t.createEndpoint(ctx, ipv4Subnets[0])

			t.netLink.AwaitRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])

			By("Deleting remote Endpoint")

			t.DeleteEndpoint(ctx, endpoint.Name)

			t.netLink.AwaitNoRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])
		})
	})

	Context("on gateway transitions", func() {
		It("should correctly update the host network dataplane for existing remote Endpoints", func(ctx context.Context) {
			By("Creating remote Endpoint")

			t.createEndpoint(ctx, ipv4Subnets[0])

			t.netLink.EnsureNoRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])

			By("Creating local gateway Endpoint")

			localEP := t.CreateLocalHostEndpoint(ctx)

			t.netLink.AwaitRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])

			By("Deleting local gateway Endpoint")

			t.DeleteEndpoint(ctx, localEP.Name)

			t.netLink.AwaitNoRule(constants.RouteAgentHostNetworkTableID, "", ipv4Subnets[0])
			t.netLink.AwaitNoGwRoutes(0, constants.RouteAgentHostNetworkTableID, t.OVNK8sMgmntIntGw)
		})
	})

	It("should not try to process NonGatewayRoutes", func() {
		assert.EnsureNoActionsForResource(&t.dynClient.Fake, "nongatewayroutes", "watch")
	})
}