Skip to content

Commit

Permalink
fix: invalidated config should retain error (#1068)
Browse files Browse the repository at this point in the history
When a TLS handshake fails, the client would invalidate the
configuration without recording the associated error. When the liveness
check runs, it would panic when trying to print the invalidated
configuration's error because the error was nil. This commit ensures
that when the proxy invalidates a configuration, it includes the error.

Fixes #1065.
  • Loading branch information
enocom authored Jan 20, 2022
1 parent 4c63ec3 commit 49d3003
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
4 changes: 3 additions & 1 deletion proxy/proxy/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func (c *Client) selectDialer() func(context.Context, string, string) (net.Conn,
}
}

func (c *Client) invalidateCfg(cfg *tls.Config, instance string) {
func (c *Client) invalidateCfg(cfg *tls.Config, instance string, err error) {
c.cacheL.RLock()
e := c.cfgCache[instance]
c.cacheL.RUnlock()
Expand All @@ -540,7 +540,9 @@ func (c *Client) invalidateCfg(cfg *tls.Config, instance string) {
if e.cfg != cfg {
return
}
err = fmt.Errorf("config invalidated after TLS handshake failed, error = %w", err)
c.cfgCache[instance] = cacheEntry{
err: err,
done: e.done,
lastRefreshed: e.lastRefreshed,
}
Expand Down
22 changes: 22 additions & 0 deletions proxy/proxy/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,29 @@ func TestClientHandshakeCanceled(t *testing.T) {
_, err := c.DialContext(ctx, instance)
validateError(t, err)
})
})

t.Run("when liveness check is called on invalidated config", func(t *testing.T) {
withTestHarness(t, func(port int) {
c := newClient(port)

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()

_, err := c.DialContext(ctx, instance)
if err == nil {
t.Fatal("expected DialContext to fail, got no error")
}

invalid := c.InvalidInstances()
if gotLen := len(invalid); gotLen != 1 {
t.Fatalf("invalid instance want = 1, got = %v", gotLen)
}
got := invalid[0]
if got.err == nil {
t.Fatal("want invalid instance error, got nil")
}
})
})

// Makes it to Handshake.
Expand Down
2 changes: 1 addition & 1 deletion proxy/proxy/connect_tls_117.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (c *Client) connectTLS(
// this file is conditionally compiled on only Go versions >= 1.17.
if err := ret.HandshakeContext(ctx); err != nil {
_ = ret.Close()
c.invalidateCfg(cfg, instance)
c.invalidateCfg(cfg, instance, err)
return nil, err
}
return ret, nil
Expand Down
2 changes: 1 addition & 1 deletion proxy/proxy/connect_tls_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (c *Client) connectTLS(
ret := tls.Client(conn, cfg)
if err := ret.Handshake(); err != nil {
_ = ret.Close()
c.invalidateCfg(cfg, instance)
c.invalidateCfg(cfg, instance, err)
return nil, err
}
return ret, nil
Expand Down

0 comments on commit 49d3003

Please sign in to comment.