derp: don't send duplicate add messagesbradfitz/dup_add

Updates #17816

Change-Id: I470f8f779beb91af9a8bea8eb12a5e6a2b12915d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

2 files changed, 28 insertions, 8 deletions

diff --git a/derp/derp_server.go b/derp/derp_server.go
index 2e17cbfe5..b372fbe4e 100644
--- a/derp/derp_server.go
+++ b/derp/derp_server.go
@@ -557,6 +557,7 @@ func (s *Server) registerClient(c *sclient) {
 	defer s.mu.Unlock()
 
 	cs, ok := s.clients[c.key]
+	isFirstConnInSet := !ok
 	if !ok {
 		c.debugLogf("register single client")
 		cs = &clientSet{}
@@ -598,7 +599,9 @@ func (s *Server) registerClient(c *sclient) {
 	}
 	s.keyOfAddr[c.remoteIPPort] = c.key
 	s.curClients.Add(1)
-	s.broadcastPeerStateChangeLocked(c.key, c.remoteIPPort, c.presentFlags(), true)
+	if isFirstConnInSet {
+		s.broadcastPeerStateChangeLocked(c.key, c.remoteIPPort, c.presentFlags(), true)
+	}
 }
 
 // broadcastPeerStateChangeLocked enqueues a message to all watchers
diff --git a/derp/derp_test.go b/derp/derp_test.go
index 9185194dd..2430a756b 100644
--- a/derp/derp_test.go
+++ b/derp/derp_test.go
@@ -550,13 +550,28 @@ type testClient struct {
 	closed bool
 }
 
-func newTestClient(t *testing.T, ts *testServer, name string, newClient func(net.Conn, key.NodePrivate, logger.Logf) (*Client, error)) *testClient {
+// testClientOpt can be one of:
+//
+//   - key.NodePrivate, to set an explicit private key
+type testClientOpt any
+
+func newTestClient(t *testing.T, ts *testServer, name string, newClient func(net.Conn, key.NodePrivate, logger.Logf) (*Client, error), opts ...testClientOpt) *testClient {
 	t.Helper()
+
+	k := key.NewNode()
+	for _, opt := range opts {
+		switch opt := opt.(type) {
+		case key.NodePrivate:
+			k = opt
+		default:
+			t.Fatalf("unknown option type %T", opt)
+		}
+	}
+
 	nc, err := net.Dial("tcp", ts.ln.Addr().String())
 	if err != nil {
 		t.Fatal(err)
 	}
-	k := key.NewNode()
 	ts.addKeyName(k.Public(), name)
 	c, err := newClient(nc, k, logger.WithPrefix(t.Logf, "client-"+name+": "))
 	if err != nil {
@@ -573,7 +588,7 @@ func newTestClient(t *testing.T, ts *testServer, name string, newClient func(net
 	return tc
 }
 
-func newRegularClient(t *testing.T, ts *testServer, name string) *testClient {
+func newRegularClient(t *testing.T, ts *testServer, name string, opts ...testClientOpt) *testClient {
 	return newTestClient(t, ts, name, func(nc net.Conn, priv key.NodePrivate, logf logger.Logf) (*Client, error) {
 		brw := bufio.NewReadWriter(bufio.NewReader(nc), bufio.NewWriter(nc))
 		c, err := NewClient(priv, nc, brw, logf)
@@ -582,8 +597,7 @@ func newRegularClient(t *testing.T, ts *testServer, name string) *testClient {
 		}
 		waitConnect(t, c)
 		return c, nil
-
-	})
+	}, opts...)
 }
 
 func newTestWatcher(t *testing.T, ts *testServer, name string) *testClient {
@@ -683,11 +697,14 @@ func TestWatch(t *testing.T) {
 	w1 := newTestWatcher(t, ts, "w1")
 	w1.wantPresent(t, w1.pub)
 
-	c1 := newRegularClient(t, ts, "c1")
+	c1Priv := key.NewNode()
+	c1 := newRegularClient(t, ts, "c1", c1Priv)
+	c1dup := newRegularClient(t, ts, "c1-dup", c1Priv)
 	w1.wantPresent(t, c1.pub)
 
 	c2 := newRegularClient(t, ts, "c2")
-	w1.wantPresent(t, c2.pub)
+	w1.wantPresent(t, c2.pub) // and not c1 again from c1dup
+	c1dup.close(t)
 
 	w2 := newTestWatcher(t, ts, "w2")
 	w1.wantPresent(t, w2.pub)