diff --git a/hscontrol/db/routes.go b/hscontrol/db/routes.go index 0a72c427..6325dacc 100644 --- a/hscontrol/db/routes.go +++ b/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() { - update, err = failoverRouteTx(tx, isLikelyConnected, route) - 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 route.IsExitRoute() { + 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 } diff --git a/hscontrol/db/routes_test.go b/hscontrol/db/routes_test.go index 7b11e136..ed9d4c04 100644 --- a/hscontrol/db/routes_test.go +++ b/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) } - mkNode = func(nid types.NodeID) types.Node { - return types.Node{ID: nid} + ipp = func(s string) netip.Prefix { return netip.MustParsePrefix(s) } + np = func(nid types.NodeID) *types.Node { + 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, diff --git a/hscontrol/types/routes.go b/hscontrol/types/routes.go index 1f6b8a77..4ef3621f 100644 --- a/hscontrol/types/routes.go +++ b/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"`