Skip to content

Commit e9016ae

Browse files
bcmmbagabcmmbaga
authored
[management] Add backward compatibility for older clients without firewall rules port range support (#4003)
Adds backward compatibility for clients with versions prior to v0.48.0 that do not support port range firewall rules. - Skips generation of firewall rules with multi-port ranges for older clients - Preserves support for single-port ranges by treating them as individual port rules, ensuring compatibility with older clients
1 parent 23b5d45 commit e9016ae

File tree

6 files changed

+246
-63
lines changed

6 files changed

+246
-63
lines changed

‎management/server/peer.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (am *DefaultAccountManager) getUserAccessiblePeers(ctx context.Context, acc
9292

9393
// fetch all the peers that have access to the user's peers
9494
for _, peer := range peers {
95-
aclPeers, _ := account.GetPeerConnectionResources(ctx, peer.ID, approvedPeersMap)
95+
aclPeers, _ := account.GetPeerConnectionResources(ctx, peer, approvedPeersMap)
9696
for _, p := range aclPeers {
9797
peersMap[p.ID] = p
9898
}
@@ -1149,7 +1149,7 @@ func (am *DefaultAccountManager) checkIfUserOwnsPeer(ctx context.Context, accoun
11491149
}
11501150

11511151
for _, p := range userPeers {
1152-
aclPeers, _ := account.GetPeerConnectionResources(ctx, p.ID, approvedPeersMap)
1152+
aclPeers, _ := account.GetPeerConnectionResources(ctx, p, approvedPeersMap)
11531153
for _, aclPeer := range aclPeers {
11541154
if aclPeer.ID == peer.ID {
11551155
return peer, nil

‎management/server/policy_test.go‎

Lines changed: 106 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
2727
ID: "peerB",
2828
IP: net.ParseIP("100.65.80.39"),
2929
Status: &nbpeer.PeerStatus{},
30+
Meta: nbpeer.PeerSystemMeta{WtVersion: "0.48.0"},
3031
},
3132
"peerC": {
3233
ID: "peerC",
@@ -63,6 +64,12 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
6364
IP: net.ParseIP("100.65.31.2"),
6465
Status: &nbpeer.PeerStatus{},
6566
},
67+
"peerK": {
68+
ID: "peerK",
69+
IP: net.ParseIP("100.32.80.1"),
70+
Status: &nbpeer.PeerStatus{},
71+
Meta: nbpeer.PeerSystemMeta{WtVersion: "0.30.0"},
72+
},
6673
},
6774
Groups: map[string]*types.Group{
6875
"GroupAll": {
@@ -111,6 +118,13 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
111118
"peerI",
112119
},
113120
},
121+
"GroupWorkflow": {
122+
ID: "GroupWorkflow",
123+
Name: "workflow",
124+
Peers: []string{
125+
"peerK",
126+
},
127+
},
114128
},
115129
Policies: []*types.Policy{
116130
{
@@ -189,6 +203,39 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
189203
},
190204
},
191205
},
206+
{
207+
ID: "RuleWorkflow",
208+
Name: "Workflow",
209+
Description: "No description",
210+
Enabled: true,
211+
Rules: []*types.PolicyRule{
212+
{
213+
ID: "RuleWorkflow",
214+
Name: "Workflow",
215+
Description: "No description",
216+
Bidirectional: true,
217+
Enabled: true,
218+
Protocol: types.PolicyRuleProtocolTCP,
219+
Action: types.PolicyTrafficActionAccept,
220+
PortRanges: []types.RulePortRange{
221+
{
222+
Start: 8088,
223+
End: 8088,
224+
},
225+
{
226+
Start: 9090,
227+
End: 9095,
228+
},
229+
},
230+
Sources: []string{
231+
"GroupWorkflow",
232+
},
233+
Destinations: []string{
234+
"GroupDMZ",
235+
},
236+
},
237+
},
238+
},
192239
},
193240
}
194241

@@ -199,14 +246,14 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
199246

200247
t.Run("check that all peers get map", func(t *testing.T) {
201248
for _, p := range account.Peers {
202-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), p.ID, validatedPeers)
203-
assert.GreaterOrEqual(t, len(peers), 2, "minimum number peers should present")
204-
assert.GreaterOrEqual(t, len(firewallRules), 2, "minimum number of firewall rules should present")
249+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), p, validatedPeers)
250+
assert.GreaterOrEqual(t, len(peers), 1, "minimum number peers should present")
251+
assert.GreaterOrEqual(t, len(firewallRules), 1, "minimum number of firewall rules should present")
205252
}
206253
})
207254

208255
t.Run("check first peer map details", func(t *testing.T) {
209-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerB", validatedPeers)
256+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerB"], validatedPeers)
210257
assert.Len(t, peers, 8)
211258
assert.Contains(t, peers, account.Peers["peerA"])
212259
assert.Contains(t, peers, account.Peers["peerC"])
@@ -364,6 +411,32 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
364411
assert.True(t, contains, "rule not found in expected rules %#v", rule)
365412
}
366413
})
414+
415+
t.Run("check port ranges support for older peers", func(t *testing.T) {
416+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerK"], validatedPeers)
417+
assert.Len(t, peers, 1)
418+
assert.Contains(t, peers, account.Peers["peerI"])
419+
420+
expectedFirewallRules := []*types.FirewallRule{
421+
{
422+
PeerIP: "100.65.31.2",
423+
Direction: types.FirewallRuleDirectionIN,
424+
Action: "accept",
425+
Protocol: "tcp",
426+
Port: "8088",
427+
PolicyID: "RuleWorkflow",
428+
},
429+
{
430+
PeerIP: "100.65.31.2",
431+
Direction: types.FirewallRuleDirectionOUT,
432+
Action: "accept",
433+
Protocol: "tcp",
434+
Port: "8088",
435+
PolicyID: "RuleWorkflow",
436+
},
437+
}
438+
assert.ElementsMatch(t, firewallRules, expectedFirewallRules)
439+
})
367440
}
368441

369442
func TestAccount_getPeersByPolicyDirect(t *testing.T) {
@@ -466,10 +539,10 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
466539
}
467540

468541
t.Run("check first peer map", func(t *testing.T) {
469-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerB", approvedPeers)
542+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerB"], approvedPeers)
470543
assert.Contains(t, peers, account.Peers["peerC"])
471544

472-
epectedFirewallRules := []*types.FirewallRule{
545+
expectedFirewallRules := []*types.FirewallRule{
473546
{
474547
PeerIP: "100.65.254.139",
475548
Direction: types.FirewallRuleDirectionIN,
@@ -487,19 +560,19 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
487560
PolicyID: "RuleSwarm",
488561
},
489562
}
490-
assert.Len(t, firewallRules, len(epectedFirewallRules))
491-
slices.SortFunc(epectedFirewallRules, sortFunc())
563+
assert.Len(t, firewallRules, len(expectedFirewallRules))
564+
slices.SortFunc(expectedFirewallRules, sortFunc())
492565
slices.SortFunc(firewallRules, sortFunc())
493566
for i := range firewallRules {
494-
assert.Equal(t, epectedFirewallRules[i], firewallRules[i])
567+
assert.Equal(t, expectedFirewallRules[i], firewallRules[i])
495568
}
496569
})
497570

498571
t.Run("check second peer map", func(t *testing.T) {
499-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerC", approvedPeers)
572+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerC"], approvedPeers)
500573
assert.Contains(t, peers, account.Peers["peerB"])
501574

502-
epectedFirewallRules := []*types.FirewallRule{
575+
expectedFirewallRules := []*types.FirewallRule{
503576
{
504577
PeerIP: "100.65.80.39",
505578
Direction: types.FirewallRuleDirectionIN,
@@ -517,21 +590,21 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
517590
PolicyID: "RuleSwarm",
518591
},
519592
}
520-
assert.Len(t, firewallRules, len(epectedFirewallRules))
521-
slices.SortFunc(epectedFirewallRules, sortFunc())
593+
assert.Len(t, firewallRules, len(expectedFirewallRules))
594+
slices.SortFunc(expectedFirewallRules, sortFunc())
522595
slices.SortFunc(firewallRules, sortFunc())
523596
for i := range firewallRules {
524-
assert.Equal(t, epectedFirewallRules[i], firewallRules[i])
597+
assert.Equal(t, expectedFirewallRules[i], firewallRules[i])
525598
}
526599
})
527600

528601
account.Policies[1].Rules[0].Bidirectional = false
529602

530603
t.Run("check first peer map directional only", func(t *testing.T) {
531-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerB", approvedPeers)
604+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerB"], approvedPeers)
532605
assert.Contains(t, peers, account.Peers["peerC"])
533606

534-
epectedFirewallRules := []*types.FirewallRule{
607+
expectedFirewallRules := []*types.FirewallRule{
535608
{
536609
PeerIP: "100.65.254.139",
537610
Direction: types.FirewallRuleDirectionOUT,
@@ -541,19 +614,19 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
541614
PolicyID: "RuleSwarm",
542615
},
543616
}
544-
assert.Len(t, firewallRules, len(epectedFirewallRules))
545-
slices.SortFunc(epectedFirewallRules, sortFunc())
617+
assert.Len(t, firewallRules, len(expectedFirewallRules))
618+
slices.SortFunc(expectedFirewallRules, sortFunc())
546619
slices.SortFunc(firewallRules, sortFunc())
547620
for i := range firewallRules {
548-
assert.Equal(t, epectedFirewallRules[i], firewallRules[i])
621+
assert.Equal(t, expectedFirewallRules[i], firewallRules[i])
549622
}
550623
})
551624

552625
t.Run("check second peer map directional only", func(t *testing.T) {
553-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerC", approvedPeers)
626+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerC"], approvedPeers)
554627
assert.Contains(t, peers, account.Peers["peerB"])
555628

556-
epectedFirewallRules := []*types.FirewallRule{
629+
expectedFirewallRules := []*types.FirewallRule{
557630
{
558631
PeerIP: "100.65.80.39",
559632
Direction: types.FirewallRuleDirectionIN,
@@ -563,11 +636,11 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
563636
PolicyID: "RuleSwarm",
564637
},
565638
}
566-
assert.Len(t, firewallRules, len(epectedFirewallRules))
567-
slices.SortFunc(epectedFirewallRules, sortFunc())
639+
assert.Len(t, firewallRules, len(expectedFirewallRules))
640+
slices.SortFunc(expectedFirewallRules, sortFunc())
568641
slices.SortFunc(firewallRules, sortFunc())
569642
for i := range firewallRules {
570-
assert.Equal(t, epectedFirewallRules[i], firewallRules[i])
643+
assert.Equal(t, expectedFirewallRules[i], firewallRules[i])
571644
}
572645
})
573646
}
@@ -748,7 +821,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
748821
t.Run("verify peer's network map with default group peer list", func(t *testing.T) {
749822
// peerB doesn't fulfill the NB posture check but is included in the destination group Swarm,
750823
// will establish a connection with all source peers satisfying the NB posture check.
751-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerB", approvedPeers)
824+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerB"], approvedPeers)
752825
assert.Len(t, peers, 4)
753826
assert.Len(t, firewallRules, 4)
754827
assert.Contains(t, peers, account.Peers["peerA"])
@@ -758,7 +831,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
758831

759832
// peerC satisfy the NB posture check, should establish connection to all destination group peer's
760833
// We expect a single permissive firewall rule which all outgoing connections
761-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerC", approvedPeers)
834+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerC"], approvedPeers)
762835
assert.Len(t, peers, len(account.Groups["GroupSwarm"].Peers))
763836
assert.Len(t, firewallRules, 1)
764837
expectedFirewallRules := []*types.FirewallRule{
@@ -775,7 +848,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
775848

776849
// peerE doesn't fulfill the NB posture check and exists in only destination group Swarm,
777850
// all source group peers satisfying the NB posture check should establish connection
778-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerE", approvedPeers)
851+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerE"], approvedPeers)
779852
assert.Len(t, peers, 4)
780853
assert.Len(t, firewallRules, 4)
781854
assert.Contains(t, peers, account.Peers["peerA"])
@@ -785,7 +858,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
785858

786859
// peerI doesn't fulfill the OS version posture check and exists in only destination group Swarm,
787860
// all source group peers satisfying the NB posture check should establish connection
788-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerI", approvedPeers)
861+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerI"], approvedPeers)
789862
assert.Len(t, peers, 4)
790863
assert.Len(t, firewallRules, 4)
791864
assert.Contains(t, peers, account.Peers["peerA"])
@@ -800,19 +873,19 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
800873

801874
// peerB doesn't satisfy the NB posture check, and doesn't exist in destination group peer's
802875
// no connection should be established to any peer of destination group
803-
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), "peerB", approvedPeers)
876+
peers, firewallRules := account.GetPeerConnectionResources(context.Background(), account.Peers["peerB"], approvedPeers)
804877
assert.Len(t, peers, 0)
805878
assert.Len(t, firewallRules, 0)
806879

807880
// peerI doesn't satisfy the OS version posture check, and doesn't exist in destination group peer's
808881
// no connection should be established to any peer of destination group
809-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerI", approvedPeers)
882+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerI"], approvedPeers)
810883
assert.Len(t, peers, 0)
811884
assert.Len(t, firewallRules, 0)
812885

813886
// peerC satisfy the NB posture check, should establish connection to all destination group peer's
814887
// We expect a single permissive firewall rule which all outgoing connections
815-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerC", approvedPeers)
888+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerC"], approvedPeers)
816889
assert.Len(t, peers, len(account.Groups["GroupSwarm"].Peers))
817890
assert.Len(t, firewallRules, len(account.Groups["GroupSwarm"].Peers))
818891

@@ -827,14 +900,14 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
827900

828901
// peerE doesn't fulfill the NB posture check and exists in only destination group Swarm,
829902
// all source group peers satisfying the NB posture check should establish connection
830-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerE", approvedPeers)
903+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerE"], approvedPeers)
831904
assert.Len(t, peers, 3)
832905
assert.Len(t, firewallRules, 3)
833906
assert.Contains(t, peers, account.Peers["peerA"])
834907
assert.Contains(t, peers, account.Peers["peerC"])
835908
assert.Contains(t, peers, account.Peers["peerD"])
836909

837-
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), "peerA", approvedPeers)
910+
peers, firewallRules = account.GetPeerConnectionResources(context.Background(), account.Peers["peerA"], approvedPeers)
838911
assert.Len(t, peers, 5)
839912
// assert peers from Group Swarm
840913
assert.Contains(t, peers, account.Peers["peerD"])

‎management/server/posture/nb_version.go‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,12 @@ func sanitizeVersion(version string) string {
2424
}
2525

2626
func (n *NBVersionCheck) Check(ctx context.Context, peer nbpeer.Peer) (bool, error) {
27-
peerVersion := sanitizeVersion(peer.Meta.WtVersion)
28-
minVersion := sanitizeVersion(n.MinVersion)
29-
30-
peerNBVersion, err := version.NewVersion(peerVersion)
31-
if err != nil {
32-
return false, err
33-
}
34-
35-
constraints, err := version.NewConstraint(">= " + minVersion)
27+
meetsMin, err := MeetsMinVersion(n.MinVersion, peer.Meta.WtVersion)
3628
if err != nil {
3729
return false, err
3830
}
3931

40-
if constraints.Check(peerNBVersion) {
32+
if meetsMin {
4133
return true, nil
4234
}
4335

@@ -60,3 +52,21 @@ func (n *NBVersionCheck) Validate() error {
6052
}
6153
return nil
6254
}
55+
56+
// MeetsMinVersion checks if the peer's version meets or exceeds the minimum required version
57+
func MeetsMinVersion(minVer, peerVer string) (bool, error) {
58+
peerVer = sanitizeVersion(peerVer)
59+
minVer = sanitizeVersion(minVer)
60+
61+
peerNBVer, err := version.NewVersion(peerVer)
62+
if err != nil {
63+
return false, err
64+
}
65+
66+
constraints, err := version.NewConstraint(">= " + minVer)
67+
if err != nil {
68+
return false, err
69+
}
70+
71+
return constraints.Check(peerNBVer), nil
72+
}

0 commit comments

Comments
 (0)