Skip to content

Commit

Permalink
Remove Account.Rules from Store engines (#1528)
Browse files Browse the repository at this point in the history
  • Loading branch information
surik authored Feb 19, 2024
1 parent cb3408a commit db3cba5
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 156 deletions.
40 changes: 21 additions & 19 deletions management/server/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ type Account struct {
UsersG []User `json:"-" gorm:"foreignKey:AccountID;references:id"`
Groups map[string]*Group `gorm:"-"`
GroupsG []Group `json:"-" gorm:"foreignKey:AccountID;references:id"`
Rules map[string]*Rule `gorm:"-"`
RulesG []Rule `json:"-" gorm:"foreignKey:AccountID;references:id"`
Policies []*Policy `gorm:"foreignKey:AccountID;references:id"`
Routes map[string]*route.Route `gorm:"-"`
RoutesG []route.Route `json:"-" gorm:"foreignKey:AccountID;references:id"`
Expand All @@ -219,6 +217,9 @@ type Account struct {
DNSSettings DNSSettings `gorm:"embedded;embeddedPrefix:dns_settings_"`
// Settings is a dictionary of Account settings
Settings *Settings `gorm:"embedded;embeddedPrefix:settings_"`
// deprecated on store and api level
Rules map[string]*Rule `json:"-" gorm:"-"`
RulesG []Rule `json:"-" gorm:"-"`
}

type UserInfo struct {
Expand Down Expand Up @@ -635,11 +636,6 @@ func (a *Account) Copy() *Account {
groups[id] = group.Copy()
}

rules := map[string]*Rule{}
for id, rule := range a.Rules {
rules[id] = rule.Copy()
}

policies := []*Policy{}
for _, policy := range a.Policies {
policies = append(policies, policy.Copy())
Expand Down Expand Up @@ -673,7 +669,6 @@ func (a *Account) Copy() *Account {
Peers: peers,
Users: users,
Groups: groups,
Rules: rules,
Policies: policies,
Routes: routes,
NameServerGroups: nsGroups,
Expand Down Expand Up @@ -1793,21 +1788,28 @@ func addAllGroup(account *Account) error {
}
account.Groups = map[string]*Group{allGroup.ID: allGroup}

defaultRule := &Rule{
ID: xid.New().String(),
id := xid.New().String()

defaultPolicy := &Policy{
ID: id,
Name: DefaultRuleName,
Description: DefaultRuleDescription,
Disabled: false,
Source: []string{allGroup.ID},
Destination: []string{allGroup.ID},
Enabled: true,
Rules: []*PolicyRule{
{
ID: id,
Name: DefaultRuleName,
Description: DefaultRuleDescription,
Enabled: true,
Sources: []string{allGroup.ID},
Destinations: []string{allGroup.ID},
Bidirectional: true,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
},
},
}
account.Rules = map[string]*Rule{defaultRule.ID: defaultRule}

// TODO: after migration we need to drop rule and create policy directly
defaultPolicy, err := RuleToPolicy(defaultRule)
if err != nil {
return fmt.Errorf("convert rule to policy: %w", err)
}
account.Policies = []*Policy{defaultPolicy}
}
return nil
Expand Down
17 changes: 0 additions & 17 deletions management/server/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,6 @@ func verifyNewAccountHasDefaultFields(t *testing.T, account *Account, createdBy
if account.Domain != domain {
t.Errorf("expecting newly created account to have domain %s, got %s", domain, account.Domain)
}

if len(account.Rules) != 1 {
t.Errorf("expecting newly created account to have 1 rule, got %d", len(account.Rules))
}

for _, rule := range account.Rules {
if rule.Name != "Default" {
t.Errorf("expecting newly created account to have Default rule, got %s", rule.Name)
}
}
}

func TestAccount_GetPeerNetworkMap(t *testing.T) {
Expand Down Expand Up @@ -1528,13 +1518,6 @@ func TestAccount_Copy(t *testing.T) {
Peers: []string{"peer1"},
},
},
Rules: map[string]*Rule{
"rule1": {
ID: "rule1",
Destination: []string{},
Source: []string{},
},
},
Policies: []*Policy{
{
ID: "policy1",
Expand Down
19 changes: 0 additions & 19 deletions management/server/file_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,6 @@ func restore(file string) (*FileStore, error) {
if account.Policies == nil {
account.Policies = make([]*Policy, 0)
}
for _, rule := range account.Rules {
policy, err := RuleToPolicy(rule)
if err != nil {
log.Errorf("unable to migrate rule to policy: %v", err)
continue
}
// don't update policies from rules, rules deprecated,
// only append not existed rules as part of the migration process
if _, ok := policies[policy.ID]; !ok {
account.Policies = append(account.Policies, policy)
}
}

// for data migration. Can be removed once most base will be with labels
existingLabels := account.getPeerDNSLabels()
Expand Down Expand Up @@ -342,13 +330,6 @@ func (s *FileStore) SaveAccount(account *Account) error {
s.PrivateDomain2AccountID[accountCopy.Domain] = accountCopy.Id
}

accountCopy.Rules = make(map[string]*Rule)
for _, policy := range accountCopy.Policies {
for _, rule := range policy.Rules {
accountCopy.Rules[rule.ID] = rule.ToRule()
}
}

return s.persist(s.storeFile)
}

Expand Down
51 changes: 8 additions & 43 deletions management/server/file_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,18 @@ func TestStore(t *testing.T) {
Name: "all",
Peers: []string{"testpeer"},
}
account.Rules["all"] = &Rule{
ID: "all",
Name: "all",
Source: []string{"all"},
Destination: []string{"all"},
Flow: TrafficFlowBidirect,
}
account.Policies = append(account.Policies, &Policy{
ID: "all",
Name: "all",
Enabled: true,
Rules: []*PolicyRule{account.Rules["all"].ToPolicyRule()},
Rules: []*PolicyRule{
{
ID: "all",
Name: "all",
Sources: []string{"all"},
Destinations: []string{"all"},
},
},
})
account.Policies = append(account.Policies, &Policy{
ID: "dmz",
Expand Down Expand Up @@ -317,41 +317,6 @@ func TestRestore(t *testing.T) {
require.Len(t, store.TokenID2UserID, 1, "failed to restore a FileStore wrong TokenID2UserID mapping length")
}

// TODO: outdated, delete this
func TestRestorePolicies_Migration(t *testing.T) {
storeDir := t.TempDir()

err := util.CopyFileContents("testdata/store_policy_migrate.json", filepath.Join(storeDir, "store.json"))
if err != nil {
t.Fatal(err)
}

store, err := NewFileStore(storeDir, nil)
if err != nil {
return
}

account := store.Accounts["bf1c8084-ba50-4ce7-9439-34653001fc3b"]
require.Len(t, account.Groups, 1, "failed to restore a FileStore file - missing Account Groups")
require.Len(t, account.Rules, 1, "failed to restore a FileStore file - missing Account Rules")
require.Len(t, account.Policies, 1, "failed to restore a FileStore file - missing Account Policies")

policy := account.Policies[0]
require.Equal(t, policy.Name, "Default", "failed to restore a FileStore file - missing Account Policies Name")
require.Equal(t, policy.Description,
"This is a default rule that allows connections between all the resources",
"failed to restore a FileStore file - missing Account Policies Description")
require.NoError(t, err, "failed to upldate query")
require.Len(t, policy.Rules, 1, "failed to restore a FileStore file - missing Account Policy Rules")
require.Equal(t, policy.Rules[0].Action, PolicyTrafficActionAccept, "failed to restore a FileStore file - missing Account Policies Action")
require.Equal(t, policy.Rules[0].Destinations,
[]string{"cfefqs706sqkneg59g3g"},
"failed to restore a FileStore file - missing Account Policies Destinations")
require.Equal(t, policy.Rules[0].Sources,
[]string{"cfefqs706sqkneg59g3g"},
"failed to restore a FileStore file - missing Account Policies Sources")
}

func TestRestoreGroups_Migration(t *testing.T) {
storeDir := t.TempDir()

Expand Down
123 changes: 77 additions & 46 deletions management/server/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,41 +83,57 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
},
},
},
Rules: map[string]*Rule{
"RuleDefault": {
Policies: []*Policy{
{
ID: "RuleDefault",
Name: "Default",
Description: "This is a default rule that allows connections between all the resources",
Source: []string{
"GroupAll",
},
Destination: []string{
"GroupAll",
Enabled: true,
Rules: []*PolicyRule{
{
ID: "RuleDefault",
Name: "Default",
Description: "This is a default rule that allows connections between all the resources",
Bidirectional: true,
Enabled: true,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
Sources: []string{
"GroupAll",
},
Destinations: []string{
"GroupAll",
},
},
},
},
"RuleSwarm": {
{
ID: "RuleSwarm",
Name: "Swarm",
Description: "",
Source: []string{
"GroupSwarm",
"GroupAll",
},
Destination: []string{
"GroupSwarm",
Description: "No description",
Enabled: true,
Rules: []*PolicyRule{
{
ID: "RuleSwarm",
Name: "Swarm",
Description: "No description",
Bidirectional: true,
Enabled: true,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
Sources: []string{
"GroupSwarm",
"GroupAll",
},
Destinations: []string{
"GroupSwarm",
},
},
},
},
},
}

rule1, err := RuleToPolicy(account.Rules["RuleDefault"])
assert.NoError(t, err)

rule2, err := RuleToPolicy(account.Rules["RuleSwarm"])
assert.NoError(t, err)

account.Policies = append(account.Policies, rule1, rule2)

t.Run("check that all peers get map", func(t *testing.T) {
for _, p := range account.Peers {
peers, firewallRules := account.getPeerConnectionResources(p.ID)
Expand Down Expand Up @@ -307,41 +323,56 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
},
},
},
Rules: map[string]*Rule{
"RuleDefault": {
Policies: []*Policy{
{
ID: "RuleDefault",
Name: "Default",
Disabled: true,
Description: "This is a default rule that allows connections between all the resources",
Source: []string{
"GroupAll",
},
Destination: []string{
"GroupAll",
Enabled: false,
Rules: []*PolicyRule{
{
ID: "RuleDefault",
Name: "Default",
Description: "This is a default rule that allows connections between all the resources",
Bidirectional: true,
Enabled: false,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
Sources: []string{
"GroupAll",
},
Destinations: []string{
"GroupAll",
},
},
},
},
"RuleSwarm": {
{
ID: "RuleSwarm",
Name: "Swarm",
Description: "",
Source: []string{
"GroupSwarm",
},
Destination: []string{
"peerF",
Description: "No description",
Enabled: true,
Rules: []*PolicyRule{
{
ID: "RuleSwarm",
Name: "Swarm",
Description: "No description",
Bidirectional: true,
Enabled: true,
Protocol: PolicyRuleProtocolALL,
Action: PolicyTrafficActionAccept,
Sources: []string{
"GroupSwarm",
},
Destinations: []string{
"peerF",
},
},
},
},
},
}

rule1, err := RuleToPolicy(account.Rules["RuleDefault"])
assert.NoError(t, err)

rule2, err := RuleToPolicy(account.Rules["RuleSwarm"])
assert.NoError(t, err)

account.Policies = append(account.Policies, rule1, rule2)

t.Run("check first peer map", func(t *testing.T) {
peers, firewallRules := account.getPeerConnectionResources("peerB")
assert.Contains(t, peers, account.Peers["peerC"])
Expand Down
Loading

0 comments on commit db3cba5

Please sign in to comment.