From efa13573ede2a970bbead71d7cabd20196681d10 Mon Sep 17 00:00:00 2001 From: pascal Date: Tue, 9 Jun 2026 20:58:20 +0200 Subject: [PATCH] fix linter issues --- management/server/affectedpeers/resolver.go | 79 +++++++---- management/server/networks/routers/manager.go | 2 + management/server/peer.go | 133 +++++++++--------- 3 files changed, 122 insertions(+), 92 deletions(-) diff --git a/management/server/affectedpeers/resolver.go b/management/server/affectedpeers/resolver.go index 32ccb67d8..7309de37b 100644 --- a/management/server/affectedpeers/resolver.go +++ b/management/server/affectedpeers/resolver.go @@ -48,52 +48,76 @@ func Load(ctx context.Context, s store.Store, accountID string, c Change) (*Snap return snap, nil } + if err := snap.loadCollections(ctx, s, accountID, c); err != nil { + return nil, err + } + if err := snap.loadGroupIndex(ctx, s, accountID); err != nil { + return nil, err + } + + return snap, nil +} + +// loadCollections reads the policy/route/nameserver/dns/router/resource/proxy +// collections a Change can touch, gated to what the walk needs. +func (snap *Snapshot) loadCollections(ctx context.Context, s store.Store, accountID string, c Change) error { hasGroupOrPeerChange := len(c.ChangedGroupIDs) > 0 || len(c.ChangedPeerIDs) > 0 || len(c.Resources) > 0 hasNetworkObject := len(c.Routers) > 0 || len(c.Resources) > 0 || len(c.Networks) > 0 - needsPolicies := hasGroupOrPeerChange || len(c.PostureCheckIDs) > 0 || len(c.Policies) > 0 || hasNetworkObject - needsRoutersResources := needsPolicies // the resource<->router bridge can fire for any of these + // the resource<->router bridge can fire for any of these + needsRoutersResources := hasGroupOrPeerChange || len(c.PostureCheckIDs) > 0 || len(c.Policies) > 0 || hasNetworkObject var err error - if needsPolicies { + if needsRoutersResources { if snap.policies, err = s.GetAccountPolicies(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err + return err + } + if snap.routers, err = s.GetNetworkRoutersByAccountID(ctx, store.LockingStrengthNone, accountID); err != nil { + return err + } + if snap.resources, err = s.GetNetworkResourcesByAccountID(ctx, store.LockingStrengthNone, accountID); err != nil { + return err } } if hasGroupOrPeerChange { if snap.routes, err = s.GetAccountRoutes(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err + return err + } + if err = snap.loadProxyServices(ctx, s, accountID); err != nil { + return err } } if len(c.ChangedGroupIDs) > 0 || len(c.ChangedPeerIDs) > 0 { if snap.nsGroups, err = s.GetAccountNameServerGroups(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err + return err } if snap.dnsSettings, err = s.GetAccountDNSSettings(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err - } - } - if needsRoutersResources { - if snap.routers, err = s.GetNetworkRoutersByAccountID(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err - } - if snap.resources, err = s.GetNetworkResourcesByAccountID(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err - } - } - if hasGroupOrPeerChange { - if snap.proxyByCluster, err = s.GetEmbeddedProxyPeerIDsByCluster(ctx, accountID); err != nil { - return nil, err - } - if len(snap.proxyByCluster) > 0 { - if snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID); err != nil { - return nil, err - } + return err } } + return nil +} +// loadProxyServices loads the embedded-proxy cluster index, and the services only +// when the account actually has embedded proxy peers. +func (snap *Snapshot) loadProxyServices(ctx context.Context, s store.Store, accountID string) error { + var err error + if snap.proxyByCluster, err = s.GetEmbeddedProxyPeerIDsByCluster(ctx, accountID); err != nil { + return err + } + if len(snap.proxyByCluster) == 0 { + return nil + } + snap.services, err = s.GetAccountServices(ctx, store.LockingStrengthNone, accountID) + return err +} + +// loadGroupIndex loads all groups (for group.Resources) and builds the +// group->member-peers index. Always needed: the bridge resolves group.Resources +// and Expand maps groups to member peers. +func (snap *Snapshot) loadGroupIndex(ctx context.Context, s store.Store, accountID string) error { groups, err := s.GetAccountGroups(ctx, store.LockingStrengthNone, accountID) if err != nil { - return nil, err + return err } snap.groups = make(map[string]*types.Group, len(groups)) snap.groupPeers = make(map[string]map[string]struct{}, len(groups)) @@ -105,8 +129,7 @@ func Load(ctx context.Context, s store.Store, accountID string, c Change) (*Snap } snap.groupPeers[g.ID] = members } - - return snap, nil + return nil } // Change describes what changed in an account. diff --git a/management/server/networks/routers/manager.go b/management/server/networks/routers/manager.go index f1b31093c..390074487 100644 --- a/management/server/networks/routers/manager.go +++ b/management/server/networks/routers/manager.go @@ -317,5 +317,7 @@ func (m *mockManager) DeleteRouter(ctx context.Context, accountID, userID, netwo } func (m *mockManager) DeleteRouterInTransaction(ctx context.Context, transaction store.Store, accountID, userID, networkID, routerID string) (*types.NetworkRouter, func(), error) { + // no-op mock: returns zero values so tests that don't exercise router deletion + // can satisfy the Manager interface without a real store. return nil, func() {}, nil } diff --git a/management/server/peer.go b/management/server/peer.go index e8990481b..baf62a7eb 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -106,39 +106,22 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK am.updatePeerLocationIfChanged(ctx, accountID, peer, realIP) } - expired := peer.Status != nil && peer.Status.LoginExpired - - if peer.AddedWithSSOLogin() { - settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) - if err != nil { - return err - } - if peer.LoginExpirationEnabled && settings.PeerLoginExpirationEnabled { - am.schedulePeerLoginExpiration(ctx, accountID) - } - if peer.InactivityExpirationEnabled && settings.PeerInactivityExpirationEnabled { - am.checkAndSchedulePeerInactivityExpiration(ctx, accountID) - } + if err = am.schedulePeerExpirations(ctx, accountID, peer); err != nil { + return err } - if expired { - changedPeerIDs := []string{peer.ID} + // A login-expired peer reconnecting, or an embedded proxy peer flipping to + // connected (which triggers SynthesizePrivateServiceZones), must refresh the + // peers reachable from it. The embedded-proxy fan-out tolerates a dispatch error. + if peer.Status != nil && peer.Status.LoginExpired { affectedPeerIDs := am.markConnectedAffectedPeers(ctx, accountID, peer.ID, nmap) - err = am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs) - if err != nil { + if err = am.networkMapController.OnPeersUpdated(ctx, accountID, []string{peer.ID}, affectedPeerIDs); err != nil { return fmt.Errorf("notify network map controller of peer update: %w", err) } } - - // An embedded proxy peer flipping to connected is the trigger for - // SynthesizePrivateServiceZones to emit DNS A records pointing at its - // tunnel IP. Fan the change out to every peer that has a synthesized - // policy or DNS-record edge from this proxy peer; otherwise their - // synth state stays stale until some other change pokes the controller. if peer.ProxyMeta.Embedded { - changedPeerIDs := []string{peer.ID} affectedPeerIDs := am.markConnectedAffectedPeers(ctx, accountID, peer.ID, nmap) - if err := am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs); err != nil { + if err := am.networkMapController.OnPeersUpdated(ctx, accountID, []string{peer.ID}, affectedPeerIDs); err != nil { log.WithContext(ctx).Warnf("notify network map controller of embedded proxy %s connect: %v", peer.ID, err) } } @@ -146,6 +129,25 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK return nil } +// schedulePeerExpirations reschedules the account's login/inactivity expiration +// timers for an SSO peer that just connected. +func (am *DefaultAccountManager) schedulePeerExpirations(ctx context.Context, accountID string, peer *nbpeer.Peer) error { + if !peer.AddedWithSSOLogin() { + return nil + } + settings, err := am.Store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) + if err != nil { + return err + } + if peer.LoginExpirationEnabled && settings.PeerLoginExpirationEnabled { + am.schedulePeerLoginExpiration(ctx, accountID) + } + if peer.InactivityExpirationEnabled && settings.PeerInactivityExpirationEnabled { + am.checkAndSchedulePeerInactivityExpiration(ctx, accountID) + } + return nil +} + // MarkPeerDisconnected marks a peer as disconnected, but only when the // stored session token matches the one passed in. A mismatch means a // newer stream has already taken ownership of the peer — disconnects from @@ -510,12 +512,6 @@ func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peer return status.NewPeerNotPartOfAccountError() } - var peer *nbpeer.Peer - var settings *types.Settings - var eventsToStore []func() - var snap *affectedpeers.Snapshot - var change affectedpeers.Change - serviceID, err := am.serviceManager.GetServiceIDByTargetID(ctx, accountID, peerID) if err != nil { return fmt.Errorf("failed to check if resource is used by service: %w", err) @@ -524,39 +520,8 @@ func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peer return status.NewPeerInUseError(peerID, serviceID) } - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { - peer, err = transaction.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID) - if err != nil { - return err - } - - settings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) - if err != nil { - return err - } - - if err = am.validatePeerDelete(ctx, transaction, accountID, peerID); err != nil { - return err - } - - // 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 - } - - eventsToStore, err = deletePeers(ctx, am, transaction, accountID, userID, []*nbpeer.Peer{peer}, settings) - if err != nil { - return fmt.Errorf("failed to delete peer: %w", err) - } - - if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil { - return fmt.Errorf("failed to increment network serial: %w", err) - } - - return nil - }) + change := affectedpeers.Change{ChangedPeerIDs: []string{peerID}} + settings, eventsToStore, snap, err := am.deletePeerInTransaction(ctx, accountID, userID, peerID, change) if err != nil { return err } @@ -577,6 +542,46 @@ func (am *DefaultAccountManager) DeletePeer(ctx context.Context, accountID, peer return nil } +// deletePeerInTransaction loads the peer + settings, captures the affected-peers +// snapshot (before the delete, while the peer's group memberships still exist), +// then deletes the peer and bumps the network serial — all in one transaction. +func (am *DefaultAccountManager) deletePeerInTransaction(ctx context.Context, accountID, userID, peerID string, change affectedpeers.Change) (*types.Settings, []func(), *affectedpeers.Snapshot, error) { + var settings *types.Settings + var eventsToStore []func() + var snap *affectedpeers.Snapshot + + err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + peer, err := transaction.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID) + if err != nil { + return err + } + + settings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) + if err != nil { + return err + } + + if err = am.validatePeerDelete(ctx, transaction, accountID, peerID); err != nil { + return err + } + + if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil { + return err + } + + if eventsToStore, err = deletePeers(ctx, am, transaction, accountID, userID, []*nbpeer.Peer{peer}, settings); err != nil { + return fmt.Errorf("failed to delete peer: %w", err) + } + + if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil { + return fmt.Errorf("failed to increment network serial: %w", err) + } + + return nil + }) + return settings, eventsToStore, snap, err +} + // GetNetworkMap returns Network map for a given peer (omits original peer from the Peers result) func (am *DefaultAccountManager) GetNetworkMap(ctx context.Context, peerID string) (*types.NetworkMap, error) { return am.networkMapController.GetNetworkMap(ctx, peerID)