From 5e0ff494a5ba19e869ee1f30cbb0822c7c2cdc05 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Thu, 4 Jun 2020 11:40:12 -0700
Subject: [PATCH] derp: change NewClient constructor to an option pattern

(The NewMeshClient constructor I added recently was gross in
retrospect at call sites, especially when it wasn't obvious that a
meshKey empty string meant a regular client)
---
 derp/derp_client.go              | 38 +++++++++++++++++++++++++-------
 derp/derp_test.go                |  2 +-
 derp/derphttp/derphttp_client.go |  2 +-
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/derp/derp_client.go b/derp/derp_client.go
index 9e537ee32..0f927a673 100644
--- a/derp/derp_client.go
+++ b/derp/derp_client.go
@@ -34,16 +34,38 @@ type Client struct {
 	readErr error // sticky read error
 }
 
-func NewClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logger.Logf) (*Client, error) {
-	noMeshKey := ""
-	return NewMeshClient(privateKey, nc, brw, logf, noMeshKey)
+// ClientOpt is an option passed to NewClient.
+type ClientOpt interface {
+	update(*clientOpt)
 }
 
-// NewMeshClient is the Client constructor for trusted clients that
-// are a peer in a cluster mesh.
+type clientOptFunc func(*clientOpt)
+
+func (f clientOptFunc) update(o *clientOpt) { f(o) }
+
+// clientOpt are the options passed to newClient.
+type clientOpt struct {
+	MeshKey string
+}
+
+// MeshKey returns a ClientOpt to pass to the DERP server during connect to get
+// access to join the mesh.
 //
-// An empty meshKey is equivalent to just using NewClient.
-func NewMeshClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logger.Logf, meshKey string) (*Client, error) {
+// An empty key means to not use a mesh key.
+func MeshKey(key string) ClientOpt { return clientOptFunc(func(o *clientOpt) { o.MeshKey = key }) }
+
+func NewClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logger.Logf, opts ...ClientOpt) (*Client, error) {
+	var opt clientOpt
+	for _, o := range opts {
+		if o == nil {
+			return nil, errors.New("nil ClientOpt")
+		}
+		o.update(&opt)
+	}
+	return newClient(privateKey, nc, brw, logf, opt)
+}
+
+func newClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logger.Logf, opt clientOpt) (*Client, error) {
 	c := &Client{
 		privateKey: privateKey,
 		publicKey:  privateKey.Public(),
@@ -51,7 +73,7 @@ func NewMeshClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf
 		nc:         nc,
 		br:         brw.Reader,
 		bw:         brw.Writer,
-		meshKey:    meshKey,
+		meshKey:    opt.MeshKey,
 	}
 	if err := c.recvServerKey(); err != nil {
 		return nil, fmt.Errorf("derp.Client: failed to receive server key: %v", err)
diff --git a/derp/derp_test.go b/derp/derp_test.go
index 0f44f217c..e7deae0fb 100644
--- a/derp/derp_test.go
+++ b/derp/derp_test.go
@@ -510,7 +510,7 @@ func newRegularClient(t *testing.T, ts *testServer, name string) *testClient {
 func newTestWatcher(t *testing.T, ts *testServer, name string) *testClient {
 	return newTestClient(t, ts, name, func(nc net.Conn, priv key.Private, logf logger.Logf) (*Client, error) {
 		brw := bufio.NewReadWriter(bufio.NewReader(nc), bufio.NewWriter(nc))
-		c, err := NewMeshClient(priv, nc, brw, logf, "mesh-key")
+		c, err := NewClient(priv, nc, brw, logf, MeshKey("mesh-key"))
 		if err != nil {
 			return nil, err
 		}
diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go
index 5e1fba498..ba166be03 100644
--- a/derp/derphttp/derphttp_client.go
+++ b/derp/derphttp/derphttp_client.go
@@ -282,7 +282,7 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien
 		return nil, 0, fmt.Errorf("GET failed: %v: %s", err, b)
 	}
 
-	derpClient, err := derp.NewMeshClient(c.privateKey, httpConn, brw, c.logf, c.MeshKey)
+	derpClient, err := derp.NewClient(c.privateKey, httpConn, brw, c.logf, derp.MeshKey(c.MeshKey))
 	if err != nil {
 		return nil, 0, err
 	}