Add support for TLS configuration in NSQ input to reach HTTPS nsqd endpoints#3903
Add support for TLS configuration in NSQ input to reach HTTPS nsqd endpoints#3903danielnelson merged 4 commits intoinfluxdata:masterfrom Scalingo:inputs/nsq/tls
Conversation
danielnelson
added
the
feat
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("fail to build tls config: %v", err) | ||
| } |
There was a problem hiding this comment.
I think this is going to act a little strange if the tls.Config isn't created succesfully, because the second time Gather is called it will not return an error here. You may want to look at the nginx input for a good example.
plugins/inputs/nsq/nsq.go
Outdated
|
|
||
| var tr = &http.Transport{ | ||
| ResponseHeaderTimeout: time.Duration(3 * time.Second), | ||
| func (n *NSQ) buildTLSConfig() (*tls.Config, error) { |
There was a problem hiding this comment.
You can use internal.GetTLSConfig to do this: https://github.com/influxdata/telegraf/blob/master/internal/internal.go#L118
plugins/inputs/nsq/nsq.go
Outdated
| func (n *NSQ) getHttpClient() *http.Client { | ||
| n.httpClientOnce.Do(func() { | ||
| tr := &http.Transport{ | ||
| ResponseHeaderTimeout: time.Duration(3 * time.Second), |
There was a problem hiding this comment.
Could you remove the ResponseHeaderTimeout and only use the Timeout on http.Client?
|
@Soulou do you have time to address the changes requested by Daniel? |
|
My apologies for the delay, @glinton. I'm going to do them before the end of August |
|
Sorry to pester you @Soulou, but just looking for an update. Thanks |
…dpoints Goal: A completely secure NSQ doesn't have any HTTP endpoint, but only HTTPS, often with x509 client authentication. This PR aims at configuring the HTTP client to correctly connects to nsqd Choices: TLS configuration build is only done once, and the first `Gather()` will return an error, if configuration is invalid, then it will be silent. The HTTP client is only built once, to limit allocations
|
Rebased and updated, really sorry for the delay @glinton |
4 participants
Required for all PRs: