Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[management] Refactor account to use store methods #2955

Draft
wants to merge 13 commits into
base: routes-get-account-refactoring
Choose a base branch
from
Draft

[management] Refactor account to use store methods #2955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
70b4628
Refactor account settings updates
bcmmbaga Nov 22, 2024
7a6ca3e
Fix tests
bcmmbaga Nov 22, 2024
2115e2c
Add tests
bcmmbaga Nov 22, 2024
ea51ce8
Remove group all checks for accounts during startup
bcmmbaga Nov 22, 2024
e17d812
Remove unused store methods
bcmmbaga Nov 25, 2024
71af7ed
Refactor new account handling
bcmmbaga Nov 25, 2024
accada3
Remove db lock on aggregate db calls
bcmmbaga Nov 25, 2024
de99624
Fix tests
bcmmbaga Nov 26, 2024
13eae9b
Remove unused function
bcmmbaga Nov 27, 2024
3186876
Add group All and default policy to client tests
bcmmbaga Nov 27, 2024
41b4e31
Fix tests
bcmmbaga Nov 27, 2024
875b8d6
Clean up sqlite policy rules after deletion
bcmmbaga Nov 27, 2024
1e24916
Add missing store tests
bcmmbaga Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/testdata/store.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ INSERT INTO accounts VALUES('bf1c8084-ba50-4ce7-9439-34653001fc3b','','2024-10-0
INSERT INTO setup_keys VALUES('','bf1c8084-ba50-4ce7-9439-34653001fc3b','A2C8E62B-38F5-4553-B31E-DD66C696CEBB','Default key','reusable','2021-08-19 20:46:20.005936822+02:00','2321-09-18 20:46:20.005936822+02:00','2021-08-19 20:46:20.005936822+02:00',0,0,'0001-01-01 00:00:00+00:00','[]',0,0);
INSERT INTO users VALUES('edafee4e-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','admin',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 21:28:24.830506+02:00','api',0,'');
INSERT INTO users VALUES('f4f6d672-63fb-11ec-90d6-0242ac120003','bf1c8084-ba50-4ce7-9439-34653001fc3b','user',0,0,'','[]',0,'0001-01-01 00:00:00+00:00','2024-10-02 21:28:24.830506+02:00','api',0,'');
INSERT INTO "groups" VALUES('cs1tnh0hhcjnqoiuebeg','bf1c8084-ba50-4ce7-9439-34653001fc3b','All','api','["cfvprsrlo1hqoo49ohog", "cg3161rlo1hs9cq94gdg", "cg05lnblo1hkg2j514p0"]',0,'');
INSERT INTO policies VALUES('cs1tnh0hhcjnqoiuebf0','bf1c8084-ba50-4ce7-9439-34653001fc3b','Default','This is a default rule that allows connections between all the resources',1,'[]');
INSERT INTO policy_rules VALUES('cs387mkv2d4bgq41b6n0','cs1tnh0hhcjnqoiuebf0','Default','This is a default rule that allows connections between all the resources',1,'accept','["cs1tnh0hhcjnqoiuebeg"]','["cs1tnh0hhcjnqoiuebeg"]',1,'all',NULL,NULL);
INSERT INTO installations VALUES(1,'');

COMMIT;
338 changes: 164 additions & 174 deletions management/server/account.go
View file
Open in desktop

Large diffs are not rendered by default.

287 changes: 143 additions & 144 deletions management/server/account_test.go
View file
Open in desktop

Large diffs are not rendered by default.

119 changes: 58 additions & 61 deletions management/server/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ func TestGetDNSSettings(t *testing.T) {
t.Error("failed to create account manager")
}

account, err := initTestDNSAccount(t, am)
accountID, err := initTestDNSAccount(t, am)
if err != nil {
t.Fatal("failed to init testing account")
}

dnsSettings, err := am.GetDNSSettings(context.Background(), account.Id, dnsAdminUserID)
dnsSettings, err := am.GetDNSSettings(context.Background(), accountID, dnsAdminUserID)
if err != nil {
t.Fatalf("Got an error when trying to retrieve the DNS settings with an admin user, err: %s", err)
}
Expand All @@ -53,16 +53,12 @@ func TestGetDNSSettings(t *testing.T) {
t.Fatal("DNS settings for new accounts shouldn't return nil")
}

account.DNSSettings = DNSSettings{
err = am.Store.SaveDNSSettings(context.Background(), LockingStrengthUpdate, accountID, &DNSSettings{
DisabledManagementGroups: []string{group1ID},
}

err = am.Store.SaveAccount(context.Background(), account)
if err != nil {
t.Error("failed to save testing account with new DNS settings")
}
})
require.NoError(t, err, "failed to update DNS settings")

dnsSettings, err = am.GetDNSSettings(context.Background(), account.Id, dnsAdminUserID)
dnsSettings, err = am.GetDNSSettings(context.Background(), accountID, dnsAdminUserID)
if err != nil {
t.Errorf("Got an error when trying to retrieve the DNS settings with an admin user, err: %s", err)
}
Expand All @@ -71,7 +67,7 @@ func TestGetDNSSettings(t *testing.T) {
t.Errorf("DNS settings should have one disabled mgmt group, groups: %s", dnsSettings.DisabledManagementGroups)
}

_, err = am.GetDNSSettings(context.Background(), account.Id, dnsRegularUserID)
_, err = am.GetDNSSettings(context.Background(), accountID, dnsRegularUserID)
if err == nil {
t.Errorf("An error should be returned when getting the DNS settings with a regular user")
}
Expand Down Expand Up @@ -126,20 +122,20 @@ func TestSaveDNSSettings(t *testing.T) {
t.Error("failed to create account manager")
}

account, err := initTestDNSAccount(t, am)
accountID, err := initTestDNSAccount(t, am)
if err != nil {
t.Error("failed to init testing account")
}

err = am.SaveDNSSettings(context.Background(), account.Id, testCase.userID, testCase.inputSettings)
err = am.SaveDNSSettings(context.Background(), accountID, testCase.userID, testCase.inputSettings)
if err != nil {
if testCase.shouldFail {
return
}
t.Error(err)
}

updatedAccount, err := am.Store.GetAccount(context.Background(), account.Id)
updatedAccount, err := am.Store.GetAccount(context.Background(), accountID)
if err != nil {
t.Errorf("should be able to retrieve updated account, got err: %s", err)
}
Expand All @@ -158,17 +154,17 @@ func TestGetNetworkMap_DNSConfigSync(t *testing.T) {
t.Error("failed to create account manager")
}

account, err := initTestDNSAccount(t, am)
accountID, err := initTestDNSAccount(t, am)
if err != nil {
t.Error("failed to init testing account")
}

peer1, err := account.FindPeerByPubKey(dnsPeer1Key)
peer1, err := am.Store.GetPeerByPeerPubKey(context.Background(), LockingStrengthShare, dnsPeer1Key)
if err != nil {
t.Error("failed to init testing account")
}

peer2, err := account.FindPeerByPubKey(dnsPeer2Key)
peer2, err := am.Store.GetPeerByPeerPubKey(context.Background(), LockingStrengthShare, dnsPeer2Key)
if err != nil {
t.Error("failed to init testing account")
}
Expand All @@ -179,11 +175,13 @@ func TestGetNetworkMap_DNSConfigSync(t *testing.T) {
require.True(t, newAccountDNSConfig.DNSConfig.ServiceEnable, "default DNS config should have local DNS service enabled")
require.Len(t, newAccountDNSConfig.DNSConfig.NameServerGroups, 0, "updated DNS config should have no nameserver groups since peer 1 is NS for the only existing NS group")

dnsSettings := account.DNSSettings.Copy()
accountDNSSettings, err := am.Store.GetAccountDNSSettings(context.Background(), LockingStrengthShare, accountID)
require.NoError(t, err, "failed to get account DNS settings")

dnsSettings := accountDNSSettings.Copy()
dnsSettings.DisabledManagementGroups = append(dnsSettings.DisabledManagementGroups, dnsGroup1ID)
account.DNSSettings = dnsSettings
err = am.Store.SaveAccount(context.Background(), account)
require.NoError(t, err)
err = am.Store.SaveDNSSettings(context.Background(), LockingStrengthUpdate, accountID, &dnsSettings)
require.NoError(t, err, "failed to update DNS settings")

updatedAccountDNSConfig, err := am.GetNetworkMap(context.Background(), peer1.ID)
require.NoError(t, err)
Expand Down Expand Up @@ -222,7 +220,7 @@ func createDNSStore(t *testing.T) (Store, error) {
return store, nil
}

func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, error) {
func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (string, error) {
t.Helper()
peer1 := &nbpeer.Peer{
Key: dnsPeer1Key,
Expand Down Expand Up @@ -257,64 +255,65 @@ func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, erro

domain := "example.com"

account := newAccountWithId(context.Background(), dnsAccountID, dnsAdminUserID, domain)

account.Users[dnsRegularUserID] = &User{
Id: dnsRegularUserID,
Role: UserRoleUser,
err := newAccountWithId(context.Background(), am.Store, dnsAccountID, dnsAdminUserID, domain)
if err != nil {
return "", err
}

err := am.Store.SaveAccount(context.Background(), account)
err = am.Store.SaveUser(context.Background(), LockingStrengthUpdate, &User{
Id: dnsRegularUserID,
AccountID: dnsAccountID,
Role: UserRoleUser,
})
if err != nil {
return nil, err
return "", err
}

savedPeer1, _, _, err := am.AddPeer(context.Background(), "", dnsAdminUserID, peer1)
if err != nil {
return nil, err
return "", err
}
_, _, _, err = am.AddPeer(context.Background(), "", dnsAdminUserID, peer2)
if err != nil {
return nil, err
return "", err
}

account, err = am.Store.GetAccount(context.Background(), account.Id)
peer1, err = am.Store.GetPeerByPeerPubKey(context.Background(), LockingStrengthShare, peer1.Key)
if err != nil {
return nil, err
return "", err
}

peer1, err = account.FindPeerByPubKey(peer1.Key)
_, err = am.Store.GetPeerByPeerPubKey(context.Background(), LockingStrengthShare, peer2.Key)
if err != nil {
return nil, err
return "", err
}

_, err = account.FindPeerByPubKey(peer2.Key)
err = am.Store.SaveGroups(context.Background(), LockingStrengthUpdate, []*group.Group{
{
ID: dnsGroup1ID,
AccountID: dnsAccountID,
Peers: []string{peer1.ID},
Name: dnsGroup1ID,
},
{
ID: dnsGroup2ID,
AccountID: dnsAccountID,
Name: dnsGroup2ID,
},
})
if err != nil {
return nil, err
}

newGroup1 := &group.Group{
ID: dnsGroup1ID,
Peers: []string{peer1.ID},
Name: dnsGroup1ID,
return "", err
}

newGroup2 := &group.Group{
ID: dnsGroup2ID,
Name: dnsGroup2ID,
}

account.Groups[newGroup1.ID] = newGroup1
account.Groups[newGroup2.ID] = newGroup2

allGroup, err := account.GetGroupAll()
allGroup, err := am.Store.GetGroupByName(context.Background(), LockingStrengthShare, dnsAccountID, "All")
if err != nil {
return nil, err
return "", err
}

account.NameServerGroups[dnsNSGroup1] = &dns.NameServerGroup{
ID: dnsNSGroup1,
Name: "ns-group-1",
err = am.Store.SaveNameServerGroup(context.Background(), LockingStrengthUpdate, &dns.NameServerGroup{
ID: dnsNSGroup1,
AccountID: dnsAccountID,
Name: "ns-group-1",
NameServers: []dns.NameServer{{
IP: netip.MustParseAddr(savedPeer1.IP.String()),
NSType: dns.UDPNameServerType,
Expand All @@ -323,14 +322,12 @@ func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, erro
Primary: true,
Enabled: true,
Groups: []string{allGroup.ID},
}

err = am.Store.SaveAccount(context.Background(), account)
})
if err != nil {
return nil, err
return "", err
}

return am.Store.GetAccount(context.Background(), account.Id)
return dnsAccountID, nil
}

func generateTestData(size int) nbdns.Config {
Expand Down
Loading
Loading