From a09c185b5f93d6cedd93934c1551d736961fcb38 Mon Sep 17 00:00:00 2001
From: Miguel Duarte Barroso <mdbarroso@redhat.com>
Date: Sat, 19 Nov 2022 19:33:05 +0100
Subject: [PATCH] controller: reject adding ifaces to host networked pods

The controller unit tests were refactored a bit - creating the k8s
client and the dummy controller - in a `JustBeforeEach` block to allow
each `When` / `Context` block to define the pod previously. This way,
there is a lot less duplicated code.

Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
---
 pkg/controller/pod.go      | 15 +++++++++
 pkg/controller/pod_test.go | 65 +++++++++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go
index c96cc280..18de8d82 100644
--- a/pkg/controller/pod.go
+++ b/pkg/controller/pod.go
@@ -243,6 +243,14 @@ func (pnc *PodNetworksController) handlePodUpdate(oldObj interface{}, newObj int
 	oldPod := oldObj.(*corev1.Pod)
 	newPod := newObj.(*corev1.Pod)
 
+	if newPod.Spec.HostNetwork {
+		klog.Warningf(
+			"rejecting to add interfaces for host networked pod: %s",
+			annotations.NamespacedName(newPod.GetNamespace(), newPod.GetName()),
+		)
+		pnc.Eventf(newPod, corev1.EventTypeWarning, "InterfaceAddRejected", rejectInterfaceAddEventFormat(newPod))
+		return
+	}
 	if !didNetworkSelectionElementsChange(oldPod, newPod) {
 		return
 	}
@@ -413,6 +421,13 @@ func removeIfaceEventFormat(pod *corev1.Pod, network *nadv1.NetworkSelectionElem
 	)
 }
 
+func rejectInterfaceAddEventFormat(pod *corev1.Pod) string {
+	return fmt.Sprintf(
+		"pod [%s]: will not add interface to host networked pod",
+		annotations.NamespacedName(pod.GetNamespace(), pod.GetName()),
+	)
+}
+
 func interfaceAttributes(networkData nadv1.NetworkSelectionElement) *multusapi.DelegateInterfaceAttributes {
 	if len(networkData.IPRequest) > 0 || networkData.MacRequest != "" {
 		return &multusapi.DelegateInterfaceAttributes{
diff --git a/pkg/controller/pod_test.go b/pkg/controller/pod_test.go
index 25693761..66064940 100644
--- a/pkg/controller/pod_test.go
+++ b/pkg/controller/pod_test.go
@@ -78,6 +78,7 @@ var _ = Describe("Dynamic Attachment controller", func() {
 				pod           *corev1.Pod
 				networkToAdd  string
 				stopChannel   chan struct{}
+				nadClient     nadclient.Interface
 			)
 
 			networkStatusNames := func(statuses []nad.NetworkStatus) []string {
@@ -90,15 +91,24 @@ var _ = Describe("Dynamic Attachment controller", func() {
 
 			BeforeEach(func() {
 				pod = podSpec(podName, namespace, networkName)
-				k8sClient = fake.NewSimpleClientset(pod)
 				networkToAdd = fmt.Sprintf("%s-2", networkName)
-				nadClient, err := newFakeNetAttachDefClient(
+
+				var err error
+				nadClient, err = newFakeNetAttachDefClient(
 					netAttachDef(networkName, namespace, dummyNetSpec(networkName, cniVersion)),
 					netAttachDef(networkToAdd, namespace, dummyNetSpec(networkToAdd, cniVersion)))
 				Expect(err).NotTo(HaveOccurred())
 				stopChannel = make(chan struct{})
 				const maxEvents = 5
 				eventRecorder = record.NewFakeRecorder(maxEvents)
+			})
+
+			AfterEach(func() {
+				close(stopChannel)
+			})
+
+			JustBeforeEach(func() {
+				k8sClient = fake.NewSimpleClientset(pod)
 				Expect(
 					newDummyPodController(
 						k8sClient,
@@ -127,12 +137,8 @@ var _ = Describe("Dynamic Attachment controller", func() {
 				)
 			})
 
-			AfterEach(func() {
-				close(stopChannel)
-			})
-
 			When("an attachment is added to the pod's network annotations", func() {
-				BeforeEach(func() {
+				JustBeforeEach(func() {
 					var err error
 					_, err = k8sClient.CoreV1().Pods(namespace).UpdateStatus(
 						context.TODO(),
@@ -169,7 +175,7 @@ var _ = Describe("Dynamic Attachment controller", func() {
 			})
 
 			When("an attachment is removed from the pod's network annotations", func() {
-				BeforeEach(func() {
+				JustBeforeEach(func() {
 					var err error
 					_, err = k8sClient.CoreV1().Pods(namespace).UpdateStatus(
 						context.TODO(),
@@ -202,6 +208,43 @@ var _ = Describe("Dynamic Attachment controller", func() {
 					}).Should(BeEmpty())
 				})
 			})
+
+			When("an attachment is added to a host networked pod", func() {
+				BeforeEach(func() {
+					pod = hostNetworkedPodSpec(podName, namespace, networkName)
+				})
+
+				JustAfterEach(func() {
+					_, err := k8sClient.CoreV1().Pods(namespace).UpdateStatus(
+						context.TODO(),
+						updatePodSpec(pod, networkName, networkToAdd),
+						metav1.UpdateOptions{})
+					Expect(err).NotTo(HaveOccurred())
+				})
+
+				It("the attachment request is ignored", func() {
+					Eventually(func() ([]nad.NetworkStatus, error) {
+						updatedPod, err := k8sClient.CoreV1().Pods(namespace).Get(context.TODO(), podName, metav1.GetOptions{})
+						if err != nil {
+							return nil, err
+						}
+						status, err := annotations.PodDynamicNetworkStatus(updatedPod)
+						if err != nil {
+							return nil, err
+						}
+						return status, nil
+					}).Should(ConsistOf(
+						ifaceStatus(namespace, networkName, "net0", "")))
+				})
+
+				It("throws an event indicating the interface add operation is rejected", func() {
+					expectedEventPayload := fmt.Sprintf(
+						"Warning InterfaceAddRejected pod [%s]: will not add interface to host networked pod",
+						annotations.NamespacedName(namespace, podName),
+					)
+					Eventually(<-eventRecorder.Events).Should(Equal(expectedEventPayload))
+				})
+			})
 		})
 	})
 })
@@ -444,3 +487,9 @@ func ifaceStatus(namespace, networkName, ifaceName, macAddress string) nad.Netwo
 		Mac:       macAddress,
 	}
 }
+
+func hostNetworkedPodSpec(name string, namespace string, networks ...string) *corev1.Pod {
+	pod := podSpec(name, namespace, networks...)
+	pod.Spec.HostNetwork = true
+	return pod
+}