diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index ee88d4ae..20e95e7b 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -122,7 +122,7 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) { // Expected: tags = ["tag:server"] (no duplicates) node4 := findNode("node4") 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 // Expected: tags = [] (unchanged) diff --git a/hscontrol/db/users_test.go b/hscontrol/db/users_test.go index cbb1c3ce..36d836f9 100644 --- a/hscontrol/db/users_test.go +++ b/hscontrol/db/users_test.go @@ -123,7 +123,7 @@ func TestDestroyUserErrors(t *testing.T) { result := db.DB.First(&survivingNode, "id = ?", node.ID) require.NoError(t, result.Error) assert.Nil(t, survivingNode.UserID) - assert.Equal(t, []string{"tag:server"}, survivingNode.Tags) + assert.Equal(t, []string{"tag:server"}, survivingNode.Tags.List()) }, }, { diff --git a/hscontrol/servertest/issues_test.go b/hscontrol/servertest/issues_test.go index 4859f794..b687d512 100644 --- a/hscontrol/servertest/issues_test.go +++ b/hscontrol/servertest/issues_test.go @@ -563,7 +563,7 @@ func TestIssuesNodeStoreConsistency(t *testing.T) { require.NoError(t, err, "node should be in database") nsRoutes := nsView.ApprovedRoutes().AsSlice() - dbRoutes := dbNode.ApprovedRoutes + dbRoutes := dbNode.ApprovedRoutes.List() assert.Equal(t, nsRoutes, dbRoutes, "NodeStore and DB should agree on approved routes") diff --git a/hscontrol/state/node_store_test.go b/hscontrol/state/node_store_test.go index 6b421754..745a8714 100644 --- a/hscontrol/state/node_store_test.go +++ b/hscontrol/state/node_store_test.go @@ -684,7 +684,7 @@ func TestNodeStoreOperations(t *testing.T) { finalNode := snapshot.nodesByID[1] assert.Equal(t, "multi-update-hostname", finalNode.Hostname) 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.Equal(t, "db-save-hostname", nodePtr.Hostname) 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 snapshot := store.data.Load() storedNode := snapshot.nodesByID[1] assert.Equal(t, "db-save-hostname", storedNode.Hostname) 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 assert.Equal(t, "concurrent-db-hostname", nodePtr1.Hostname) 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-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-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 snapshot := store.data.Load() diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 60512ebe..63cdffbb 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -120,7 +120,7 @@ type Node struct { NodeKey key.NodePublic `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"` @@ -150,7 +150,7 @@ type Node struct { // When non-empty, the node is "tagged" and tags define its identity. // Empty for user-owned nodes. // 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 // 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 // announces at the moment. // See [Node.Hostinfo] - ApprovedRoutes []netip.Prefix `gorm:"column:approved_routes;serializer:json"` + ApprovedRoutes Prefixes `gorm:"column:approved_routes;serializer:json"` CreatedAt time.Time UpdatedAt time.Time diff --git a/hscontrol/types/slices.go b/hscontrol/types/slices.go new file mode 100644 index 00000000..faf29d1b --- /dev/null +++ b/hscontrol/types/slices.go @@ -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) } diff --git a/hscontrol/types/types_clone.go b/hscontrol/types/types_clone.go index 6739beed..a14dc6fd 100644 --- a/hscontrol/types/types_clone.go +++ b/hscontrol/types/types_clone.go @@ -86,7 +86,7 @@ var _NodeCloneNeedsRegeneration = Node(struct { MachineKey key.MachinePublic NodeKey key.NodePublic DiscoKey key.DiscoPublic - Endpoints []netip.AddrPort + Endpoints AddrPorts Hostinfo *tailcfg.Hostinfo IPv4 *netip.Addr IPv6 *netip.Addr @@ -95,12 +95,12 @@ var _NodeCloneNeedsRegeneration = Node(struct { UserID *uint User *User RegisterMethod string - Tags []string + Tags Strings AuthKeyID *uint64 AuthKey *PreAuthKey Expiry *time.Time LastSeen *time.Time - ApprovedRoutes []netip.Prefix + ApprovedRoutes Prefixes CreatedAt time.Time UpdatedAt time.Time DeletedAt *time.Time diff --git a/hscontrol/types/types_view.go b/hscontrol/types/types_view.go index 6684ecd6..f8beb9a9 100644 --- a/hscontrol/types/types_view.go +++ b/hscontrol/types/types_view.go @@ -275,7 +275,7 @@ var _NodeViewNeedsRegeneration = Node(struct { MachineKey key.MachinePublic NodeKey key.NodePublic DiscoKey key.DiscoPublic - Endpoints []netip.AddrPort + Endpoints AddrPorts Hostinfo *tailcfg.Hostinfo IPv4 *netip.Addr IPv6 *netip.Addr @@ -284,12 +284,12 @@ var _NodeViewNeedsRegeneration = Node(struct { UserID *uint User *User RegisterMethod string - Tags []string + Tags Strings AuthKeyID *uint64 AuthKey *PreAuthKey Expiry *time.Time LastSeen *time.Time - ApprovedRoutes []netip.Prefix + ApprovedRoutes Prefixes CreatedAt time.Time UpdatedAt time.Time DeletedAt *time.Time