Skip to content

Commit

Permalink
fix: invalidated config should retain error
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 committed Jan 10, 2022
1 parent d1c2542 commit 3904508
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
3 changes: 2 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 @@ -541,6 +541,7 @@ func (c *Client) invalidateCfg(cfg *tls.Config, instance string) {
return
}
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)
}
i := invalid[0]
if want := "deadline exceeded"; !strings.Contains(i.err.Error(), want) {
t.Fatalf("invalid instance error want = %v, got = %v", want, i.Error())
}
})
})

// 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 3904508

Please sign in to comment.