fix derp issue

Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby · Dec 8, 2023 · 9dbac20 · 9dbac20
1 parent 8b3721b
commit 9dbac20
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 4 deletions.
diff --git a/go.mod b/go.mod
@@ -35,6 +35,7 @@ require (
 	github.com/tcnksm/go-latest v0.0.0-20170313132115-e3007ae9052e
 	go4.org/netipx v0.0.0-20230824141953-6213f710f925
 	golang.org/x/crypto v0.16.0
+	golang.org/x/exp v0.0.0-20231127185646-65229373498e
 	golang.org/x/net v0.19.0
 	golang.org/x/oauth2 v0.15.0
 	golang.org/x/sync v0.5.0
@@ -146,7 +147,6 @@ require (
 	github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go4.org/mem v0.0.0-20220726221520-4f986261bf13 // indirect
-	golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect
 	golang.org/x/mod v0.14.0 // indirect
 	golang.org/x/sys v0.15.0 // indirect
 	golang.org/x/term v0.15.0 // indirect

diff --git a/hscontrol/derp/server/derp_server.go b/hscontrol/derp/server/derp_server.go
@@ -18,6 +18,7 @@ import (
 	"tailscale.com/net/stun"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
+	"tailscale.com/types/logger"
 )
 
 // fastStartHeader is the header (with value "1") that signals to the HTTP
@@ -33,13 +34,19 @@ type DERPServer struct {
 	tailscaleDERP *derp.Server
 }
 
+func derpLogf() logger.Logf {
+	return func(format string, args ...any) {
+		log.Debug().Caller().Msgf(format, args...)
+	}
+}
+
 func NewDERPServer(
 	serverURL string,
 	derpKey key.NodePrivate,
 	cfg *types.DERPConfig,
 ) (*DERPServer, error) {
 	log.Trace().Caller().Msg("Creating new embedded DERP server")
-	server := derp.NewServer(derpKey, log.Debug().Msgf) // nolint // zerolinter complains
+	server := derp.NewServer(derpKey, derpLogf()) // nolint // zerolinter complains
 
 	return &DERPServer{
 		serverURL:     serverURL,

diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go
@@ -402,6 +402,21 @@ func (node *Node) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.PeerC
 		ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP
 	}
 
+	if req.Hostinfo != nil && req.Hostinfo.NetInfo != nil {
+		// If there is no stored Hostinfo or NetInfo, use
+		// the new PreferredDERP.
+		if node.Hostinfo == nil {
+			ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP
+		} else if node.Hostinfo.NetInfo == nil {
+			ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP
+		} else {
+			// If there is a PreferredDERP check if it has changed.
+			if node.Hostinfo.NetInfo.PreferredDERP != req.Hostinfo.NetInfo.PreferredDERP {
+				ret.DERPRegion = req.Hostinfo.NetInfo.PreferredDERP
+			}
+		}
+	}
+
 	// TODO(kradalby): Find a good way to compare updates
 	ret.Endpoints = req.Endpoints
 
@@ -432,8 +447,17 @@ func (node *Node) ApplyPeerChange(change *tailcfg.PeerChange) {
 	// This might technically not be useful as we replace
 	// the whole hostinfo blob when it has changed.
 	if change.DERPRegion != 0 {
-		if node.Hostinfo != nil &&
-			node.Hostinfo.NetInfo != nil {
+		if node.Hostinfo == nil {
+			node.Hostinfo = &tailcfg.Hostinfo{
+				NetInfo: &tailcfg.NetInfo{
+					PreferredDERP: change.DERPRegion,
+				},
+			}
+		} else if node.Hostinfo.NetInfo == nil {
+			node.Hostinfo.NetInfo = &tailcfg.NetInfo{
+				PreferredDERP: change.DERPRegion,
+			}
+		} else {
 			node.Hostinfo.NetInfo.PreferredDERP = change.DERPRegion
 		}
 	}

diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go
@@ -5,7 +5,9 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 )
 
 func Test_NodeCanAccess(t *testing.T) {
@@ -232,3 +234,135 @@ func TestNodeFQDN(t *testing.T) {
 		})
 	}
 }
+
+func TestPeerChangeFromMapRequest(t *testing.T) {
+	nKeys := []key.NodePublic{
+		key.NewNode().Public(),
+		key.NewNode().Public(),
+		key.NewNode().Public(),
+	}
+
+	dKeys := []key.DiscoPublic{
+		key.NewDisco().Public(),
+		key.NewDisco().Public(),
+		key.NewDisco().Public(),
+	}
+
+	tests := []struct {
+		name   string
+		node   Node
+		mapReq tailcfg.MapRequest
+		want   tailcfg.PeerChange
+	}{
+		{
+			name: "preferred-derp-changed",
+			node: Node{
+				ID:        1,
+				NodeKey:   nKeys[0],
+				DiscoKey:  dKeys[0],
+				Endpoints: []netip.AddrPort{},
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 998,
+					},
+				},
+			},
+			mapReq: tailcfg.MapRequest{
+				NodeKey:  nKeys[0],
+				DiscoKey: dKeys[0],
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 999,
+					},
+				},
+			},
+			want: tailcfg.PeerChange{
+				NodeID:     1,
+				DERPRegion: 999,
+			},
+		},
+		{
+			name: "preferred-derp-no-changed",
+			node: Node{
+				ID:        1,
+				NodeKey:   nKeys[0],
+				DiscoKey:  dKeys[0],
+				Endpoints: []netip.AddrPort{},
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 100,
+					},
+				},
+			},
+			mapReq: tailcfg.MapRequest{
+				NodeKey:  nKeys[0],
+				DiscoKey: dKeys[0],
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 100,
+					},
+				},
+			},
+			want: tailcfg.PeerChange{
+				NodeID:     1,
+				DERPRegion: 0,
+			},
+		},
+		{
+			name: "preferred-derp-no-mapreq-netinfo",
+			node: Node{
+				ID:        1,
+				NodeKey:   nKeys[0],
+				DiscoKey:  dKeys[0],
+				Endpoints: []netip.AddrPort{},
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 200,
+					},
+				},
+			},
+			mapReq: tailcfg.MapRequest{
+				NodeKey:  nKeys[0],
+				DiscoKey: dKeys[0],
+				Hostinfo: &tailcfg.Hostinfo{},
+			},
+			want: tailcfg.PeerChange{
+				NodeID:     1,
+				DERPRegion: 0,
+			},
+		},
+		{
+			name: "preferred-derp-no-node-netinfo",
+			node: Node{
+				ID:        1,
+				NodeKey:   nKeys[0],
+				DiscoKey:  dKeys[0],
+				Endpoints: []netip.AddrPort{},
+				Hostinfo:  &tailcfg.Hostinfo{},
+			},
+			mapReq: tailcfg.MapRequest{
+				NodeKey:  nKeys[0],
+				DiscoKey: dKeys[0],
+				Hostinfo: &tailcfg.Hostinfo{
+					NetInfo: &tailcfg.NetInfo{
+						PreferredDERP: 200,
+					},
+				},
+			},
+			want: tailcfg.PeerChange{
+				NodeID:     1,
+				DERPRegion: 200,
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := tc.node.PeerChangeFromMapRequest(tc.mapReq)
+
+			if diff := cmp.Diff(tc.want, got, cmpopts.IgnoreFields(tailcfg.PeerChange{}, "LastSeen")); diff != "" {
+				t.Errorf("Patch unexpected result (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
diff --git a/integration/embedded_derp_test.go b/integration/embedded_derp_test.go
@@ -44,6 +44,9 @@ func TestDERPServerScenario(t *testing.T) {
 	headscaleConfig["HEADSCALE_DERP_SERVER_REGION_NAME"] = "Headscale Embedded DERP"
 	headscaleConfig["HEADSCALE_DERP_SERVER_STUN_LISTEN_ADDR"] = "0.0.0.0:3478"
 	headscaleConfig["HEADSCALE_DERP_SERVER_PRIVATE_KEY_PATH"] = "/tmp/derp.key"
+	// Envknob for enabling DERP debug logs
+	headscaleConfig["DERP_DEBUG_LOGS"] = "true"
+	headscaleConfig["DERP_PROBER_DEBUG_LOGS"] = "true"
 
 	err = scenario.CreateHeadscaleEnv(
 		spec,