fix: revert "direct embedded derp traffic directly to the server"

kylecarbs · kylecarbs · commit 497477753d29 · 2023-03-23T14:05:29.000Z
Not sure why this is breaking deployments, but it seems likely to be the cause. See #6746.
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -259,7 +259,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
 	stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
 	t.Cleanup(stunCleanup)
 
-	derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug)))
+	derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp")))
 	derpServer.SetMeshKey("test-key")
 
 	// match default with cli default
diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -1,7 +1,6 @@
 package coderd
 
 import (
-	"bufio"
 	"context"
 	"database/sql"
 	"encoding/json"
@@ -418,29 +417,51 @@ func (api *API) dialWorkspaceAgentTailnet(r *http.Request, agentID uuid.UUID) (*
 	ctx := r.Context()
 	clientConn, serverConn := net.Pipe()
 
+	derpMap := api.DERPMap.Clone()
+	for _, region := range derpMap.Regions {
+		if !region.EmbeddedRelay {
+			continue
+		}
+		var node *tailcfg.DERPNode
+		for _, n := range region.Nodes {
+			if n.STUNOnly {
+				continue
+			}
+			node = n
+			break
+		}
+		if node == nil {
+			continue
+		}
+		// TODO: This should dial directly to execute the
+		// DERP server instead of contacting localhost.
+		//
+		// This requires modification of Tailscale internals
+		// to pipe through a proxy function per-region, so
+		// this is an easy and mostly reliable hack for now.
+		cloned := node.Clone()
+		// Add p for proxy.
+		// This first node supports TLS.
+		cloned.Name += "p"
+		cloned.IPv4 = "127.0.0.1"
+		cloned.InsecureForTests = true
+		region.Nodes = append(region.Nodes, cloned.Clone())
+		// This second node forces HTTP.
+		cloned.Name += "-http"
+		cloned.ForceHTTP = true
+		region.Nodes = append(region.Nodes, cloned)
+	}
+
 	conn, err := tailnet.NewConn(&tailnet.Options{
 		Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
-		DERPMap:   api.DERPMap,
+		DERPMap:   derpMap,
 		Logger:    api.Logger.Named("tailnet"),
 	})
 	if err != nil {
 		_ = clientConn.Close()
 		_ = serverConn.Close()
 		return nil, xerrors.Errorf("create tailnet conn: %w", err)
 	}
-	conn.SetDERPRegionDialer(func(_ context.Context, region *tailcfg.DERPRegion) net.Conn {
-		if !region.EmbeddedRelay {
-			return nil
-		}
-		left, right := net.Pipe()
-		go func() {
-			defer left.Close()
-			defer right.Close()
-			brw := bufio.NewReadWriter(bufio.NewReader(right), bufio.NewWriter(right))
-			api.DERPServer.Accept(ctx, right, brw, r.RemoteAddr)
-		}()
-		return left
-	})
 
 	sendNodes, _ := tailnet.ServeCoordinator(clientConn, func(node []*tailnet.Node) error {
 		err = conn.UpdateNodes(node, true)
diff --git a/go.mod b/go.mod
@@ -40,7 +40,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
 
 // There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
 // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
-replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230321171725-fed359a0cafa
+replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230307022319-1e5e724a3949
 
 // Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
 // See: https://github.com/coder/coder/issues/3371
diff --git a/go.sum b/go.sum
@@ -378,8 +378,12 @@ github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d h1:09JG37IgTB6n3ouX9
 github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d/go.mod h1:r+1J5i/989wt6CUeNSuvFKKA9hHuKKPMxdzDbTuvwwk=
 github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko=
 github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
-github.com/coder/tailscale v1.1.1-0.20230321171725-fed359a0cafa h1:EjRGgTz7BUECmbV8jHTi1/rKdDjJESGSlm1Jp7evvCQ=
-github.com/coder/tailscale v1.1.1-0.20230321171725-fed359a0cafa/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
+github.com/coder/tailscale v1.1.1-0.20230307022319-1e5e724a3949 h1:8WfMfRTDaEpnmhCJWfFQ7JHz19GyP+EgFgLGu5ngdek=
+github.com/coder/tailscale v1.1.1-0.20230307022319-1e5e724a3949/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
+github.com/coder/terraform-provider-coder v0.6.15 h1:Llvh4RwxSQ/goy7ToTOeHf3tdEz+79qbyOh61hNnJs0=
+github.com/coder/terraform-provider-coder v0.6.15/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
+github.com/coder/terraform-provider-coder v0.6.17 h1:YvjM5nQx5RO+gXsYIv++CkiWCuJueQdJaPrsjnkZ4XQ=
+github.com/coder/terraform-provider-coder v0.6.17/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
 github.com/coder/terraform-provider-coder v0.6.21 h1:TIH6+/VQFreT8q/CkRvpHtbIeM5cOAhuDS5Sh1Nm21Q=
 github.com/coder/terraform-provider-coder v0.6.21/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
 github.com/coder/wgtunnel v0.1.5 h1:WP3sCj/3iJ34eKvpMQEp1oJHvm24RYh0NHbj1kfUKfs=
diff --git a/tailnet/conn.go b/tailnet/conn.go
@@ -355,11 +355,6 @@ func (c *Conn) SetDERPMap(derpMap *tailcfg.DERPMap) {
 	c.wireguardEngine.SetNetworkMap(&netMapCopy)
 }
 
-// SetDERPRegionDialer updates the dialer to use for connecting to DERP regions.
-func (c *Conn) SetDERPRegionDialer(dialer func(ctx context.Context, region *tailcfg.DERPRegion) net.Conn) {
-	c.magicConn.SetDERPRegionDialer(dialer)
-}
-
 func (c *Conn) RemoveAllPeers() error {
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
