diff --git a/management/server/affectedpeers/resolver.go b/management/server/affectedpeers/resolver.go index d96d342f4..e142e262a 100644 --- a/management/server/affectedpeers/resolver.go +++ b/management/server/affectedpeers/resolver.go @@ -79,7 +79,10 @@ func Load(ctx context.Context, s store.Store, accountID string, c Change) (*Snap return nil, err } } - if len(c.ChangedGroupIDs) > 0 { + // A changed peer is resolved to its groups during the walk (see + // seedChangedGroupsFromPeers), so the nameserver/DNS group walkers can fire for + // peer changes too — load those tables whenever groups or peers changed. + if len(c.ChangedGroupIDs) > 0 || len(c.ChangedPeerIDs) > 0 { if snap.nsGroups, err = s.GetAccountNameServerGroups(ctx, store.LockingStrengthNone, accountID); err != nil { return nil, err } @@ -225,10 +228,33 @@ func newResolver(ctx context.Context, snap *Snapshot, accountID string, c Change resourceIDs: toSet(c.ResourceIDs), networkIDs: toSet(c.NetworkIDs), } + // A changed peer affects every entity referencing a group it belongs to, so + // seed the changed-group set with the peer's memberships from the snapshot's + // group->peers index. Callers pass only ChangedPeerIDs; the peer->group lookup + // is the resolver's job, not theirs. + r.seedChangedGroupsFromPeers() r.matchedPolicies = append(r.matchedPolicies, c.Policies...) return r } +// seedChangedGroupsFromPeers adds, for each changed peer, the groups it belongs +// to into changedGroupSet, so the group-driven walkers (policies, routes, +// nameservers, DNS, routers) fire for memberships — not only for entities that +// reference the peer directly. +func (r *resolver) seedChangedGroupsFromPeers() { + if len(r.changedPeerSet) == 0 { + return + } + for groupID, members := range r.snap.groupPeers { + for pID := range r.changedPeerSet { + if _, ok := members[pID]; ok { + r.changedGroupSet[groupID] = struct{}{} + break + } + } + } +} + func (r *resolver) walk() { r.collectFromExplicitPolicies() r.collectFromExplicitRoutes(r.change.Routes) diff --git a/management/server/peer.go b/management/server/peer.go index bf5e69dfe..e1fe08d1b 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -538,12 +538,9 @@ func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peer return err } - // Load before delete: pre-state still has the peer's group memberships. - groupIDs, err := transaction.GetGroupIDsByPeerIDs(ctx, accountID, []string{peerID}) - if err != nil { - return fmt.Errorf("failed to get group IDs for peer: %w", err) - } - change = affectedpeers.Change{ChangedGroupIDs: groupIDs} + // Load before delete so the snapshot still has the peer's group memberships; + // the resolver derives them from the peer ID during the walk. + change = affectedpeers.Change{ChangedPeerIDs: []string{peerID}} if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil { return err } @@ -1577,17 +1574,12 @@ func affectedPeerIDsFromNetworkMap(nmap *types.NetworkMap, selfPeerID string) [] return ids } -// resolveAffectedPeersForPeerChanges resolves changed peer IDs into the full set of affected peer IDs. +// resolveAffectedPeersForPeerChanges resolves changed peer IDs into the full set +// of affected peer IDs. The resolver derives each peer's group memberships during +// the walk, so the caller passes only the changed peer IDs. func (am *DefaultAccountManager) resolveAffectedPeersForPeerChanges(ctx context.Context, s store.Store, accountID string, changedPeerIDs []string) []string { - groupIDs, err := s.GetGroupIDsByPeerIDs(ctx, accountID, changedPeerIDs) - if err != nil { - log.WithContext(ctx).Errorf("failed to get group IDs for changed peers: %v", err) - return nil - } - return am.ResolveAffectedPeers(ctx, s, accountID, affectedpeers.Change{ - ChangedGroupIDs: groupIDs, - ChangedPeerIDs: changedPeerIDs, + ChangedPeerIDs: changedPeerIDs, }) } diff --git a/management/server/user.go b/management/server/user.go index 35536b43a..412f15ce7 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -1302,12 +1302,9 @@ func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, accountI for _, peer := range userPeers { peerIDs = append(peerIDs, peer.ID) } - // Load before delete: pre-state still has the peers' group memberships. - groupIDs, err := transaction.GetGroupIDsByPeerIDs(ctx, accountID, peerIDs) - if err != nil { - return fmt.Errorf("failed to get group IDs for user peers: %w", err) - } - change = affectedpeers.Change{ChangedGroupIDs: groupIDs} + // Load before delete so the snapshot still has the peers' group + // memberships; the resolver derives them from the peer IDs during the walk. + change = affectedpeers.Change{ChangedPeerIDs: peerIDs} if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil { return err }