Skip to content

Commit 570f9b0

Browse files
jaer-tsunjaer-tsun
authored
Prevent Namespace Race (#463)
* poll api-server version for a minute before panicking * always add namespace set, when adding nw policy * create the ns set in add pod, if add namespace has not been called yet
1 parent 43746bc commit 570f9b0

File tree

4 files changed

+44
-23
lines changed

4 files changed

+44
-23
lines changed

‎npm/npm.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,21 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
136136
iptMgr := iptm.NewIptablesManager()
137137
iptMgr.UninitNpmChains()
138138

139-
podInformer := informerFactory.Core().V1().Pods()
140-
nsInformer := informerFactory.Core().V1().Namespaces()
141-
npInformer := informerFactory.Networking().V1().NetworkPolicies()
139+
var (
140+
podInformer = informerFactory.Core().V1().Pods()
141+
nsInformer = informerFactory.Core().V1().Namespaces()
142+
npInformer = informerFactory.Networking().V1().NetworkPolicies()
143+
serverVersion *version.Info
144+
err error
145+
)
142146

143-
serverVersion, err := clientset.ServerVersion()
147+
for ticker, start := time.NewTicker(1 * time.Second).C, time.Now(); time.Since(start) < time.Minute * 1; {
148+
<-ticker
149+
serverVersion, err = clientset.ServerVersion()
150+
if err == nil {
151+
break
152+
}
153+
}
144154
if err != nil {
145155
log.Logf("Error: failed to retrieving kubernetes version")
146156
panic(err.Error)

‎npm/pod.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {
3737

3838
// Add the pod to ipset
3939
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
40+
41+
// Add pod namespace if it doesn't exist
42+
if _, exists := npMgr.nsMap[podNs]; !exists {
43+
log.Printf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs))
44+
if err = ipsMgr.CreateSet(podNs); err != nil {
45+
log.Printf("Error creating ipset %s", podNs)
46+
return err
47+
}
48+
}
49+
4050
// Add the pod to its namespace's ipset.
4151
log.Printf("Adding pod %s to ipset %s", podIP, podNs)
4252
if err = ipsMgr.AddToSet(podNs, podIP); err != nil {
@@ -60,13 +70,6 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {
6070
}
6171
}
6272

63-
ns, err := newNs(podNs)
64-
if err != nil {
65-
log.Errorf("Error: failed to create namespace %s", podNs)
66-
return err
67-
}
68-
npMgr.nsMap[podNs] = ns
69-
7073
return nil
7174
}
7275

‎npm/translatePolicy.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,8 @@ func translateIngress(ns string, targetSelector metav1.LabelSelector, rules []ne
155155

156156
labelsWithOps, _, _ := parseSelector(&targetSelector)
157157
ops, labels := GetOperatorsAndLabels(labelsWithOps)
158-
if len(ops) == 1 && len(labels) == 1 {
159-
if ops[0] == "" && labels[0] == "" {
160-
// targetSelector is empty. Select all pods within the namespace
161-
labels[0] = "ns-" + ns
162-
}
163-
}
164158
sets = append(sets, labels...)
159+
sets = append(sets, "ns-" + ns)
165160

166161
targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesDstFlag, false)
167162
targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false)
@@ -643,13 +638,9 @@ func translateEgress(ns string, targetSelector metav1.LabelSelector, rules []net
643638

644639
labelsWithOps, _, _ := parseSelector(&targetSelector)
645640
ops, labels := GetOperatorsAndLabels(labelsWithOps)
646-
if len(ops) == 1 && len(labels) == 1 {
647-
if ops[0] == "" && labels[0] == "" {
648-
// targetSelector is empty. Select all pods within the namespace
649-
labels[0] = "ns-" + ns
650-
}
651-
}
652641
sets = append(sets, labels...)
642+
sets = append(sets, "ns-" + ns)
643+
653644
targetSelectorIptEntrySpec := craftPartialIptEntrySpecFromOpsAndLabels(ns, ops, labels, util.IptablesSrcFlag, false)
654645
targetSelectorComment := craftPartialIptablesCommentFromSelector(ns, &targetSelector, false)
655646
for _, rule := range rules {

‎npm/translatePolicy_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,7 @@ func TestTranslateIngress(t *testing.T) {
607607
expectedSets := []string{
608608
"context:dev",
609609
"testNotIn:frontend",
610+
"ns-testnamespace",
610611
"app:db",
611612
"testIn:frontend",
612613
"region:northpole",
@@ -884,6 +885,7 @@ func TestTranslateEgress(t *testing.T) {
884885
expectedSets := []string{
885886
"context:dev",
886887
"testNotIn:frontend",
888+
"ns-testnamespace",
887889
"app:db",
888890
"testIn:frontend",
889891
"region:northpole",
@@ -1139,6 +1141,7 @@ func TestTranslatePolicy(t *testing.T) {
11391141

11401142
expectedSets = []string{
11411143
"app:backend",
1144+
"ns-testnamespace",
11421145
"app:frontend",
11431146
}
11441147
if !reflect.DeepEqual(sets, expectedSets) {
@@ -1263,6 +1266,7 @@ func TestTranslatePolicy(t *testing.T) {
12631266

12641267
expectedSets = []string{
12651268
"app:frontend",
1269+
"ns-testnamespace",
12661270
}
12671271
if !reflect.DeepEqual(sets, expectedSets) {
12681272
t.Errorf("translatedPolicy failed @ ALLOW-all-TO-app:frontend-FROM-all-namespaces-policy sets comparison")
@@ -1337,6 +1341,7 @@ func TestTranslatePolicy(t *testing.T) {
13371341

13381342
expectedSets = []string{
13391343
"app:frontend",
1344+
"ns-testnamespace",
13401345
}
13411346
if !reflect.DeepEqual(sets, expectedSets) {
13421347
t.Errorf("translatedPolicy failed @ ALLOW-none-TO-app:frontend-policy sets comparison")
@@ -1521,6 +1526,7 @@ func TestTranslatePolicy(t *testing.T) {
15211526
sets, lists, iptEntries = translatePolicy(allowAllNsToFrontendPolicy)
15221527
expectedSets = []string{
15231528
"app:frontend",
1529+
"ns-testnamespace",
15241530
}
15251531
if !reflect.DeepEqual(sets, expectedSets) {
15261532
t.Errorf("translatedPolicy failed @ ALLOW-all-namespaces-TO-app:frontend-policy sets comparison")
@@ -1666,6 +1672,7 @@ func TestTranslatePolicy(t *testing.T) {
16661672

16671673
expectedSets = []string{
16681674
"app:frontend",
1675+
"ns-testnamespace",
16691676
}
16701677
if !reflect.DeepEqual(sets, expectedSets) {
16711678
t.Errorf("translatedPolicy failed @ ALLOW-ns-namespace:dev-AND-!ns-namespace:test0-AND-!ns-namespace:test1-TO-app:frontend-policy sets comparison")
@@ -1827,6 +1834,7 @@ func TestTranslatePolicy(t *testing.T) {
18271834
"k0",
18281835
"k1:v0",
18291836
"k1:v1",
1837+
"ns-testnamespace",
18301838
}
18311839
if !reflect.DeepEqual(sets, expectedSets) {
18321840
t.Errorf("translatedPolicy failed @ AllOW-ALL-TO-k0-AND-k1:v0-AND-k1:v1-AND-app:frontend-policy sets comparison")
@@ -2031,6 +2039,7 @@ func TestTranslatePolicy(t *testing.T) {
20312039

20322040
expectedSets = []string{
20332041
"app:frontend",
2042+
"ns-testnamespace",
20342043
"app:backend",
20352044
}
20362045
if !reflect.DeepEqual(sets, expectedSets) {
@@ -2165,6 +2174,7 @@ func TestTranslatePolicy(t *testing.T) {
21652174

21662175
expectedSets = []string{
21672176
"app:backdoor",
2177+
"ns-dangerous",
21682178
}
21692179
if !reflect.DeepEqual(sets, expectedSets) {
21702180
t.Errorf("translatedPolicy failed @ ALLOW-ALL-TO-app:backdoor-policy sets comparison")
@@ -2251,6 +2261,7 @@ func TestTranslatePolicy(t *testing.T) {
22512261

22522262
expectedSets = []string{
22532263
"app:frontend",
2264+
"ns-testnamespace",
22542265
"app:backend",
22552266
}
22562267
if !reflect.DeepEqual(sets, expectedSets) {
@@ -2381,6 +2392,7 @@ func TestTranslatePolicy(t *testing.T) {
23812392
expectedSets = []string{
23822393
"app:k8s",
23832394
"team:aks",
2395+
"ns-acn",
23842396
"program:cni",
23852397
"team:acn",
23862398
"binary:cns",
@@ -2561,6 +2573,7 @@ func TestTranslatePolicy(t *testing.T) {
25612573

25622574
expectedSets = []string{
25632575
"app:backend",
2576+
"ns-testnamespace",
25642577
}
25652578
if !reflect.DeepEqual(sets, expectedSets) {
25662579
t.Errorf("translatedPolicy failed @ ALLOW-none-FROM-app:backend-policy sets comparison")
@@ -2612,6 +2625,7 @@ func TestTranslatePolicy(t *testing.T) {
26122625

26132626
expectedSets = []string{
26142627
"app:backend",
2628+
"ns-testnamespace",
26152629
}
26162630
if !reflect.DeepEqual(sets, expectedSets) {
26172631
t.Errorf("translatedPolicy failed @ ALLOW-all-FROM-app:backend-policy sets comparison")
@@ -2752,6 +2766,7 @@ func TestTranslatePolicy(t *testing.T) {
27522766

27532767
expectedSets = []string{
27542768
"app:frontend",
2769+
"ns-testnamespace",
27552770
}
27562771
if !reflect.DeepEqual(sets, expectedSets) {
27572772
t.Errorf("translatedPolicy failed @ ALLOW-ALL-FROM-app:frontend-TCP-PORT-53-OR-UDP-PORT-53-policy sets comparison")
@@ -2969,6 +2984,7 @@ func TestTranslatePolicy(t *testing.T) {
29692984

29702985
expectedSets = []string{
29712986
"role:db",
2987+
"ns-default",
29722988
"role:frontend",
29732989
}
29742990
if !reflect.DeepEqual(sets, expectedSets) {
@@ -3303,6 +3319,7 @@ func TestDropPrecedenceOverAllow(t *testing.T) {
33033319
expectedSets = []string{
33043320
"app:test",
33053321
"testIn:pod-A",
3322+
"ns-default",
33063323
"testIn:pod-B",
33073324
"testIn:pod-C",
33083325
}

0 commit comments

Comments
 (0)