@brian-brazil I'm having trouble implementing prometheus/common#129 into prometheus. I think the reimplementation of basicAuthRoundTripper breaks the ability to compose timeouts with the Authorization header.

Inside util/httpclient/client.go I have:

// NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the
// given config.HTTPClientConfig. The name is used as go-conntrack metric label.
func NewRoundTripperFromConfig(cfg config_util.HTTPClientConfig, name string) (http.RoundTripper, error) {
	client, err := config_util.NewHTTPClientFromConfig(&cfg)
	if err != nil {
		return nil, err
	}

	// The only timeout we care about is the configured scrape timeout.
	// It is applied on request. So we leave out any timings here.
	if client.Transport == nil {
		client.Transport = &http.Transport{}
	}
	// Apply http.Transport specific settings
	rt, ok := client.Transport.(*http.Transport)
	if ok {
		rt.Proxy = http.ProxyURL(cfg.ProxyURL.URL)
		rt.MaxIdleConns = 20000
		rt.MaxIdleConnsPerHost = 1000 // see https://github.com/golang/go/issues/13801
		rt.DisableKeepAlives = false
		rt.DisableCompression = true
		// 5 minutes is typically above the maximum sane scrape interval. So we can
		// use keepalive for all configurations.
		rt.IdleConnTimeout = 5 * time.Minute
		rt.DialContext = conntrack.NewDialContextFunc(
			conntrack.DialWithTracing(),
			conntrack.DialWithName(name),
		)
		client.Transport = rt
	}

	return client.Transport, nil
}

Prometheus isn't making successful authed requests though.

Hang on. It's a problem with my credentials.

After copying httputil over to the common repo I manually vendored it back into prometheus and then updated the imports/calls.

I'm going to test this out manually to verify the marathon case works.

Edit: It's working.

@brian-brazil Thanks for the prometheus/common merge. I cleaned up this branch and revendored that change here. I'm going to test it locally again just to double check, but this should be g2g.

Edit: It's working for me.

@brian-brazil Updated.

That test failed before on this branch too, but it's passing for me locally. All tests pass locally for me.

$ go test -race github.com/prometheus/prometheus/discovery/kubernetes -count 1
ok  	github.com/prometheus/prometheus/discovery/kubernetes	1.474s

That test is known to be flaky.

Thanks!

This broke basic auth configs without a username due to the new code now requiring them, see #4173.

Hi folks, what is the static on this PR? Our users could really benefit from this security fix. I can't see it documented in the published site yet? https://prometheus.io/docs/alerting/configuration/#http_config

Thanks,

Alex cc @LucasRoesler

@alexellis the option is already available in AlertManager v0.15.3 but it is true that it isn't documented. As it was implemented in github.com/prometheus/commong, it got added to AlertManager in stealth mode ;-)