From af7e7a45604dd7fe5e0c64d5e2c907a3108c7c36 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 13 May 2026 09:53:21 +0000 Subject: [PATCH] db: remove unused SetApprovedRoutes and SetTags helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both helpers existed to write the literal "[]" when clearing a slice column — a workaround for GORM's struct-Updates skipping nil slices. The State path goes exclusively through persistNodeToDB, which is now correct end-to-end thanks to the named IsZero slice types, so the helpers are dead in production. The remaining callers were tests. TestSetTags is dropped — TestSetTags_* in hscontrol/grpcv1_test.go already covers the State path that production uses. TestAutoApproveRoutes now writes routes via DB.Save on the loaded node, which is the path gRPC SetApprovedRoutes drives in production. Updates #3110 --- hscontrol/db/node.go | 84 --------------------------------------- hscontrol/db/node_test.go | 56 ++------------------------ 2 files changed, 4 insertions(+), 136 deletions(-) diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index e6bd9cf0..1b46ca61 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -1,11 +1,9 @@ package db import ( - "encoding/json" "errors" "fmt" "net/netip" - "slices" "sort" "strconv" "sync" @@ -17,7 +15,6 @@ import ( "github.com/juanfont/headscale/hscontrol/util/zlog/zf" "github.com/rs/zerolog/log" "gorm.io/gorm" - "tailscale.com/net/tsaddr" "tailscale.com/types/key" "tailscale.com/util/dnsname" ) @@ -188,87 +185,6 @@ func GetNodeByNodeKey( return &mach, nil } -func (hsdb *HSDatabase) SetTags( - nodeID types.NodeID, - tags []string, -) error { - return hsdb.Write(func(tx *gorm.DB) error { - return SetTags(tx, nodeID, tags) - }) -} - -// SetTags takes a NodeID and update the forced tags. -// It will overwrite any tags with the new list. -func SetTags( - tx *gorm.DB, - nodeID types.NodeID, - tags []string, -) error { - if len(tags) == 0 { - // if no tags are provided, we remove all tags - err := tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("tags", "[]").Error - if err != nil { - return fmt.Errorf("removing tags: %w", err) - } - - return nil - } - - slices.Sort(tags) - tags = slices.Compact(tags) - - b, err := json.Marshal(tags) - if err != nil { - return err - } - - err = tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("tags", string(b)).Error - if err != nil { - return fmt.Errorf("updating tags: %w", err) - } - - return nil -} - -// SetApprovedRoutes takes a Node struct pointer and updates the approved routes. -func SetApprovedRoutes( - tx *gorm.DB, - nodeID types.NodeID, - routes []netip.Prefix, -) error { - if len(routes) == 0 { - // if no routes are provided, we remove all - err := tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("approved_routes", "[]").Error - if err != nil { - return fmt.Errorf("removing approved routes: %w", err) - } - - return nil - } - - // When approving exit routes, ensure both IPv4 and IPv6 are included - // If either 0.0.0.0/0 or ::/0 is being approved, both should be approved - hasIPv4Exit := slices.Contains(routes, tsaddr.AllIPv4()) - hasIPv6Exit := slices.Contains(routes, tsaddr.AllIPv6()) - - if hasIPv4Exit && !hasIPv6Exit { - routes = append(routes, tsaddr.AllIPv6()) - } else if hasIPv6Exit && !hasIPv4Exit { - routes = append(routes, tsaddr.AllIPv4()) - } - - b, err := json.Marshal(routes) - if err != nil { - return err - } - - if err := tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("approved_routes", string(b)).Error; err != nil { //nolint:noinlineerr - return fmt.Errorf("updating approved routes: %w", err) - } - - return nil -} - // SetLastSeen sets a node's last seen field indicating that we // have recently communicating with this node. func (hsdb *HSDatabase) SetLastSeen(nodeID types.NodeID, lastSeen time.Time) error { diff --git a/hscontrol/db/node_test.go b/hscontrol/db/node_test.go index f9ba7b36..7bbcf3d9 100644 --- a/hscontrol/db/node_test.go +++ b/hscontrol/db/node_test.go @@ -178,55 +178,6 @@ func TestDisableNodeExpiry(t *testing.T) { assert.Nil(t, nodeFromDB.Expiry, "expiry should be nil after disabling") } -func TestSetTags(t *testing.T) { - db, err := newSQLiteTestDB() - require.NoError(t, err) - - user, err := db.CreateUser(types.User{Name: "test"}) - require.NoError(t, err) - - pak, err := db.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) - require.NoError(t, err) - - pakID := pak.ID - - _, err = db.getNode(types.UserID(user.ID), "testnode") - require.Error(t, err) - - nodeKey := key.NewNode() - machineKey := key.NewMachine() - - node := &types.Node{ - ID: 0, - MachineKey: machineKey.Public(), - NodeKey: nodeKey.Public(), - Hostname: "testnode", - UserID: &user.ID, - RegisterMethod: util.RegisterMethodAuthKey, - AuthKeyID: &pakID, - } - - trx := db.DB.Save(node) - require.NoError(t, trx.Error) - - // assign simple tags - sTags := []string{"tag:test", "tag:foo"} - err = db.SetTags(node.ID, sTags) - require.NoError(t, err) - node, err = db.getNode(types.UserID(user.ID), "testnode") - require.NoError(t, err) - assert.Equal(t, sTags, node.Tags) - - // assign duplicate tags, expect no errors but no doubles in DB - eTags := []string{"tag:bar", "tag:test", "tag:unknown", "tag:test"} - err = db.SetTags(node.ID, eTags) - require.NoError(t, err) - node, err = db.getNode(types.UserID(user.ID), "testnode") - require.NoError(t, err) - assert.Equal(t, []string{"tag:bar", "tag:test", "tag:unknown"}, node.Tags) -} - - func TestAutoApproveRoutes(t *testing.T) { tests := []struct { name string @@ -409,13 +360,15 @@ func TestAutoApproveRoutes(t *testing.T) { assert.Equal(t, tt.expectChange, changed1) if changed1 { - err = SetApprovedRoutes(adb.DB, node.ID, newRoutes1) + node.ApprovedRoutes = types.Prefixes(newRoutes1) + err = adb.DB.Save(&node).Error require.NoError(t, err) } newRoutes2, changed2 := policy.ApproveRoutesWithPolicy(pm, nodeTagged.View(), nodeTagged.ApprovedRoutes, tt.routes) if changed2 { - err = SetApprovedRoutes(adb.DB, nodeTagged.ID, newRoutes2) + nodeTagged.ApprovedRoutes = types.Prefixes(newRoutes2) + err = adb.DB.Save(&nodeTagged).Error require.NoError(t, err) } @@ -625,7 +578,6 @@ func TestListEphemeralNodes(t *testing.T) { assert.Equal(t, nodeEph.Hostname, ephemeralNodes[0].Hostname) } - func TestListPeers(t *testing.T) { // Setup test database db, err := newSQLiteTestDB()