Repository navigation
Commit 6ae7fcf
authored
pkg/netpol: explain one policy's effect on one connection (#1747)
## What this is, in one paragraph
When Radar says "this NetworkPolicy blocks that connection", it has to
be right — a wrong answer sends someone editing the wrong YAML.
Kubernetes policies are unusual: they only ever *allow*; a policy never
blocks anything by itself. A pod is blocked when *some* policy selects
it and *none* of them allow the connection. So the honest question to
ask of a single policy is not "does it block?" but "does it allow this
connection, or not, or can't we tell?". This PR adds that question to
`pkg/netpol` as `Explain`, and rebuilds the existing service-level
verdict (`Evaluate`) on top of it so the two can never disagree.
## What a reviewer without NetworkPolicy expertise can check
- **Every answer is one of four words**: `NotApplicable` (this policy
doesn't concern this pod or direction), `Admits` (a rule allows it — the
reason names the rule), `DoesNotAdmit` (no rule allows it),
`Undecidable` (Radar can't tell — the reason says what it couldn't
check). There is deliberately no `Denies`.
- **"Undecidable" is never used as a hedge.** It appears only when a
specific fact is missing: the peer's namespace labels aren't readable,
the peer pod no longer exists, a selector is malformed, the connection's
port is unknown, or the address a rule would be checked against might
have been rewritten by the network (a real Kubernetes behaviour — see
the two `ipBlock` rules below). Each case has its own test.
- **The reasons are user-facing text** — they appear verbatim under a
dropped flow in the Traffic view — so they're written as an operator
reads a flow: *"rule 1 allows incoming TCP/80 from pods labeled
app=client in namespace demo"*, *"requires incoming traffic to be
explicitly allowed, but has no allow rules"*, *"the network may change
the source address before this rule sees it, so Radar can't tell whether
10.0.0.12 falls in this range"*.
## The rules worth knowing (and where the old evaluator got them wrong)
| Kubernetes behaviour | What `Explain` does | Old `/evaluate` handler |
|---|---|---|
| A policy with no `policyTypes` still isolates ingress | Treats it as
isolating ingress | Ignored it → false "allowed" |
| Rule ports matter; a named port resolves on the destination pod |
Compares protocol + port; named port needs the pod | Ignored ports
entirely |
| `namespaceSelector` matches the *Namespace object's labels* | Needs
the Namespace; without it ⇒ undecidable | Compared namespace *names* →
cross-namespace never matched |
| `ipBlock` on **ingress**: a hit is decisive, a miss isn't (source may
be SNATed) | Hit ⇒ admits; miss ⇒ undecidable | Never matched |
| `ipBlock` on **egress to a pod**: the plugin may see the Service IP,
not the pod's | Undecidable either way | Never matched |
| `ipBlock` on **egress to outside the cluster**: nothing rewrites it |
Hit admits; miss is decisive | Never matched |
| Node / host-network endpoints: plugin-defined | Undecidable | Treated
as pods |
These were confirmed live on an enforcing EKS cluster before this PR:
the old handler said "allowed" for a connection that timed out, and
"allowed for the wrong reason" for one that worked.
## What changed in code
- `pkg/netpol/explain.go` — `Explain(policy, direction, selectedPod,
peer, port, protocol) → {Effect, Reason}`. One rule walk; peers
described in words; three-valued folding so an open question never
becomes a "no".
- `pkg/netpol/evaluate.go` — `Evaluate` now folds `Explain` per policy.
`Peer` gains `External` and `IP` (an observed address wins over a pod's
address set). Verdicts unchanged.
- Tests: 25 `Explain` cases + a fold-consistency test that checks
`Evaluate` and `Explain` agree on every fixture; the 33 existing
`Evaluate` cases still pass. `internal/trace` (Service reachability) and
`internal/prometheus` (why Prometheus is unreachable) consume the
package and pass unchanged.
## Testing
`go test ./pkg/netpol ./internal/trace ./internal/prometheus` ✓. Live:
the same fixture pods and policies on `us-east-1-nonprod`, all six
`kubectl exec curl` outcomes matched by `Explain` with the correct rule
credited; causality proven by removing and restoring the isolating
policy.1 parent 9f742a4 commit 6ae7fcf
3 files changed
Lines changed: 744 additions & 96 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
20 | 30 | | |
21 | 31 | | |
22 | 32 | | |
| |||
121 | 131 | | |
122 | 132 | | |
123 | 133 | | |
124 | | - | |
| 134 | + | |
125 | 135 | | |
126 | 136 | | |
127 | 137 | | |
| |||
151 | 161 | | |
152 | 162 | | |
153 | 163 | | |
154 | | - | |
| 164 | + | |
155 | 165 | | |
156 | 166 | | |
157 | 167 | | |
158 | 168 | | |
159 | 169 | | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
| 170 | + | |
168 | 171 | | |
169 | 172 | | |
170 | 173 | | |
| |||
178 | 181 | | |
179 | 182 | | |
180 | 183 | | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
| 184 | + | |
188 | 185 | | |
189 | 186 | | |
190 | 187 | | |
191 | 188 | | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
192 | 202 | | |
193 | 203 | | |
194 | 204 | | |
| |||
202 | 212 | | |
203 | 213 | | |
204 | 214 | | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | | - | |
212 | | - | |
213 | | - | |
214 | | - | |
215 | | - | |
216 | | - | |
217 | | - | |
218 | 215 | | |
219 | 216 | | |
220 | 217 | | |
| |||
262 | 259 | | |
263 | 260 | | |
264 | 261 | | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | | - | |
300 | | - | |
301 | | - | |
302 | | - | |
303 | | - | |
304 | | - | |
305 | | - | |
306 | | - | |
307 | | - | |
308 | | - | |
309 | | - | |
310 | | - | |
311 | | - | |
312 | | - | |
313 | | - | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | | - | |
324 | | - | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
329 | 269 | | |
330 | 270 | | |
331 | 271 | | |
332 | 272 | | |
333 | 273 | | |
| 274 | + | |
334 | 275 | | |
335 | | - | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
336 | 283 | | |
337 | 284 | | |
338 | 285 | | |
| |||
351 | 298 | | |
352 | 299 | | |
353 | 300 | | |
354 | | - | |
| 301 | + | |
355 | 302 | | |
356 | 303 | | |
357 | 304 | | |
| |||
0 commit comments