[management] Fix duplicate resource routes when routing peer is part of the source group (#3095)

* Remove duplicate resource routes when routing peer is part of the source group

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

* Add tests

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

---------

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
This commit is contained in:
Bethuel Mmbaga 2024-12-20 21:10:53 +03:00 committed by GitHub
parent 82b4e58ad0
commit 7ee7ada273
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 69 additions and 4 deletions

View File

@ -2175,6 +2175,7 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
peerJIp = "100.65.29.65"
peerKIp = "100.65.29.66"
peerMIp = "100.65.29.67"
peerOIp = "100.65.29.68"
)
account := &types.Account{
@ -2256,6 +2257,20 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
IP: net.ParseIP(peerMIp),
Status: &nbpeer.PeerStatus{},
},
"peerN": {
ID: "peerN",
IP: net.ParseIP("100.65.20.18"),
Key: "peerN",
Status: &nbpeer.PeerStatus{},
Meta: nbpeer.PeerSystemMeta{
GoOS: "linux",
},
},
"peerO": {
ID: "peerO",
IP: net.ParseIP(peerOIp),
Status: &nbpeer.PeerStatus{},
},
},
Groups: map[string]*types.Group{
"router1": {
@ -2330,6 +2345,14 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
Name: "Pipeline",
Peers: []string{"peerM"},
},
"metrics": {
ID: "metrics",
Name: "Metrics",
Peers: []string{"peerN", "peerO"},
Resources: []types.Resource{
{ID: "resource6"},
},
},
},
Networks: []*networkTypes.Network{
{
@ -2352,6 +2375,10 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
ID: "network5",
Name: "Pipeline Network",
},
{
ID: "network6",
Name: "Metrics Network",
},
},
NetworkRouters: []*routerTypes.NetworkRouter{
{
@ -2389,6 +2416,13 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
Masquerade: false,
Metric: 9999,
},
{
ID: "router6",
NetworkID: "network6",
Peer: "peerN",
Masquerade: false,
Metric: 9999,
},
},
NetworkResources: []*resourceTypes.NetworkResource{
{
@ -2426,6 +2460,13 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
Type: "host",
Prefix: netip.MustParsePrefix("10.12.12.1/32"),
},
{
ID: "resource6",
NetworkID: "network6",
Name: "Resource 6",
Type: "domain",
Domain: "*.google.com",
},
},
Policies: []*types.Policy{
{
@ -2527,6 +2568,24 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
},
},
},
{
ID: "policyResource6",
Name: "policyResource6",
Enabled: true,
Rules: []*types.PolicyRule{
{
ID: "ruleResource6",
Name: "ruleResource6",
Bidirectional: true,
Enabled: true,
Protocol: types.PolicyRuleProtocolTCP,
Action: types.PolicyTrafficActionAccept,
Ports: []string{"9090"},
Sources: []string{"metrics"},
Destinations: []string{"metrics"},
},
},
},
},
}
@ -2553,6 +2612,10 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
// which is part of the destination in the policies (policyResource3 and policyResource4)
policies = account.GetPoliciesForNetworkResource("resource4")
assert.Len(t, policies, 2, "resource4 should have exactly 2 policy applied via access control groups")
// Test case: Resource6 is applied to the access control groups (metrics),
policies = account.GetPoliciesForNetworkResource("resource6")
assert.Len(t, policies, 1, "resource6 should have exactly 1 policy applied via access control groups")
})
t.Run("validate routing peer firewall rules for network resources", func(t *testing.T) {
@ -2663,5 +2726,9 @@ func TestAccount_GetPeerNetworkResourceFirewallRules(t *testing.T) {
_, routes, sourcePeers = account.GetNetworkResourcesRoutesToSync(context.Background(), "peerM", resourcePoliciesMap, resourceRoutersMap)
assert.Len(t, routes, 1)
assert.Len(t, sourcePeers, 0)
_, routes, sourcePeers = account.GetNetworkResourcesRoutesToSync(context.Background(), "peerN", resourcePoliciesMap, resourceRoutersMap)
assert.Len(t, routes, 1)
assert.Len(t, sourcePeers, 2)
})
}

View File

@ -1330,10 +1330,8 @@ func (a *Account) GetNetworkResourcesRoutesToSync(ctx context.Context, peerID st
// routing peer should be able to connect with all source peers
if addSourcePeers {
allSourcePeers = append(allSourcePeers, group.Peers...)
}
// add routes for the resource if the peer is in the distribution group
if slices.Contains(group.Peers, peerID) {
} else if slices.Contains(group.Peers, peerID) {
// add routes for the resource if the peer is in the distribution group
for peerId, router := range networkRoutingPeers {
routes = append(routes, a.getNetworkResourcesRoutes(resource, peerId, router, resourcePolicies)...)
}