Skip to content

Commit 6a3846a

Browse files
pascal-fischerpascal-fischer
authored
[management] Remove save account calls (#4349)
1 parent 7cd5dca commit 6a3846a

File tree

8 files changed

+92
-34
lines changed

8 files changed

+92
-34
lines changed

‎management/server/account.go‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,20 +1952,19 @@ func (am *DefaultAccountManager) GetOrCreateAccountByPrivateDomain(ctx context.C
19521952
return nil, false, status.Errorf(status.Internal, "failed to get or create new account by private domain")
19531953
}
19541954

1955-
func (am *DefaultAccountManager) UpdateToPrimaryAccount(ctx context.Context, accountId string) (*types.Account, error) {
1956-
var account *types.Account
1955+
func (am *DefaultAccountManager) UpdateToPrimaryAccount(ctx context.Context, accountId string) error {
19571956
err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
19581957
var err error
1959-
account, err = transaction.GetAccount(ctx, accountId)
1958+
ok, domain, err := transaction.IsPrimaryAccount(ctx, accountId)
19601959
if err != nil {
19611960
return err
19621961
}
19631962

1964-
if account.IsDomainPrimaryAccount {
1963+
if ok {
19651964
return nil
19661965
}
19671966

1968-
existingPrimaryAccountID, err := transaction.GetAccountIDByPrivateDomain(ctx, store.LockingStrengthNone, account.Domain)
1967+
existingPrimaryAccountID, err := transaction.GetAccountIDByPrivateDomain(ctx, store.LockingStrengthNone, domain)
19691968

19701969
// error is not a not found error
19711970
if handleNotFound(err) != nil {
@@ -1981,9 +1980,7 @@ func (am *DefaultAccountManager) UpdateToPrimaryAccount(ctx context.Context, acc
19811980
return status.Errorf(status.Internal, "cannot update account to primary")
19821981
}
19831982

1984-
account.IsDomainPrimaryAccount = true
1985-
1986-
if err := transaction.SaveAccount(ctx, account); err != nil {
1983+
if err := transaction.MarkAccountPrimary(ctx, accountId); err != nil {
19871984
log.WithContext(ctx).WithFields(log.Fields{
19881985
"accountId": accountId,
19891986
}).Errorf("failed to update account to primary: %v", err)
@@ -1993,10 +1990,10 @@ func (am *DefaultAccountManager) UpdateToPrimaryAccount(ctx context.Context, acc
19931990
return nil
19941991
})
19951992
if err != nil {
1996-
return nil, err
1993+
return err
19971994
}
19981995

1999-
return account, nil
1996+
return nil
20001997
}
20011998

20021999
// propagateUserGroupMemberships propagates all account users' group memberships to their peers.
@@ -2067,14 +2064,12 @@ func (am *DefaultAccountManager) reallocateAccountPeerIPs(ctx context.Context, t
20672064
Mask: net.CIDRMask(newNetworkRange.Bits(), newNetworkRange.Addr().BitLen()),
20682065
}
20692066

2070-
account, err := transaction.GetAccount(ctx, accountID)
2067+
err := transaction.UpdateAccountNetwork(ctx, accountID, newIPNet)
20712068
if err != nil {
20722069
return err
20732070
}
20742071

2075-
account.Network.Net = newIPNet
2076-
2077-
peers, err := transaction.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, "", "")
2072+
peers, err := transaction.GetAccountPeers(ctx, store.LockingStrengthUpdate, accountID, "", "")
20782073
if err != nil {
20792074
return err
20802075
}
@@ -2094,10 +2089,6 @@ func (am *DefaultAccountManager) reallocateAccountPeerIPs(ctx context.Context, t
20942089
takenIPs = append(takenIPs, newIP)
20952090
}
20962091

2097-
if err = transaction.SaveAccount(ctx, account); err != nil {
2098-
return err
2099-
}
2100-
21012092
for _, peer := range peers {
21022093
if err = transaction.SavePeer(ctx, accountID, peer); err != nil {
21032094
return status.Errorf(status.Internal, "save updated peer %s: %v", peer.ID, err)

‎management/server/account/manager.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"time"
88

99
nbdns "github.com/netbirdio/netbird/dns"
10-
"github.com/netbirdio/netbird/shared/management/domain"
1110
"github.com/netbirdio/netbird/management/server/activity"
1211
nbcache "github.com/netbirdio/netbird/management/server/cache"
1312
nbcontext "github.com/netbirdio/netbird/management/server/context"
@@ -18,6 +17,7 @@ import (
1817
"github.com/netbirdio/netbird/management/server/types"
1918
"github.com/netbirdio/netbird/management/server/users"
2019
"github.com/netbirdio/netbird/route"
20+
"github.com/netbirdio/netbird/shared/management/domain"
2121
)
2222

2323
type ExternalCacheManager nbcache.UserDataCache
@@ -120,7 +120,7 @@ type Manager interface {
120120
SyncUserJWTGroups(ctx context.Context, userAuth nbcontext.UserAuth) error
121121
GetStore() store.Store
122122
GetOrCreateAccountByPrivateDomain(ctx context.Context, initiatorId, domain string) (*types.Account, bool, error)
123-
UpdateToPrimaryAccount(ctx context.Context, accountId string) (*types.Account, error)
123+
UpdateToPrimaryAccount(ctx context.Context, accountId string) error
124124
GetOwnerInfo(ctx context.Context, accountId string) (*types.UserInfo, error)
125125
GetCurrentUserInfo(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error)
126126
}

‎management/server/account_test.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3250,11 +3250,13 @@ func Test_GetCreateAccountByPrivateDomain(t *testing.T) {
32503250
assert.Equal(t, 0, len(account2.Users))
32513251
assert.Equal(t, 0, len(account2.SetupKeys))
32523252

3253-
account, err = manager.UpdateToPrimaryAccount(ctx, account.Id)
3253+
err = manager.UpdateToPrimaryAccount(ctx, account.Id)
3254+
assert.NoError(t, err)
3255+
account, err = manager.Store.GetAccount(ctx, account.Id)
32543256
assert.NoError(t, err)
32553257
assert.True(t, account.IsDomainPrimaryAccount)
32563258

3257-
_, err = manager.UpdateToPrimaryAccount(ctx, account2.Id)
3259+
err = manager.UpdateToPrimaryAccount(ctx, account2.Id)
32583260
assert.Error(t, err, "should not be able to update a second account to primary")
32593261
}
32603262

@@ -3275,7 +3277,9 @@ func Test_UpdateToPrimaryAccount(t *testing.T) {
32753277
assert.False(t, account.IsDomainPrimaryAccount)
32763278
assert.Equal(t, domain, account.Domain)
32773279

3278-
account, err = manager.UpdateToPrimaryAccount(ctx, account.Id)
3280+
err = manager.UpdateToPrimaryAccount(ctx, account.Id)
3281+
assert.NoError(t, err)
3282+
account, err = manager.Store.GetAccount(ctx, account.Id)
32793283
assert.NoError(t, err)
32803284
assert.True(t, account.IsDomainPrimaryAccount)
32813285

‎management/server/integrated_validator.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,23 @@ func (am *DefaultAccountManager) UpdateIntegratedValidator(ctx context.Context,
5050
defer unlock()
5151

5252
return am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
53-
a, err := transaction.GetAccount(ctx, accountID)
53+
settings, err := transaction.GetAccountSettings(ctx, store.LockingStrengthUpdate, accountID)
5454
if err != nil {
5555
return err
5656
}
5757

5858
var extra *types.ExtraSettings
5959

60-
if a.Settings.Extra != nil {
61-
extra = a.Settings.Extra
60+
if settings.Extra != nil {
61+
extra = settings.Extra
6262
} else {
6363
extra = &types.ExtraSettings{}
64-
a.Settings.Extra = extra
64+
settings.Extra = extra
6565
}
6666

6767
extra.IntegratedValidator = validator
6868
extra.IntegratedValidatorGroups = groups
69-
return transaction.SaveAccount(ctx, a)
69+
return transaction.SaveAccountSettings(ctx, accountID, settings)
7070
})
7171
}
7272

‎management/server/mock_server/account_mock.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"google.golang.org/grpc/status"
1111

1212
nbdns "github.com/netbirdio/netbird/dns"
13-
"github.com/netbirdio/netbird/shared/management/domain"
1413
"github.com/netbirdio/netbird/management/server/account"
1514
"github.com/netbirdio/netbird/management/server/activity"
1615
nbcontext "github.com/netbirdio/netbird/management/server/context"
@@ -21,6 +20,7 @@ import (
2120
"github.com/netbirdio/netbird/management/server/types"
2221
"github.com/netbirdio/netbird/management/server/users"
2322
"github.com/netbirdio/netbird/route"
23+
"github.com/netbirdio/netbird/shared/management/domain"
2424
)
2525

2626
var _ account.Manager = (*MockAccountManager)(nil)
@@ -114,7 +114,7 @@ type MockAccountManager struct {
114114
DeleteSetupKeyFunc func(ctx context.Context, accountID, userID, keyID string) error
115115
BuildUserInfosForAccountFunc func(ctx context.Context, accountID, initiatorUserID string, accountUsers []*types.User) (map[string]*types.UserInfo, error)
116116
GetStoreFunc func() store.Store
117-
UpdateToPrimaryAccountFunc func(ctx context.Context, accountId string) (*types.Account, error)
117+
UpdateToPrimaryAccountFunc func(ctx context.Context, accountId string) error
118118
GetOwnerInfoFunc func(ctx context.Context, accountID string) (*types.UserInfo, error)
119119
GetCurrentUserInfoFunc func(ctx context.Context, userAuth nbcontext.UserAuth) (*users.UserInfoWithPermissions, error)
120120
GetAccountMetaFunc func(ctx context.Context, accountID, userID string) (*types.AccountMeta, error)
@@ -933,11 +933,11 @@ func (am *MockAccountManager) GetOrCreateAccountByPrivateDomain(ctx context.Cont
933933
return nil, false, status.Errorf(codes.Unimplemented, "method GetOrCreateAccountByPrivateDomainFunc is not implemented")
934934
}
935935

936-
func (am *MockAccountManager) UpdateToPrimaryAccount(ctx context.Context, accountId string) (*types.Account, error) {
936+
func (am *MockAccountManager) UpdateToPrimaryAccount(ctx context.Context, accountId string) error {
937937
if am.UpdateToPrimaryAccountFunc != nil {
938938
return am.UpdateToPrimaryAccountFunc(ctx, accountId)
939939
}
940-
return nil, status.Errorf(codes.Unimplemented, "method UpdateToPrimaryAccount is not implemented")
940+
return status.Errorf(codes.Unimplemented, "method UpdateToPrimaryAccount is not implemented")
941941
}
942942

943943
func (am *MockAccountManager) GetOwnerInfo(ctx context.Context, accountId string) (*types.UserInfo, error) {

‎management/server/store/sql_store.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2832,3 +2832,57 @@ func getDebuggingCtx(grpcCtx context.Context) (context.Context, context.CancelFu
28322832
}()
28332833
return ctx, cancel
28342834
}
2835+
2836+
func (s *SqlStore) IsPrimaryAccount(ctx context.Context, accountID string) (bool, string, error) {
2837+
var info types.PrimaryAccountInfo
2838+
result := s.db.Model(&types.Account{}).
2839+
Select("is_domain_primary_account, domain").
2840+
Where(idQueryCondition, accountID).
2841+
Take(&info)
2842+
2843+
if result.Error != nil {
2844+
return false, "", status.Errorf(status.Internal, "failed to get account info: %v", result.Error)
2845+
}
2846+
2847+
return info.IsDomainPrimaryAccount, info.Domain, nil
2848+
}
2849+
2850+
func (s *SqlStore) MarkAccountPrimary(ctx context.Context, accountID string) error {
2851+
result := s.db.Model(&types.Account{}).
2852+
Where(idQueryCondition, accountID).
2853+
Update("is_domain_primary_account", true)
2854+
if result.Error != nil {
2855+
log.WithContext(ctx).Errorf("failed to mark account as primary: %s", result.Error)
2856+
return status.Errorf(status.Internal, "failed to mark account as primary")
2857+
}
2858+
2859+
if result.RowsAffected == 0 {
2860+
return status.NewAccountNotFoundError(accountID)
2861+
}
2862+
2863+
return nil
2864+
}
2865+
2866+
type accountNetworkPatch struct {
2867+
Network *types.Network `gorm:"embedded;embeddedPrefix:network_"`
2868+
}
2869+
2870+
func (s *SqlStore) UpdateAccountNetwork(ctx context.Context, accountID string, ipNet net.IPNet) error {
2871+
patch := accountNetworkPatch{
2872+
Network: &types.Network{Net: ipNet},
2873+
}
2874+
2875+
result := s.db.WithContext(ctx).
2876+
Model(&types.Account{}).
2877+
Where(idQueryCondition, accountID).
2878+
Updates(&patch)
2879+
2880+
if result.Error != nil {
2881+
log.WithContext(ctx).Errorf("failed to update account network: %v", result.Error)
2882+
return status.Errorf(status.Internal, "failed to update account network")
2883+
}
2884+
if result.RowsAffected == 0 {
2885+
return status.NewAccountNotFoundError(accountID)
2886+
}
2887+
return nil
2888+
}

‎management/server/store/store.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ type Store interface {
202202
GetPeerByIP(ctx context.Context, lockStrength LockingStrength, accountID string, ip net.IP) (*nbpeer.Peer, error)
203203
GetPeerIdByLabel(ctx context.Context, lockStrength LockingStrength, accountID string, hostname string) (string, error)
204204
GetAccountGroupPeers(ctx context.Context, lockStrength LockingStrength, accountID string) (map[string]map[string]struct{}, error)
205+
IsPrimaryAccount(ctx context.Context, accountID string) (bool, string, error)
206+
MarkAccountPrimary(ctx context.Context, accountID string) error
207+
UpdateAccountNetwork(ctx context.Context, accountID string, ipNet net.IPNet) error
205208
}
206209

207210
const (

‎management/server/types/account.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ import (
1616
log "github.com/sirupsen/logrus"
1717

1818
nbdns "github.com/netbirdio/netbird/dns"
19-
"github.com/netbirdio/netbird/shared/management/domain"
2019
resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types"
2120
routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
2221
networkTypes "github.com/netbirdio/netbird/management/server/networks/types"
2322
nbpeer "github.com/netbirdio/netbird/management/server/peer"
2423
"github.com/netbirdio/netbird/management/server/posture"
25-
"github.com/netbirdio/netbird/shared/management/status"
2624
"github.com/netbirdio/netbird/management/server/telemetry"
2725
"github.com/netbirdio/netbird/management/server/util"
2826
"github.com/netbirdio/netbird/route"
27+
"github.com/netbirdio/netbird/shared/management/domain"
28+
"github.com/netbirdio/netbird/shared/management/status"
2929
)
3030

3131
const (
@@ -89,6 +89,12 @@ type Account struct {
8989
Onboarding AccountOnboarding `gorm:"foreignKey:AccountID;references:id;constraint:OnDelete:CASCADE"`
9090
}
9191

92+
// this class is used by gorm only
93+
type PrimaryAccountInfo struct {
94+
IsDomainPrimaryAccount bool
95+
Domain string
96+
}
97+
9298
// Subclass used in gorm to only load network and not whole account
9399
type AccountNetwork struct {
94100
Network *Network `gorm:"embedded;embeddedPrefix:network_"`

0 commit comments

Comments
 (0)