From 39045081f4add5bc10ccdf9df0684140b6a19771 Mon Sep 17 00:00:00 2001
From: Eno Compton <enocom@google.com>
Date: Mon, 10 Jan 2022 15:11:46 -0700
Subject: [PATCH] fix: invalidated config should retain error

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.
---
 proxy/proxy/client.go            |  3 ++-
 proxy/proxy/client_test.go       | 22 ++++++++++++++++++++++
 proxy/proxy/connect_tls_117.go   |  2 +-
 proxy/proxy/connect_tls_other.go |  2 +-
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/proxy/proxy/client.go b/proxy/proxy/client.go
index 84c992b2e..e5565e7f3 100644
--- a/proxy/proxy/client.go
+++ b/proxy/proxy/client.go
@@ -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()
@@ -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,
 	}
diff --git a/proxy/proxy/client_test.go b/proxy/proxy/client_test.go
index 0d380d60c..a5b3bd515 100644
--- a/proxy/proxy/client_test.go
+++ b/proxy/proxy/client_test.go
@@ -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.
diff --git a/proxy/proxy/connect_tls_117.go b/proxy/proxy/connect_tls_117.go
index fcba5906f..a1d580217 100644
--- a/proxy/proxy/connect_tls_117.go
+++ b/proxy/proxy/connect_tls_117.go
@@ -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
diff --git a/proxy/proxy/connect_tls_other.go b/proxy/proxy/connect_tls_other.go
index 94c5cb3f5..574bcd564 100644
--- a/proxy/proxy/connect_tls_other.go
+++ b/proxy/proxy/connect_tls_other.go
@@ -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