Skip to content

Commit 3799358

Browse files
vakalapavakalapa
authored
[NPM] Clearing NPM MARKs on packets before accepting them (#823)
* First pass at creating a new chain to accept and clear marks npm added * Adding rules to defualt chains
1 parent dc7ff48 commit 3799358

File tree

6 files changed

+76
-6
lines changed

6 files changed

+76
-6
lines changed

‎npm/iptm/helper.go‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
func getAllChainsAndRules() [][]string {
1111
funcList := []func() [][]string{
1212
getAzureNPMChainRules,
13+
getAzureNPMAcceptChainRules,
1314
getAzureNPMIngressChainRules,
1415
getAzureNPMIngressPortChainRules,
1516
getAzureNPMIngressFromChainRules,
@@ -44,7 +45,7 @@ func getAzureNPMChainRules() [][]string {
4445
{
4546
util.IptablesAzureChain,
4647
util.IptablesJumpFlag,
47-
util.IptablesAccept,
48+
util.IptablesAzureAcceptChain,
4849
util.IptablesModuleFlag,
4950
util.IptablesMarkVerb,
5051
util.IptablesMarkFlag,
@@ -57,7 +58,7 @@ func getAzureNPMChainRules() [][]string {
5758
{
5859
util.IptablesAzureChain,
5960
util.IptablesJumpFlag,
60-
util.IptablesAccept,
61+
util.IptablesAzureAcceptChain,
6162
util.IptablesModuleFlag,
6263
util.IptablesMarkVerb,
6364
util.IptablesMarkFlag,
@@ -70,7 +71,7 @@ func getAzureNPMChainRules() [][]string {
7071
{
7172
util.IptablesAzureChain,
7273
util.IptablesJumpFlag,
73-
util.IptablesAccept,
74+
util.IptablesAzureAcceptChain,
7475
util.IptablesModuleFlag,
7576
util.IptablesMarkVerb,
7677
util.IptablesMarkFlag,
@@ -96,6 +97,32 @@ func getAzureNPMChainRules() [][]string {
9697
}
9798
}
9899

100+
// getAzureNPMAcceptChainRules clears all marks and accepts packets
101+
func getAzureNPMAcceptChainRules() [][]string {
102+
return [][]string{
103+
{
104+
util.IptablesAzureAcceptChain,
105+
util.IptablesJumpFlag,
106+
util.IptablesMark,
107+
util.IptablesSetMarkFlag,
108+
util.IptablesAzureClearMarkHex,
109+
util.IptablesModuleFlag,
110+
util.IptablesCommentModuleFlag,
111+
util.IptablesCommentFlag,
112+
fmt.Sprintf("Clear-AZURE-NPM-MARKS"),
113+
},
114+
{
115+
util.IptablesAzureAcceptChain,
116+
util.IptablesJumpFlag,
117+
util.IptablesAccept,
118+
util.IptablesModuleFlag,
119+
util.IptablesCommentModuleFlag,
120+
util.IptablesCommentFlag,
121+
fmt.Sprintf("ACCEPT-All-packets"),
122+
},
123+
}
124+
}
125+
99126
// getAzureNPMIngressChainRules returns rules for AZURE-NPM-INGRESS-PORT
100127
func getAzureNPMIngressChainRules() [][]string {
101128
return [][]string{

‎npm/iptm/iptm.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ var (
2828
// IptablesAzureChainList contains list of all NPM chains
2929
IptablesAzureChainList = []string{
3030
util.IptablesAzureChain,
31+
util.IptablesAzureAcceptChain,
3132
util.IptablesAzureIngressChain,
3233
util.IptablesAzureEgressChain,
3334
util.IptablesAzureIngressPortChain,
@@ -182,8 +183,15 @@ func (iptMgr *IptablesManager) UninitNpmChains() error {
182183
return err
183184
}
184185

186+
// For backward compatibility, we should be cleaning older chains
187+
allAzureChains := append(
188+
IptablesAzureChainList,
189+
util.IptablesAzureTargetSetsChain,
190+
util.IptablesAzureIngressWrongDropsChain,
191+
)
192+
185193
iptMgr.OperationFlag = util.IptablesFlushFlag
186-
for _, chain := range IptablesAzureChainList {
194+
for _, chain := range allAzureChains {
187195
entry := &IptEntry{
188196
Chain: chain,
189197
}
@@ -193,7 +201,7 @@ func (iptMgr *IptablesManager) UninitNpmChains() error {
193201
}
194202
}
195203

196-
for _, chain := range IptablesAzureChainList {
204+
for _, chain := range allAzureChains {
197205
if err := iptMgr.DeleteChain(chain); err != nil {
198206
return err
199207
}

‎npm/namespace.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
131131
ns, err := newNs(nsName)
132132
if err != nil {
133133
metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to create namespace %s with err: %v", nsName, err)
134+
return err
134135
}
135136
setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion())
136137

‎npm/nwpolicy.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
103103
ns, err = newNs(npNs)
104104
if err != nil {
105105
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating namespace %s with err: %v", npNs, err)
106+
return err
106107
}
107108
npMgr.NsMap[npNs] = ns
108109
}
@@ -145,6 +146,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
145146
addedPolicy, err = addPolicy(oldPolicy, npObj)
146147
if err != nil {
147148
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: adding policy %s to %s with err: %v", npName, oldPolicy.ObjectMeta.Name, err)
149+
return err
148150
}
149151
}
150152

@@ -161,28 +163,33 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
161163
log.Logf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
162164
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetNetHashFlag})); err != nil {
163165
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset %s with err: %v", set, err)
166+
return err
164167
}
165168
}
166169
for _, set := range namedPorts {
167170
log.Logf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
168171
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetIPPortHashFlag})); err != nil {
169172
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset named port %s with err: %v", set, err)
173+
return err
170174
}
171175
}
172176
for _, list := range lists {
173177
if err = ipsMgr.CreateList(list); err != nil {
174178
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset list %s with err: %v", list, err)
179+
return err
175180
}
176181
}
177182
if err = npMgr.InitAllNsList(); err != nil {
178183
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: initializing all-namespace ipset list with err: %v", err)
184+
return err
179185
}
180186
createCidrsRule("in", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, ingressIPCidrs, ipsMgr)
181187
createCidrsRule("out", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, egressIPCidrs, ipsMgr)
182188
iptMgr := allNs.iptMgr
183189
for _, iptEntry := range iptEntries {
184190
if err = iptMgr.Add(iptEntry); err != nil {
185191
metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to apply iptables rule. Rule: %+v with err: %v", iptEntry, err)
192+
return err
186193
}
187194
}
188195

@@ -227,6 +234,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
227234
for _, iptEntry := range iptEntries {
228235
if err = iptMgr.Delete(iptEntry); err != nil {
229236
metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to apply iptables rule. Rule: %+v with err: %v", iptEntry, err)
237+
return err
230238
}
231239
}
232240

@@ -239,6 +247,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
239247
deductedPolicy, err := deductPolicy(oldPolicy, npObj)
240248
if err != nil {
241249
metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: deducting policy %s from %s with err: %v", npName, oldPolicy.ObjectMeta.Name, err)
250+
return err
242251
}
243252

244253
if deductedPolicy == nil {

‎npm/pod.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,36 +215,42 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error {
215215
npMgr.NsMap[podNs], err = newNs(podNs)
216216
if err != nil {
217217
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to create namespace %s with err: %v", podNs, err)
218+
return err
218219
}
219220
log.Logf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs))
220221
if err = ipsMgr.CreateSet(podNs, append([]string{util.IpsetNetHashFlag})); err != nil {
221222
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: creating ipset %s with err: %v", podNs, err)
223+
return err
222224
}
223225
}
224226

225227
// Add the pod to its namespace's ipset.
226228
log.Logf("Adding pod %s to ipset %s", podIP, podNs)
227229
if err = ipsMgr.AddToSet(podNs, podIP, util.IpsetNetHashFlag, podUID); err != nil {
228230
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to namespace ipset with err: %v", err)
231+
return err
229232
}
230233

231234
// Add the pod to its label's ipset.
232235
for podLabelKey, podLabelVal := range podLabels {
233236
log.Logf("Adding pod %s to ipset %s", podIP, podLabelKey)
234237
if err = ipsMgr.AddToSet(podLabelKey, podIP, util.IpsetNetHashFlag, podUID); err != nil {
235238
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to label ipset with err: %v", err)
239+
return err
236240
}
237241

238242
label := podLabelKey + ":" + podLabelVal
239243
log.Logf("Adding pod %s to ipset %s", podIP, label)
240244
if err = ipsMgr.AddToSet(label, podIP, util.IpsetNetHashFlag, podUID); err != nil {
241245
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to label ipset with err: %v", err)
246+
return err
242247
}
243248
}
244249

245250
// Add pod's named ports from its ipset.
246251
if err = appendNamedPortIpsets(ipsMgr, podContainerPorts, podUID, podIP, false); err != nil {
247252
metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to namespace ipset with err: %v", err)
253+
return err
248254
}
249255

250256
// add the Pod info to the podMap
@@ -291,17 +297,20 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
291297
npMgr.NsMap[newPodObjNs], err = newNs(newPodObjNs)
292298
if err != nil {
293299
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to create namespace %s with err: %v", newPodObjNs, err)
300+
return err
294301
}
295302
log.Logf("Creating set: %v, hashedSet: %v", newPodObjNs, util.GetHashedName(newPodObjNs))
296303
if err = ipsMgr.CreateSet(newPodObjNs, append([]string{util.IpsetNetHashFlag})); err != nil {
297304
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error creating ipset %s with err: %v", newPodObjNs, err)
305+
return err
298306
}
299307
}
300308

301309
cachedPodObj, exists := npMgr.PodMap[podKey]
302310
if !exists {
303311
if addErr := npMgr.AddPod(newPodObj); addErr != nil {
304312
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod during update with error %+v", addErr)
313+
return addErr
305314
}
306315
return nil
307316
}
@@ -328,6 +337,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
328337
if newPodObj.Status.Phase == v1.PodSucceeded || newPodObj.Status.Phase == v1.PodFailed {
329338
if delErr := npMgr.DeletePod(newPodObj); delErr != nil {
330339
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod during update with error %+v", delErr)
340+
return delErr
331341
}
332342

333343
return nil
@@ -369,10 +379,12 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
369379
)
370380
if err = ipsMgr.DeleteFromSet(cachedPodObj.Namespace, cachedPodIP, cachedPodObj.PodUID); err != nil {
371381
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from namespace ipset with err: %v", err)
382+
return err
372383
}
373384
// Add the pod to its namespace's ipset.
374385
if err = ipsMgr.AddToSet(newPodObjNs, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.PodUID); err != nil {
375386
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to namespace ipset with err: %v", err)
387+
return err
376388
}
377389
} else {
378390
//if no change in labels then return
@@ -392,6 +404,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
392404
log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podIPSetName)
393405
if err = ipsMgr.DeleteFromSet(podIPSetName, cachedPodIP, cachedPodObj.PodUID); err != nil {
394406
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from label ipset with err: %v", err)
407+
return err
395408
}
396409
}
397410

@@ -400,6 +413,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
400413
log.Logf("Adding pod %s to ipset %s", newPodObjIP, addIPSetName)
401414
if err = ipsMgr.AddToSet(addIPSetName, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.PodUID); err != nil {
402415
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to label ipset with err: %v", err)
416+
return err
403417
}
404418
}
405419

@@ -411,10 +425,12 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error {
411425
// Delete cached pod's named ports from its ipset.
412426
if err = appendNamedPortIpsets(ipsMgr, cachedPodObj.ContainerPorts, cachedPodObj.PodUID, cachedPodIP, true); err != nil {
413427
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from namespace ipset with err: %v", err)
428+
return err
414429
}
415430
// Add new pod's named ports from its ipset.
416431
if err = appendNamedPortIpsets(ipsMgr, newPodPorts, cachedPodObj.PodUID, newPodObjIP, false); err != nil {
417432
metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to namespace ipset with err: %v", err)
433+
return err
418434
}
419435
}
420436

@@ -470,25 +486,29 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error {
470486
// Delete the pod from its namespace's ipset.
471487
if err = ipsMgr.DeleteFromSet(podNs, cachedPodIP, podUID); err != nil {
472488
metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from namespace ipset with err: %v", err)
489+
return err
473490
}
474491

475492
// Delete the pod from its label's ipset.
476493
for podLabelKey, podLabelVal := range podLabels {
477494
log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podLabelKey)
478495
if err = ipsMgr.DeleteFromSet(podLabelKey, cachedPodIP, podUID); err != nil {
479496
metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from label ipset with err: %v", err)
497+
return err
480498
}
481499

482500
label := podLabelKey + ":" + podLabelVal
483501
log.Logf("Deleting pod %s from ipset %s", cachedPodIP, label)
484502
if err = ipsMgr.DeleteFromSet(label, cachedPodIP, podUID); err != nil {
485503
metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from label ipset with err: %v", err)
504+
return err
486505
}
487506
}
488507

489508
// Delete pod's named ports from its ipset. Delete is TRUE
490509
if err = appendNamedPortIpsets(ipsMgr, containerPorts, podUID, cachedPodIP, true); err != nil {
491510
metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from namespace ipset with err: %v", err)
511+
return err
492512
}
493513

494514
delete(npMgr.PodMap, podKey)

‎npm/util/const.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const (
6666
IptablesCommentFlag string = "--comment"
6767
IptablesAddCommentFlag
6868
IptablesAzureChain string = "AZURE-NPM"
69+
IptablesAzureAcceptChain string = "AZURE-NPM-ACCEPT"
6970
IptablesAzureKubeSystemChain string = "AZURE-NPM-KUBE-SYSTEM"
7071
IptablesAzureIngressChain string = "AZURE-NPM-INGRESS"
7172
IptablesAzureIngressPortChain string = "AZURE-NPM-INGRESS-PORT"
@@ -76,10 +77,13 @@ const (
7677
IptablesKubeServicesChain string = "KUBE-SERVICES"
7778
IptablesForwardChain string = "FORWARD"
7879
IptablesInputChain string = "INPUT"
79-
IptablesAzureIngressDropsChain string = "AZURE-NPM-INRGESS-DROPS"
80+
IptablesAzureIngressDropsChain string = "AZURE-NPM-INGRESS-DROPS"
8081
IptablesAzureEgressDropsChain string = "AZURE-NPM-EGRESS-DROPS"
8182
// Below chain exists only in NPM before v1.2.6
83+
// TODO delete this below set while cleaning up
8284
IptablesAzureTargetSetsChain string = "AZURE-NPM-TARGET-SETS"
85+
// Below chain existing only in NPM before v1.2.7
86+
IptablesAzureIngressWrongDropsChain string = "AZURE-NPM-INRGESS-DROPS"
8387
// Below chains exists only for before Azure-NPM:v1.0.27
8488
// and should be removed after a baking period.
8589
IptablesAzureIngressFromNsChain string = "AZURE-NPM-INGRESS-FROM-NS"
@@ -95,6 +99,7 @@ const (
9599
// IptablesAzureEgressMarkHex is for checking the absolute value of the mark
96100
IptablesAzureEgressMarkHex string = "0x1000"
97101
IptablesAzureAcceptMarkHex string = "0x3000"
102+
IptablesAzureClearMarkHex string = "0x0"
98103
)
99104

100105
//ipset related constants.

0 commit comments

Comments
 (0)