Remove Account.Rules from Store engines (#1528)

management/server/account.go

@@ -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"`
@@ -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 {
@@ -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())
@@ -673,7 +669,6 @@ func (a *Account) Copy() *Account {
 		Peers:                  peers,
 		Users:                  users,
 		Groups:                 groups,
-		Rules:                  rules,
 		Policies:               policies,
 		Routes:                 routes,
 		NameServerGroups:       nsGroups,
@@ -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,
+			Enabled:     true,
-			Source:      []string{allGroup.ID},
+			Rules: []*PolicyRule{
-			Destination: []string{allGroup.ID},
+				{
+					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

management/server/account_test.go

@@ -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) {
@@ -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",

management/server/file_store.go

@@ -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()
@@ -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)
 }
 

management/server/file_store_test.go

@@ -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",
@@ -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()
 

management/server/policy_test.go

@@ -83,41 +83,57 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
 				},
 			},
 		},
-		Rules: map[string]*Rule{
+		Policies: []*Policy{
-			"RuleDefault": {
+			{
 				ID:          "RuleDefault",
 				Name:        "Default",
 				Description: "This is a default rule that allows connections between all the resources",
-				Source: []string{
+				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",
 						},
-				Destination: []string{
+						Destinations: []string{
 							"GroupAll",
 						},
 					},
-			"RuleSwarm": {
+				},
+			},
+			{
 				ID:          "RuleSwarm",
 				Name:        "Swarm",
-				Description: "",
+				Description: "No description",
-				Source: []string{
+				Enabled:     true,
+				Rules: []*PolicyRule{
+					{
+						ID:            "RuleSwarm",
+						Name:          "Swarm",
+						Description:   "No description",
+						Bidirectional: true,
+						Enabled:       true,
+						Protocol:      PolicyRuleProtocolALL,
+						Action:        PolicyTrafficActionAccept,
+						Sources: []string{
 							"GroupSwarm",
 							"GroupAll",
 						},
-				Destination: []string{
+						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)
@@ -307,41 +323,56 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
 				},
 			},
 		},
-		Rules: map[string]*Rule{
+		Policies: []*Policy{
-			"RuleDefault": {
+			{
 				ID:          "RuleDefault",
 				Name:        "Default",
-				Disabled:    true,
 				Description: "This is a default rule that allows connections between all the resources",
-				Source: []string{
+				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",
 						},
-				Destination: []string{
+						Destinations: []string{
 							"GroupAll",
 						},
 					},
-			"RuleSwarm": {
+				},
+			},
+			{
 				ID:          "RuleSwarm",
 				Name:        "Swarm",
-				Description: "",
+				Description: "No description",
-				Source: []string{
+				Enabled:     true,
+				Rules: []*PolicyRule{
+					{
+						ID:            "RuleSwarm",
+						Name:          "Swarm",
+						Description:   "No description",
+						Bidirectional: true,
+						Enabled:       true,
+						Protocol:      PolicyRuleProtocolALL,
+						Action:        PolicyTrafficActionAccept,
+						Sources: []string{
 							"GroupSwarm",
 						},
-				Destination: []string{
+						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"])

management/server/sqlite_store.go

@@ -156,11 +156,6 @@ func (s *SqliteStore) SaveAccount(account *Account) error {
 		account.GroupsG = append(account.GroupsG, *group)
 	}
 
-	for id, rule := range account.Rules {
-		rule.ID = id
-		account.RulesG = append(account.RulesG, *rule)
-	}
-
 	for id, route := range account.Routes {
 		route.ID = id
 		account.RoutesG = append(account.RoutesG, *route)
@@ -356,7 +351,6 @@ func (s *SqliteStore) GetAllAccounts() (all []*Account) {
 
 func (s *SqliteStore) GetAccount(accountID string) (*Account, error) {
 	var account Account
-
 	result := s.db.Model(&account).
 		Preload("UsersG.PATsG"). // have to be specifies as this is nester reference
 		Preload(clause.Associations).
@@ -403,12 +397,6 @@ func (s *SqliteStore) GetAccount(accountID string) (*Account, error) {
 	}
 	account.GroupsG = nil
 
-	account.Rules = make(map[string]*Rule, len(account.RulesG))
-	for _, rule := range account.RulesG {
-		account.Rules[rule.ID] = rule.Copy()
-	}
-	account.RulesG = nil
-
 	account.Routes = make(map[string]*route.Route, len(account.RoutesG))
 	for _, route := range account.RoutesG {
 		account.Routes[route.ID] = route.Copy()