Skip to content

Commit 8a90118

Browse files
authored
Traffic flow policy panel: evaluate dropped flows with pkg/netpol (#1752)
## What this is, in one paragraph In the Traffic view, when a flow was **dropped**, Radar shows a small panel: *was a network policy responsible, which one, and what do the policies say right now?* Until now that panel had its own home-grown reading of NetworkPolicy and gave confidently wrong answers on real clusters (it said "allowed" for a connection that timed out). This PR replaces it with the shared evaluator from #1747, only claims what the evidence supports, and phrases everything the way an operator reads a flow. Stacked on #1747. ## What you'll see (three real captures, kind + Cilium 1.18.5) **1. The common case — a default-deny policy, nothing allows the client.** Cilium reports only "blocked by policy" and doesn't name one, so Radar's own check carries the answer: > Dropped: blocked by network policy — no rule allowed it · `POLICY_DENIED` > ● **No current NetworkPolicy allows this traffic into demo/echo-…** · checked against policies as they are now, flow seen 10:41:11 > ⛨ 1 policy applies to demo/echo-… > ● `deny-echo-ingress` (NetworkPolicy) — no matching allow rule > requires incoming traffic to be explicitly allowed, but has no allow rules **2. An explicit Cilium deny rule.** Here the plugin *does* name the policy, so that is the verdict; Radar's check is a note under it — and it says honestly that Kubernetes policies alone would have allowed the traffic: > ● **Blocked by CiliumNetworkPolicy demo/deny-client-explicit** · reported by the network plugin when the packet was dropped > Current check: Radar can't complete the current NetworkPolicy check · flow seen 10:43:05 > a CiliumNetworkPolicy also applies to this pod, and Radar reads only Kubernetes NetworkPolicies; Kubernetes NetworkPolicies alone would allow it > ⛨ 3 policies apply · *NetworkPolicies combine their allow rules: a policy with no matching rule doesn't override another policy's allow.* > ● `allow-client-to-echo` — allows this traffic · rule 1 allows incoming TCP/80 from pods labeled app=client in namespace demo > ● `deny-echo-ingress` — no matching allow rule · … > ● `deny-client-explicit` (CiliumNetworkPolicy) — can't evaluate **3. A drop that isn't about policy** (packet to an address nothing owns): > Destination: 10.244.0.99 (outside the cluster) > Dropped: no route to the destination address · `STALE_OR_UNROUTABLE_IP` > Not a policy decision, so current NetworkPolicies were not checked ## How to judge it without being a NetworkPolicy expert - **Radar never guesses.** If it can't establish something — the pod is gone, the flow record lacks a direction or protocol, the caller can't read the peer's namespace, a Cilium policy is in play — it says *what* it couldn't check, in the panel. The word "denied" only appears when every applicable policy was examined and none allows the traffic. - **The plugin's record beats Radar's reading.** When Hubble names the policy that dropped the packet, that's the headline. Radar's check of *today's* policies is shown under it, with the flow's time, because policies may have changed since. - **Cilium clusters are handled honestly.** Cilium merges its own allow rules with Kubernetes ones (verified live: a Cilium allow-all let traffic through a Kubernetes default-deny). So if any Cilium policy governs the pod in that direction, Radar lists it and states its Kubernetes-only reading *as a reading*: "No Kubernetes NetworkPolicy allows this traffic — but another policy kind also applies and could". Address ranges (`ipBlock`) are never decisive for a pod at the other end, because plugins disagree on whether they match pod addresses at all (the reference implementation does, Cilium by default does not). - **Nobody sees data they couldn't read themselves.** Listing policies and reading the pod are required (403 otherwise); a peer pod or Namespace the caller can't read is left "not visible to you" rather than fetched from Radar's cache on their behalf — and the panel says so. The plugin's own attribution is filtered the same way: a policy name the caller couldn't list is withheld before the flow is delivered, and the panel says "blocked by a policy you don't have permission to read". ## What changed in code **Backend** — `GET /api/network-policies/evaluate` rebuilt on `netpol.Explain`. - One named pod at the enforcing end; the peer resolved by the flow's endpoint kind (pod by name · external address → judged against `ipBlock`s · node/host → undecidable · unidentified or any other kind → unresolved, never a same-named pod). - Direction, port and protocol taken from the flow; missing direction or non-TCP/UDP/SCTP protocol ⇒ undecidable; malformed port ⇒ 400. - Per-caller gates on policies (list), the pod (get), the peer pod (get) and Namespace labels (get); a namespace the cache doesn't cover ⇒ undecidable. - `CiliumNetworkPolicy` / `CiliumClusterwideNetworkPolicy` discovered independently by group; those whose `endpointSelector` selects the pod *and* carry rules for the flow's direction are listed as rows and cap the verdict. Selectors are matched against the Cilium identity (pod labels + namespace, service account, Namespace labels). Requirements are ANDed, so a definite miss on any answerable key means the policy doesn't select the pod; only when everything answerable matches does a key Radar can't reproduce — the cluster name, or Namespace labels it couldn't read — leave the selector undecidable rather than a miss. - Response: `verdict admitted|denied|undecidable|no-policy`, `reason`, `coreVerdict` (the Kubernetes-only reading when the verdict was capped), `evaluated{direction,pod,peer,port,protocol}`, rows with `effect admits|does_not_admit|undecidable` and `Explain`'s reason. **Hubble ingestion** — flows carry the plugin's own attribution (`policyVerdict{allowedBy, deniedBy, withheld}`, references filtered per caller on REST and SSE); non-pod endpoints are classified from Cilium's reserved identities as `Host`, `External` or `Unknown` (node evidence wins over a `cidr:` label). The traffic graph draws all three where external traffic goes. Older relays that fill only the numeric drop code are read too. **Panel** — sends the endpoints by kind, port and protocol; evaluates only policy drops (`POLICY_DENIED`, `POLICY_DENY`, or the plugin naming a policy — the two codes are matched exactly, they are not substrings of each other); the plugin's named deny is the headline (an allow the plugin also recorded on a dropped flow — at another layer or the other end — is a note, never the verdict) → scoped current check with its check time (refreshed while the row is open) → rows in plain words; drop reasons phrased for an operator with the raw code beside them; non-pod endpoints show the address first. **Helm chart** — the Cloud cluster-read role grants `get/list/watch` on the two Cilium policy kinds under `rbac.crdGroups.cilium`, so Cloud tiers keep decisive verdicts on Cilium clusters (the panel gates on the caller listing them). Needs the usual re-vendor into the deployment repo. ## Testing - `go test`: handler table (15 cases: the four old wrong answers, `ipBlock` hit/miss/egress, host peer, missing direction/protocol/pod, peer gone, non-pod kind, no-policy), per-caller gating (5), namespace outside the cache, 14 Cilium cases (allow rescues a core deny ⇒ capped with `coreVerdict`; egress-only leaves ingress alone; CCNP; CCNP-only cluster; `specs[]`/`matchExpressions`; service-account and namespace-label selectors; cluster-name selector; a definite miss beats an unknown key; a later selecting spec; caller can't list Cilium kinds ⇒ undecidable; nodeSelector-only); attribution redaction per caller; Hubble kinds incl. both label orders, `*_by` mapping, numeric drop-code fallback; `pkg/netpol` pod-source `ipBlock` cases. `make tsc` ✓; vitest incl. `isPolicyDropReason` and `isExternalKind`; helm unittest for the chart grant and its gate. - **Live on kind + Cilium 1.18.5 with Hubble**, each against `kubectl exec curl` ground truth: default-deny (timeout → panel 1); allow + Cilium `ingressDeny` (timeout, Hubble names the CNP → panel 2); default-deny + Cilium `fromEntities: [all]` (**200** → undecidable naming the CNP; the pre-fix contract would have said "denied"); egress-only CCNP (ingress verdict stands); CNP selecting by service account (selected); unallocated IP (`STALE_OR_UNROUTABLE_IP` → panel 3). - Visual test at 1920×1080, dark and light; no console errors. ## Not in this PR Ranking/collapsing the rows, "what would need to allow this" in prose with a link to the policy, and "the policy has changed since the drop" detection — the next slice. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes RBAC grants, traffic API response shape, and policy-evaluation semantics shown to operators; mistakes could mislead debugging but the design favors undecidable over guessing. > > **Overview** > Rebuilds the Traffic view **dropped-flow policy panel** so it stops using a hand-rolled NetworkPolicy reader and instead calls **`GET /api/network-policies/evaluate`**, backed by **`netpol.Explain`** with conservative, evidence-only verdicts (`admitted` / `denied` / `no-policy` / `undecidable`, plus **`coreVerdict`** when Cilium caps the answer). > > The evaluate API now takes **flow-shaped query params** (required **direction**, enforcing pod name, peer kind/IP, port, protocol), enforces **per-caller RBAC** (list policies, get pods/namespaces), and **detects governing CiliumNetworkPolicy / clusterwide policies**—listing them and marking the overall result undecidable when they apply. **`pkg/netpol`** treats **Host** peers and **ipBlock vs pod** matches as undecidable where plugins disagree. > > **Hubble ingestion** adds **`policyVerdict`** on flows, classifies non-pod endpoints as **Host / External / Unknown**, and **redacts policy names** the viewer cannot list (REST and SSE), with a **`withheld`** count for denied refs. The UI panel prioritizes **plugin attribution** on policy drops, runs a **current NetworkPolicy check** under it, and only evaluates when drop reason or denied refs indicate policy. > > **Helm** grants Cloud cluster-read **`get/list/watch`** on Cilium policy CRDs when **`rbac.crdGroups.cilium`** is enabled, with chart tests for the gate. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 681d97b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 6f8e047 commit 8a90118

20 files changed

Lines changed: 1781 additions & 391 deletions

‎deploy/helm/radar/templates/cloud-rbac-cluster-read.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,17 @@ rules:
126126
resources: ["*"]
127127
verbs: ["get", "list", "watch"]
128128
{{- end }}
129+
{{- if .Values.rbac.crdGroups.cilium }}
130+
# The Traffic view's policy panel gates per-CALLER on listing Cilium
131+
# policies: a Cilium policy that governs the pod makes the core
132+
# NetworkPolicy reading inconclusive, so a caller who cannot list them gets
133+
# an undecidable answer on every flow. Declared policy, no secrets — the same
134+
# class as the core NetworkPolicies the built-in view role already reads.
135+
# Gated on the SA's own cilium collection for the same reason as karpenter.
136+
- apiGroups: ["cilium.io"]
137+
resources: ["ciliumnetworkpolicies", "ciliumclusterwidenetworkpolicies"]
138+
verbs: ["get", "list", "watch"]
139+
{{- end }}
129140
{{- if .Values.rbac.metrics }}
130141
# On clusters with a standard metrics-server install this is usually
131142
# redundant — its aggregated-metrics-reader role carries aggregate-to-view

‎deploy/helm/radar/tests/cloud_rbac_cluster_read_test.yaml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,47 @@ tests:
108108
verbs: ["get", "list", "watch"]
109109
any: true
110110

111+
- it: grants Cilium policy reads the Traffic policy panel gates on
112+
set:
113+
cloud:
114+
enabled: true
115+
token: dummy
116+
url: "wss://cloud.example"
117+
clusterName: test-cluster
118+
documentSelector:
119+
path: kind
120+
value: ClusterRole
121+
asserts:
122+
- contains:
123+
path: rules
124+
content:
125+
apiGroups: ["cilium.io"]
126+
resources: ["ciliumnetworkpolicies", "ciliumclusterwidenetworkpolicies"]
127+
verbs: ["get", "list", "watch"]
128+
any: true
129+
130+
- it: follows the Cilium collection gate
131+
set:
132+
cloud:
133+
enabled: true
134+
token: dummy
135+
url: "wss://cloud.example"
136+
clusterName: test-cluster
137+
rbac:
138+
crdGroups:
139+
cilium: false
140+
documentSelector:
141+
path: kind
142+
value: ClusterRole
143+
asserts:
144+
- notContains:
145+
path: rules
146+
content:
147+
apiGroups: ["cilium.io"]
148+
resources: ["ciliumnetworkpolicies", "ciliumclusterwidenetworkpolicies"]
149+
verbs: ["get", "list", "watch"]
150+
any: true
151+
111152
- it: follows the Prometheus collection gate
112153
set:
113154
cloud:

0 commit comments

Comments
 (0)