From b69cfd83cab81a713c73eb377d0b4e162ea0c1f6 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 1 Feb 2021 14:56:23 -0800 Subject: [PATCH 1/3] Ignoring hostnetwork pods from being added into Ipsets --- npm/pod.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/npm/pod.go b/npm/pod.go index 1c2a11560c..cad89c0d55 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -27,7 +27,8 @@ func isInvalidPodUpdate(oldPodObj, newPodObj *corev1.Pod) (isInvalidUpdate bool) oldPodObj.Status.Phase == newPodObj.Status.Phase && oldPodObj.Status.PodIP == newPodObj.Status.PodIP && newPodObj.ObjectMeta.DeletionTimestamp == nil && - newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil + newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil && + newPodObj.Spec.HostNetwork //Ignore if HostNetwork pod isInvalidUpdate = isInvalidUpdate && reflect.DeepEqual(oldPodObj.ObjectMeta.Labels, newPodObj.ObjectMeta.Labels) return @@ -61,6 +62,12 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { } } + // Ignore adding the HostNetwork pod to any ipsets. + if podObj.Spec.HostNetwork { + log.Logf("HostNetwork POD IGNORED: [%s%s/%s/%s%+v%s]", podUid, podNs, podName, podNodeName, podLabels, podIP) + return nil + } + // Add the pod to its namespace's ipset. log.Logf("Adding pod %s to ipset %s", podIP, podNs) if err = ipsMgr.AddToSet(podNs, podIP, util.IpsetNetHashFlag, podUid); err != nil { From 4a68db7edfecf535a279519353f03b1df8b720c9 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 1 Feb 2021 15:38:18 -0800 Subject: [PATCH 2/3] generalizing the check on hostnetwork pod --- npm/pod.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/npm/pod.go b/npm/pod.go index cad89c0d55..afb4e830b1 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -21,14 +21,17 @@ func isSystemPod(podObj *corev1.Pod) bool { return podObj.ObjectMeta.Namespace == util.KubeSystemFlag } +func isHostNetworkPod(podObj *corev1.Pod) bool { + return podObj.Spec.HostNetwork +} + func isInvalidPodUpdate(oldPodObj, newPodObj *corev1.Pod) (isInvalidUpdate bool) { isInvalidUpdate = oldPodObj.ObjectMeta.Namespace == newPodObj.ObjectMeta.Namespace && oldPodObj.ObjectMeta.Name == newPodObj.ObjectMeta.Name && oldPodObj.Status.Phase == newPodObj.Status.Phase && oldPodObj.Status.PodIP == newPodObj.Status.PodIP && newPodObj.ObjectMeta.DeletionTimestamp == nil && - newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil && - newPodObj.Spec.HostNetwork //Ignore if HostNetwork pod + newPodObj.ObjectMeta.DeletionGracePeriodSeconds == nil isInvalidUpdate = isInvalidUpdate && reflect.DeepEqual(oldPodObj.ObjectMeta.Labels, newPodObj.ObjectMeta.Labels) return @@ -63,7 +66,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { } // Ignore adding the HostNetwork pod to any ipsets. - if podObj.Spec.HostNetwork { + if isHostNetworkPod(podObj) { log.Logf("HostNetwork POD IGNORED: [%s%s/%s/%s%+v%s]", podUid, podNs, podName, podNodeName, podLabels, podIP) return nil } @@ -147,6 +150,17 @@ func (npMgr *NetworkPolicyManager) UpdatePod(oldPodObj, newPodObj *corev1.Pod) e newPodObjNs, newPodObjName, newPodObjLabel, newPodObjPhase, newPodObjIP, ) + // today K8s does not allow updating HostNetwork flag for an existing Pod. So NPM can safely + // check on the oldPodObj for hostNework value + if isHostNetworkPod(oldPodObj) { + log.Logf( + "POD UPDATING ignored for HostNetwork Pod:\n old pod: [%s/%s/%+v/%s/%s]\n new pod: [%s/%s/%+v/%s/%s]", + oldPodObjNs, oldPodObjName, oldPodObjLabel, oldPodObjPhase, oldPodObjIP, + newPodObjNs, newPodObjName, newPodObjLabel, newPodObjPhase, newPodObjIP, + ) + return nil + } + // Todo: Update if cached ip and podip changed and it is not a delete event if err = npMgr.DeletePod(oldPodObj); err != nil { From bd5c2c8cf37cef2cb6fdbb3d49ce8cdde6121480 Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 2 Feb 2021 10:24:13 -0800 Subject: [PATCH 3/3] Adding tests for add, update and delete hostnetwork pods --- npm/pod.go | 22 +++--- npm/pod_test.go | 180 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 11 deletions(-) diff --git a/npm/pod.go b/npm/pod.go index afb4e830b1..01f6f44b1c 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -126,6 +126,17 @@ func (npMgr *NetworkPolicyManager) UpdatePod(oldPodObj, newPodObj *corev1.Pod) e return nil } + // today K8s does not allow updating HostNetwork flag for an existing Pod. So NPM can safely + // check on the oldPodObj for hostNework value + if isHostNetworkPod(oldPodObj) { + log.Logf( + "POD UPDATING ignored for HostNetwork Pod:\n old pod: [%s/%s/%+v/%s/%s]\n new pod: [%s/%s/%+v/%s/%s]", + oldPodObj.ObjectMeta.Namespace, oldPodObj.ObjectMeta.Name, oldPodObj.Status.PodIP, + newPodObj.ObjectMeta.Namespace, newPodObj.ObjectMeta.Name, newPodObj.Status.PodIP, + ) + return nil + } + if isInvalidPodUpdate(oldPodObj, newPodObj) { return nil } @@ -150,17 +161,6 @@ func (npMgr *NetworkPolicyManager) UpdatePod(oldPodObj, newPodObj *corev1.Pod) e newPodObjNs, newPodObjName, newPodObjLabel, newPodObjPhase, newPodObjIP, ) - // today K8s does not allow updating HostNetwork flag for an existing Pod. So NPM can safely - // check on the oldPodObj for hostNework value - if isHostNetworkPod(oldPodObj) { - log.Logf( - "POD UPDATING ignored for HostNetwork Pod:\n old pod: [%s/%s/%+v/%s/%s]\n new pod: [%s/%s/%+v/%s/%s]", - oldPodObjNs, oldPodObjName, oldPodObjLabel, oldPodObjPhase, oldPodObjIP, - newPodObjNs, newPodObjName, newPodObjLabel, newPodObjPhase, newPodObjIP, - ) - return nil - } - // Todo: Update if cached ip and podip changed and it is not a delete event if err = npMgr.DeletePod(oldPodObj); err != nil { diff --git a/npm/pod_test.go b/npm/pod_test.go index 2f114ba55d..5ffe957974 100644 --- a/npm/pod_test.go +++ b/npm/pod_test.go @@ -202,3 +202,183 @@ func TestDeletePod(t *testing.T) { } npMgr.Unlock() } + +func TestAddHostNetworkPod(t *testing.T) { + npMgr := &NetworkPolicyManager{ + nsMap: make(map[string]*namespace), + podMap: make(map[string]string), + TelemetryEnabled: false, + } + + allNs, err := newNs(util.KubeAllNamespacesFlag) + if err != nil { + panic(err.Error) + } + npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs + + ipsMgr := ipsm.NewIpsetManager() + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestAddHostNetworkPod failed @ ipsMgr.Save") + } + + defer func() { + if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestAddHostNetworkPod failed @ ipsMgr.Restore") + } + }() + + podObj := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + Labels: map[string]string{ + "app": "test-pod", + }, + }, + Status: corev1.PodStatus{ + Phase: "Running", + PodIP: "1.2.3.4", + }, + Spec: corev1.PodSpec{ + HostNetwork: true, + }, + } + + npMgr.Lock() + if err := npMgr.AddPod(podObj); err != nil { + t.Errorf("TestAddHostNetworkPod failed @ AddPod") + } + + if len(npMgr.podMap) >= 1 { + t.Errorf("TestAddHostNetworkPod failed @ podMap length check") + } + npMgr.Unlock() +} + +func TestUpdateHostNetworkPod(t *testing.T) { + npMgr := &NetworkPolicyManager{ + nsMap: make(map[string]*namespace), + podMap: make(map[string]string), + TelemetryEnabled: false, + } + + allNs, err := newNs(util.KubeAllNamespacesFlag) + if err != nil { + panic(err.Error) + } + npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs + + ipsMgr := ipsm.NewIpsetManager() + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestUpdateHostNetworkPod failed @ ipsMgr.Save") + } + + defer func() { + if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestUpdateHostNetworkPod failed @ ipsMgr.Restore") + } + }() + + // HostNetwork check is done on the oldPodObj, + // so intentionally not adding hostnet true in newPodObj + oldPodObj := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-test-pod", + Namespace: "test-namespace", + Labels: map[string]string{ + "app": "old-test-pod", + }, + }, + Status: corev1.PodStatus{ + Phase: "Running", + PodIP: "1.2.3.4", + }, + Spec: corev1.PodSpec{ + HostNetwork: true, + }, + } + + newPodObj := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-test-pod", + Namespace: "test-namespace", + Labels: map[string]string{ + "app": "new-test-pod", + }, + }, + Status: corev1.PodStatus{ + Phase: "Running", + PodIP: "4.3.2.1", + }, + } + + npMgr.Lock() + if err := npMgr.AddPod(oldPodObj); err != nil { + t.Errorf("TestUpdateHostNetworkPod failed @ AddPod") + } + + if err := npMgr.UpdatePod(oldPodObj, newPodObj); err != nil { + t.Errorf("TestUpdateHostNetworkPod failed @ UpdatePod") + } + + if len(npMgr.podMap) >= 1 { + t.Errorf("TestUpdateHostNetworkPod failed @ podMap length check") + } + npMgr.Unlock() +} + +func TestDeleteHostNetworkPod(t *testing.T) { + npMgr := &NetworkPolicyManager{ + nsMap: make(map[string]*namespace), + podMap: make(map[string]string), + TelemetryEnabled: false, + } + + allNs, err := newNs(util.KubeAllNamespacesFlag) + if err != nil { + panic(err.Error) + } + npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs + + ipsMgr := ipsm.NewIpsetManager() + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestDeleteHostNetworkPod failed @ ipsMgr.Save") + } + + defer func() { + if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestDeleteHostNetworkPod failed @ ipsMgr.Restore") + } + }() + + podObj := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + Labels: map[string]string{ + "app": "test-pod", + }, + }, + Status: corev1.PodStatus{ + Phase: "Running", + PodIP: "1.2.3.4", + }, + Spec: corev1.PodSpec{ + HostNetwork: true, + }, + } + + npMgr.Lock() + if err := npMgr.AddPod(podObj); err != nil { + t.Errorf("TestDeleteHostNetworkPod failed @ AddPod") + } + + if len(npMgr.podMap) >= 1 { + t.Errorf("TestDeleteHostNetworkPod failed @ podMap length check") + } + + if err := npMgr.DeletePod(podObj); err != nil { + t.Errorf("TestDeleteHostNetworkPod failed @ DeletePod") + } + npMgr.Unlock() +}