fix(exporter): forward Authorization header from zxp to zot metrics endpoint#4019
Open
AkashKumar7902 wants to merge 1 commit into
Open
fix(exporter): forward Authorization header from zxp to zot metrics endpoint#4019AkashKumar7902 wants to merge 1 commit into
AkashKumar7902 wants to merge 1 commit into
Conversation
…ndpoint zxp scrapes zot's /metrics endpoint to translate the JSON payload into Prometheus format, but until now it always sent an unauthenticated request. With htpasswd or any other authentication enabled on zot, the scrape returned 401 and the JSON decoder silently produced an empty MetricsInfo, so zxp only emitted go_*/process_* metrics and the zot_up gauge was never set to 0 to surface the failure. This change: - Replaces the global prometheus.Register + promhttp.Handler() wiring in zxp with a per-request handler that builds a fresh registry, copies the inbound Authorization header(s), and registers a collector clone scoped to that request via Collector.WithHeaders. - Adds MetricsClient.WithHeaders so the underlying scrape carries the forwarded credentials to zot, and applies any configured headers on every outgoing request in makeGETRequest. - Treats non-2xx responses from zot as scrape failures so zot_up flips to 0 (and the error is logged) instead of silently decoding an HTTP error body as metrics. Fixes project-zot#3263 Signed-off-by: Akash Kumar <meakash7902@gmail.com>
Contributor
|
@AkashKumar7902 do you plan to work on these PRs? Some of them have been moved out of draft state (ones we like). |
Contributor
Author
|
@rchincha yes |
Contributor
|
@AkashKumar7902 pls fix the merge conflicts |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4019 +/- ##
==========================================
+ Coverage 91.42% 91.44% +0.01%
==========================================
Files 197 197
Lines 28152 28179 +27
==========================================
+ Hits 25739 25768 +29
+ Misses 1562 1560 -2
Partials 851 851 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
What type of PR is this?
bug
Which issue does this PR fix:
Fixes #3263
What does this PR do / Why do we need it:
When zot is configured with authentication (e.g. htpasswd), the
zxpnode exporter could not scrape zot's/metricsendpoint because it always issued an unauthenticated request. The JSON decoder silently consumed the 401 error body, so the resulting Prometheus output was missing everyzot_*metric (onlygo_*/process_*showed up) andzot_upnever flipped to 0 to surface the failure.This PR makes the exporter forward the
Authorizationheader from the inbound/metricsrequest to the upstream zot scrape:pkg/exporter/api/exporter.goswaps the globalprometheus.Register+promhttp.Handler()wiring for a per-request handler. Each request now builds a freshprometheus.Registryand registers a collector clone scoped to that request viaCollector.WithHeaders, which carries the forwardedAuthorizationheader(s) into the scrape.pkg/extensions/monitoring/minimal_client.goaddsMetricsClient.WithHeadersand copies any configured headers onto every outgoing request inmakeGETRequest. Non-2xx responses are now treated as scrape failures instead of being decoded as JSON, sozot_upcorrectly reports 0 and the underlying error is logged.Behavior with no auth on zot is unchanged because no
Authorizationheader is forwarded when none is sent.If an issue # is not available please add repro steps and logs showing the issue:
N/A — see #3263 for repro steps and logs.
Testing done on this change:
TestMetricsHandlerForwardsAuthorizationHeaderandTestMetricsHandlerReportsZotDownWithoutAuthorizationHeaderinpkg/exporter/api/exporter_internal_test.gowhich spin up an httptest server that requires a Basic Authorization header. The first test asserts the exporter forwards the header (200 +zot_up 1); the second asserts that without the header the scrape is treated as a failure (zot_up 0).TestMetricsClientForwardsHeadersAndRejectsNonSuccessStatusinpkg/extensions/monitoring/minimal_client_test.gowhich verifies that an unauthenticatedGetMetrics()call now returnsunexpected status code 401(instead of decoding the error body) and that callingWithHeadersproduces a client whose request carries theAuthorizationheader.go build ./pkg/exporter/... ./pkg/extensions/monitoring/...go test ./pkg/exporter/api/... ./pkg/extensions/monitoring/...— both packages pass locally.gofmt -landgo vetclean on the touched files.Automation added to e2e:
No e2e change. The fix is fully covered by the new unit/internal tests against an httptest server that mirrors a 401-protected zot. An e2e test that boots a real htpasswd-protected zot under zxp can be tracked separately if desired.
Will this break upgrades or downgrades?
No. The wire format of the
/metricsendpoint is unchanged. Existing zxp deployments that do not pass anAuthorizationheader continue to work exactly as before; the only behavior change is that scrape failures (any non-2xx from zot) now correctly surface aszot_up 0rather than an empty success.Does this PR introduce any user-facing change?:
Yes — operators running
zxpagainst a zot with authentication enabled can now scrape it by forwarding their Prometheus credentials (e.g.basic_authin the Prometheus scrape config pointed atzxp). zxp passes the inboundAuthorizationheader straight through to zot.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.