feat(prometheus): custom HTTP headers for auth-protected backends#732
Merged
Conversation
…ends `--prometheus-url` was effectively unusable against hosted or enterprise Prometheus-compatible backends that require header-based authentication (Bearer tokens, X-Scope-OrgID multi-tenancy headers, etc.) — users had to put a reverse proxy in front just to inject auth. Add a `prometheusHeaders` map to `~/.radar/config.json` and an equivalent repeatable `--prometheus-header Key=Value` CLI flag. Headers are applied to all Prometheus requests, in both the metrics-API client (`internal/prometheus`) and the traffic/Caretta query path (`internal/traffic/caretta`) — otherwise the topology traffic view would silently 401 against the same authed endpoint. Helm chart exposes `traffic.prometheusHeaders` as a map that renders into repeated `--prometheus-header` args. CLI flags override file defaults outright (kubectl-style) rather than merging. Resolves #683.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Address review findings on PR #732: - Reject CR/LF and invalid header-token bytes in --prometheus-header at parse time via httpguts (was silently corrupting requests or failing opaquely at first-query time; CRLF in a value is the classic header- injection vector). Promotes golang.org/x/net from indirect to direct. - Surface 401/403 from probe() in both prometheus/client.go and traffic/caretta.go via errorlog — otherwise a misconfigured Bearer token shows up as "Prometheus not found" after discovery falls through every candidate. - Quote --prometheus-header args in the Helm deployment template; matches the | quote pattern used for auth.secret, oidc.clientSecret, cloud.token in the same file. - Add caretta_test.go covering applyHeaders on both queryPrometheusRaw and tryMetricsEndpointLocked — guards the parallel implementation against drift from internal/prometheus. - Add main_test.go covering headerFlag: Key=Value parsing edge cases (including '=' in Bearer tokens), CRLF / invalid-name rejection, and the kubectl-style "first CLI flag wipes file defaults" latch. - Trim WHAT-not-WHY comments on struct fields and test docstrings. - gofmt diagnostics.go and bootstrap.go (struct alignment after the new bool field).
…alURL in Reinitialize Reinitialize held clientMu exclusively but read globalClient.headers and globalClient.manualURL without taking the per-client mutex. SetHeaders releases clientMu before acquiring c.mu to write headers, opening a race window go test -race would flag. SetURL (the method) writes manualURL under c.mu on the HTTP handler path, completely independent of clientMu — same race. Take globalClient.mu.RLock around the snapshot. Also copyHeaders so the new client doesn't alias the old client's map.
…intLocked Bugbot caught a real deadlock: tryMetricsEndpointLocked is called under c.mu.Lock() (write) by discoverPrometheus / Connect / tryClusterAddrLocked / discoverMetricsServiceDynamic. My applyHeaders took c.mu.RLock() — sync.RWMutex isn't reentrant, so every Caretta user (not just header-config users) would have wedged on the first metrics-endpoint probe. The lock was over-engineered defense against a mutation that doesn't exist: CarettaSource.headers is assigned exactly once inside manager.go's initOnce.Do and never written again (context switches construct a fresh source). Removing the lock both fixes the deadlock and matches the actual concurrency story. Update the test to acquire c.mu.Lock before calling tryMetricsEndpointLocked — the original test invoked it directly and that's why the deadlock didn't surface.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3adeafe. Configure here.
…d headers - Strip PrometheusHeaders from GET /api/config response (file + effective); diagnostics already masks them as a presence bool, /api/config was the remaining plaintext leak. PUT preserves the on-disk value so a UI round-trip can't silently wipe the user's auth headers. - Apply httpguts.ValidHeaderFieldName/Value to headers loaded from ~/.radar/config.json (the CLI Set path already validates). Invalid entries are dropped with a startup log line instead of failing at request time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #683.
--prometheus-urlwas effectively unusable against hosted or enterprise Prometheus-compatible backends (Grafana Cloud, Mimir, VictoriaMetrics Cluster, Thanos) that require header-based authentication — Bearer tokens,X-Scope-OrgIDmulti-tenancy headers, etc. Workaround was running a reverse proxy in front of Radar just to inject auth.This PR adds custom headers that flow through to every Prometheus request.
What changed
Config + CLI
prometheusHeaders: { ... }map in~/.radar/config.json--prometheus-header Key=ValueCLI flag (repeatable). CLI flags override file defaults outright rather than merging — matches kubectl semantics.Wire-level
internal/prometheus/client.go— headers attached indoQueryandprobe, preserved across context-switch reinit.internal/traffic/caretta.go— same treatment in all 3 Prometheus request sites. Without this the topology traffic view would silently 401 against the same authed endpoint.Helm chart
traffic.prometheusHeaders: {}renders into repeated--prometheus-headerargs on the deployment, properly quoted (matches the pattern used forauth.secret/cloud.tokenin the same template). For secret-bearing values, the chart README recommends external secret stores (sealed-secrets, external-secrets).Misc
PUT /api/configautomatically (the dialog preserves unknown fields), so editing the config file or using the CLI is the v1 path.Safety / correctness
httpguts.ValidHeaderFieldName/Valueat parse time — applied both to CLI flags and to headers loaded from~/.radar/config.json. Rejects CR/LF in values (classic header-injection vector) and invalid characters in keys, instead of lettingnet/httpeither refuse opaquely or silently corrupt the request. File-loaded headers that fail validation are dropped with a clear startup log line rather than producing cryptic transport errors at first request.errorlog.Record(...)makes the auth failure visible in the diagnostics overlay.prometheus.Reinitialize— was readingglobalClient.headers/manualURLwhile holding onlyclientMu;SetHeadersandSetURL-the-method write those underc.muon independent code paths. Now snapshots underc.mu.RLock+copyHeaders. Verified clean undergo test -race.applyHeaderstookc.mu.RLock(), but it's called fromtryMetricsEndpointLockedwhich runs underc.mu.Lock()(write). Go'sRWMutexisn't reentrant → would have deadlocked every Caretta user on the first metrics probe. Removed the lock entirely;CarettaSource.headersis set once insideinitOnce.Doand never mutated.Secret handling
Headers are stored in plain text in
~/.radar/config.json. This matches the file's existing trust level (kubeconfig paths, etc.) but it's called out in the docs so users with stricter requirements know to template from a secret store rather than checking values into their Helm values file.GET /api/configredactsPrometheusHeadersfrom both thefileandeffectivepayloads — values would otherwise be returned in plaintext to anyone with API access (matters for auth-enabled deployments and embedded uses like Radar Hub; localhost-defaultauth-mode=nonewas already practically safe but the inconsistency vs. the diagnostics endpoint, which already masks them as a presence bool, was the bug).PUT /api/configpreserves the on-disk value so a UI round-trip through the redacted GET can't silently wipe auth headers.Tests
internal/config: round-trip test extended to cover the new field.internal/prometheus: headers reach the wire on bothdoQueryandprobe; noAuthorizationheader sent when none is configured (avoids tripping picky reverse proxies).internal/traffic: newcaretta_test.gocoveringapplyHeadersonqueryPrometheusRaw+tryMetricsEndpointLocked— the latter acquires the write lock first so the production lock shape is reproduced (and the deadlock regression can't come back).cmd/explorer: newmain_test.gowith table-driven coverage ofheaderFlag—Key=Valueparsing including=in Bearer-token values, CRLF / invalid-name rejection, the kubectl-style "first CLI flag wipes file defaults" latch, and defensive-copy semantics onvalue().go test -race. Helm template render verified manually.