fix deletion of exit routes without nodes (#2286)

Fixes #2259

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

hscontrol/db/routes.go

@@ -117,13 +117,13 @@ func EnableRoute(tx *gorm.DB, id uint64) (*types.StateUpdate, error) {
 	if route.IsExitRoute() {
 		return enableRoutes(
 			tx,
-			&route.Node,
+			route.Node,
 			tsaddr.AllIPv4(),
 			tsaddr.AllIPv6(),
 		)
 	}
 
-	return enableRoutes(tx, &route.Node, netip.Prefix(route.Prefix))
+	return enableRoutes(tx, route.Node, netip.Prefix(route.Prefix))
 }
 
 func DisableRoute(tx *gorm.DB,
@@ -154,7 +154,7 @@ func DisableRoute(tx *gorm.DB,
 			return nil, err
 		}
 	} else {
-		routes, err = GetNodeRoutes(tx, &node)
+		routes, err = GetNodeRoutes(tx, node)
 		if err != nil {
 			return nil, err
 		}
@@ -201,24 +201,26 @@ func DeleteRoute(
 		return nil, err
 	}
 
+	if route.Node == nil {
+		// If the route is not assigned to a node, just delete it,
+		// there are no updates to be sent as no nodes are
+		// dependent on it
+		if err := tx.Unscoped().Delete(&route).Error; err != nil {
+			return nil, err
+		}
+		return nil, nil
+	}
+
 	var routes types.Routes
 	node := route.Node
 
 	// Tailscale requires both IPv4 and IPv6 exit routes to
 	// be enabled at the same time, as per
 	// https://github.com/juanfont/headscale/issues/804#issuecomment-1399314002
+	// This means that if we delete a route which is an exit route, delete both.
 	var update []types.NodeID
-	if !route.IsExitRoute() {
+	if route.IsExitRoute() {
-		update, err = failoverRouteTx(tx, isLikelyConnected, route)
+		routes, err = GetNodeRoutes(tx, node)
-		if err != nil {
-			return nil, nil
-		}
-
-		if err := tx.Unscoped().Delete(&route).Error; err != nil {
-			return nil, err
-		}
-	} else {
-		routes, err = GetNodeRoutes(tx, &node)
 		if err != nil {
 			return nil, err
 		}
@@ -233,13 +235,22 @@ func DeleteRoute(
 		if err := tx.Unscoped().Delete(&routesToDelete).Error; err != nil {
 			return nil, err
 		}
+	} else {
+		update, err = failoverRouteTx(tx, isLikelyConnected, route)
+		if err != nil {
+			return nil, nil
+		}
+
+		if err := tx.Unscoped().Delete(&route).Error; err != nil {
+			return nil, err
+		}
 	}
 
 	// If update is empty, it means that one was not created
 	// by failover (as a failover was not necessary), create
 	// one and return to the caller.
 	if routes == nil {
-		routes, err = GetNodeRoutes(tx, &node)
+		routes, err = GetNodeRoutes(tx, node)
 		if err != nil {
 			return nil, err
 		}

hscontrol/db/routes_test.go

@@ -290,23 +290,18 @@ func (s *Suite) TestDeleteRoutes(c *check.C) {
 }
 
 var (
-	ipp    = func(s string) netip.Prefix { return netip.MustParsePrefix(s) }
+	ipp = func(s string) netip.Prefix { return netip.MustParsePrefix(s) }
-	mkNode = func(nid types.NodeID) types.Node {
+	np  = func(nid types.NodeID) *types.Node {
-		return types.Node{ID: nid}
+		return &types.Node{ID: nid}
 	}
 )
 
-var np = func(nid types.NodeID) *types.Node {
-	no := mkNode(nid)
-	return &no
-}
-
 var r = func(id uint, nid types.NodeID, prefix netip.Prefix, enabled, primary bool) types.Route {
 	return types.Route{
 		Model: gorm.Model{
 			ID: id,
 		},
-		Node:      mkNode(nid),
+		Node:      np(nid),
 		Prefix:    prefix,
 		Enabled:   enabled,
 		IsPrimary: primary,
@@ -693,7 +688,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix:    ipp("10.0.0.0/24"),
-				Node:      types.Node{},
+				Node:      &types.Node{},
 				IsPrimary: false,
 			},
 			routes:  types.Routes{},
@@ -707,7 +702,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix:    ipp("0.0.0.0/0"),
-				Node:      types.Node{},
+				Node:      &types.Node{},
 				IsPrimary: true,
 			},
 			routes:  types.Routes{},
@@ -721,7 +716,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: true,
@@ -732,7 +727,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -748,7 +743,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: true,
@@ -760,7 +755,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -771,7 +766,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 2,
 					},
 					IsPrimary: false,
@@ -795,7 +790,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: false,
@@ -807,7 +802,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -818,7 +813,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 2,
 					},
 					IsPrimary: false,
@@ -835,7 +830,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 2,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 2,
 				},
 				IsPrimary: true,
@@ -847,7 +842,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: false,
@@ -858,7 +853,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 2,
 					},
 					IsPrimary: true,
@@ -869,7 +864,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 3,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 3,
 					},
 					IsPrimary: false,
@@ -893,7 +888,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: true,
@@ -905,7 +900,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -917,7 +912,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 4,
 					},
 					IsPrimary: false,
@@ -938,7 +933,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: true,
@@ -950,7 +945,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -962,7 +957,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 4,
 					},
 					IsPrimary: false,
@@ -973,7 +968,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 3,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 2,
 					},
 					IsPrimary: true,
@@ -998,7 +993,7 @@ func TestFailoverRouteTx(t *testing.T) {
 					ID: 1,
 				},
 				Prefix: ipp("10.0.0.0/24"),
-				Node: types.Node{
+				Node: &types.Node{
 					ID: 1,
 				},
 				IsPrimary: true,
@@ -1010,7 +1005,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 1,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 1,
 					},
 					IsPrimary: true,
@@ -1022,7 +1017,7 @@ func TestFailoverRouteTx(t *testing.T) {
 						ID: 2,
 					},
 					Prefix: ipp("10.0.0.0/24"),
-					Node: types.Node{
+					Node: &types.Node{
 						ID: 2,
 					},
 					IsPrimary: false,
@@ -1075,7 +1070,7 @@ func TestFailoverRoute(t *testing.T) {
 			Model: gorm.Model{
 				ID: id,
 			},
-			Node: types.Node{
+			Node: &types.Node{
 				ID: nid,
 			},
 			Prefix:    prefix,

hscontrol/types/routes.go

@@ -14,7 +14,7 @@ type Route struct {
 	gorm.Model
 
 	NodeID uint64
-	Node   Node
+	Node   *Node
 
 	// TODO(kradalby): change this custom type to netip.Prefix
 	Prefix netip.Prefix `gorm:"serializer:text"`