Repository navigation
Commit e914705
authored
Show Prometheus discovery progress, pre-warm on context switch, refuse headers without a URL (#1736)
## Why
Users saw a **Discover Prometheus** button even though discovery is
automatic. Two gaps:
1. Discovery already ran in the background at boot, but `prom.Status`
had no in-flight state, so any metrics view opened during the 5–10 s run
(4 s direct-probe budget + port-forward off-cluster) rendered the manual
CTA. Clicking it only joined the run already underway.
2. After a context switch nothing started discovery at all —
`Reinitialize` builds a fresh client and stops there.
Separately: `--prometheus-header` values were sent to *every*
auto-discovered candidate during probing. A mislabelled or hostile
Service that looks like Prometheus would receive the operator's bearer
token.
## What
**Discovery status.** `prom.Status` gains `discovering`, owned by the
detached discovery run itself — waiters come and go on their own
deadlines, the flag follows the run (gate wait included). The last run's
outcome, including a timeout, is reported as `error` when idle, so
charts and Settings say *why* there is no connection instead of showing
a bare button. Both are generation-keyed: an older run can never
describe a newer configuration, and a cached-endpoint probe that
finishes after a configuration change neither returns the superseded
endpoint nor tears down what the new generation connected.
**Pre-warm everywhere a cluster comes up.** `Prewarm()` runs at boot and
from the context-switch and namespace-rescope callbacks. A cluster with
no Prometheus no longer records an errorlog warning on every start; the
status carries the reason.
**Headers require a URL.** With headers configured and no URL, discovery
refuses to run — in the Prometheus client and the Caretta traffic source
— the status names the rule, and boot logs a warning (not fatal). `PUT
/api/integrations/prometheus` rejects saving that pair, checked against
what will actually be in effect: the submitted headers, else the running
client's (flags, env and file already folded in), plus env-sourced
headers on disk that re-resolve at the next start. The Helm chart's
`values.schema.json` enforces the same dependency so `helm
install`/`lint` fails on `traffic.prometheusHeaders*` without
`traffic.prometheusUrl`.
**Atomic live configuration.** A URL + header change applies in one step
with the cached connection dropped alongside it (`prometheus.Configure`,
`traffic.SetMetricsConfig`), and only the forward that served the
dropped endpoint is stopped. The traffic subsystem is rebuilt on such a
change — serialized with context operations — because a live source
copies URL and headers at construction; without the rebuild, rotated or
cleared credentials would keep being used until the next context switch.
**Frontend.** Status polls every 2 s while a run is in flight or before
the server's run has reported (a status read before the boot/switch
pre-warm starts is transient — every run ends in `connected` or
`error`); both chart CTA sites render "Discovering Prometheus…" instead
of the button; `useAutoPromConnect` marks discovering optimistically
before its POST and undoes it if the request itself fails, so an
unreachable server can't leave the spinner up. Settings → Overview →
Metrics shows Discovering / Connected / the failure reason.
`@skyhook-io/k8s-ui` `PrometheusChartsView` gains an optional
`discovering` prop (additive).
## Testing
- **Go** (`internal/prometheus`, `internal/server`, `internal/traffic`,
`pkg/prom`): status before / during / after a run; discovering follows
the flight when every waiter cancels; failure outcome surfaces when
idle; timeout and generation rules; headers gate never touches the
cluster (fake client asserts no list); new credentials never reach the
previous endpoint through `Configure`; a stale cached probe cannot tear
down a newer connection; apply-handler 400 matrix (submitted / live /
env-on-disk headers). **Vitest**: poll cadence. `helm lint` with and
without headers/URL.
- **Live, local Radar against an EKS cluster** with
`monitoring/prometheus-operated`: opening a Pod drawer inside the boot
window shows "Discovering Prometheus…" and flips to populated charts ~1
s later with no click; a header context switch to a cluster without
Prometheus shows *Not connected — no Prometheus service found in
cluster* in Settings, and switching back pre-warms and connects (~6 s)
while the open drawer waits on the same run. Started with
`--prometheus-header` and no URL: warning logged, no enumeration or
probe, the drawer and Settings carry the rule, `PUT` with an empty URL
returns 400, and clearing the headers in the same `PUT` discovers and
connects.
## Notes
- Local and in-cluster both pre-warm; in-cluster this is a `list
services` plus in-cluster GETs, no port-forward.
- A context switch performed server-side (API, CAPI connect) reaches an
already-open tab on the regular 60 s status cadence; the header switcher
invalidates queries itself. Unchanged by this PR.
- Stacked follow-up: #1737 names the NetworkPolicy behind an in-cluster
unreachable candidate.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes metrics discovery and how Prometheus auth headers are applied;
misconfigurations that relied on headers without a URL will stop
discovering until fixed, but the change reduces credential leakage to
unintended endpoints.
>
> **Overview**
> Improves **Prometheus metrics UX** and **credential safety** when
Radar auto-discovers metrics backends.
>
> **Discovery progress** — `prom.Status` now exposes `discovering` (tied
to the in-flight discovery run, not individual HTTP waiters) and
surfaces the last failure (including timeouts) as `error` when idle.
Boot and **context/namespace switches** call `Prewarm()` so discovery
starts automatically; the UI polls every 2s while unsettled and shows
“Discovering Prometheus…” instead of a manual CTA.
>
> **Headers require an explicit URL** — Shared rule via
`pkg/prom.HeadersRequireURL` / `ErrHeadersRequireURL`: with auth headers
but no `prometheusUrl`, discovery is skipped (Prometheus client and
Caretta traffic), Settings/`PUT /api/integrations/prometheus` reject
saving that combo, and Helm `values.schema.json` enforces
`traffic.prometheusUrl` when headers are set. Docs call out that
credentials are never sent to auto-discovered endpoints.
>
> **Safer live config** — URL and headers apply atomically (`Configure`,
`SetMetricsConfig`) with the cached connection dropped in the same step;
live Prometheus integration changes **restart the traffic subsystem** so
rotated headers cannot keep hitting an old endpoint.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
52216a7. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 11278fa commit e914705
22 files changed
Lines changed: 784 additions & 81 deletions
File tree
- deploy/helm/radar
- docs
- internal
- app
- k8s
- prometheus
- server
- traffic
- packages/k8s-ui/src/components/charts
- pkg/prom
- web/src
- api
- components
- resource
- settings
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
207 | 207 | | |
208 | 208 | | |
209 | 209 | | |
210 | | - | |
211 | | - | |
| 210 | + | |
| 211 | + | |
212 | 212 | | |
213 | 213 | | |
214 | 214 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
362 | 362 | | |
363 | 363 | | |
364 | 364 | | |
365 | | - | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
366 | 376 | | |
367 | 377 | | |
368 | 378 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
533 | 533 | | |
534 | 534 | | |
535 | 535 | | |
| 536 | + | |
| 537 | + | |
536 | 538 | | |
537 | 539 | | |
538 | 540 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
| 118 | + | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
522 | 522 | | |
523 | 523 | | |
524 | 524 | | |
525 | | - | |
526 | | - | |
| 525 | + | |
| 526 | + | |
527 | 527 | | |
528 | 528 | | |
529 | 529 | | |
| |||
557 | 557 | | |
558 | 558 | | |
559 | 559 | | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
560 | 568 | | |
561 | 569 | | |
562 | 570 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| 32 | + | |
32 | 33 | | |
33 | 34 | | |
34 | 35 | | |
| |||
292 | 293 | | |
293 | 294 | | |
294 | 295 | | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
295 | 299 | | |
296 | 300 | | |
297 | 301 | | |
| |||
319 | 323 | | |
320 | 324 | | |
321 | 325 | | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
322 | 331 | | |
323 | 332 | | |
324 | 333 | | |
| |||
578 | 587 | | |
579 | 588 | | |
580 | 589 | | |
581 | | - | |
582 | | - | |
583 | | - | |
584 | | - | |
585 | | - | |
586 | | - | |
587 | | - | |
588 | | - | |
589 | | - | |
590 | | - | |
591 | | - | |
592 | | - | |
593 | | - | |
594 | | - | |
595 | | - | |
| 590 | + | |
596 | 591 | | |
597 | 592 | | |
598 | 593 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
268 | 268 | | |
269 | 269 | | |
270 | 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 | + | |
271 | 297 | | |
272 | 298 | | |
273 | 299 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
| 8 | + | |
7 | 9 | | |
8 | 10 | | |
9 | 11 | | |
| |||
83 | 85 | | |
84 | 86 | | |
85 | 87 | | |
86 | | - | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
87 | 91 | | |
88 | 92 | | |
89 | 93 | | |
| |||
0 commit comments