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

api: create fresh http client for unix sockets #8602

Merged
merged 2 commits into from
Sep 6, 2020
Merged

api: create fresh http client for unix sockets #8602

merged 2 commits into from
Sep 6, 2020

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Sep 2, 2020

The HTTP client over unix socket needs a special transport object,
and in doing so we need to create a fresh HTTP client so that we
pickup the environment variable based configuration options.

The HTTP client over unix socket needs a special transport object,
and in doing so we need to create a fresh HTTP client so that we
pickup the environment variable based configuration options.
@shoenig
Copy link
Member Author

shoenig commented Sep 2, 2020

This problem was uncovered when trying to run a connect-native task in Nomad using bridge networking. Nomad creates a unix socket through which the task can proxy communications with consul. However, it breaks when TLS is enabled.

2020/09/02 14:51:56 environment CONSUL_HTTP_ADDR = unix:///alloc/tmp/consul_http.sock
2020/09/02 14:51:56 environment CONSUL_NAMESPACE = <unset>
2020/09/02 14:51:56 environment CONSUL_CACERT = /secrets/consul_ca_file.pem
2020/09/02 14:51:56 environment CONSUL_CLIENT_CERT = /secrets/consul_cert_file.pem
2020/09/02 14:51:56 environment CONSUL_CLIENT_KEY = /secrets/consul_key_file.pem
2020/09/02 14:51:56 environment CONSUL_HTTP_SSL = true
2020/09/02 14:51:56 environment CONSUL_HTTP_SSL_VERIFY = true
2020/09/02 14:51:56 environment CONSUL_TLS_SERVER_NAME = localhost
2020/09/02 14:51:56 environment CONSUL_GRPC_ADDR = <unset>
2020/09/02 14:51:56 environment CONSUL_HTTP_TOKEN_FILE = <unset>
2020/09/02 14:51:56 starting generator API, listening on 0.0.0.0:26967
2020-09-02T14:51:56.328Z [ERROR] connect.watch: Watch errored: service=uuid-api type=connect_leaf error="Get "https://%2Falloc%2Ftmp%2Fconsul_http.sock/v1/agent/connect/ca/leaf/uuid-api": x509: certificate is valid for server.dc1.consul, localhost, not /alloc/tmp/consul_http.sock" retry=5s
2020-09-02T14:51:56.328Z [ERROR] connect.watch: Watch errored: service=uuid-api type=connect_roots error="Get "https://%2Falloc%2Ftmp%2Fconsul_http.sock/v1/agent/connect/ca/roots": x509: certificate is valid for server.dc1.consul, localhost, not /alloc/tmp/consul_http.sock" retry=5s
2020-09-02T14:52:01.329Z [ERROR] connect.watch: Watch errored: service=uuid-api type=connect_roots error="Get "https://%2Falloc%2Ftmp%2Fconsul_http.sock/v1/agent/connect/ca/roots": x509: certificate is valid for server.dc1.consul, localhost, not /alloc/tmp/consul_http.sock" retry=20s
2020-09-02T14:52:01.330Z [ERROR] connect.watch: Watch errored: service=uuid-api type=connect_leaf error="Get "https://%2Falloc%2Ftmp%2Fconsul_http.sock/v1/agent/connect/ca/leaf/uuid-api": x509: certificate is valid for server.dc1.consul, localhost, not /alloc/tmp/consul_http.sock" retry=20s

Note that the environment variable CONSUL_TLS_SEVER_NAME is set to localhost, but the Consul API library is ignoring the value and using the unix socket path as the server name, which of course is invalid.

With this change, the same connect native test program works correctly.

2020/09/02 15:11:45 environment CONSUL_HTTP_ADDR = unix:///alloc/tmp/consul_http.sock
2020/09/02 15:11:45 environment CONSUL_NAMESPACE = <unset>
2020/09/02 15:11:45 environment CONSUL_CACERT = /secrets/consul_ca_file.pem
2020/09/02 15:11:45 environment CONSUL_CLIENT_CERT = /secrets/consul_cert_file.pem
2020/09/02 15:11:45 environment CONSUL_CLIENT_KEY = /secrets/consul_key_file.pem
2020/09/02 15:11:45 environment CONSUL_HTTP_SSL = true
2020/09/02 15:11:45 environment CONSUL_HTTP_SSL_VERIFY = true
2020/09/02 15:11:45 environment CONSUL_TLS_SERVER_NAME = localhost
2020/09/02 15:11:45 environment CONSUL_GRPC_ADDR = <unset>
2020/09/02 15:11:45 environment CONSUL_HTTP_TOKEN_FILE = <unset>
2020/09/02 15:11:45 starting generator API, listening on 0.0.0.0:29859
$ curl -s -o /dev/null -w '%{response_code}\n' localhost:9800
200

Change suggested by @mkeeler, thanks!

Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Could you add a changelog entry in the .changelog directory. The file name should be 8602.txt and probably look similar to this one https://github.com/hashicorp/consul/blob/master/.changelog/7628.txt

@mkeeler
Copy link
Member

mkeeler commented Sep 2, 2020

Actually @shoenig I will just fix up the changelog.

@mkeeler mkeeler merged commit 9fab3fe into master Sep 6, 2020
@mkeeler mkeeler deleted the b-unix-tls branch September 6, 2020 16:27
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 9fab3fe onto release/1.8.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Sep 6, 2020
Co-authored-by: Matt Keeler <mkeeler@users.noreply.github.com>
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 this pull request may close these issues.

None yet

3 participants