From f8388fdf2618e90d02f7d78cb447e20a9afe7662 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Tue, 2 Jul 2024 10:50:19 +0200 Subject: [PATCH 1/3] Add stack trace when saving empty domains added temporary domain check for existing accounts to trace where the issue is originated refactor save account due to complexity score --- management/server/sql_store.go | 87 ++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/management/server/sql_store.go b/management/server/sql_store.go index 2153a011edc..b7f868bc365 100644 --- a/management/server/sql_store.go +++ b/management/server/sql_store.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "runtime/debug" "strings" "sync" "time" @@ -128,38 +129,10 @@ func (s *SqlStore) AcquireAccountReadLock(accountID string) (unlock func()) { func (s *SqlStore) SaveAccount(account *Account) error { start := time.Now() - for _, key := range account.SetupKeys { - account.SetupKeysG = append(account.SetupKeysG, *key) - } - - for id, peer := range account.Peers { - peer.ID = id - account.PeersG = append(account.PeersG, *peer) - } - - for id, user := range account.Users { - user.Id = id - for id, pat := range user.PATs { - pat.ID = id - user.PATsG = append(user.PATsG, *pat) - } - account.UsersG = append(account.UsersG, *user) - } - - for id, group := range account.Groups { - group.ID = id - account.GroupsG = append(account.GroupsG, *group) - } - - for id, route := range account.Routes { - route.ID = id - account.RoutesG = append(account.RoutesG, *route) - } + // todo: remove this check after the issue is resolved + s.checkAccountDomainBeforeSave(account.Id, account.Domain) - for id, ns := range account.NameServerGroups { - ns.ID = id - account.NameServerGroupsG = append(account.NameServerGroupsG, *ns) - } + generateAccountSQLTypes(account) err := s.db.Transaction(func(tx *gorm.DB) error { result := tx.Select(clause.Associations).Delete(account.Policies, "account_id = ?", account.Id) @@ -196,6 +169,58 @@ func (s *SqlStore) SaveAccount(account *Account) error { return err } +// generateAccountSQLTypes generates the GORM compatible types for the account +func generateAccountSQLTypes(account *Account) { + for _, key := range account.SetupKeys { + account.SetupKeysG = append(account.SetupKeysG, *key) + } + + for id, peer := range account.Peers { + peer.ID = id + account.PeersG = append(account.PeersG, *peer) + } + + for id, user := range account.Users { + user.Id = id + for id, pat := range user.PATs { + pat.ID = id + user.PATsG = append(user.PATsG, *pat) + } + account.UsersG = append(account.UsersG, *user) + } + + for id, group := range account.Groups { + group.ID = id + account.GroupsG = append(account.GroupsG, *group) + } + + for id, route := range account.Routes { + route.ID = id + account.RoutesG = append(account.RoutesG, *route) + } + + for id, ns := range account.NameServerGroups { + ns.ID = id + account.NameServerGroupsG = append(account.NameServerGroupsG, *ns) + } +} + +// checkAccountDomainBeforeSave temporary method to troubleshoot an issue with domains getting blank +func (s *SqlStore) checkAccountDomainBeforeSave(accountID, newDomain string) { + var acc Account + var domain string + result := s.db.Model(&acc).Select("domain").Where("id = ?", accountID).First(&domain) + if result.Error != nil { + if !errors.Is(result.Error, gorm.ErrRecordNotFound) { + log.Errorf("error when getting account %s from the store to check domain: %s", accountID, result.Error) + } + return + } + if domain != "" && newDomain == "" { + log.Warnf("saving an account with empty domain when there was a domain set. Previous domain %s, Account ID: %s", domain, accountID, debug.Stack()) + } +} + func (s *SqlStore) DeleteAccount(account *Account) error { start := time.Now() From f0ba419cba95a13847756dcb399cffe16f087b1c Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Tue, 2 Jul 2024 10:56:21 +0200 Subject: [PATCH 2/3] add id query condition const --- management/server/sql_store.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/management/server/sql_store.go b/management/server/sql_store.go index b7f868bc365..1772974d595 100644 --- a/management/server/sql_store.go +++ b/management/server/sql_store.go @@ -31,6 +31,7 @@ import ( const ( storeSqliteFileName = "store.db" + idQueryCondition = "id = ?" ) // SqlStore represents an account storage backed by a Sql DB persisted to disk @@ -209,7 +210,7 @@ func generateAccountSQLTypes(account *Account) { func (s *SqlStore) checkAccountDomainBeforeSave(accountID, newDomain string) { var acc Account var domain string - result := s.db.Model(&acc).Select("domain").Where("id = ?", accountID).First(&domain) + result := s.db.Model(&acc).Select("domain").Where(idQueryCondition, accountID).First(&domain) if result.Error != nil { if !errors.Is(result.Error, gorm.ErrRecordNotFound) { log.Errorf("error when getting account %s from the store to check domain: %s", accountID, result.Error) @@ -262,7 +263,7 @@ func (s *SqlStore) SaveInstallationID(ID string) error { func (s *SqlStore) GetInstallationID() string { var installation installation - if result := s.db.First(&installation, "id = ?", s.installationPK); result.Error != nil { + if result := s.db.First(&installation, idQueryCondition, s.installationPK); result.Error != nil { return "" } @@ -370,7 +371,7 @@ func (s *SqlStore) GetTokenIDByHashedToken(hashedToken string) (string, error) { func (s *SqlStore) GetUserByTokenID(tokenID string) (*User, error) { var token PersonalAccessToken - result := s.db.First(&token, "id = ?", tokenID) + result := s.db.First(&token, idQueryCondition, tokenID) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, status.Errorf(status.NotFound, "account not found: index lookup failed") @@ -384,7 +385,7 @@ func (s *SqlStore) GetUserByTokenID(tokenID string) (*User, error) { } var user User - result = s.db.Preload("PATsG").First(&user, "id = ?", token.UserID) + result = s.db.Preload("PATsG").First(&user, idQueryCondition, token.UserID) if result.Error != nil { return nil, status.Errorf(status.NotFound, "account not found: index lookup failed") } @@ -419,7 +420,7 @@ func (s *SqlStore) GetAccount(accountID string) (*Account, error) { result := s.db.Model(&account). Preload("UsersG.PATsG"). // have to be specifies as this is nester reference Preload(clause.Associations). - First(&account, "id = ?", accountID) + First(&account, idQueryCondition, accountID) if result.Error != nil { log.Errorf("error when getting account %s from the store: %s", accountID, result.Error) if errors.Is(result.Error, gorm.ErrRecordNotFound) { @@ -483,7 +484,7 @@ func (s *SqlStore) GetAccount(accountID string) (*Account, error) { func (s *SqlStore) GetAccountByUser(userID string) (*Account, error) { var user User - result := s.db.Select("account_id").First(&user, "id = ?", userID) + result := s.db.Select("account_id").First(&user, idQueryCondition, userID) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, status.Errorf(status.NotFound, "account not found: index lookup failed") @@ -500,7 +501,7 @@ func (s *SqlStore) GetAccountByUser(userID string) (*Account, error) { func (s *SqlStore) GetAccountByPeerID(peerID string) (*Account, error) { var peer nbpeer.Peer - result := s.db.Select("account_id").First(&peer, "id = ?", peerID) + result := s.db.Select("account_id").First(&peer, idQueryCondition, peerID) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { return nil, status.Errorf(status.NotFound, "account not found: index lookup failed") @@ -553,7 +554,7 @@ func (s *SqlStore) GetAccountIDByPeerPubKey(peerKey string) (string, error) { func (s *SqlStore) GetAccountIDByUserID(userID string) (string, error) { var user User var accountID string - result := s.db.Model(&user).Select("account_id").Where("id = ?", userID).First(&accountID) + result := s.db.Model(&user).Select("account_id").Where(idQueryCondition, userID).First(&accountID) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { return "", status.Errorf(status.NotFound, "account not found: index lookup failed") @@ -595,7 +596,7 @@ func (s *SqlStore) GetPeerByPeerPubKey(peerKey string) (*nbpeer.Peer, error) { func (s *SqlStore) GetAccountSettings(accountID string) (*Settings, error) { var accountSettings AccountSettings - if err := s.db.Model(&Account{}).Where("id = ?", accountID).First(&accountSettings).Error; err != nil { + if err := s.db.Model(&Account{}).Where(idQueryCondition, accountID).First(&accountSettings).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { return nil, status.Errorf(status.NotFound, "settings not found") } From c8049f7afc0ee57648130611559684f4c467a256 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Tue, 2 Jul 2024 11:02:57 +0200 Subject: [PATCH 3/3] fix log positional args --- management/server/sql_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/sql_store.go b/management/server/sql_store.go index 1772974d595..35f09d60c31 100644 --- a/management/server/sql_store.go +++ b/management/server/sql_store.go @@ -218,7 +218,7 @@ func (s *SqlStore) checkAccountDomainBeforeSave(accountID, newDomain string) { return } if domain != "" && newDomain == "" { - log.Warnf("saving an account with empty domain when there was a domain set. Previous domain %s, Account ID: %s", domain, accountID, debug.Stack()) + log.Warnf("saving an account with empty domain when there was a domain set. Previous domain %s, Account ID: %s, Trace: %s", domain, accountID, debug.Stack()) } }