Skip to content

feat: add httpHeaders to ServiceMonitor and PodMonitor endpoints - #8729

Open
somaz94 wants to merge 2 commits into
prometheus-operator:mainfrom
somaz94:feat/scrape-http-headers
Open

somaz94 wants to merge 2 commits into
prometheus-operator:mainfrom
somaz94:feat/scrape-http-headers

Conversation

@somaz94

@somaz94 somaz94 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Prometheus supports http_headers in scrape configurations since v2.55.0, but there was no way to set them from a ServiceMonitor or a PodMonitor.

This adds an httpHeaders field to the ServiceMonitor endpoints and the PodMonitor pod metrics endpoints. Only plain values are supported for now, secrets and files can be added later as optional fields without breaking the API. Header names reserved by Prometheus (Authorization, Host, ...) are rejected during the resource selection, reusing config.ReservedHeaders from prometheus/common.

For context: #7003 added http_headers to oauth2, remote_read, remote_write and ScrapeConfig, but not to the ServiceMonitor/PodMonitor endpoints this issue is about. Probe and ScrapeConfig endpoints can follow the same pattern in separate pull requests.

Closes: #7189

Type of change

  • FEATURE (non-breaking change which adds functionality)

Verification

make generate, make check and make test-unit pass locally.

New unit tests:

  • TestServiceMonitorEndpointHTTPHeaders and TestPodMonitorEndpointHTTPHeaders with golden files for v2.55.0 (rendered) and v2.54.0 (dropped with a warning).
  • New cases in TestSelectServiceMonitors and TestSelectPodMonitors for a valid header, a reserved header name and a header without value.

No end-to-end test since the change only affects configuration generation, which the golden files cover.

Changelog entry

Add `httpHeaders` field to the ServiceMonitor and PodMonitor endpoints to set custom HTTP headers on scrape requests. It requires Prometheus >= v2.55.0.

Generative AI (Claude Code) was used to write parts of this change. I reviewed and tested it myself.

@somaz94
somaz94 marked this pull request as ready for review July 30, 2026 02:33
@somaz94
somaz94 requested a review from a team as a code owner July 30, 2026 02:33

// values defines the values of the HTTP header.
//
// +kubebuilder:validation:MinItems=1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:items:MinLength=1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 2079ca2b8 — thanks. items:MinLength=1 matches the existing convention in prometheus_types.go, probe_types.go and scrapeconfig_types.go, so the rendered schema now has items: {minLength: 1, type: string} alongside the existing minItems: 1.

//
// It requires Prometheus >= v2.55.0.
//
// +listType=map

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +listType=map
// +kubebuilder:validation:MinItems=1
// +listType=map

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 2079ca2b8, and I added the same MinItems=1 to Endpoint.HTTPHeaders in types.go as well — the ServiceMonitor field is identical, and marking only PodMetricsEndpoint would have left the two CRDs with different validation. Regenerated artifacts are in the same commit (CRDs, jsonnet, bundle.yaml); api.md is unchanged because that generator doesn't render validation constraints.

One thing I left out deliberately: HTTPHeader.Validate() rejects an empty values list but doesn't check for empty strings, so semantic validation is now slightly looser than the schema. Happy to tighten it if you'd like, but I kept this commit scoped to the markers.

@simonpasquier

Copy link
Copy Markdown
Contributor

it needs a rebase.

@somaz94
somaz94 force-pushed the feat/scrape-http-headers branch from 2079ca2 to 977c2a0 Compare September 4, 2026 01:31
Prometheus supports custom HTTP headers in scrape configurations since
v2.55.0 but the operator had no way to set them for ServiceMonitor and
PodMonitor endpoints.

Header names which are managed by Prometheus itself are rejected during
the resource selection, using the reserved list from prometheus/common.

Fixes prometheus-operator#7189

Signed-off-by: somaz <genius5711@gmail.com>
Signed-off-by: somaz <genius5711@gmail.com>
@somaz94
somaz94 force-pushed the feat/scrape-http-headers branch from 977c2a0 to 42ff3a9 Compare September 4, 2026 06:42
@somaz94

somaz94 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main, no conflicts. Thanks!

@slashpai

slashpai commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Test need to be fixed

=== RUN   TestAllNS/x/AMMinReadySeconds
    alertmanager_test.go:2643: 
        	Error Trace:	/home/runner/work/prometheus-operator/prometheus-operator/test/e2e/alertmanager_test.go:2643
        	Error:      	Received unexpected error:
        	            	failed to update Alertmanager: alertmanager allns-x-amminreadyseconds-tktvc1-1-e94615f8/basic-am failed to become available: expected 3 replicas, got 0: context deadline exceeded
        	Test:       	TestAllNS/x/AMMinReadySeconds

@somaz94

somaz94 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

That test doesn't touch anything this PR changes — the diff is ServiceMonitor/PodMonitor only, with no alertmanager paths — and the same job passed on this branch five hours earlier in run 33826114913. In the failing run 54 of the 55 alertmanager subtests passed, and the job has failed on three unrelated branches this week (a dependabot bump, the msteams e2e case, the global-config e2e case), each time on a different subtest with a context deadline. That reads like a timing flake rather than a regression from this change. Could you re-run it? I can't trigger a re-run from the fork, and I'd rather not push a no-op commit if it's avoidable.

@simonpasquier

Copy link
Copy Markdown
Contributor

Thanks for doing this and sorry in advance for the late reply. Storing header values directly into the custom resources poses a security risk since users may accidentally leak sensitive data (e.g. API keys) into them. Though the risk is low, it exists (Prometheus supports both clear-text, file and secret values) and I'd prefer that we adopt a safe approach by using secret key references.

@somaz94

somaz94 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense. Prometheus's own Header type supports values, secrets and files side by side, so I'd add a secrets field of []SecretKeySelector next to values on HTTPHeader and resolve it through the assets store the same way proxyConnectHeader already does. Do you want values dropped entirely, or kept for non-sensitive headers like the X-Scope-OrgID: tenant-a multi-tenancy case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http_headers in PodMonitor (PodMetricsEndpoint) and ServiceMonitor (Endpoint)

4 participants