diff --git a/management/server/affected_peers_coverage_test.go b/management/server/affected_peers_coverage_test.go index 998676c5e..cf9b83617 100644 --- a/management/server/affected_peers_coverage_test.go +++ b/management/server/affected_peers_coverage_test.go @@ -107,7 +107,7 @@ func TestAffectedPeers_DependencyCoverageMatrix(t *testing.T) { affected := resolveAffected(t, s.manager.Store, s.accountID, change) assert.ElementsMatch(t, affected, mustContain, "expected peer to be affected") - assert.NotElementsMatch(t, affected, mustExclude, "peer must not be affected") + assert.NotContains(t, affected, mustExclude, "peer must not be affected") }) } } diff --git a/management/server/affectedpeers/resolver.go b/management/server/affectedpeers/resolver.go index 5ab8a96f7..42b99c461 100644 --- a/management/server/affectedpeers/resolver.go +++ b/management/server/affectedpeers/resolver.go @@ -11,6 +11,8 @@ package affectedpeers import ( "context" + "maps" + "slices" log "github.com/sirupsen/logrus" @@ -774,26 +776,29 @@ func getGroupsAndPeersFromPolicyViaGroups(policy *types.Policy, groupSet map[str // i.e. if a peer is present in the policy rule sourceResources, return destination group IDs and the destinationResource from the rule // and vice-versa func getGroupsAndPeersFromPolicyViaPeers(policy *types.Policy, changedSet map[string]struct{}) ([]string, []string) { - var groupIds, peerIds []string + peerIds := make(map[string]struct{}) + var groupIds []string if len(changedSet) == 0 { - return peerIds, groupIds + return []string{}, groupIds } for _, rule := range policy.Rules { if isDirectPeerInSet(rule.SourceResource, changedSet) { groupIds = append(groupIds, rule.Destinations...) - peerIds = append(peerIds, rule.SourceResource.ID) + peerIds[rule.SourceResource.ID] = struct{}{} if rule.DestinationResource.Type == types.ResourceTypePeer && rule.DestinationResource.ID != "" { - peerIds = append(peerIds, rule.DestinationResource.ID) + peerIds[rule.DestinationResource.ID] = struct{}{} } - } else if isDirectPeerInSet(rule.DestinationResource, changedSet) { + } + // it's possible that the changeSet contains peer ids of both source and destination resources + if isDirectPeerInSet(rule.DestinationResource, changedSet) { groupIds = append(groupIds, rule.Sources...) - peerIds = append(peerIds, rule.DestinationResource.ID) + peerIds[rule.DestinationResource.ID] = struct{}{} if rule.SourceResource.Type == types.ResourceTypePeer && rule.SourceResource.ID != "" { - peerIds = append(peerIds, rule.SourceResource.ID) + peerIds[rule.SourceResource.ID] = struct{}{} } } } - return peerIds, groupIds + return slices.Collect(maps.Keys(peerIds)), groupIds } func policyReferencesPostureChecks(policy *types.Policy, ids map[string]struct{}) bool { diff --git a/management/server/affectedpeers/resolver_test.go b/management/server/affectedpeers/resolver_test.go index 62f426549..08a73c264 100644 --- a/management/server/affectedpeers/resolver_test.go +++ b/management/server/affectedpeers/resolver_test.go @@ -210,43 +210,55 @@ func TestPolicyReferencesDirectPeers(t *testing.T) { Sources: []string{"sg6"}, Destinations: []string{"dg6"}, }, + { + SourceResource: types.Resource{Type: types.ResourceTypePeer, ID: "p7"}, + DestinationResource: types.Resource{Type: types.ResourceTypePeer, ID: "r7"}, + Sources: []string{"sg7"}, + Destinations: []string{"dg7"}, + }, }} var tests = []struct { name string - inGroups map[string]struct{} + changedPeerIds map[string]struct{} expectedPeerIds []string expectedGroupIds []string }{ { name: "match sources", - inGroups: map[string]struct{}{"p1": {}, "p2": {}}, + changedPeerIds: map[string]struct{}{"p1": {}, "p2": {}}, expectedPeerIds: []string{"p1", "p2", "r1", "r2"}, expectedGroupIds: []string{"dg1", "dg2"}, }, { name: "match destinations", - inGroups: map[string]struct{}{"r1": {}, "r2": {}}, + changedPeerIds: map[string]struct{}{"r1": {}, "r2": {}}, expectedPeerIds: []string{"r1", "r2", "p1", "p2"}, expectedGroupIds: []string{"sg1", "sg2"}, }, { name: "wrong opposing peer types, only changed peer ids and groups on the opposing end of the rule", - inGroups: map[string]struct{}{"p3": {}, "r4": {}}, + changedPeerIds: map[string]struct{}{"p3": {}, "r4": {}}, expectedPeerIds: []string{"p3", "r4"}, expectedGroupIds: []string{"dg3", "sg4"}, }, { name: "wrong peer type, no matching peer ids", - inGroups: map[string]struct{}{"p5": {}, "r6": {}}, + changedPeerIds: map[string]struct{}{"p5": {}, "r6": {}}, expectedPeerIds: []string{}, expectedGroupIds: []string{}, }, + { + name: "changed peers on both sides of the policy", + changedPeerIds: map[string]struct{}{"p7": {}, "r7": {}}, + expectedPeerIds: []string{"p7", "r7"}, + expectedGroupIds: []string{"sg7", "dg7"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - peerIds, groupIds := getGroupsAndPeersFromPolicyViaPeers(policy, tt.inGroups) + peerIds, groupIds := getGroupsAndPeersFromPolicyViaPeers(policy, tt.changedPeerIds) assert.ElementsMatch(t, peerIds, tt.expectedPeerIds) assert.ElementsMatch(t, groupIds, tt.expectedGroupIds) })