Repository: GoogleCloudPlatform/cloudsql-proxy (go)

Issue:
Panic in liveness endpoint
## Bug Description

When this google cloud incident started https://status.cloud.google.com/incidents/NMcnk6aE8xMHHwRGmyry, our cloudsql proxy started being killed by liveness probes. They are configured according to this recommendation https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/main/examples/k8s-health-check#cloud-sql-proxy-health-checks. That's fine and expected, but the error message was very confusing:

```
[Health Check] Liveness failed: %!v(PANIC=Error method: runtime error: invalid memory address or nil pointer dereference)
```

There seems to be a panic somewhere in cloudsql proxy, but no more information was printed.

## Environment

1. OS type and version: GKE
2. Cloud SQL Proxy version (`./cloud_sql_proxy -version`): gcr.io/cloudsql-docker/gce-proxy:1.27.0-buster@sha256:5d3d9bb25542b5f786cdb983d052b5f9145c9abd0b7128688f545ae9ce060fca


Pull request:
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.

Patch:
diff --git a/proxy/proxy/client.go b/proxy/proxy/client.go
index e240a9f..6a913af 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()
@@ -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,
 	}
diff --git a/proxy/proxy/connect_tls_117.go b/proxy/proxy/connect_tls_117.go
index fcba590..a1d5802 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 94c5cb3..574bcd5 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


Test patch:
diff --git a/proxy/proxy/client_test.go b/proxy/proxy/client_test.go
index 906f52f..123b5ae 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)
+			}
+			got := invalid[0]
+			if got.err == nil {
+				t.Fatal("want invalid instance error, got nil")
+			}
+		})
 	})
 
 	// Makes it to Handshake.