Skip to content

Commit a9c2430

Browse files
vakalapavakalapa
authored
[NPM] [bug] Adding sort in iptable comment for deterministic behavior in Flake UT (#924)
* Adding sort in comment for deterministic behavior * Fixing some other UTs' comments * addressing some comments
1 parent 45f3668 commit a9c2430

File tree

2 files changed

+25
-22
lines changed

2 files changed

+25
-22
lines changed

‎npm/translatePolicy.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
package npm
44

55
import (
6+
"sort"
67
"strconv"
8+
"strings"
79

810
"github.com/Azure/azure-container-networking/log"
911
"github.com/Azure/azure-container-networking/npm/iptm"
@@ -191,7 +193,7 @@ func craftPartialIptablesCommentFromSelector(ns string, selector *metav1.LabelSe
191193
ops = append(ops, op)
192194
}
193195

194-
var comment, prefix, postfix string
196+
var prefix, postfix string
195197
if isNamespaceSelector {
196198
prefix = "ns-"
197199
} else {
@@ -200,12 +202,13 @@ func craftPartialIptablesCommentFromSelector(ns string, selector *metav1.LabelSe
200202
}
201203
}
202204

205+
comments := []string{}
203206
for i := range labelsWithoutOps {
204-
comment += prefix + ops[i] + labelsWithoutOps[i]
205-
comment += "-AND-"
207+
comments = append(comments, prefix+ops[i]+labelsWithoutOps[i])
206208
}
207209

208-
return comment[:len(comment)-len("-AND-")] + postfix
210+
sort.Strings(comments)
211+
return strings.Join(comments, "-AND-") + postfix
209212
}
210213

211214
func appendSelectorLabelsToLists(lists, listLabelsWithMembers map[string][]string, isNamespaceSelector bool) {

‎npm/translatePolicy_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ func TestCraftPartialIptablesCommentFromSelector(t *testing.T) {
343343
},
344344
}
345345
comment = craftPartialIptablesCommentFromSelector("testnamespace", selector, false)
346-
expectedComment = "k0:v0-AND-!k2-AND-k1:v10:v11-IN-ns-testnamespace"
346+
expectedComment = "!k2-AND-k0:v0-AND-k1:v10:v11-IN-ns-testnamespace"
347347
if comment != expectedComment {
348348
t.Errorf("TestCraftPartialIptablesCommentFromSelector failed @ normal selector comparison")
349349
t.Errorf("comment:\n%v", comment)
@@ -371,7 +371,7 @@ func TestCraftPartialIptablesCommentFromSelector(t *testing.T) {
371371
},
372372
}
373373
comment = craftPartialIptablesCommentFromSelector("", nsSelector, true)
374-
expectedComment = "ns-k0:v0-AND-ns-!k2-AND-ns-k1:v10:v11"
374+
expectedComment = "ns-!k2-AND-ns-k0:v0-AND-ns-k1:v10:v11"
375375
if comment != expectedComment {
376376
t.Errorf("TestCraftPartialIptablesCommentFromSelector failed @ namespace selector comparison")
377377
t.Errorf("comment:\n%v", comment)
@@ -425,7 +425,7 @@ func TestGetDefaultDropEntries(t *testing.T) {
425425
util.IptablesModuleFlag,
426426
util.IptablesCommentModuleFlag,
427427
util.IptablesCommentFlag,
428-
"DROP-ALL-TO-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
428+
"DROP-ALL-TO-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
429429
},
430430
},
431431
}
@@ -465,7 +465,7 @@ func TestGetDefaultDropEntries(t *testing.T) {
465465
util.IptablesModuleFlag,
466466
util.IptablesCommentModuleFlag,
467467
util.IptablesCommentFlag,
468-
"DROP-ALL-FROM-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
468+
"DROP-ALL-FROM-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
469469
},
470470
},
471471
}
@@ -505,7 +505,7 @@ func TestGetDefaultDropEntries(t *testing.T) {
505505
util.IptablesModuleFlag,
506506
util.IptablesCommentModuleFlag,
507507
util.IptablesCommentFlag,
508-
"DROP-ALL-TO-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
508+
"DROP-ALL-TO-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
509509
},
510510
},
511511
{
@@ -532,7 +532,7 @@ func TestGetDefaultDropEntries(t *testing.T) {
532532
util.IptablesModuleFlag,
533533
util.IptablesCommentModuleFlag,
534534
util.IptablesCommentFlag,
535-
"DROP-ALL-FROM-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
535+
"DROP-ALL-FROM-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
536536
},
537537
},
538538
}
@@ -717,7 +717,7 @@ func TestTranslateIngress(t *testing.T) {
717717
util.IptablesModuleFlag,
718718
util.IptablesCommentModuleFlag,
719719
util.IptablesCommentFlag,
720-
"ALLOW-app:db-AND-testIn:frontend-IN-ns-testnamespace-AND-TCP-PORT-6783-TO-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
720+
"ALLOW-app:db-AND-testIn:frontend-IN-ns-testnamespace-AND-TCP-PORT-6783-TO-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
721721
},
722722
},
723723
{
@@ -760,7 +760,7 @@ func TestTranslateIngress(t *testing.T) {
760760
util.IptablesModuleFlag,
761761
util.IptablesCommentModuleFlag,
762762
util.IptablesCommentFlag,
763-
"ALLOW-ns-ns:dev-AND-ns-testIn:frontendns-AND-TCP-PORT-6783-TO-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
763+
"ALLOW-ns-ns:dev-AND-ns-testIn:frontendns-AND-TCP-PORT-6783-TO-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
764764
},
765765
},
766766
{
@@ -814,7 +814,7 @@ func TestTranslateIngress(t *testing.T) {
814814
util.IptablesModuleFlag,
815815
util.IptablesCommentModuleFlag,
816816
util.IptablesCommentFlag,
817-
"ALLOW-ns-planet:earth-AND-ns-keyExists-AND-region:northpole-AND-!k-AND-TCP-PORT-6783-TO-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
817+
"ALLOW-ns-keyExists-AND-ns-planet:earth-AND-!k-AND-region:northpole-AND-TCP-PORT-6783-TO-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
818818
},
819819
},
820820
}
@@ -1000,7 +1000,7 @@ func TestTranslateEgress(t *testing.T) {
10001000
util.IptablesModuleFlag,
10011001
util.IptablesCommentModuleFlag,
10021002
util.IptablesCommentFlag,
1003-
"ALLOW-app:db-AND-testIn:frontend-IN-ns-testnamespace-AND-TCP-PORT-6783-FROM-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
1003+
"ALLOW-app:db-AND-testIn:frontend-IN-ns-testnamespace-AND-TCP-PORT-6783-FROM-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
10041004
},
10051005
},
10061006
{
@@ -1043,7 +1043,7 @@ func TestTranslateEgress(t *testing.T) {
10431043
util.IptablesModuleFlag,
10441044
util.IptablesCommentModuleFlag,
10451045
util.IptablesCommentFlag,
1046-
"ALLOW-ns-ns:dev-AND-ns-testIn:frontendns-AND-TCP-PORT-6783-FROM-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace",
1046+
"ALLOW-ns-ns:dev-AND-ns-testIn:frontendns-AND-TCP-PORT-6783-FROM-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace",
10471047
},
10481048
},
10491049
{
@@ -1097,7 +1097,7 @@ func TestTranslateEgress(t *testing.T) {
10971097
util.IptablesModuleFlag,
10981098
util.IptablesCommentModuleFlag,
10991099
util.IptablesCommentFlag,
1100-
"ALLOW-context:dev-AND-!testNotIn:frontend-IN-ns-testnamespace-TO-ns-planet:earth-AND-ns-keyExists-AND-region:northpole-AND-!k-AND-TCP-PORT-6783",
1100+
"ALLOW-!testNotIn:frontend-AND-context:dev-IN-ns-testnamespace-TO-ns-keyExists-AND-ns-planet:earth-AND-!k-AND-region:northpole-AND-TCP-PORT-6783",
11011101
},
11021102
},
11031103
}
@@ -1588,7 +1588,7 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) {
15881588
util.IptablesModuleFlag,
15891589
util.IptablesCommentModuleFlag,
15901590
util.IptablesCommentFlag,
1591-
"ALLOW-ns-namespace:dev-AND-ns-!namespace:test0-TO-app:frontend-IN-ns-testnamespace",
1591+
"ALLOW-ns-!namespace:test0-AND-ns-namespace:dev-TO-app:frontend-IN-ns-testnamespace",
15921592
},
15931593
},
15941594
{
@@ -1622,7 +1622,7 @@ func TestAllowNamespaceDevToAppFrontend(t *testing.T) {
16221622
util.IptablesModuleFlag,
16231623
util.IptablesCommentModuleFlag,
16241624
util.IptablesCommentFlag,
1625-
"ALLOW-ns-namespace:dev-AND-ns-!namespace:test1-TO-app:frontend-IN-ns-testnamespace",
1625+
"ALLOW-ns-!namespace:test1-AND-ns-namespace:dev-TO-app:frontend-IN-ns-testnamespace",
16261626
},
16271627
},
16281628
{
@@ -1731,7 +1731,7 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) {
17311731
util.IptablesModuleFlag,
17321732
util.IptablesCommentModuleFlag,
17331733
util.IptablesCommentFlag,
1734-
"ALLOW-all-namespaces-TO-app:frontend-AND-!k0-AND-k1:v0:v1-IN-ns-testnamespace",
1734+
"ALLOW-all-namespaces-TO-!k0-AND-app:frontend-AND-k1:v0:v1-IN-ns-testnamespace",
17351735
},
17361736
},
17371737
{
@@ -1763,7 +1763,7 @@ func TestAllowAllToK0AndK1AndAppFrontend(t *testing.T) {
17631763
util.IptablesModuleFlag,
17641764
util.IptablesCommentModuleFlag,
17651765
util.IptablesCommentFlag,
1766-
"DROP-ALL-TO-app:frontend-AND-!k0-AND-k1:v0:v1-IN-ns-testnamespace",
1766+
"DROP-ALL-TO-!k0-AND-app:frontend-AND-k1:v0:v1-IN-ns-testnamespace",
17671767
},
17681768
},
17691769
}
@@ -2473,7 +2473,7 @@ func TestAllowMultiplePodSelectors(t *testing.T) {
24732473
util.IptablesModuleFlag,
24742474
util.IptablesCommentModuleFlag,
24752475
util.IptablesCommentFlag,
2476-
"ALLOW-ns-!ns:netpol-4537-x-AND-pod:b:c-AND-app:test:int-TO-pod:a:x-IN-ns-netpol-4537-x",
2476+
"ALLOW-ns-!ns:netpol-4537-x-AND-app:test:int-AND-pod:b:c-TO-pod:a:x-IN-ns-netpol-4537-x",
24772477
},
24782478
},
24792479
{
@@ -2512,7 +2512,7 @@ func TestAllowMultiplePodSelectors(t *testing.T) {
25122512
util.IptablesModuleFlag,
25132513
util.IptablesCommentModuleFlag,
25142514
util.IptablesCommentFlag,
2515-
"ALLOW-ns-!ns:netpol-4537-y-AND-pod:b:c-AND-app:test:int-TO-pod:a:x-IN-ns-netpol-4537-x",
2515+
"ALLOW-ns-!ns:netpol-4537-y-AND-app:test:int-AND-pod:b:c-TO-pod:a:x-IN-ns-netpol-4537-x",
25162516
},
25172517
},
25182518
}

0 commit comments

Comments
 (0)