diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index bf462d44..7a914224 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -24,7 +24,7 @@ import ( "tailscale.com/util/multierr" ) -// ErrInvalidTagOwner is returned when a tag owner is not an Alias type. +// ErrInvalidTagOwner is returned when a tag owner is not an [Alias] type. var ErrInvalidTagOwner = errors.New("tag owner is not an Alias") type PolicyManager struct { @@ -86,8 +86,8 @@ type filterAndPolicy struct { } // validateUserReferences surfaces ambiguous user@ tokens at policy load so -// duplicate DB rows fail loudly instead of silently dropping rules (#3160). -// Missing-user tokens stay tolerant (#2863). Empty users → no-op for +// duplicate DB rows fail loudly instead of silently dropping rules. +// Missing-user tokens stay tolerant. Empty users → no-op for // syntax-only checks. func validateUserReferences(pol *Policy, users types.Users) error { if pol == nil || len(users) == 0 { @@ -170,7 +170,7 @@ func validateUserReferences(pol *Policy, users types.Users) error { return multierr.New(errs...) } -// NewPolicyManager creates a new PolicyManager from a policy file and a list of users and nodes. +// NewPolicyManager creates a new [PolicyManager] from a policy file and a list of users and nodes. // It returns an error if the policy file is invalid. // The policy manager will update the filter rules based on the users and nodes. func NewPolicyManager(b []byte, users []types.User, nodes views.Slice[types.NodeView]) (*PolicyManager, error) { @@ -360,7 +360,7 @@ func (pm *PolicyManager) updateLocked() (bool, error) { return true, nil } -// SSHPolicy returns the tailcfg.SSHPolicy for node, compiling and +// SSHPolicy returns the [tailcfg.SSHPolicy] for node, compiling and // caching on first access. Rules use SessionDuration = 0 (no // auto-approval) and emit check URLs of the form // /machine/ssh/action/{src}/to/{dst}?local_user={local_user} per the @@ -531,6 +531,11 @@ func (pm *PolicyManager) Filter() ([]tailcfg.FilterRule, []matcher.Match) { // For global filters, it uses the global filter matchers for all nodes. // For autogroup:self policies (empty global filter), it builds per-node // peer maps using each node's specific filter rules. +// +// Compared to [policy.ReduceNodes], which builds the list per node, we end +// up with doing the full work for every node O(n^2), while this will reduce +// the list as we see relationships while building the map, making it +// O(n^2/2) in the end, but with less work per node. func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[types.NodeID][]types.NodeView { if pm == nil { return nil @@ -546,9 +551,6 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ ret := make(map[types.NodeID][]types.NodeView, nodes.Len()) // Build the map of all peers according to the matchers. - // Compared to ReduceNodes, which builds the list per node, we end up with doing - // the full work for every node O(n^2), while this will reduce the list as we see - // relationships while building the map, making it O(n^2/2) in the end, but with less work per node. for i := range nodes.Len() { for j := i + 1; j < nodes.Len(); j++ { if nodes.At(i).ID() == nodes.At(j).ID() { @@ -667,8 +669,8 @@ func (pm *PolicyManager) filterForNodeLocked( // If the policy uses autogroup:self, this returns node-specific compiled rules. // Otherwise, it returns the global filter reduced for this node. // -// Cache is invalidated by updateLocked on policy reload, node-set -// change, or tag-state change. +// Cache is invalidated by [PolicyManager.updateLocked] on policy reload, +// node-set change, or tag-state change. func (pm *PolicyManager) FilterForNode(node types.NodeView) ([]tailcfg.FilterRule, error) { if pm == nil { return nil, nil @@ -682,14 +684,14 @@ func (pm *PolicyManager) FilterForNode(node types.NodeView) ([]tailcfg.FilterRul // MatchersForNode returns the matchers for peer relationship determination for a specific node. // These are UNREDUCED matchers - they include all rules where the node could be either source or destination. -// This is different from FilterForNode which returns REDUCED rules for packet filtering. +// This is different from [PolicyManager.FilterForNode] which returns REDUCED rules for packet filtering. // // For global policies: returns the global matchers (same for all nodes) // For autogroup:self: returns node-specific matchers from unreduced compiled rules. // // Per-node results are cached and invalidated on policy/node updates -// so BuildPeerMap's O(N²) slow path avoids recomputing matchers for -// every pair. +// so [PolicyManager.BuildPeerMap]'s O(N²) slow path avoids recomputing +// matchers for every pair. func (pm *PolicyManager) MatchersForNode(node types.NodeView) ([]matcher.Match, error) { if pm == nil { return nil, nil @@ -832,9 +834,9 @@ func (pm *PolicyManager) nodesHavePolicyAffectingChanges(newNodes views.Slice[ty // NodeCanHaveTag checks if a node can have the specified tag during client-initiated // registration or reauth flows (e.g., tailscale up --advertise-tags). // -// This function is NOT used by the admin API's SetNodeTags - admins can set any -// existing tag on any node by calling State.SetNodeTags directly, which bypasses -// this authorization check. +// This function is NOT used by the admin API's [state.State.SetNodeTags] - admins can +// set any existing tag on any node by calling [state.State.SetNodeTags] directly, +// which bypasses this authorization check. func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool { if pm == nil || pm.pol == nil { return false @@ -874,7 +876,7 @@ func (pm *PolicyManager) NodeCanHaveTag(node types.NodeView, tag string) bool { } // userMatchesOwner checks if a user matches a tag owner entry. -// This is used as a fallback when the node's IP is not in the tagOwnerMap. +// This is used as a fallback when the node's IP is not in the [PolicyManager.tagOwnerMap]. func (pm *PolicyManager) userMatchesOwner(user types.UserView, owner Owner) bool { switch o := owner.(type) { case *Username: @@ -984,11 +986,14 @@ func (pm *PolicyManager) NodeCanApproveRoute(node types.NodeView, route netip.Pr // ViaRoutesForPeer computes via grant effects for a viewer-peer pair. // For each via grant where the viewer matches the source, it checks whether the // peer advertises any of the grant's destination prefixes. If the peer has the -// via tag, those prefixes go into Include; otherwise into Exclude. +// via tag, those prefixes go into [types.ViaRouteResult.Include]; otherwise +// into [types.ViaRouteResult.Exclude]. // -// Performance note: this holds pm.mu for its full duration. Hot -// callers should memoise by (policy-hash, viewer-id) rather than -// invoking this per-pair. +// Performance note: this holds [PolicyManager.mu] for its full duration. Hot +// callers should memoise by (policy-hash, viewer-id) rather than invoking +// this per-pair. +// +//nolint:gocyclo // three-pass via-grant resolution (match, primary election, regular-overlap) func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.ViaRouteResult { var result types.ViaRouteResult @@ -1045,11 +1050,12 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via continue } - // Filter rules and AllowedIPs are different layers. The filter - // rule carries the dst (the authorisation surface). AllowedIPs - // carries the advertised route (the routing fact the viewer - // needs to pick this peer). This loop builds the AllowedIPs - // side, so it emits routes — not dst prefixes. + // Filter rules and [tailcfg.Node.AllowedIPs] are different layers. + // The filter rule carries the dst (the authorisation surface). + // [tailcfg.Node.AllowedIPs] carries the advertised route (the + // routing fact the viewer needs to pick this peer). This loop + // builds the AllowedIPs side, so it emits routes — not dst + // prefixes. peerSubnetRoutes := peer.SubnetRoutes() var matchedPrefixes []netip.Prefix @@ -1109,11 +1115,11 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via // Include. The others move to Exclude. This mirrors HA // primary election scoped to the via tag group. // - // Unlike the global PrimaryRoutes election (routes/primary.go), - // which picks one primary across ALL advertisers of a prefix, - // this election is scoped to the via tag. Two via grants with - // different tags (e.g., tag:ha-a vs tag:ha-b) each elect their - // own winner independently. + // Unlike the global [tailcfg.Node.PrimaryRoutes] election + // (routes/primary.go), which picks one primary across ALL + // advertisers of a prefix, this election is scoped to the via tag. + // Two via grants with different tags (e.g., tag:ha-a vs tag:ha-b) + // each elect their own winner independently. // // Only process via grants where the viewer matches the source, // otherwise grants for other viewer groups would incorrectly @@ -1165,8 +1171,9 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via // When a regular grant also covers a prefix that a via grant // included, defer to global HA primary election (UsePrimary). // When a regular grant covers a prefix that a via grant excluded - // (peer lacks via tag), remove the exclusion so RoutesForPeer - // can apply normal ReduceRoutes + primary logic. + // (peer lacks via tag), remove the exclusion so + // [state.State.RoutesForPeer] can apply normal + // [policy.ReduceRoutes] + primary logic. for i, grant := range grants { if len(grant.Via) > 0 { continue @@ -1437,7 +1444,7 @@ func (pm *PolicyManager) invalidateNodeCache(newNodes views.Slice[types.NodeView } // invalidateGlobalPolicyCache invalidates only nodes whose properties affecting -// ReduceFilterRules changed. For global policies, each node's filter is independent. +// [policyutil.ReduceFilterRules] changed. For global policies, each node's filter is independent. func (pm *PolicyManager) invalidateGlobalPolicyCache(newNodes views.Slice[types.NodeView]) { oldNodeMap := make(map[types.NodeID]types.NodeView) for _, node := range pm.nodes.All() { @@ -1514,8 +1521,8 @@ func flattenTags(tagOwners TagOwners, tag Tag, visiting map[Tag]bool, chain []Ta return result, nil } -// flattenTagOwners flattens all TagOwners by resolving nested tags and detecting cycles. -// It will return a new TagOwners map where all the Tag types have been resolved to their underlying Owners. +// flattenTagOwners flattens all [TagOwners] by resolving nested tags and detecting cycles. +// It will return a new [TagOwners] map where all the [Tag] types have been resolved to their underlying [Owners]. func flattenTagOwners(tagOwners TagOwners) (TagOwners, error) { ret := make(TagOwners) @@ -1536,9 +1543,9 @@ func flattenTagOwners(tagOwners TagOwners) (TagOwners, error) { return ret, nil } -// resolveTagOwners resolves the TagOwners to a map of Tag to netipx.IPSet. -// The resulting map can be used to quickly look up the IPSet for a given Tag. -// It is intended for internal use in a PolicyManager. +// resolveTagOwners resolves the [TagOwners] to a map of [Tag] to [netipx.IPSet]. +// The resulting map can be used to quickly look up the IPSet for a given [Tag]. +// It is intended for internal use in a [PolicyManager]. func resolveTagOwners(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (map[Tag]*netipx.IPSet, error) { if p == nil { return make(map[Tag]*netipx.IPSet), nil @@ -1690,14 +1697,16 @@ func (pm *PolicyManager) NodeCapMaps() map[types.NodeID]tailcfg.NodeCapMap { } // NodesWithChangedCapMap returns the IDs of nodes whose nodeAttrs -// CapMap shifted across one or more updateLocked calls since the -// last drain. The buffer drains on return. The mapper calls this -// once per ReloadPolicy to decide which nodes need a SelfUpdate. +// CapMap shifted across one or more [PolicyManager.updateLocked] calls +// since the last drain. The buffer drains on return. The mapper calls +// this once per [state.State.ReloadPolicy] to decide which nodes need +// a [change.SelfUpdate]. // -// refreshNodeAttrsLocked APPENDS to the buffer; the drain returns -// the union of every change since the previous read. A concurrent -// SetUsers/SetNodes between SetPolicy and a drain cannot silently -// lose the policy-reload diff. +// [PolicyManager.refreshNodeAttrsLocked] APPENDS to the buffer; the drain +// returns the union of every change since the previous read. A concurrent +// [PolicyManager.SetUsers]/[PolicyManager.SetNodes] between +// [PolicyManager.SetPolicy] and a drain cannot silently lose the +// policy-reload diff. func (pm *PolicyManager) NodesWithChangedCapMap() []types.NodeID { if pm == nil { return nil diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 72a13168..30b06ca1 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -95,7 +95,7 @@ var ( // nodeAttrUnsupportedCaps lists caps that headscale parses but cannot act on // today. Each entry maps to the tracking issue an operator can follow. The // caps are accepted by Tailscale SaaS, but delivering them via headscale -// without the matching server-side machinery would be misleading -- nodes +// without the matching server-side machinery would be misleading — nodes // would advertise a feature that does not work. Reject at policy load and // point operators at the issue. var nodeAttrUnsupportedCaps = map[tailcfg.NodeCapability]string{ @@ -402,11 +402,12 @@ func (u *Username) CanBeAutoApprover() bool { return true } -// resolveUser attempts to find a user in the provided [types.Users] slice that matches the Username. -// It prioritizes matching the ProviderIdentifier, and if not found, it falls back to matching the Email or Name. +// resolveUser attempts to find a user in the provided [types.Users] slice that matches the [Username]. +// It prioritizes matching the [types.User.ProviderIdentifier], and if not found, it falls back to matching +// the [types.User.Email] or [types.User.Name]. // If no matching user is found, it returns an error indicating no user matching. // If multiple matching users are found, it returns an error indicating multiple users matching. -// It returns the matched types.User and a nil error if exactly one match is found. +// It returns the matched [types.User] and a nil error if exactly one match is found. func (u *Username) resolveUser(users types.Users) (types.User, error) { var potentialUsers types.Users @@ -718,9 +719,9 @@ func (p *Prefix) UnmarshalJSON(b []byte) error { return nil } -// Resolve resolves the Prefix to an IPSet. The IPSet will contain all the IP -// addresses that the Prefix represents within Headscale. It is the product -// of the Prefix and the Policy, Users, and Nodes. +// Resolve resolves the [Prefix] to an [netipx.IPSet]. The [netipx.IPSet] will +// contain all the IP addresses that the [Prefix] represents within Headscale. +// It is the product of the [Prefix] and the [Policy], [types.Users], and [types.Nodes]. // // See [Policy], [types.Users], and [types.Nodes] for more details. func (p *Prefix) Resolve(_ *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { @@ -864,16 +865,16 @@ type Alias interface { UnmarshalJSON(b []byte) error // String renders the alias back to its policy-file form. Implementations - // are expected to return a value that round-trips through parseAlias for + // are expected to return a value that round-trips through [parseAlias] for // any alias the parser accepted, so callers can use it as a stable // identity in rendered errors and logs. String() string - // Resolve resolves the Alias to an IPSet. The IPSet will contain all the IP - // addresses that the Alias represents within Headscale. It is the product - // of the Alias and the Policy, Users and Nodes. - // This is an interface definition and the implementation is independent of - // the Alias type. + // Resolve resolves the [Alias] to a [netipx.IPSet]. The [netipx.IPSet] will + // contain all the IP addresses that the [Alias] represents within Headscale. + // It is the product of the [Alias] and the [Policy], [types.Users] and + // [types.Nodes]. This is an interface definition and the implementation is + // independent of the [Alias] type. Resolve(pol *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) resolve(pol *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) @@ -1177,7 +1178,7 @@ func buildIPSetMultiErr(ipBuilder *netipx.IPSetBuilder, errs []error) (*netipx.I return ips, multierr.New(append(errs, err)...) } -// Helper function to unmarshal a JSON string into either an AutoApprover or Owner pointer. +// Helper function to unmarshal a JSON string into either an [AutoApprover] or [Owner] pointer. func unmarshalPointer[T any]( b []byte, parseFunc func(string) (T, error), @@ -1349,7 +1350,7 @@ func parseOwner(s string) (Owner, error) { type Usernames []Username -// Groups are a map of Group to a list of Username. +// Groups are a map of [Group] to a list of [Username]. type Groups map[Group]Usernames func (g *Groups) Contains(group *Group) error { @@ -1366,8 +1367,8 @@ func (g *Groups) Contains(group *Group) error { return fmt.Errorf("%w: %q", ErrGroupNotDefined, group) } -// UnmarshalJSON overrides the default JSON unmarshalling for Groups to ensure -// that each group name is validated using the isGroup function. This ensures +// UnmarshalJSON overrides the default JSON unmarshalling for [Groups] to ensure +// that each group name is validated using the [isGroup] function. This ensures // that all group names conform to the expected format, which is always prefixed // with "group:". If any group name is invalid, an error is returned. func (g *Groups) UnmarshalJSON(b []byte) error { @@ -1541,7 +1542,7 @@ func (to TagOwners) MarshalJSON() ([]byte, error) { return json.Marshal(rawTagOwners) } -// TagOwners are a map of Tag to a list of the UserEntities that own the tag. +// TagOwners are a map of [Tag] to a list of the UserEntities that own the tag. type TagOwners map[Tag]Owners func (to TagOwners) Contains(tagOwner *Tag) error { @@ -1587,9 +1588,9 @@ func (ap AutoApproverPolicy) MarshalJSON() ([]byte, error) { return json.Marshal(&obj) } -// resolveAutoApprovers resolves the AutoApprovers to a map of netip.Prefix to netipx.IPSet. +// resolveAutoApprovers resolves the [AutoApprovers] to a map of [netip.Prefix] to [netipx.IPSet]. // The resulting map can be used to quickly look up if a node can self-approve a route. -// It is intended for internal use in a PolicyManager. +// It is intended for internal use in a [PolicyManager]. func resolveAutoApprovers(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (map[netip.Prefix]*netipx.IPSet, *netipx.IPSet, error) { if p == nil { return nil, nil, nil @@ -1653,14 +1654,14 @@ func resolveAutoApprovers(p *Policy, users types.Users, nodes views.Slice[types. return ret, exitNodeSet, nil } -// Action represents the action to take for an ACL rule. +// Action represents the action to take for an [ACL] rule. type Action string const ( ActionAccept Action = "accept" ) -// SSHAction represents the action to take for an SSH rule. +// SSHAction represents the action to take for an [SSH] rule. type SSHAction string const ( @@ -1668,12 +1669,12 @@ const ( SSHActionCheck SSHAction = "check" ) -// String returns the string representation of the Action. +// String returns the string representation of the [Action]. func (a *Action) String() string { return string(*a) } -// UnmarshalJSON implements JSON unmarshaling for Action. +// UnmarshalJSON implements JSON unmarshaling for [Action]. func (a *Action) UnmarshalJSON(b []byte) error { str := strings.Trim(string(b), `"`) switch str { @@ -1686,12 +1687,12 @@ func (a *Action) UnmarshalJSON(b []byte) error { return nil } -// MarshalJSON implements JSON marshaling for Action. +// MarshalJSON implements JSON marshaling for [Action]. func (a *Action) MarshalJSON() ([]byte, error) { return json.Marshal(string(*a)) } -// String returns the string representation of the SSHAction. +// String returns the string representation of the [SSHAction]. func (a *SSHAction) String() string { return string(*a) } @@ -1715,7 +1716,7 @@ func (a *SSHAction) UnmarshalJSON(b []byte) error { return nil } -// MarshalJSON implements JSON marshaling for SSHAction. +// MarshalJSON implements JSON marshaling for [SSHAction]. func (a *SSHAction) MarshalJSON() ([]byte, error) { return json.Marshal(string(*a)) } @@ -1741,12 +1742,12 @@ const ( ProtocolNameWildcard Protocol = "*" ) -// String returns the string representation of the Protocol. +// String returns the string representation of the [Protocol]. func (p *Protocol) String() string { return string(*p) } -// Description returns the human-readable description of the Protocol. +// Description returns the human-readable description of the [Protocol]. func (p *Protocol) Description() string { switch *p { case ProtocolNameICMP: @@ -1784,8 +1785,8 @@ func (p *Protocol) Description() string { } } -// toIANAProtocolNumbers converts a Protocol to its IANA protocol numbers. -// Since validation happens during UnmarshalJSON, this method should not fail for valid Protocol values. +// toIANAProtocolNumbers converts a [Protocol] to its IANA protocol numbers. +// Since validation happens during [Protocol.UnmarshalJSON], this method should not fail for valid [Protocol] values. func (p *Protocol) toIANAProtocolNumbers() []int { switch *p { case "": @@ -1829,13 +1830,13 @@ func (p *Protocol) toIANAProtocolNumbers() []int { } } -// UnmarshalJSON implements JSON unmarshaling for Protocol. +// UnmarshalJSON implements JSON unmarshaling for [Protocol]. // // Tailscale accepts both named ("tcp") and numeric IANA ("6") forms. // Storing whichever form the user wrote leaves downstream code with // two equivalents to handle separately, and any consumer that // branches on the named form would silently mishandle the numeric -// equivalent. Canonicalising to the named form here makes Protocol +// equivalent. Canonicalising to the named form here makes [Protocol] // hold one value post-parse — every downstream consumer sees the // same form regardless of what the user wrote. func (p *Protocol) UnmarshalJSON(b []byte) error { @@ -1859,7 +1860,7 @@ func (p *Protocol) UnmarshalJSON(b []byte) error { return nil } -// validate checks if the Protocol is valid. +// validate checks if the [Protocol] is valid. func (p *Protocol) validate() error { switch *p { case "", ProtocolNameICMP, ProtocolNameIGMP, ProtocolNameIPv4, ProtocolNameIPInIP, @@ -1891,7 +1892,7 @@ func (p *Protocol) validate() error { } } -// MarshalJSON implements JSON marshaling for Protocol. +// MarshalJSON implements JSON marshaling for [Protocol]. func (p *Protocol) MarshalJSON() ([]byte, error) { return json.Marshal(string(*p)) } @@ -1913,7 +1914,7 @@ const ( ProtocolFC = 133 // Fibre Channel ) -// ProtocolNumberToName maps IANA protocol numbers to their protocol name strings. +// ProtocolNumberToName maps IANA protocol numbers to their [Protocol] name strings. var ProtocolNumberToName = map[int]Protocol{ ProtocolICMP: ProtocolNameICMP, ProtocolIGMP: ProtocolNameIGMP, @@ -1937,7 +1938,7 @@ type ACL struct { Destinations []AliasWithPorts `json:"dst"` } -// UnmarshalJSON implements custom unmarshalling for ACL that ignores fields starting with '#'. +// UnmarshalJSON implements custom unmarshalling for [ACL] that ignores fields starting with '#'. // headscale-admin uses # in some field names to add metadata, so we will ignore // those to ensure it doesnt break. // https://github.com/GoodiesHQ/headscale-admin/blob/214a44a9c15c92d2b42383f131b51df10c84017c/src/lib/common/acl.svelte.ts#L38 @@ -2004,7 +2005,7 @@ type NodeAttrGrant struct { IPPool []netip.Prefix `json:"ipPool,omitempty"` } -// aclToGrants converts an ACL rule to one or more equivalent Grant rules. +// aclToGrants converts an [ACL] rule to one or more equivalent [Grant] rules. func aclToGrants(acl ACL) []Grant { ret := make([]Grant, 0, len(acl.Destinations)) @@ -2076,8 +2077,8 @@ func aclToGrants(acl ACL) []Grant { // TODO(kradalby): // Add validation method checking: // All users exists -// All groups and users are valid tag TagOwners -// Everything referred to in ACLs exists in other +// All groups and users are valid tag [TagOwners] +// Everything referred to in [ACL]s exists in other // entities. type Policy struct { // validated is set if the policy has been validated. @@ -2098,7 +2099,7 @@ type Policy struct { RandomizeClientPort bool `json:"randomizeClientPort,omitempty"` } -// MarshalJSON is deliberately not implemented for Policy. +// MarshalJSON is deliberately not implemented for [Policy]. // We use the default JSON marshalling behavior provided by the Go runtime. var ( @@ -2178,7 +2179,7 @@ func validateAutogroupForDst(dst *AutoGroup) error { // autogroup:self / autogroup:internet / autogroup:danger-all are rejected — // none of them describes a stable identity set that a node-level attribute // can attach to. autogroup:admin / autogroup:owner are rejected one layer -// up: AutoGroup.UnmarshalJSON returns ErrInvalidAutogroup at parse time +// up: [AutoGroup.UnmarshalJSON] returns [ErrInvalidAutogroup] at parse time // because those values aren't in the allowed set, so the policy never // reaches this validator. func validateAutogroupForNodeAttrs(ag *AutoGroup) error { @@ -2195,7 +2196,7 @@ func validateAutogroupForNodeAttrs(ag *AutoGroup) error { // validateNodeAttrIPPool rejects ipPool entries outside the CGNAT range or // overlapping the Tailscale-reserved subranges (MagicDNS, Quad100/IPN). A -// prefix is considered "within" CGNAT when it is at least as specific as +// [netip.Prefix] is considered "within" CGNAT when it is at least as specific as // 100.64.0.0/10 and its first address lies inside it. func validateNodeAttrIPPool(prefix netip.Prefix) error { cgnat := tsaddr.CGNATRange() @@ -2248,9 +2249,9 @@ func validateAutogroupForSSHDst(dst *AutoGroup) error { // validateSSHSrcDstCombination validates that SSH source/destination combinations // follow Tailscale's security model: -// - Destination can be: tags, autogroup:self (if source is users/groups), or same-user -// - Tags/autogroup:tagged CANNOT SSH to user destinations -// - Username destinations require the source to be that same single user only. +// - Destination can be: tags, autogroup:self (if source is users/groups), or same-user +// - Tags/autogroup:tagged CANNOT SSH to user destinations +// - [Username] destinations require the source to be that same single user only. func validateSSHSrcDstCombination(sources SSHSrcAliases, destinations SSHDstAliases) error { // Categorize source types srcHasTaggedEntities := false @@ -2303,11 +2304,11 @@ func validateSSHSrcDstCombination(sources SSHSrcAliases, destinations SSHDstAlia return nil } -// validateACLSrcDstCombination validates that ACL source/destination combinations +// validateACLSrcDstCombination validates that [ACL] source/destination combinations // follow Tailscale's security model: -// - autogroup:self destinations require ALL sources to be users, groups, autogroup:member, or wildcard (*) -// - Tags, autogroup:tagged, hosts, and raw IPs are NOT valid sources for autogroup:self -// - Wildcard (*) is allowed because autogroup:self evaluation narrows it per-node to the node's own IPs. +// - autogroup:self destinations require ALL sources to be users, groups, autogroup:member, or wildcard (*) +// - Tags, autogroup:tagged, hosts, and raw IPs are NOT valid sources for autogroup:self +// - Wildcard (*) is allowed because autogroup:self evaluation narrows it per-node to the node's own IPs. func validateACLSrcDstCombination(sources Aliases, destinations []AliasWithPorts) error { // Check if any destination is autogroup:self hasAutogroupSelf := false @@ -2381,11 +2382,11 @@ var tailscaleCapAllowlist = map[tailcfg.PeerCapability]bool{ tailcfg.PeerCapabilityTsIDP: true, // tailscale.com/cap/tsidp } -// validateGrantSrcDstCombination validates grant-specific source/destination -// combinations. Grants are stricter than ACLs: wildcard (*) sources are NOT +// validateGrantSrcDstCombination validates [Grant]-specific source/destination +// combinations. [Grant]s are stricter than [ACL]s: wildcard (*) sources are NOT // allowed with autogroup:self destinations because * includes tags, and tags -// cannot use autogroup:self. ACLs allow this combination because ACL -// autogroup:self evaluation narrows it per-node, but grants reject it at +// cannot use autogroup:self. [ACL]s allow this combination because ACL +// autogroup:self evaluation narrows it per-node, but [Grant]s reject it at // validation time. func validateGrantSrcDstCombination(sources Aliases, destinations Aliases) error { hasAutogroupSelf := false @@ -2917,14 +2918,14 @@ func (p *Policy) validate() error { // SSHCheckPeriod represents the check period for SSH "check" mode rules. // nil means not specified (runtime default of 12h applies). -// Always=true means "always" (check on every request). -// Duration is an explicit period (min 1m, max 168h). +// [SSHCheckPeriod.Always]=true means "always" (check on every request). +// [SSHCheckPeriod.Duration] is an explicit period (min 1m, max 168h). type SSHCheckPeriod struct { Always bool Duration time.Duration } -// UnmarshalJSON implements JSON unmarshaling for SSHCheckPeriod. +// UnmarshalJSON implements JSON unmarshaling for [SSHCheckPeriod]. func (p *SSHCheckPeriod) UnmarshalJSON(b []byte) error { str := strings.Trim(string(b), `"`) if str == "always" { @@ -2943,7 +2944,7 @@ func (p *SSHCheckPeriod) UnmarshalJSON(b []byte) error { return nil } -// MarshalJSON implements JSON marshaling for SSHCheckPeriod. +// MarshalJSON implements JSON marshaling for [SSHCheckPeriod]. func (p SSHCheckPeriod) MarshalJSON() ([]byte, error) { if p.Always { return []byte(`"always"`), nil @@ -2980,11 +2981,11 @@ type SSH struct { AcceptEnv []string `json:"acceptEnv,omitempty"` } -// SSHSrcAliases is a list of aliases that can be used as sources in an SSH rule. +// SSHSrcAliases is a list of aliases that can be used as sources in an [SSH] rule. // It can be a list of usernames, groups, tags or autogroups. type SSHSrcAliases []Alias -// MarshalJSON marshals the Groups to JSON. +// MarshalJSON marshals the [Groups] to JSON. func (g *Groups) MarshalJSON() ([]byte, error) { if *g == nil { return []byte("{}"), nil @@ -3049,7 +3050,7 @@ func (a *SSHDstAliases) UnmarshalJSON(b []byte) error { return nil } -// MarshalJSON marshals the SSHDstAliases to JSON. +// MarshalJSON marshals the [SSHDstAliases] to JSON. func (a SSHDstAliases) MarshalJSON() ([]byte, error) { if a == nil { return []byte("[]"), nil @@ -3078,7 +3079,7 @@ func (a SSHDstAliases) MarshalJSON() ([]byte, error) { return json.Marshal(aliases) } -// MarshalJSON marshals the SSHSrcAliases to JSON. +// MarshalJSON marshals the [SSHSrcAliases] to JSON. func (a *SSHSrcAliases) MarshalJSON() ([]byte, error) { if a == nil || *a == nil { return []byte("[]"), nil @@ -3123,7 +3124,7 @@ func (a *SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[ return newResolvedAddresses(buildIPSetMultiErr(&ips, errs)) } -// SSHDstAliases is a list of aliases that can be used as destinations in an SSH rule. +// SSHDstAliases is a list of aliases that can be used as destinations in an [SSH] rule. // It can be a list of usernames, tags or autogroups. type SSHDstAliases []Alias @@ -3170,20 +3171,20 @@ func (u SSHUsers) LocalpartEntries() []SSHUser { }) } -type SSHUser string +type SSHUser string //nolint:recvcheck // UnmarshalJSON requires pointer receiver; string-newtype methods use value receivers by convention func (u SSHUser) String() string { return string(u) } -// IsLocalpart returns true if the SSHUser has the literal `localpart:` +// IsLocalpart returns true if the [SSHUser] has the literal `localpart:` // prefix. It is a syntactic check only — non-canonical shapes still // pass. func (u SSHUser) IsLocalpart() bool { return strings.HasPrefix(string(u), SSHUserLocalpartPrefix) } -// IsCanonicalLocalpart reports whether the SSHUser parses as the +// IsCanonicalLocalpart reports whether the [SSHUser] parses as the // canonical `localpart:*@` form that resolution acts on. func (u SSHUser) IsCanonicalLocalpart() bool { if !u.IsLocalpart() { @@ -3225,14 +3226,14 @@ func (u SSHUser) ParseLocalpart() (string, error) { return domain, nil } -// MarshalJSON marshals the SSHUser to JSON. +// MarshalJSON marshals the [SSHUser] to JSON. func (u SSHUser) MarshalJSON() ([]byte, error) { return json.Marshal(string(u)) } // UnmarshalJSON trims surrounding whitespace per element. A whitespace- // only entry collapses to `""` and surfaces as `user "" is not valid` in -// the per-rule Validate() pass. +// the per-rule [Policy.validate] pass. func (u *SSHUser) UnmarshalJSON(b []byte) error { var s string if err := json.Unmarshal(b, &s); err != nil { //nolint:noinlineerr @@ -3244,7 +3245,7 @@ func (u *SSHUser) UnmarshalJSON(b []byte) error { return nil } -// unmarshalPolicy takes a byte slice and unmarshals it into a Policy struct. +// unmarshalPolicy takes a byte slice and unmarshals it into a [Policy] struct. // In addition to unmarshalling, it will also validate the policy. // This is the only entrypoint of reading a policy from a file or other source. func unmarshalPolicy(b []byte) (*Policy, error) { @@ -3296,8 +3297,8 @@ func unmarshalPolicy(b []byte) (*Policy, error) { return &policy, nil } -// validateProtocolPortCompatibility checks that only TCP, UDP, and SCTP protocols -// can have specific ports. All other protocols should only use wildcard ports. +// validateProtocolPortCompatibility checks that only TCP, UDP, and SCTP [Protocol]s +// can have specific ports. All other [Protocol]s should only use wildcard ports. func validateProtocolPortCompatibility(protocol Protocol, destinations []AliasWithPorts) error { // Only TCP, UDP, and SCTP support specific ports supportsSpecificPorts := protocol == ProtocolNameTCP || protocol == ProtocolNameUDP || protocol == ProtocolNameSCTP || protocol == "" @@ -3323,7 +3324,7 @@ func validateProtocolPortCompatibility(protocol Protocol, destinations []AliasWi // follow: a tests entry describes one connection attempt to one specific // destination port over a connection-oriented protocol and asserts // whether that attempt is allowed or denied. The same shapes remain -// valid inside ACL or Grant destinations where the rule does not apply. +// valid inside [ACL] or [Grant] destinations where the rule does not apply. func validateTests(pol *Policy, tests []PolicyTest) error { var errs []error @@ -3364,10 +3365,10 @@ func validateTests(pol *Policy, tests []PolicyTest) error { // validateTestDestination enforces that a tests-block dst describes one // connection attempt to one specific host on one specific port. SaaS // rejects three shapes that violate the rule: autogroup:internet (routed -// by exit-node AllowedIPs, not the packet filter); multi-port +// by exit-node [tailcfg.Node.AllowedIPs], not the packet filter); multi-port // (range/list/wildcard, no single allow/deny answer); and CIDR ranges // — both raw `/N` syntax and `hosts:`-table aliases whose RHS is a -// multi-host prefix. Bare IP literals reach this function as *Prefix +// multi-host prefix. Bare IP literals reach this function as *[Prefix] // /32 or /128 just like explicit `/32` / `/128` does, so the CIDR // check inspects the raw input string for `/` rather than the parsed // alias type. diff --git a/hscontrol/servertest/routes_test.go b/hscontrol/servertest/routes_test.go index 85f81820..4f224192 100644 --- a/hscontrol/servertest/routes_test.go +++ b/hscontrol/servertest/routes_test.go @@ -17,6 +17,8 @@ import ( // TestRoutes verifies that route advertisements and approvals // propagate correctly through the control plane to all peers. +// +//nolint:gocyclo // table-driven test driver with many independent subtests func TestRoutes(t *testing.T) { t.Parallel() @@ -71,14 +73,14 @@ func TestRoutes(t *testing.T) { RoutableIPs: []netip.Prefix{routePrefix}, }) - // Send a non-streaming update to push the new hostinfo. + // Send a non-streaming update to push the new [tailcfg.Hostinfo]. ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() _ = c1.Direct().SendUpdate(ctx) // The observer should eventually see the advertised routes - // in the peer's hostinfo. + // in the peer's [tailcfg.Hostinfo]. c2.WaitForCondition(t, "advertised route in hostinfo", 15*time.Second, func(nm *netmap.NetworkMap) bool { @@ -113,7 +115,7 @@ func TestRoutes(t *testing.T) { c1.WaitForPeers(t, 1, 10*time.Second) c2.WaitForPeers(t, 1, 10*time.Second) - // Step 1: Advertise the route by updating hostinfo. + // Step 1: Advertise the route by updating [tailcfg.Hostinfo]. c1.Direct().SetHostinfo(&tailcfg.Hostinfo{ BackendLogID: "servertest-fullrt-advertiser", Hostname: "fullrt-advertiser", @@ -125,7 +127,7 @@ func TestRoutes(t *testing.T) { _ = c1.Direct().SendUpdate(ctx) - // Wait for the server to process the hostinfo update + // Wait for the server to process the [tailcfg.Hostinfo] update // by waiting for observer to see the advertised route. c2.WaitForCondition(t, "hostinfo update propagated", 10*time.Second, @@ -214,18 +216,18 @@ func TestRoutes(t *testing.T) { } }) - // Reproduces https://github.com/juanfont/headscale/issues/3203: + // Reproduces the HA secondary recovery race: // HA tracking loses the secondary subnet router after all routers serving // the route have been offline simultaneously and one of them returns. // // Two assertions split the failure surface: // R1 — server-side primary route state restores after reconnect. - // R2 — observer's netmap shows the reconnected router online with + // R2 — observer's [netmap.NetworkMap] shows the reconnected router online with // the route in its primary set. - // If R1 fails the bug is in state.Connect / primaryRoutes; if R1 passes - // and R2 fails the bug is in change broadcast / mapBatcher. + // If R1 fails the bug is in [state.State.Connect] / primaryRoutes; if R1 passes + // and R2 fails the bug is in change broadcast / [mapper.Batcher]. // - // Caveat: servertest's Reconnect re-registers via TryLogin in addition + // Caveat: [TestClient.Reconnect] re-registers via [controlclient.Direct.TryLogin] in addition // to starting a new poll session. Production reconnects after a brief // network outage may bypass re-registration. If this test passes on // main, fall back to the integration variant noted in the plan @@ -247,7 +249,7 @@ func TestRoutes(t *testing.T) { obs.WaitForPeers(t, 2, 10*time.Second) - // Both routers advertise the same route via their hostinfo. + // Both routers advertise the same route via their [tailcfg.Hostinfo]. advertise := func(c *servertest.TestClient, name string) { t.Helper() c.Direct().SetHostinfo(&tailcfg.Hostinfo{ @@ -305,8 +307,8 @@ func TestRoutes(t *testing.T) { // 3. Reconnect r2 (cable plugged back in). r2.Reconnect(t) - // Hostinfo is part of the controlclient.Direct state; the Reconnect - // helper re-registers via TryLogin which carries the same Hostinfo + // [tailcfg.Hostinfo] is part of the [controlclient.Direct] state; the [TestClient.Reconnect] + // helper re-registers via [controlclient.Direct.TryLogin] which carries the same [tailcfg.Hostinfo] // that was set above. Push it again to be sure the announced route // is registered in the new session. advertise(r2, "ha3203-router2") @@ -343,4 +345,4 @@ func TestRoutes(t *testing.T) { }) } -// findNodeID is defined in issues_test.go. +// [findNodeID] is defined in issues_test.go. diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index be824b36..dce769b7 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2,11 +2,11 @@ // coordinating between subsystems like database, IP allocation, // policy management, and DERP routing. // -// The central type State owns a copy-on-write NodeStore -// (node_store.go), a PrimaryRoutes HA ledger, the PolicyManager, and a -// pingTracker for PingRequest correlation. Cross-subsystem operations -// (node updates, policy evaluation, IP allocation) go through State -// rather than directly to the database. +// The central type [State] owns a copy-on-write [NodeStore] +// (node_store.go), a PrimaryRoutes HA ledger, the [policy.PolicyManager], +// and a [pingTracker] for [tailcfg.PingRequest] correlation. +// Cross-subsystem operations (node updates, policy evaluation, IP +// allocation) go through [State] rather than directly to the database. package state @@ -71,7 +71,7 @@ var ErrNodeNotFound = errors.New("node not found") // ErrInvalidNodeView is returned when an invalid node view is provided. var ErrInvalidNodeView = errors.New("invalid node view provided") -// ErrNodeNotInNodeStore is returned when a node no longer exists in the NodeStore. +// ErrNodeNotInNodeStore is returned when a node no longer exists in the [NodeStore]. var ErrNodeNotInNodeStore = errors.New("node no longer exists in NodeStore") // ErrNodeNameNotUnique is returned when a node name is not unique. @@ -122,6 +122,9 @@ type sshCheckPair struct { // State manages Headscale's core state, coordinating between database, policy management, // IP allocation, and DERP routing. All methods are thread-safe. +// +// See [policy.PolicyManager] for policy evaluation and [NodeStore] for the +// in-memory node cache. type State struct { // cfg holds the current Headscale configuration cfg *types.Config @@ -169,7 +172,7 @@ type State struct { sshCheckMu sync.RWMutex } -// NewState creates and initializes a new State instance, setting up the database, +// NewState creates and initializes a new [State] instance, setting up the database, // IP allocator, DERP map, policy manager, and loading existing users and nodes. func NewState(cfg *types.Config) (*State, error) { cacheExpiration := registerCacheExpiration @@ -226,7 +229,7 @@ func NewState(cfg *types.Config) (*State, error) { return nil, fmt.Errorf("initializing policy manager: %w", err) } - // Apply defaults for NodeStore batch configuration if not set. + // Apply defaults for [NodeStore] batch configuration if not set. // This ensures tests that create Config directly (without viper) still work. batchSize := cfg.Tuning.NodeStoreBatchSize if batchSize == 0 { @@ -238,7 +241,7 @@ func NewState(cfg *types.Config) (*State, error) { batchTimeout = defaultNodeStoreBatchTimeout } - // PolicyManager.BuildPeerMap handles both global and per-node filter complexity. + // [policy.PolicyManager.BuildPeerMap] handles both global and per-node filter complexity. // This moves the complex peer relationship logic into the policy package where it belongs. nodeStore := NewNodeStore( nodes, @@ -264,7 +267,7 @@ func NewState(cfg *types.Config) (*State, error) { }, nil } -// Close gracefully shuts down the State instance and releases all resources. +// Close gracefully shuts down the [State] instance and releases all resources. func (s *State) Close() error { s.pings.drain() s.nodeStore.Stop() @@ -288,7 +291,7 @@ func (s *State) DERPMap() tailcfg.DERPMapView { } // ReloadPolicy reloads the access control policy and triggers auto-approval if changed. -// Returns true if the policy changed. +// Returns the resulting [change.Change] slice when the policy or routes changed. func (s *State) ReloadPolicy() ([]change.Change, error) { pol, err := hsdb.PolicyBytes(s.db.DB, s.cfg) if err != nil { @@ -304,18 +307,18 @@ func (s *State) ReloadPolicy() ([]change.Change, error) { // approvals don't persist if checkPeriod rules are modified or removed. s.ClearSSHCheckAuth() - // Rebuild peer maps after policy changes because the peersFunc in NodeStore - // uses the PolicyManager's filters. Without this, nodes won't see newly allowed - // peers until a node is added/removed, causing autogroup:self policies to not - // propagate correctly when switching between policy types. + // Rebuild peer maps after policy changes because the peersFunc in [NodeStore] + // uses the [policy.PolicyManager]'s filters. Without this, nodes won't see + // newly allowed peers until a node is added/removed, causing autogroup:self + // policies to not propagate correctly when switching between policy types. s.nodeStore.RebuildPeerMaps() //nolint:prealloc // cs starts with one element and may grow cs := []change.Change{change.PolicyChange()} // Per-node selective self refresh for nodeAttrs. A broadcast - // PolicyChange() re-renders peer lists and packet filters but - // never repopulates a node's own [tailcfg.Node.CapMap]; that + // [change.PolicyChange] re-renders peer lists and packet filters + // but never repopulates a node's own [tailcfg.Node.CapMap]; that // lives on the self entry only. The drain returns every node ID // whose cap output shifted across recent updateLocked calls — // refreshNodeAttrsLocked appends rather than overwrites so a @@ -476,18 +479,19 @@ func (s *State) ListAllUsers() ([]types.User, error) { // persistNodeToDB saves the given node state to the database. // This function must receive the exact node state to save to ensure consistency between -// NodeStore and the database. It verifies the node still exists in NodeStore to prevent -// race conditions where a node might be deleted between UpdateNode returning and -// persistNodeToDB being called. +// [NodeStore] and the database. It verifies the node still exists in [NodeStore] to +// prevent race conditions where a node might be deleted between [NodeStore.UpdateNode] +// returning and persistNodeToDB being called. func (s *State) persistNodeToDB(node types.NodeView) (types.NodeView, change.Change, error) { if !node.Valid() { return types.NodeView{}, change.Change{}, ErrInvalidNodeView } - // Verify the node still exists in NodeStore before persisting to database. - // Without this check, we could hit a race condition where UpdateNode returns a valid - // node from a batch update, then the node gets deleted (e.g., ephemeral node logout), - // and persistNodeToDB would incorrectly re-insert the deleted node into the database. + // Verify the node still exists in [NodeStore] before persisting to database. + // Without this check, we could hit a race condition where [NodeStore.UpdateNode] + // returns a valid node from a batch update, then the node gets deleted (e.g., + // ephemeral node logout), and persistNodeToDB would incorrectly re-insert the + // deleted node into the database. _, exists := s.nodeStore.GetNode(node.ID()) if !exists { log.Warn(). @@ -523,12 +527,12 @@ func (s *State) persistNodeToDB(node types.NodeView) (types.NodeView, change.Cha } func (s *State) SaveNode(node types.NodeView) (types.NodeView, change.Change, error) { - // Update NodeStore first + // Update [NodeStore] first nodePtr := node.AsStruct() resultNode := s.nodeStore.PutNode(*nodePtr) - // Then save to database using the result from PutNode + // Then save to database using the result from [NodeStore.PutNode] return s.persistNodeToDB(resultNode) } @@ -563,14 +567,15 @@ func (s *State) DeleteNode(node types.NodeView) (change.Change, error) { // Connect marks a node connected and returns the resulting changes // plus a session epoch. The caller must pass the epoch back to -// Disconnect so deferred grace-period disconnects from a previous -// poll session are dropped (see poll.go). +// [State.Disconnect] so deferred grace-period disconnects from a +// previous poll session are dropped (see poll.go). func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) { prevRoutes := s.nodeStore.PrimaryRoutes() // Reconnecting clears Unhealthy: the node just proved basic // connectivity by completing the Noise handshake. var epoch uint64 + node, ok := s.nodeStore.UpdateNode(id, func(n *types.Node) { n.SessionEpoch++ epoch = n.SessionEpoch @@ -597,17 +602,20 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) { } // Disconnect marks the node offline. epoch must match the value -// Connect returned for this session; otherwise the call no-ops so a -// deferred disconnect from an older session cannot overwrite state set -// by a newer Connect. The check and the IsOnline write share an -// UpdateNode closure, making them atomic against concurrent Connects. +// [State.Connect] returned for this session; otherwise the call no-ops +// so a deferred disconnect from an older session cannot overwrite state +// set by a newer [State.Connect]. The check and the IsOnline write share +// an [NodeStore.UpdateNode] closure, making them atomic against +// concurrent connects. func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, error) { var stale bool + node, ok := s.nodeStore.UpdateNode(id, func(n *types.Node) { if n.SessionEpoch != epoch { stale = true return } + now := time.Now() n.LastSeen = &now n.IsOnline = new(false) @@ -631,7 +639,7 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro log.Info().EmbedObject(node).Msg("node disconnected") - // Persist LastSeen best-effort: NodeStore already reflects offline + // Persist LastSeen best-effort: [NodeStore] already reflects offline // and peers still need the change notifications below. _, c, err := s.persistNodeToDB(node) if err != nil { @@ -807,11 +815,11 @@ func (s *State) ListEphemeralNodes() views.Slice[types.NodeView] { // SetNodeExpiry updates the expiration time for a node. // If expiry is nil, the node's expiry is disabled (node will never expire). func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.NodeView, change.Change, error) { - // Update NodeStore before database to ensure consistency. The NodeStore update is - // blocking and will be the source of truth for the batcher. The database update must - // make the exact same change. If the database update fails, the NodeStore change will - // remain, but since we return an error, no change notification will be sent to the - // batcher, preventing inconsistent state propagation. + // Update [NodeStore] before database to ensure consistency. The [NodeStore] update + // is blocking and will be the source of truth for the batcher. The database update + // must make the exact same change. If the database update fails, the [NodeStore] + // change will remain, but since we return an error, no change notification will be + // sent to the batcher, preventing inconsistent state propagation. n, ok := s.nodeStore.UpdateNode(nodeID, func(node *types.Node) { node.Expiry = expiry }) @@ -877,9 +885,9 @@ func (s *State) SetNodeTags(nodeID types.NodeID, tags []string) (types.NodeView, // Log the operation logTagOperation(existingNode, validatedTags) - // Update NodeStore before database to ensure consistency. The NodeStore update is - // blocking and will be the source of truth for the batcher. The database update must - // make the exact same change. + // Update [NodeStore] before database to ensure consistency. The [NodeStore] update + // is blocking and will be the source of truth for the batcher. The database update + // must make the exact same change. n, ok := s.nodeStore.UpdateNode(nodeID, func(node *types.Node) { node.Tags = validatedTags // Tagged nodes are owned by their tags, not a user. @@ -974,7 +982,7 @@ func (s *State) BackfillNodeIPs() ([]string, error) { return nil, err } - // Refresh NodeStore after IP changes to ensure consistency + // Refresh [NodeStore] after IP changes to ensure consistency if len(changes) > 0 { nodes, err := s.db.ListNodes() if err != nil { @@ -996,7 +1004,7 @@ func (s *State) BackfillNodeIPs() ([]string, error) { node.Hostinfo.NetInfo = netInfo } // TODO(kradalby): This should just update the IP addresses, nothing else in the node store. - // We should avoid PutNode here. + // We should avoid [NodeStore.PutNode] here. _ = s.nodeStore.PutNode(*node) } } @@ -1061,7 +1069,7 @@ func (s *State) MatchersForNode(node types.NodeView) ([]matcher.Match, error) { } // NodeCapMap returns the policy-derived CapMap for the given node, suitable -// for merging into tailcfg.Node.CapMap when the node is rendered as self or +// for merging into [tailcfg.Node.CapMap] when the node is rendered as self or // as someone else's peer. func (s *State) NodeCapMap(id types.NodeID) tailcfg.NodeCapMap { return s.polMan.NodeCapMap(id) @@ -1103,8 +1111,9 @@ func (s *State) AutoApproveRoutes(nv types.NodeView) (change.Change, error) { Strs("routes.approved.new", util.PrefixesToString(approved)). Msg("Single node auto-approval detected route changes") - // Persist the auto-approved routes to database and NodeStore via SetApprovedRoutes - // This ensures consistency between database and NodeStore + // Persist the auto-approved routes to database and [NodeStore] via + // [State.SetApprovedRoutes]. This ensures consistency between database + // and [NodeStore]. _, c, err := s.SetApprovedRoutes(nv.ID(), approved) if err != nil { log.Error(). @@ -1342,11 +1351,11 @@ func (s *State) CreateNodeForTest(user *types.User, hostname ...string) *types.N return s.db.CreateNodeForTest(user, hostname...) } -// PutNodeInStoreForTest writes a test node into the in-memory NodeStore -// so handlers backed by NodeStore lookups (e.g. GetNodeByID) can see it. -// CreateNodeForTest only saves to the database, which is fine for tests -// that exercise the DB layer directly but insufficient for handler tests -// that go through State. +// PutNodeInStoreForTest writes a test node into the in-memory [NodeStore] +// so handlers backed by [NodeStore] lookups (e.g. [State.GetNodeByID]) can +// see it. [State.CreateNodeForTest] only saves to the database, which is +// fine for tests that exercise the DB layer directly but insufficient for +// handler tests that go through [State]. func (s *State) PutNodeInStoreForTest(node types.Node) types.NodeView { return s.nodeStore.PutNode(node) } @@ -1462,7 +1471,7 @@ type newNodeParams struct { // authNodeUpdateParams contains parameters for updating an existing node during auth. type authNodeUpdateParams struct { - // Node to update; must be valid and in NodeStore. + // Node to update; must be valid and in [NodeStore]. ExistingNode types.NodeView // Cached registration payload from the originating client request. RegData *types.RegistrationData @@ -1481,7 +1490,7 @@ type authNodeUpdateParams struct { } // applyAuthNodeUpdate applies common update logic for re-authenticating or converting -// an existing node. It updates the node in NodeStore, processes RequestTags, and +// an existing node. It updates the node in [NodeStore], processes RequestTags, and // persists changes to the database. func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView, error) { regData := params.RegData @@ -1509,8 +1518,9 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView oldTags := params.ExistingNode.Tags().AsSlice() - // Validate tags BEFORE calling UpdateNode to ensure we don't modify NodeStore - // if validation fails. This maintains consistency between NodeStore and database. + // Validate tags BEFORE calling [NodeStore.UpdateNode] to ensure we don't modify + // [NodeStore] if validation fails. This maintains consistency between [NodeStore] + // and database. rejectedTags := s.validateRequestTags(params.ExistingNode, requestTags) if len(rejectedTags) > 0 { return types.NodeView{}, fmt.Errorf( @@ -1520,7 +1530,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView ) } - // Update existing node in NodeStore - validation passed, safe to mutate + // Update existing node in [NodeStore] - validation passed, safe to mutate updatedNodeView, ok := s.nodeStore.UpdateNode(params.ExistingNode.ID(), func(node *types.Node) { node.NodeKey = regData.NodeKey node.DiscoKey = regData.DiscoKey @@ -1536,10 +1546,11 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView node.Endpoints = regData.Endpoints // Do NOT reset IsOnline here. Online status is managed exclusively by - // Connect()/Disconnect() in the poll session lifecycle. Resetting it - // during re-registration causes a false offline blip: the change - // notification triggers a map regeneration showing the node as offline - // to peers, even though Connect() will immediately set it back to true. + // [State.Connect]/[State.Disconnect] in the poll session lifecycle. + // Resetting it during re-registration causes a false offline blip: the + // change notification triggers a map regeneration showing the node as + // offline to peers, even though [State.Connect] will immediately set it + // back to true. node.LastSeen = new(time.Now()) // On conversion (tagged → user) we set the new register method. @@ -1634,7 +1645,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView return updatedNodeView, nil } -// createAndSaveNewNode creates a new node, allocates IPs, saves to DB, and adds to NodeStore. +// createAndSaveNewNode creates a new node, allocates IPs, saves to DB, and adds to [NodeStore]. // It preserves netinfo from an existing node if one is provided (for faster DERP connectivity). func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, error) { // Preserve NetInfo from existing node if available @@ -1655,7 +1666,7 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro Hostinfo: params.Hostinfo, Endpoints: params.Endpoints, LastSeen: new(time.Now()), - IsOnline: new(false), // Explicitly offline until Connect() is called + IsOnline: new(false), // Explicitly offline until [State.Connect] is called RegisterMethod: params.RegisterMethod, Expiry: params.Expiry, } @@ -1748,14 +1759,14 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro nodeToRegister.IPv4 = ipv4 nodeToRegister.IPv6 = ipv6 - // Seed GivenName from the sanitised raw hostname. NodeStore.PutNode + // Seed GivenName from the sanitised raw hostname. [NodeStore.PutNode] // bumps on collision and falls back to "node" if the sanitised // result is empty (pure non-ASCII / punctuation input). if nodeToRegister.GivenName == "" { nodeToRegister.GivenName = dnsname.SanitizeHostname(nodeToRegister.Hostname) } - // New node - database first to get ID, then NodeStore + // New node - database first to get ID, then [NodeStore] savedNode, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) { err := tx.Save(&nodeToRegister).Error if err != nil { @@ -1775,12 +1786,12 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro return types.NodeView{}, err } - // Add to NodeStore after database creates the ID + // Add to [NodeStore] after database creates the ID return s.nodeStore.PutNode(*savedNode), nil } // validateRequestTags validates that the requested tags are permitted for the node. -// This should be called BEFORE UpdateNode to ensure we don't modify NodeStore +// This should be called BEFORE [NodeStore.UpdateNode] to ensure we don't modify [NodeStore] // if validation fails. Returns the list of rejected tags (empty if all valid). func (s *State) validateRequestTags(node types.NodeView, requestTags []string) []string { // Empty tags = clear tags, always permitted @@ -2085,6 +2096,7 @@ func (s *State) findExistingNodeForPAK( return types.NodeView{}, false } +//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering func (s *State) HandleNodeFromPreAuthKey( regReq tailcfg.RegisterRequest, machineKey key.MachinePublic, @@ -2201,8 +2213,9 @@ func (s *State) HandleNodeFromPreAuthKey( node.AuthKey = pak node.AuthKeyID = &pak.ID // Do NOT reset IsOnline here. Online status is managed exclusively by - // Connect()/Disconnect() in the poll session lifecycle. Resetting it - // during re-registration causes a false offline blip to peers. + // [State.Connect]/[State.Disconnect] in the poll session lifecycle. + // Resetting it during re-registration causes a false offline blip + // to peers. node.LastSeen = new(time.Now()) // Tagged nodes keep their existing expiry (disabled). @@ -2237,7 +2250,7 @@ func (s *State) HandleNodeFromPreAuthKey( // Only mark the key used on the *first* registration. On // re-registration the same key is already used and the - // atomic compare-and-set in UsePreAuthKey would otherwise + // atomic compare-and-set in [hsdb.UsePreAuthKey] would otherwise // reject it as "authkey already used". This is the path // behind issue #2830 where containers restart with the // same one-shot key. @@ -2478,7 +2491,7 @@ func (s *State) autoApproveNodes() ([]change.Change, error) { // isAutoDerivedGivenName reports whether given matches what // dnsname.SanitizeHostname(hostname) would produce, optionally with a -// NodeStore collision-bump "-N" suffix. It is used to detect whether a +// [NodeStore] collision-bump "-N" suffix. It is used to detect whether a // GivenName has been admin-renamed (in which case it must not be // overwritten by client-side hostname changes). func isAutoDerivedGivenName(given, hostname string) bool { @@ -2498,10 +2511,10 @@ func isAutoDerivedGivenName(given, hostname string) bool { } // UpdateNodeFromMapRequest is the sync point where Hostinfo changes, -// endpoint updates, and route advertisements from a MapRequest land in -// the NodeStore. It produces a change.Change summarising what actually -// moved so downstream subsystems (mapper, policy, primary routes) can -// react accordingly. +// endpoint updates, and route advertisements from a [tailcfg.MapRequest] +// land in the [NodeStore]. It produces a [change.Change] summarising +// what actually moved so downstream subsystems (mapper, policy, primary +// routes) can react accordingly. // // TODO(kradalby): This is essentially a patch update that could be sent directly to nodes, // which means we could shortcut the whole change thing if there are no other important updates. @@ -2528,7 +2541,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest // Hostinfo + auto-approval that follows shifted any prefix. prevRoutes := s.nodeStore.PrimaryRoutes() - // We need to ensure we update the node as it is in the NodeStore at + // We need to ensure we update the node as it is in the [NodeStore] at // the time of the request. updatedNode, ok := s.nodeStore.UpdateNode(id, func(currentNode *types.Node) { peerChange := currentNode.PeerChangeFromMapRequest(req) @@ -2555,7 +2568,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest return } - // Calculate route approval before NodeStore update to avoid calling View() inside callback + // Calculate route approval before [NodeStore] update to avoid calling View() inside callback var hasNewRoutes bool if hi := req.Hostinfo; hi != nil { hasNewRoutes = len(hi.RoutableIPs) > 0 @@ -2617,7 +2630,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest currentNode.Hostname = req.Hostinfo.Hostname if autoDerived { currentNode.GivenName = dnsname.SanitizeHostname(req.Hostinfo.Hostname) - // NodeStore.UpdateNode auto-bumps GivenName on collision. + // [NodeStore.UpdateNode] auto-bumps GivenName on collision. } } @@ -2655,13 +2668,13 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest Strs(zf.AutoApprovedRoutes, util.PrefixesToString(autoApprovedRoutes)). Msg("Persisting auto-approved routes from MapRequest") - // SetApprovedRoutes will update both database and PrimaryRoutes table + // [State.SetApprovedRoutes] will update both database and PrimaryRoutes table _, c, err := s.SetApprovedRoutes(id, autoApprovedRoutes) if err != nil { return change.Change{}, fmt.Errorf("persisting auto-approved routes: %w", err) } - // If SetApprovedRoutes resulted in a policy change, return it + // If [State.SetApprovedRoutes] resulted in a policy change, return it if !c.IsEmpty() { return c, nil } @@ -2702,7 +2715,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest return buildMapRequestChangeResponse(id, updatedNode, hostinfoChanged, endpointChanged, derpChanged) } -// buildMapRequestChangeResponse determines the appropriate response type for a MapRequest update. +// buildMapRequestChangeResponse determines the appropriate response type for a [tailcfg.MapRequest] update. // Hostinfo changes require a full update, while endpoint/DERP changes can use lightweight patches. func buildMapRequestChangeResponse( id types.NodeID,