grafana / dskit

Distributed systems kit
Apache License 2.0
480 stars 68 forks source link

consider consul with TLS #346

Open mxab opened 1 year ago

mxab commented 1 year ago

Hi,

I'm currently trying to setup loki with consul.

As you use the consul.NewClient but pass in a custom httpclient

Consul ignores its environment variables (CONSUL_CACERT, CONSUL_CLIENT_CERT, CONSUL_CLIENT_KEY) that point to the TLS parts https://github.com/hashicorp/consul/blob/main/api/api.go#L706

It looks like the only reason that you pass in a custom client is to use the clean http transport and the timeout.

the transport could be passed in seperatly https://github.com/hashicorp/consul/blob/main/api/api.go#L678

for the timeout you could do this afterwards

So instead of

client, err := consul.NewClient(&consul.Config{
        Address: cfg.Host,
        Token:   cfg.ACLToken.String(),
        Scheme:  "http",
        HttpClient: &http.Client{
            Transport: cleanhttp.DefaultPooledTransport(),
            // See https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/
            Timeout: cfg.HTTPClientTimeout,
        },
    })

Something like this:


config := &consul.Config{
        Address: cfg.Host,
        Token:   cfg.ACLToken.String(),
        Scheme:  "http", 
                Transport: cleanhttp.DefaultPooledTransport(),
    }
client, err := consul.NewClient(config)

// not nice, but should work I guess :)
config.HttpClient.Timeout =  cfg.HTTPClientTimeout
ivantopo commented 2 weeks ago

Just wanted to ping on this issue as we are now also securing access to Consul and this is the only blocker we have for completely disabling HTTP on Consul.