Skip to content

http client config: Allow overwriting Host Header #13157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

http client config: Allow overwriting Host Header #13157

wants to merge 1 commit into from

Conversation

jkroepke
Copy link
Member

@jkroepke jkroepke commented Nov 17, 2023 •
edited
Loading

This PR allow to overwrite the HTTP Host header via headers

remote_write:
- name: remote
  url: https://20.0.0.1/api/v1/write
  headers:
    Host: prometheus.example.com
  tls_config:
    server_name: prometheus.example.com # this already works

@jkroepke
http client config: Allow overwriting Host Header
0377b56 
Signed-off-by: Jan-Otto Kröpke <joe@cloudeteer.de>
@jkroepke jkroepke mentioned this pull request Nov 17, 2023
Closed
@machine424
Copy link
Member

machine424 commented Nov 17, 2023 •
edited
Loading

I think that code will never be reached because of:

prometheus/config/config.go

Lines 43 to 47 in f997c72

reservedHeaders = map[string]struct{}{
// NOTE: authorization is checked specially,
// see RemoteWriteConfig.UnmarshalYAML.
// "authorization": {},
"host": {},

Prometheus does not accept setting such header in the configuration:

prometheus/config/config.go

Lines 1031 to 1047 in f997c72

func (c *RemoteWriteConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
*c = DefaultRemoteWriteConfig
type plain RemoteWriteConfig
if err := unmarshal((*plain)(c)); err != nil {
return err
}
if c.URL == nil {
return errors.New("url for remote_write is empty")
}
for _, rlcfg := range c.WriteRelabelConfigs {
if rlcfg == nil {
return errors.New("empty or null relabeling rule in remote write config")
}
}
if err := validateHeaders(c.Headers); err != nil {
return err
}

Have you tried proxy_url?

@jkroepke
Copy link
Member Author

jkroepke commented Nov 17, 2023 •
edited
Loading

I think that code will never be reached because of:

Ah. I see. I'm coming from https://github.com/grafana/agent which is using the prometheus libaries without the config validations.

I did not tried proxy_url. I expect with proxy, the HTTP CONNECT will be used to etablish an https connection over an proxy with is not supported by prometheus.

In my case, there is no proxy between client and server.

@machine424 What did you think about removing the host header from the reservedHeaders? Same for User-Agent, since Prometheus Agents exists, I would give each agent a distinct User-Agent

@machine424
Copy link
Member

I don't fully understand what you're trying to achieve by setting Host, could you elaborate?

Regarding User-Agent, I think importers can already customize it https://github.com/prometheus/prometheus/pull/7832/files and I don't think that making it customizable for a prometheus instance is a good decision as the header isn't meant to fully identify the client https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent, so this may be confusing and misleading. Maybe another header (a custom one maybe) should be used for that.

@jkroepke
Copy link
Member Author

I don't fully understand what you're trying to achieve by setting Host, could you elaborate?

Yes:

With Prometheus, I have to write metrics to a endpoint, for example https://prometheus.example.com/api/v1/write.

This issue, I have to do this in enterprise-grade environments. In such environments DNS can also have some issues which are impossible so fix or solve. I have to provide a DNS agonistic solution here is barely base on IP addresses.

As workaround, I would like to configure Prometheus to call the IP directly, for example: https://20.0.0.1/api/v1/write. The issue is that the remote Prometheus is listen on nginx Virtual Host and only response, if the Host header is prometheus.example.com. By default, the Host is extracted from the URL which is 20.0.0.1 in this example and I would like to overwrite the default behavior.

An other solution would be /etc/hosts, but it does require high OS permission (admin).

@roidelapluie
Copy link
Member

I will look at this, but better, look at generic HTTP headers config.

@jkroepke
Copy link
Member Author

Thanks @roidelapluie, looking forward.

@cstyan
Copy link
Member

cstyan commented Nov 20, 2023

The config does say

# Custom HTTP headers to be sent along with each remote write request.
# Be aware that headers that are set by Prometheus itself can't be overwritten.
headers:
  [ [<string>](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#string): [<string>](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#string) ... ]

but honestly I don't know what those headers are that Prometheus sets itself. At least in the client.Store that remote write calls it should be

 210   │     httpReq.Header.Add("Content-Encoding", "snappy")
 211   │     httpReq.Header.Set("Content-Type", "application/x-protobuf")
 212   │     httpReq.Header.Set("User-Agent", UserAgent)

and

 222   │         httpReq.Header.Set("Retry-Attempt", strconv.Itoa(attempt))

@jkroepke
Copy link
Member Author

but honestly I don't know what those headers are that Prometheus sets itself

Various headers are set by RoundTrip functions that defined on the http client.

prometheus/storage/remote/client.go

Lines 182 to 187 in aaf5d64

func (t *injectHeadersRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
for key, value := range t.headers {
req.Header.Set(key, value)
}
return t.RoundTripper.RoundTrip(req)
}

prometheus/storage/remote/azuread/azuread.go

Lines 182 to 192 in aaf5d64

// RoundTrip sets Authorization header for requests.
func (rt *azureADRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
accessToken, err := rt.tokenProvider.getAccessToken(req.Context())
if err != nil {
return nil, err
}
bearerAccessToken := "Bearer " + accessToken
req.Header.Set("Authorization", bearerAccessToken)
return rt.next.RoundTrip(req)
}

Other less specific RoundTrip are defined in prometheus/common

https://github.com/prometheus/common/blob/6ff04000d9b52262fe36f82f51ab43caa45fb8e6/config/http_config.go#L1176-L1180

Ref: https://github.com/prometheus/common/blob/6ff04000d9b52262fe36f82f51ab43caa45fb8e6/config/http_config.go#L498

After looking a bit deeper in the code, I feel it make more sense to add a dedicated hostname property to prometheus/common http client. I guess that would resolve the my issue on the root level. Any opinion for this idea? it would also resolve my issue in other software components like Grafana Loki.

@jkroepke jkroepke mentioned this pull request Dec 8, 2023
Merged
@jkroepke
Copy link
Member Author

jkroepke commented Dec 8, 2023

Superseded by prometheus/common#549

@jkroepke jkroepke closed this Dec 8, 2023
@jkroepke jkroepke deleted the host-header branch December 8, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csmarchbanks csmarchbanks Awaiting requested review from csmarchbanks

@cstyan cstyan Awaiting requested review from cstyan cstyan is a code owner

@bwplotka bwplotka Awaiting requested review from bwplotka bwplotka is a code owner

@tomwilkie tomwilkie Awaiting requested review from tomwilkie tomwilkie is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkroepke @machine424 @roidelapluie @cstyan