Skip to content
This repository was archived by the owner on Mar 5, 2024. It is now read-only.

Fix TLS when a port is specified #86

Merged
merged 1 commit into from
Jun 8, 2018

Conversation

idiamond-stripe
Copy link
Contributor

@idiamond-stripe idiamond-stripe commented Jun 7, 2018

This fixes a bug in the TLS config that does not allow users to set custom ports. This manifests with a timeout error.

  • Fix TLS config by properly splitting address into host and port
  • Add nicer error messages around creating GRPC connection
  • Clean up gateway code a bit

- Fix TLS config by properly splitting address into host and port
- Add nicer error messages around creating GRPC connection
- Clean up gateway code a bit
@pingles
Copy link
Contributor

pingles commented Jun 7, 2018

@idiamond-stripe perfect, thanks! I think @kevtaylor was hitting a problem related to the address stuff recently. This looks like it may sort it.

As with others- I'd like to test this in some of our testing clusters before I merge but all looks good. Thanks for helping to tidy it up too!

@pingles pingles added this to the 3.0 milestone Jun 7, 2018
@pingles pingles merged commit 8f577a5 into uswitch:master Jun 8, 2018
@pingles
Copy link
Contributor

pingles commented Jun 8, 2018

This fix will potentially break users that already have certs generated with port numbers in the server name:

INFO: 2018/06/08 09:30:50 grpc: failed dns SRV record lookup due to lookup _grpclb._tcp.localhost on 100.64.0.10:53: no such host.
INFO: 2018/06/08 09:30:50 balancerWrapper: got update addr from Notify: [{127.0.0.1:443 <nil>} {[::1]:443 <nil>}]
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: new subconn: [{[::1]:443 0  <nil>}]
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: new subconn: [{127.0.0.1:443 0  <nil>}]
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e270, CONNECTING
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e2c0, CONNECTING
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e2c0, TRANSIENT_FAILURE
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e2c0, CONNECTING
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e270, TRANSIENT_FAILURE
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e270, CONNECTING
INFO: 2018/06/08 09:30:50 ccBalancerWrapper: updating state and picker called by balancer: CONNECTING, 0xc4203b42a0
WARNING: 2018/06/08 09:30:50 Failed to dial 127.0.0.1:443: connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for kiam-server:443, localhost:443, localhost:9610, not localhost"; please retry.
INFO: 2018/06/08 09:30:50 balancerWrapper: handle subconn state change: 0xc42023e2c0, SHUTDOWN

I'm going to edit the documentation around generating TLS certificates so they're up-to-date and will make a mental note to ensure that the release notes for 3.0 (which this will go into) highlight this as a potentially breaking change.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants