-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Add hostname parameter #823
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,6 +120,23 @@ func probeHandler(w http.ResponseWriter, r *http.Request, c *config.Config, logg | |
return | ||
} | ||
|
||
if module.Prober == "http" { | ||
paramHost := params.Get("hostname") | ||
if paramHost != "" { | ||
if module.HTTP.Headers == nil { | ||
module.HTTP.Headers = make(map[string]string) | ||
} else { | ||
for name, value := range module.HTTP.Headers { | ||
if strings.Title(name) == "Host" && value != paramHost { | ||
http.Error(w, fmt.Sprintf("Host header defined both in module configuration (%s) and with parameter 'hostname' (%s)", value, paramHost), http.StatusBadRequest) | ||
return | ||
} | ||
} | ||
} | ||
module.HTTP.Headers["Host"] = paramHost | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is mutating the actual module, so all subsequent calls will get the same host header, if the hostname parameter is not set. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the review! I agree with the comments and I'll push a fix soon. However this one puzzles me because I cannot achieve the described behavior. I'll look into this anyway but it will be nice if you help me to fall into the problem. I tried myself but my tests failed to prove that parameter mutated the module. Here is a pastebin with the requests that I made; the exporter was not restarted during those tests and both debug output and server responses show that the module has not been changed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To reproduce, please add an unrelated HTTP header to the config |
||
} | ||
} | ||
|
||
sl := newScrapeLogger(logger, moduleName, target) | ||
level.Info(sl).Log("msg", "Beginning probe", "probe", module.Prober, "timeout_seconds", timeoutSeconds) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -341,9 +341,20 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr | |
|
||
httpClientConfig := module.HTTP.HTTPClientConfig | ||
if len(httpClientConfig.TLSConfig.ServerName) == 0 { | ||
// If there is no `server_name` in tls_config, use | ||
// the hostname of the target. | ||
httpClientConfig.TLSConfig.ServerName = targetHost | ||
// If there is no `server_name` in tls_config it makes | ||
// sense to use the host header value to avoid possible | ||
// TLS handshake problems. | ||
changed := false | ||
for name, value := range httpConfig.Headers { | ||
if strings.Title(name) == "Host" { | ||
httpClientConfig.TLSConfig.ServerName = value | ||
changed = true | ||
} | ||
} | ||
if !changed { | ||
// Otherwise use the hostname of the target. | ||
httpClientConfig.TLSConfig.ServerName = targetHost | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could set it first so we do not need !changed |
||
} | ||
} | ||
client, err := pconfig.NewClientFromConfig(httpClientConfig, "http_probe", pconfig.WithKeepAlivesDisabled()) | ||
if err != nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not really find this code elegant, maybe moving it to a dedicated function could help making it nicer.