From bb08adcbac4c74718a96eb5222c049796a232456 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Sat, 20 Jul 2024 20:36:36 +0300 Subject: [PATCH] Remove condition check for network serial update --- management/server/account.go | 8 ++------ management/server/dns.go | 8 +++----- management/server/group.go | 24 ++++++------------------ management/server/nameserver.go | 23 ++++++----------------- management/server/peer.go | 14 ++++---------- management/server/policy.go | 28 ++++++++++++++++++++++++++-- management/server/posture_checks.go | 11 ++++------- 7 files changed, 51 insertions(+), 65 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 1c457915e..278f08026 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1724,17 +1724,13 @@ func (am *DefaultAccountManager) GetAccountFromToken(ctx context.Context, claims account.UserGroupsAddToPeers(claims.UserId, addNewGroups...) account.UserGroupsRemoveFromPeers(claims.UserId, removeOldGroups...) - updateAccountPeers := areGroupChangesAffectPeers(account, addNewGroups) || areGroupChangesAffectPeers(account, removeOldGroups) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() if err := am.Store.SaveAccount(ctx, account); err != nil { log.WithContext(ctx).Errorf("failed to save account: %v", err) } else { log.WithContext(ctx).Tracef("user %s: JWT group membership changed, updating account peers", claims.UserId) - if updateAccountPeers { + if areGroupChangesAffectPeers(account, addNewGroups) || areGroupChangesAffectPeers(account, removeOldGroups) { am.updateAccountPeers(ctx, account) } unlock() diff --git a/management/server/dns.go b/management/server/dns.go index e68628561..964063099 100644 --- a/management/server/dns.go +++ b/management/server/dns.go @@ -92,11 +92,7 @@ func (am *DefaultAccountManager) SaveDNSSettings(ctx context.Context, accountID addedGroups := difference(dnsSettingsToSave.DisabledManagementGroups, oldSettings.DisabledManagementGroups) removedGroups := difference(oldSettings.DisabledManagementGroups, dnsSettingsToSave.DisabledManagementGroups) - updateAccountPeers := (areGroupChangesAffectPeers(account, addedGroups) && anyGroupHasPeers(account, addedGroups)) || - areGroupChangesAffectPeers(account, removedGroups) && anyGroupHasPeers(account, removedGroups) - if updateAccountPeers { - account.Network.IncSerial() - } + account.Network.IncSerial() if err = am.Store.SaveAccount(ctx, account); err != nil { return err } @@ -113,6 +109,8 @@ func (am *DefaultAccountManager) SaveDNSSettings(ctx context.Context, accountID am.StoreEvent(ctx, userID, accountID, accountID, activity.GroupRemovedFromDisabledManagementGroups, meta) } + updateAccountPeers := (areGroupChangesAffectPeers(account, addedGroups) && anyGroupHasPeers(account, addedGroups)) || + areGroupChangesAffectPeers(account, removedGroups) && anyGroupHasPeers(account, removedGroups) if updateAccountPeers { am.updateAccountPeers(ctx, account) } diff --git a/management/server/group.go b/management/server/group.go index 51cd35a96..15aaed690 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -170,16 +170,12 @@ func (am *DefaultAccountManager) SaveGroups(ctx context.Context, accountID, user newGroupIDs = append(newGroupIDs, newGroup.ID) } - updateAccountPeers := areGroupChangesAffectPeers(account, newGroupIDs) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() if err = am.Store.SaveGroups(account.Id, account.Groups); err != nil { return err } - if updateAccountPeers { + if areGroupChangesAffectPeers(account, newGroupIDs) { am.updateAccountPeers(ctx, account) } @@ -329,16 +325,12 @@ func (am *DefaultAccountManager) GroupAddPeer(ctx context.Context, accountID, gr group.Peers = append(group.Peers, peerID) } - updateAccountPeers := areGroupChangesAffectPeers(account, []string{group.ID}) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() if err = am.Store.SaveAccount(ctx, account); err != nil { return err } - if updateAccountPeers { + if areGroupChangesAffectPeers(account, []string{group.ID}) { am.updateAccountPeers(ctx, account) } @@ -360,11 +352,7 @@ func (am *DefaultAccountManager) GroupDeletePeer(ctx context.Context, accountID, return status.Errorf(status.NotFound, "group with ID %s not found", groupID) } - updateAccountPeers := areGroupChangesAffectPeers(account, []string{group.ID}) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() for i, itemID := range group.Peers { if itemID == peerID { group.Peers = append(group.Peers[:i], group.Peers[i+1:]...) @@ -374,7 +362,7 @@ func (am *DefaultAccountManager) GroupDeletePeer(ctx context.Context, accountID, } } - if updateAccountPeers { + if areGroupChangesAffectPeers(account, []string{group.ID}) { am.updateAccountPeers(ctx, account) } diff --git a/management/server/nameserver.go b/management/server/nameserver.go index 41eb9a40a..4a0314e98 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -79,16 +79,12 @@ func (am *DefaultAccountManager) CreateNameServerGroup(ctx context.Context, acco account.NameServerGroups[newNSGroup.ID] = newNSGroup - updateAccountPeers := anyGroupHasPeers(account, newNSGroup.Groups) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() if err := am.Store.SaveAccount(ctx, account); err != nil { return nil, err } - if updateAccountPeers { + if anyGroupHasPeers(account, newNSGroup.Groups) { am.updateAccountPeers(ctx, account) } am.StoreEvent(ctx, userID, newNSGroup.ID, accountID, activity.NameserverGroupCreated, newNSGroup.EventMeta()) @@ -116,17 +112,14 @@ func (am *DefaultAccountManager) SaveNameServerGroup(ctx context.Context, accoun } oldNSGroup := account.NameServerGroups[nsGroupToSave.ID] - updateAccountPeers := anyGroupHasPeers(account, nsGroupToSave.Groups) || anyGroupHasPeers(account, oldNSGroup.Groups) - if updateAccountPeers { - account.Network.IncSerial() - } account.NameServerGroups[nsGroupToSave.ID] = nsGroupToSave + account.Network.IncSerial() if err = am.Store.SaveAccount(ctx, account); err != nil { return err } - if updateAccountPeers { + if anyGroupHasPeers(account, nsGroupToSave.Groups) || anyGroupHasPeers(account, oldNSGroup.Groups) { am.updateAccountPeers(ctx, account) } am.StoreEvent(ctx, userID, nsGroupToSave.ID, accountID, activity.NameserverGroupUpdated, nsGroupToSave.EventMeta()) @@ -151,16 +144,12 @@ func (am *DefaultAccountManager) DeleteNameServerGroup(ctx context.Context, acco } delete(account.NameServerGroups, nsGroupID) - updateAccountPeers := anyGroupHasPeers(account, nsGroup.Groups) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() if err := am.Store.SaveAccount(ctx, account); err != nil { return err } - if updateAccountPeers { + if anyGroupHasPeers(account, nsGroup.Groups) { am.updateAccountPeers(ctx, account) } am.StoreEvent(ctx, userID, nsGroup.ID, accountID, activity.NameserverGroupDeleted, nsGroup.EventMeta()) diff --git a/management/server/peer.go b/management/server/peer.go index 5bb008a40..681843df4 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -212,16 +212,14 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user } account.UpdatePeer(peer) - expired, _ := peer.LoginExpired(account.Settings.PeerLoginExpiration) - if expired && peer.LoginExpirationEnabled { - account.Network.IncSerial() - } + account.Network.IncSerial() err = am.Store.SaveAccount(ctx, account) if err != nil { return nil, err } + expired, _ := peer.LoginExpired(account.Settings.PeerLoginExpiration) if expired && peer.LoginExpirationEnabled { am.updateAccountPeers(ctx, account) } @@ -501,11 +499,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s account.Peers[newPeer.ID] = newPeer - updateAccountPeers := areGroupChangesAffectPeers(account, groupsToAdd) - if updateAccountPeers { - account.Network.IncSerial() - } - + account.Network.IncSerial() err = am.Store.SaveAccount(ctx, account) if err != nil { return nil, nil, nil, err @@ -523,7 +517,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s am.StoreEvent(ctx, opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta) - if updateAccountPeers { + if areGroupChangesAffectPeers(account, groupsToAdd) { am.updateAccountPeers(ctx, account) } diff --git a/management/server/policy.go b/management/server/policy.go index a70d7f0ed..ca5146a5d 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -191,6 +191,16 @@ func (p *Policy) UpgradeAndFix() { } } +// AreAllRuleGroupsEmpty checks if all rule groups in the policy are effectively empty. +func (p *Policy) AreAllRuleGroupsEmpty() bool { + for _, rule := range p.Rules { + if len(rule.Sources) != 0 && len(rule.Destinations) != 0 { + return false + } + } + return true +} + // FirewallRule is a rule of the firewall. type FirewallRule struct { // PeerIP of the peer @@ -364,7 +374,9 @@ func (am *DefaultAccountManager) SavePolicy(ctx context.Context, accountID, user } am.StoreEvent(ctx, userID, policy.ID, accountID, action, policy.EventMeta()) - am.updateAccountPeers(ctx, account) + if !policy.AreAllRuleGroupsEmpty() { + am.updateAccountPeers(ctx, account) + } return nil } @@ -391,7 +403,9 @@ func (am *DefaultAccountManager) DeletePolicy(ctx context.Context, accountID, po am.StoreEvent(ctx, userID, policy.ID, accountID, activity.PolicyRemoved, policy.EventMeta()) - am.updateAccountPeers(ctx, account) + if !policy.AreAllRuleGroupsEmpty() { + am.updateAccountPeers(ctx, account) + } return nil } @@ -561,3 +575,13 @@ func getPostureChecks(account *Account, postureChecksID string) *posture.Checks } return nil } + +// isPolicyRuleGroupsEmpty checks if a given policy has rules with empty sources and destinations. +func isPolicyRuleGroupsEmpty(policy *Policy) bool { + for _, rule := range policy.Rules { + if len(rule.Sources) != 0 && len(rule.Destinations) != 0 { + return false + } + } + return true +} diff --git a/management/server/posture_checks.go b/management/server/posture_checks.go index 6677a8247..6e3f7ebac 100644 --- a/management/server/posture_checks.go +++ b/management/server/posture_checks.go @@ -70,23 +70,20 @@ func (am *DefaultAccountManager) SavePostureChecks(ctx context.Context, accountI return status.Errorf(status.PreconditionFailed, "Posture check name should be unique") } - updateAccountPeers := false action := activity.PostureCheckCreated if exists { action = activity.PostureCheckUpdated - - updateAccountPeers, _ = isPostureCheckLinkedToPolicy(account, postureChecks.ID) - if updateAccountPeers { - account.Network.IncSerial() - } } + account.Network.IncSerial() if err = am.Store.SaveAccount(ctx, account); err != nil { return err } am.StoreEvent(ctx, userID, postureChecks.ID, accountID, action, postureChecks.EventMeta()) - if updateAccountPeers { + + updateAccountPeers, _ := isPostureCheckLinkedToPolicy(account, postureChecks.ID) + if exists && updateAccountPeers { am.updateAccountPeers(ctx, account) }