Skip to content
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

consider consul with TLS #346

Open
mxab opened this issue Aug 4, 2023 · 1 comment · May be fixed by #348
Open

consider consul with TLS #346

mxab opened this issue Aug 4, 2023 · 1 comment · May be fixed by #348

Comments

@mxab
Copy link

mxab commented Aug 4, 2023

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants