Skip to content

Commit

Permalink
Add stack trace when saving empty domains (#2228)
Browse files Browse the repository at this point in the history 
added temporary domain check for existing accounts to trace where the issue originated

Refactor save account due to complexity score
  • Loading branch information
@mlsmaycon
mlsmaycon authored Jul 2, 2024
1 parent 4517da8 commit eab6183
Showing 1 changed file with 65 additions and 39 deletions.
104 changes: 65 additions & 39 deletions management/server/sql_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"runtime"
"runtime/debug"
"strings"
"sync"
"time"
Expand All @@ -30,6 +31,7 @@ import (

const (
storeSqliteFileName = "store.db"
idQueryCondition = "id = ?"
)

// SqlStore represents an account storage backed by a Sql DB persisted to disk
Expand Down Expand Up @@ -128,38 +130,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)
Expand Down Expand Up @@ -196,6 +170,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(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)
}
return
}
if domain != "" && newDomain == "" {
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())
}
}

func (s *SqlStore) DeleteAccount(account *Account) error {
start := time.Now()

Expand Down Expand Up @@ -237,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 ""
}

Expand Down Expand Up @@ -345,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")
Expand All @@ -359,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")
}
Expand Down Expand Up @@ -394,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) {
Expand Down Expand Up @@ -458,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")
Expand All @@ -475,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")
Expand Down Expand Up @@ -528,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")
Expand Down Expand Up @@ -570,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")
}
Expand Down

0 comments on commit eab6183

Please sign in to comment.