From bc77411eaa28df5cbca1e88538602b9feb896fd3 Mon Sep 17 00:00:00 2001 From: Ben Foster Date: Thu, 31 Oct 2024 11:40:25 -0400 Subject: [PATCH] Fix false positives on pods with network policies fixes #311 --- internal/lint/pod.go | 4 +- internal/lint/pod_test.go | 37 +++++++-------- internal/lint/testdata/core/pod/3.yaml | 62 +++++++++++++++++++++++++- internal/lint/testdata/core/sa/2.yaml | 6 +++ internal/lint/testdata/net/np/3.yaml | 24 +++++++++- 5 files changed, 108 insertions(+), 25 deletions(-) diff --git a/internal/lint/pod.go b/internal/lint/pod.go index 22cdbc9e..b5a9e927 100644 --- 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 index 3c0d0d0f..2a4a809d 100644 --- a/internal/lint/pod_test.go +++ b/internal/lint/pod_test.go @@ -33,7 +33,7 @@ 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)) @@ -41,6 +41,9 @@ func TestPodNPLint(t *testing.T) { 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 index 49b1419f..c1080376 100644 --- 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 \ No newline at end of file + - 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 \ No newline at end of file diff --git a/internal/lint/testdata/core/sa/2.yaml b/internal/lint/testdata/core/sa/2.yaml index a71e07c1..7540ca0c 100644 --- 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 \ No newline at end of file diff --git a/internal/lint/testdata/net/np/3.yaml b/internal/lint/testdata/net/np/3.yaml index 3ae4bad3..74820b54 100644 --- a/internal/lint/testdata/net/np/3.yaml +++ b/internal/lint/testdata/net/np/3.yaml @@ -34,4 +34,26 @@ items: - {} policyTypes: - Ingress - - Egress \ No newline at end of file + - 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 \ No newline at end of file