diff --git a/hscontrol/mapper/batcher.go b/hscontrol/mapper/batcher.go index 37f3cc24..0a1e30d0 100644 --- a/hscontrol/mapper/batcher.go +++ b/hscontrol/mapper/batcher.go @@ -92,6 +92,11 @@ func generateMapResponse(nc nodeConnection, mapper *mapper, r change.Change) (*t return nil, nil //nolint:nilnil // No response needed for other nodes when self-only } + // Check if this is a self-update (the changed node is the receiving node). + // When true, ensure the response includes the node's self info so it sees + // its own attribute changes (e.g., tags changed via admin API). + isSelfUpdate := r.OriginNode != 0 && r.OriginNode == nodeID + var ( mapResp *tailcfg.MapResponse err error @@ -110,7 +115,12 @@ func generateMapResponse(nc nodeConnection, mapper *mapper, r change.Change) (*t } removedPeers := nc.computePeerDiff(currentPeerIDs) - mapResp, err = mapper.policyChangeResponse(nodeID, version, removedPeers, currentPeers) + // Include self node when this is a self-update (e.g., node's own tags changed) + // so the node sees its updated self info along with new packet filters. + mapResp, err = mapper.policyChangeResponse(nodeID, version, removedPeers, currentPeers, isSelfUpdate) + } else if isSelfUpdate { + // Non-policy self-update: just send the self node info + mapResp, err = mapper.selfMapResponse(nodeID, version) } else { mapResp, err = mapper.buildFromChange(nodeID, version, &r) } diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index 759c9568..616d470f 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -187,13 +187,18 @@ func (m *mapper) selfMapResponse( // - PeersChanged for remaining peers (their AllowedIPs may have changed due to policy) // - Updated PacketFilters // - Updated SSHPolicy (SSH rules may reference users/groups that changed) +// - Optionally, the node's own self info (when includeSelf is true) // This avoids the issue where an empty Peers slice is interpreted by Tailscale // clients as "no change" rather than "no peers". +// When includeSelf is true, the node's self info is included so that a node +// whose own attributes changed (e.g., tags via admin API) sees its updated +// self info along with the new packet filters. func (m *mapper) policyChangeResponse( nodeID types.NodeID, capVer tailcfg.CapabilityVersion, removedPeers []tailcfg.NodeID, currentPeers views.Slice[types.NodeView], + includeSelf bool, ) (*tailcfg.MapResponse, error) { builder := m.NewMapResponseBuilder(nodeID). WithDebugType(policyResponseDebug). @@ -201,6 +206,10 @@ func (m *mapper) policyChangeResponse( WithPacketFilters(). WithSSHPolicy() + if includeSelf { + builder = builder.WithSelfNode() + } + if len(removedPeers) > 0 { // Convert tailcfg.NodeID to types.NodeID for WithPeersRemoved removedIDs := make([]types.NodeID, len(removedPeers)) diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 3b916ad6..0a5fabdb 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -719,7 +719,18 @@ func (s *State) SetNodeTags(nodeID types.NodeID, tags []string) (types.NodeView, return types.NodeView{}, change.Change{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, nodeID) } - return s.persistNodeToDB(n) + nodeView, c, err := s.persistNodeToDB(n) + if err != nil { + return nodeView, c, err + } + + // Set OriginNode so the mapper knows to include self info for this node. + // When tags change, persistNodeToDB returns PolicyChange which doesn't set OriginNode, + // so the mapper's self-update check fails and the node never sees its new tags. + // Setting OriginNode ensures the node gets a self-update with the new tags. + c.OriginNode = nodeID + + return nodeView, c, nil } // SetApprovedRoutes sets the network routes that a node is approved to advertise. diff --git a/hscontrol/types/change/change.go b/hscontrol/types/change/change.go index 307ef690..a76fb7c4 100644 --- a/hscontrol/types/change/change.go +++ b/hscontrol/types/change/change.go @@ -70,6 +70,18 @@ func (r Change) Merge(other Change) Change { merged.PeersRemoved = uniqueNodeIDs(append(r.PeersRemoved, other.PeersRemoved...)) merged.PeerPatches = append(r.PeerPatches, other.PeerPatches...) + // Preserve OriginNode for self-update detection. + // If either change has OriginNode set, keep it so the mapper + // can detect self-updates and send the node its own changes. + if merged.OriginNode == 0 { + merged.OriginNode = other.OriginNode + } + + // Preserve TargetNode for targeted responses. + if merged.TargetNode == 0 { + merged.TargetNode = other.TargetNode + } + if r.Reason != "" && other.Reason != "" && r.Reason != other.Reason { merged.Reason = r.Reason + "; " + other.Reason } else if other.Reason != "" { diff --git a/hscontrol/types/change/change_test.go b/hscontrol/types/change/change_test.go index 30330584..9f181dd6 100644 --- a/hscontrol/types/change/change_test.go +++ b/hscontrol/types/change/change_test.go @@ -233,6 +233,36 @@ func TestChange_Merge(t *testing.T) { r2: Change{Reason: "update"}, want: Change{Reason: "update"}, }, + { + name: "OriginNode preserved from first", + r1: Change{OriginNode: 42}, + r2: Change{IncludePolicy: true}, + want: Change{OriginNode: 42, IncludePolicy: true}, + }, + { + name: "OriginNode preserved from second when first is zero", + r1: Change{IncludePolicy: true}, + r2: Change{OriginNode: 42}, + want: Change{OriginNode: 42, IncludePolicy: true}, + }, + { + name: "OriginNode first wins when both set", + r1: Change{OriginNode: 1}, + r2: Change{OriginNode: 2}, + want: Change{OriginNode: 1}, + }, + { + name: "TargetNode preserved from first", + r1: Change{TargetNode: 42}, + r2: Change{IncludeSelf: true}, + want: Change{TargetNode: 42, IncludeSelf: true}, + }, + { + name: "TargetNode preserved from second when first is zero", + r1: Change{IncludeSelf: true}, + r2: Change{TargetNode: 42}, + want: Change{TargetNode: 42, IncludeSelf: true}, + }, } for _, tt := range tests {