Fix false positives on pods with network policies

fixes derailed#311
bpfoster · Oct 31, 2024 · bc77411 · bc77411
1 parent 13f3b9c
commit bc77411
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 25 deletions.
diff --git a/internal/lint/pod.go b/internal/lint/pod.go
@@ -117,10 +117,10 @@ func (s *Pod) checkNPs(ctx context.Context, pod *v1.Pod) {
 			continue
 		}
 		if labelsMatch(&np.Spec.PodSelector, pod.Labels) {
-			if s.checkIngresses(ctx, pod, np.Spec.Ingress) {
+			if np.Spec.Ingress != nil {
 				matches[0]++
 			}
-			if s.checkEgresses(ctx, pod, np.Spec.Egress) {
+			if np.Spec.Egress != nil {
 				matches[1]++
 			}
 		}

diff --git a/internal/lint/pod_test.go b/internal/lint/pod_test.go
@@ -33,14 +33,17 @@ func TestPodNPLint(t *testing.T) {
 
 	po := NewPod(test.MakeCollector(t), dba)
 	assert.Nil(t, po.Lint(test.MakeContext("v1/pods", "pods")))
-	assert.Equal(t, 2, len(po.Outcome()))
+	assert.Equal(t, 3, len(po.Outcome()))
 
 	ii := po.Outcome()["ns1/p1"]
 	assert.Equal(t, 1, len(ii))
 	assert.Equal(t, `[POP-1204] Pod Egress is not secured by a network policy`, ii[0].Message)
 
 	ii = po.Outcome()["ns2/p2"]
 	assert.Equal(t, 0, len(ii))
+
+	ii = po.Outcome()["ns3/p3"]
+	assert.Equal(t, 0, len(ii))
 }
 
 func TestPodCheckSecure(t *testing.T) {
@@ -133,25 +136,21 @@ func TestPodLint(t *testing.T) {
 	assert.Equal(t, 0, len(ii))
 
 	ii = po.Outcome()["default/p2"]
-	assert.Equal(t, 6, len(ii))
+	assert.Equal(t, 4, len(ii))
 	assert.Equal(t, `[POP-207] Pod is in an unhappy phase ()`, ii[0].Message)
 	assert.Equal(t, `[POP-208] Unmanaged pod detected. Best to use a controller`, ii[1].Message)
-	assert.Equal(t, `[POP-1204] Pod Ingress is not secured by a network policy`, ii[2].Message)
-	assert.Equal(t, `[POP-1204] Pod Egress is not secured by a network policy`, ii[3].Message)
-	assert.Equal(t, `[POP-206] Pod has no associated PodDisruptionBudget`, ii[4].Message)
-	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[5].Message)
+	assert.Equal(t, `[POP-206] Pod has no associated PodDisruptionBudget`, ii[2].Message)
+	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[3].Message)
 
 	ii = po.Outcome()["default/p3"]
-	assert.Equal(t, 6, len(ii))
+	assert.Equal(t, 4, len(ii))
 	assert.Equal(t, `[POP-105] Liveness uses a port#, prefer a named port`, ii[0].Message)
 	assert.Equal(t, `[POP-105] Readiness uses a port#, prefer a named port`, ii[1].Message)
-	assert.Equal(t, `[POP-1204] Pod Ingress is not secured by a network policy`, ii[2].Message)
-	assert.Equal(t, `[POP-1204] Pod Egress is not secured by a network policy`, ii[3].Message)
-	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[4].Message)
-	assert.Equal(t, `[POP-109] CPU Current/Request (2000m/1000m) reached user 80% threshold (200%)`, ii[5].Message)
+	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[2].Message)
+	assert.Equal(t, `[POP-109] CPU Current/Request (2000m/1000m) reached user 80% threshold (200%)`, ii[3].Message)
 
 	ii = po.Outcome()["default/p4"]
-	assert.Equal(t, 15, len(ii))
+	assert.Equal(t, 13, len(ii))
 	assert.Equal(t, `[POP-204] Pod is not ready [0/1]`, ii[0].Message)
 	assert.Equal(t, `[POP-204] Pod is not ready [0/2]`, ii[1].Message)
 	assert.Equal(t, `[POP-100] Untagged docker image in use`, ii[2].Message)
@@ -163,20 +162,16 @@ func TestPodLint(t *testing.T) {
 	assert.Equal(t, `[POP-113] Container image "zorg:latest" is not hosted on an allowed docker registry`, ii[8].Message)
 	assert.Equal(t, `[POP-107] No resource limits defined`, ii[9].Message)
 	assert.Equal(t, `[POP-208] Unmanaged pod detected. Best to use a controller`, ii[10].Message)
-	assert.Equal(t, `[POP-1204] Pod Ingress is not secured by a network policy`, ii[11].Message)
-	assert.Equal(t, `[POP-1204] Pod Egress is not secured by a network policy`, ii[12].Message)
-	assert.Equal(t, `[POP-300] Uses "default" ServiceAccount`, ii[13].Message)
-	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[14].Message)
+	assert.Equal(t, `[POP-300] Uses "default" ServiceAccount`, ii[11].Message)
+	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[12].Message)
 
 	ii = po.Outcome()["default/p5"]
-	assert.Equal(t, 7, len(ii))
+	assert.Equal(t, 5, len(ii))
 	assert.Equal(t, `[POP-113] Container image "blee:v1.2" is not hosted on an allowed docker registry`, ii[0].Message)
 	assert.Equal(t, `[POP-106] No resources requests/limits defined`, ii[1].Message)
 	assert.Equal(t, `[POP-102] No probes defined`, ii[2].Message)
-	assert.Equal(t, `[POP-1204] Pod Ingress is not secured by a network policy`, ii[3].Message)
-	assert.Equal(t, `[POP-1204] Pod Egress is not secured by a network policy`, ii[4].Message)
-	assert.Equal(t, `[POP-209] Pod is managed by multiple PodDisruptionBudgets (pdb4, pdb4-1)`, ii[5].Message)
-	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[6].Message)
+	assert.Equal(t, `[POP-209] Pod is managed by multiple PodDisruptionBudgets (pdb4, pdb4-1)`, ii[3].Message)
+	assert.Equal(t, `[POP-301] Connects to API Server? ServiceAccount token is mounted`, ii[4].Message)
 }
 
 // ----------------------------------------------------------------------------

diff --git a/internal/lint/testdata/core/pod/3.yaml b/internal/lint/testdata/core/pod/3.yaml
@@ -121,4 +121,64 @@ items:
     conditions:
     phase: Running
     podIPs:
-    - ip: 172.1.0.3
+    - ip: 172.1.0.3
+- apiVersion: v1
+  kind: Pod
+  metadata:
+    name: p3
+    namespace: ns3
+    labels:
+      app: p3
+    ownerReferences:
+      - apiVersion: apps/v1
+        controller: true
+        kind: DaemonSet
+        name: rs4
+  spec:
+    serviceAccountName: sa3
+    tolerations:
+      - key: t1
+        operator: Exists
+        effect: NoSchedule
+    containers:
+      - name: c1
+        image: alpine:v1.0
+        livenessProbe:
+          httpGet:
+            path: /healthz
+            port: http
+          initialDelaySeconds: 3
+          periodSeconds: 3
+        readinessProbe:
+          httpGet:
+            path: /healthz
+            port: http
+          initialDelaySeconds: 3
+          periodSeconds: 3
+        resources:
+          requests:
+            cpu: 1
+            memory: 1Mi
+          limits:
+            cpu: 1
+            memory: 1Mi
+        ports:
+          - containerPort: 9090
+            name: http
+            protocol: TCP
+        env:
+          - name: env1
+            valueFrom:
+              configMapKeyRef:
+                name: cm1
+                key: ns
+          - name: env2
+            valueFrom:
+              secretKeyRef:
+                name: sec1
+                key: k1
+  status:
+    conditions:
+    phase: Running
+    podIPs:
+      - ip: 172.1.0.3
diff --git a/internal/lint/testdata/core/sa/2.yaml b/internal/lint/testdata/core/sa/2.yaml
@@ -12,4 +12,10 @@ items:
     metadata:
       name: sa2
       namespace: ns2
+    automountServiceAccountToken: false
+  - apiVersion: v1
+    kind: ServiceAccount
+    metadata:
+      name: sa3
+      namespace: ns3
     automountServiceAccountToken: false
diff --git a/internal/lint/testdata/net/np/3.yaml b/internal/lint/testdata/net/np/3.yaml
@@ -34,4 +34,26 @@ items:
     - {}
     policyTypes:
     - Ingress
-    - Egress
+    - Egress
+- apiVersion: networking.k8s.io/v1
+  kind: NetworkPolicy
+  metadata:
+    name: pod-selecting-np
+    namespace: ns3
+  spec:
+    podSelector:
+      matchLabels:
+        app: p3
+    ingress:
+      - from:
+          - namespaceSelector:
+              matchLabels:
+                app: p1
+    egress:
+      - to:
+          - namespaceSelector:
+              matchLabels:
+                app: p1
+    policyTypes:
+      - Ingress
+      - Egress