types: persist Node JSON slices via named IsZero types

Endpoints, Tags and ApprovedRoutes serialize as JSON on Node. GORM's
struct Updates path skips fields it considers zero, and reflect treats
a nil slice as zero — clearing any of these columns via the State
persist path would leave the previous value in the database.

Introduce Strings, Prefixes and AddrPorts as named slice types whose
IsZero() always reports false, so GORM keeps the column in the UPDATE
regardless of the slice being nil or empty. JSON marshalling is
unchanged: nil serializes to null, empty to []. List() returns the
underlying unnamed slice for callers (mainly testify assertions over
reflect.DeepEqual) that distinguish the named type from its base.

Regenerated types_clone.go and types_view.go follow the field-type
swap. Test assertions across hscontrol/{db,state,servertest} updated
to call .List() where reflect.DeepEqual previously matched the raw
slice type.

Fixes #3110
This commit is contained in:
Kristoffer Dalby 2026-05-13 09:53:01 +00:00
parent e78a24b892
commit 7a20db9f49
8 changed files with 64 additions and 18 deletions

View File

@ -122,7 +122,7 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) {
// Expected: tags = ["tag:server"] (no duplicates) // Expected: tags = ["tag:server"] (no duplicates)
node4 := findNode("node4") node4 := findNode("node4")
require.NotNil(t, node4, "node4 should exist") require.NotNil(t, node4, "node4 should exist")
assert.Equal(t, []string{"tag:server"}, node4.Tags, "node4 should have tag:server without duplicates") assert.Equal(t, []string{"tag:server"}, node4.Tags.List(), "node4 should have tag:server without duplicates") //nolint:goconst // descriptive test assertions read better with the literal inline
// Node 5: user2 has no RequestTags // Node 5: user2 has no RequestTags
// Expected: tags = [] (unchanged) // Expected: tags = [] (unchanged)

View File

@ -123,7 +123,7 @@ func TestDestroyUserErrors(t *testing.T) {
result := db.DB.First(&survivingNode, "id = ?", node.ID) result := db.DB.First(&survivingNode, "id = ?", node.ID)
require.NoError(t, result.Error) require.NoError(t, result.Error)
assert.Nil(t, survivingNode.UserID) assert.Nil(t, survivingNode.UserID)
assert.Equal(t, []string{"tag:server"}, survivingNode.Tags) assert.Equal(t, []string{"tag:server"}, survivingNode.Tags.List())
}, },
}, },
{ {

View File

@ -563,7 +563,7 @@ func TestIssuesNodeStoreConsistency(t *testing.T) {
require.NoError(t, err, "node should be in database") require.NoError(t, err, "node should be in database")
nsRoutes := nsView.ApprovedRoutes().AsSlice() nsRoutes := nsView.ApprovedRoutes().AsSlice()
dbRoutes := dbNode.ApprovedRoutes dbRoutes := dbNode.ApprovedRoutes.List()
assert.Equal(t, nsRoutes, dbRoutes, assert.Equal(t, nsRoutes, dbRoutes,
"NodeStore and DB should agree on approved routes") "NodeStore and DB should agree on approved routes")

View File

@ -684,7 +684,7 @@ func TestNodeStoreOperations(t *testing.T) {
finalNode := snapshot.nodesByID[1] finalNode := snapshot.nodesByID[1]
assert.Equal(t, "multi-update-hostname", finalNode.Hostname) assert.Equal(t, "multi-update-hostname", finalNode.Hostname)
assert.Equal(t, "multi-update-givenname", finalNode.GivenName) assert.Equal(t, "multi-update-givenname", finalNode.GivenName)
assert.Equal(t, []string{"tag1", "tag2"}, finalNode.Tags) assert.Equal(t, []string{"tag1", "tag2"}, finalNode.Tags.List())
}, },
}, },
}, },
@ -722,14 +722,14 @@ func TestNodeStoreOperations(t *testing.T) {
assert.NotNil(t, nodePtr) assert.NotNil(t, nodePtr)
assert.Equal(t, "db-save-hostname", nodePtr.Hostname) assert.Equal(t, "db-save-hostname", nodePtr.Hostname)
assert.Equal(t, "db-save-given", nodePtr.GivenName) assert.Equal(t, "db-save-given", nodePtr.GivenName)
assert.Equal(t, []string{"db-tag1", "db-tag2"}, nodePtr.Tags) assert.Equal(t, []string{"db-tag1", "db-tag2"}, nodePtr.Tags.List())
// Verify the snapshot also reflects the same state // Verify the snapshot also reflects the same state
snapshot := store.data.Load() snapshot := store.data.Load()
storedNode := snapshot.nodesByID[1] storedNode := snapshot.nodesByID[1]
assert.Equal(t, "db-save-hostname", storedNode.Hostname) assert.Equal(t, "db-save-hostname", storedNode.Hostname)
assert.Equal(t, "db-save-given", storedNode.GivenName) assert.Equal(t, "db-save-given", storedNode.GivenName)
assert.Equal(t, []string{"db-tag1", "db-tag2"}, storedNode.Tags) assert.Equal(t, []string{"db-tag1", "db-tag2"}, storedNode.Tags.List())
}, },
}, },
{ {
@ -792,15 +792,15 @@ func TestNodeStoreOperations(t *testing.T) {
// All should have the complete final state // All should have the complete final state
assert.Equal(t, "concurrent-db-hostname", nodePtr1.Hostname) assert.Equal(t, "concurrent-db-hostname", nodePtr1.Hostname)
assert.Equal(t, "concurrent-db-given", nodePtr1.GivenName) assert.Equal(t, "concurrent-db-given", nodePtr1.GivenName)
assert.Equal(t, []string{"concurrent-tag"}, nodePtr1.Tags) assert.Equal(t, []string{"concurrent-tag"}, nodePtr1.Tags.List())
assert.Equal(t, "concurrent-db-hostname", nodePtr2.Hostname) assert.Equal(t, "concurrent-db-hostname", nodePtr2.Hostname)
assert.Equal(t, "concurrent-db-given", nodePtr2.GivenName) assert.Equal(t, "concurrent-db-given", nodePtr2.GivenName)
assert.Equal(t, []string{"concurrent-tag"}, nodePtr2.Tags) assert.Equal(t, []string{"concurrent-tag"}, nodePtr2.Tags.List())
assert.Equal(t, "concurrent-db-hostname", nodePtr3.Hostname) assert.Equal(t, "concurrent-db-hostname", nodePtr3.Hostname)
assert.Equal(t, "concurrent-db-given", nodePtr3.GivenName) assert.Equal(t, "concurrent-db-given", nodePtr3.GivenName)
assert.Equal(t, []string{"concurrent-tag"}, nodePtr3.Tags) assert.Equal(t, []string{"concurrent-tag"}, nodePtr3.Tags.List())
// Verify consistency with stored state // Verify consistency with stored state
snapshot := store.data.Load() snapshot := store.data.Load()

View File

@ -120,7 +120,7 @@ type Node struct {
NodeKey key.NodePublic `gorm:"serializer:text"` NodeKey key.NodePublic `gorm:"serializer:text"`
DiscoKey key.DiscoPublic `gorm:"serializer:text"` DiscoKey key.DiscoPublic `gorm:"serializer:text"`
Endpoints []netip.AddrPort `gorm:"serializer:json"` Endpoints AddrPorts `gorm:"serializer:json"`
Hostinfo *tailcfg.Hostinfo `gorm:"column:host_info;serializer:json"` Hostinfo *tailcfg.Hostinfo `gorm:"column:host_info;serializer:json"`
@ -150,7 +150,7 @@ type Node struct {
// When non-empty, the node is "tagged" and tags define its identity. // When non-empty, the node is "tagged" and tags define its identity.
// Empty for user-owned nodes. // Empty for user-owned nodes.
// Tags cannot be removed once set (one-way transition). // Tags cannot be removed once set (one-way transition).
Tags []string `gorm:"column:tags;serializer:json"` Tags Strings `gorm:"column:tags;serializer:json"`
// When a node has been created with a PreAuthKey, we need to // When a node has been created with a PreAuthKey, we need to
// prevent the preauthkey from being deleted before the node. // prevent the preauthkey from being deleted before the node.
@ -169,7 +169,7 @@ type Node struct {
// as a subnet router. They are not necessarily the routes that the node // as a subnet router. They are not necessarily the routes that the node
// announces at the moment. // announces at the moment.
// See [Node.Hostinfo] // See [Node.Hostinfo]
ApprovedRoutes []netip.Prefix `gorm:"column:approved_routes;serializer:json"` ApprovedRoutes Prefixes `gorm:"column:approved_routes;serializer:json"`
CreatedAt time.Time CreatedAt time.Time
UpdatedAt time.Time UpdatedAt time.Time

46
hscontrol/types/slices.go Normal file
View File

@ -0,0 +1,46 @@
package types
import "net/netip"
// The named slice types below are used for GORM-persisted Node columns
// that serialise as JSON. GORM v2's struct-based Updates skips fields
// it considers zero — for unnamed slice types that is nil — and the
// default reflect.Value.IsZero treats a nil slice as zero. By giving
// each slice an IsZero() that always returns false, the column is
// always included in UPDATE statements regardless of whether the
// caller is clearing the field. JSON marshalling is unchanged: a nil
// value serialises to null and an empty value serialises to [].
//
// The .List() helpers return the underlying unnamed slice for the
// places (mainly testify assertions over reflect.DeepEqual) where the
// distinction between the named and unnamed type matters.
// Strings is a []string with a GORM-friendly IsZero.
type Strings []string
// IsZero implements GORM's zeroer interface to keep the column in the
// UPDATE set even when the slice is nil or empty.
func (Strings) IsZero() bool { return false }
// List returns the underlying []string.
func (s Strings) List() []string { return []string(s) }
// Prefixes is a []netip.Prefix with a GORM-friendly IsZero.
type Prefixes []netip.Prefix
// IsZero implements GORM's zeroer interface to keep the column in the
// UPDATE set even when the slice is nil or empty.
func (Prefixes) IsZero() bool { return false }
// List returns the underlying []netip.Prefix.
func (s Prefixes) List() []netip.Prefix { return []netip.Prefix(s) }
// AddrPorts is a []netip.AddrPort with a GORM-friendly IsZero.
type AddrPorts []netip.AddrPort
// IsZero implements GORM's zeroer interface to keep the column in the
// UPDATE set even when the slice is nil or empty.
func (AddrPorts) IsZero() bool { return false }
// List returns the underlying []netip.AddrPort.
func (s AddrPorts) List() []netip.AddrPort { return []netip.AddrPort(s) }

View File

@ -86,7 +86,7 @@ var _NodeCloneNeedsRegeneration = Node(struct {
MachineKey key.MachinePublic MachineKey key.MachinePublic
NodeKey key.NodePublic NodeKey key.NodePublic
DiscoKey key.DiscoPublic DiscoKey key.DiscoPublic
Endpoints []netip.AddrPort Endpoints AddrPorts
Hostinfo *tailcfg.Hostinfo Hostinfo *tailcfg.Hostinfo
IPv4 *netip.Addr IPv4 *netip.Addr
IPv6 *netip.Addr IPv6 *netip.Addr
@ -95,12 +95,12 @@ var _NodeCloneNeedsRegeneration = Node(struct {
UserID *uint UserID *uint
User *User User *User
RegisterMethod string RegisterMethod string
Tags []string Tags Strings
AuthKeyID *uint64 AuthKeyID *uint64
AuthKey *PreAuthKey AuthKey *PreAuthKey
Expiry *time.Time Expiry *time.Time
LastSeen *time.Time LastSeen *time.Time
ApprovedRoutes []netip.Prefix ApprovedRoutes Prefixes
CreatedAt time.Time CreatedAt time.Time
UpdatedAt time.Time UpdatedAt time.Time
DeletedAt *time.Time DeletedAt *time.Time

View File

@ -275,7 +275,7 @@ var _NodeViewNeedsRegeneration = Node(struct {
MachineKey key.MachinePublic MachineKey key.MachinePublic
NodeKey key.NodePublic NodeKey key.NodePublic
DiscoKey key.DiscoPublic DiscoKey key.DiscoPublic
Endpoints []netip.AddrPort Endpoints AddrPorts
Hostinfo *tailcfg.Hostinfo Hostinfo *tailcfg.Hostinfo
IPv4 *netip.Addr IPv4 *netip.Addr
IPv6 *netip.Addr IPv6 *netip.Addr
@ -284,12 +284,12 @@ var _NodeViewNeedsRegeneration = Node(struct {
UserID *uint UserID *uint
User *User User *User
RegisterMethod string RegisterMethod string
Tags []string Tags Strings
AuthKeyID *uint64 AuthKeyID *uint64
AuthKey *PreAuthKey AuthKey *PreAuthKey
Expiry *time.Time Expiry *time.Time
LastSeen *time.Time LastSeen *time.Time
ApprovedRoutes []netip.Prefix ApprovedRoutes Prefixes
CreatedAt time.Time CreatedAt time.Time
UpdatedAt time.Time UpdatedAt time.Time
DeletedAt *time.Time DeletedAt *time.Time