Change init of consul client to respect TLS

[mxab]

This PR changes the initialisation of the consul client.

As during the creation of a new consul client a lot of environment variables are taken as fallback default configs, passing in a own HttpClient prevents on picking the consul specific TLS environment variables.

Instead of passing in the complete new HttpClient it only passes the transport and sets the timeout on the HttpClient afterwards

Fixes #346

Checklist

• Tests updated
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[CLAassistant]

All committers have signed the CLA.

[bboreham]

Can you be a little more specific about "a lot of environment variables"?

After this PR the only comment is pointing to a justification for something rather different; can you add one on why the timeout is post-initialization?

[mxab]

Sorry for the confusing PR comment and I can also understand that the timeout parameters post-init looks strange :)

I'm trying to use loki with our consul which uses mTLS with self signed certificates.

I saw that you are using the official consul client to work with the KV store.

When using consul's NewClient(...

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,
  },
})

the init method sets already a lot of defaults it detects from environment variables if not explicitly provided as init parameters.

Some environment variables that would take already affect would be e.g. CONSUL_NAMESPACE , CONSUL_HTTP_TOKEN_FILE or even CONSUL_HTTP_ADDR, CONSUL_HTTP_TOKEN if the passed arguments would be empty ("")

The TLS related variables (CONSUL_CACERT, CONSUL_CLIENT_CERT, ...) are unfortunatelly not taken into account as they're skipped when a custom http client is passed in.

If you pass in the transport instead of the client, they would be used as far as I see it, hence the PR to align the usage of env vars.

The only downside is that that you would have to "sneak" in the timeout parameter after the init, which I fully understand looks very wired.
Preferably I would like to change consul clients parameters to pass in the timeout directly, but I thought I try this one first :)

[pracucci]

Hi! 👋 We recently merged #947 which changed the CI configuration for unit tests. To get CI passing on this PR, please rebase on the latest main branch:

git fetch origin main
git rebase origin/main
git push --force-with-lease

Thanks!