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

Support configuring ServerName for HTTP health checks #9473

Closed
cbroglie opened this issue Dec 28, 2020 · 0 comments · Fixed by #9475
Closed

Support configuring ServerName for HTTP health checks #9473

cbroglie opened this issue Dec 28, 2020 · 0 comments · Fixed by #9475
Labels
theme/health-checks Health Check functionality type/enhancement Proposed improvement or new feature

Comments

@cbroglie
Copy link
Contributor

cbroglie commented Dec 28, 2020

Feature Description

Some TLS servers require SNI, but the Golang HTTP client doesn't include it in the ClientHello when connecting to an IP address.

Use Case(s)

We ran into this use case while trying to health check external services using consul-esm. Things work fine if we register them using hostnames as addresses, but if we use IP addresses the SNI is not set and handshakes fail.

In #3936 there was a suggestion to add a new TLSServerName field to the check definition: #3936 (comment). I believe the same TLSServerName field could be used here to optionally set TLSClientConfig.ServerName, and the change would be backwards compatible. If that sounds reasonable, I'd be happy to provide a PR addressing this.

@jsosulska jsosulska added theme/health-checks Health Check functionality type/enhancement Proposed improvement or new feature labels Feb 5, 2021
dnephin pushed a commit to cbroglie/consul that referenced this issue Mar 16, 2021
Some TLS servers require SNI, but the Golang HTTP client doesn't
include it in the ClientHello when connecting to an IP address. This
change adds a new TLSServerName field to health check definitions to
optionally set it. This fixes hashicorp#9473.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/health-checks Health Check functionality type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants