10000 feat: automatically use websockets if DERP upgrade is unavailable · coder/coder@589ead1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 589ead1

Browse files
kylecarbskylecarbs
committed
feat: automatically use websockets if DERP upgrade is unavailable
This might be our biggest hangup for deployments at the moment... Load balancers by default do not support the DERP protocol, so many of our prospects and customers run into failing workspace connections. This automatically swaps to use WebSockets, and reports the reason to coderd. In a future contribution, a warning will appear by the agent if it was forced to use WebSockets instead of DERP.
1 parent 8850ce0 commit 589ead1

File tree

9 files changed

+285
-28
lines changed

9 files changed

+285
-28
lines changed

‎coderd/coderd.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,9 @@ func New(options *Options) *API {
284284
// replicas or instances of this middleware.
285285
apiRateLimiter := httpmw.RateLimit(options.APIRateLimit, time.Minute)
286286

287+
derpHandler := derphttp.Handler(api.DERPServer)
288+
derpHandler, api.derpCloseFunc = tailnet.AddWebsocketSupport(api.DERPServer, derpHandler)
289+
287290
r.Use(
288291
httpmw.Recover(api.Logger),
289292
tracing.StatusWriterMiddleware,
@@ -363,7 +366,7 @@ func New(options *Options) *API {
363366
r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
364367
r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
365368
r.Route("/derp", func(r chi.Router) {
366-
r.Get("/", derphttp.Handler(api.DERPServer).ServeHTTP)
369+
r.Get("/", derpHandler.ServeHTTP)
367370
// This is used when UDP is blocked, and latency must be checked via HTTP(s).
368371
r.Get("/latency-check", func(w http.ResponseWriter, r *http.Request) {
369372
w.WriteHeader(http.StatusOK)
@@ -726,6 +729,7 @@ type API struct {
726729

727730
WebsocketWaitMutex sync.Mutex
728731
WebsocketWaitGroup sync.WaitGroup
732+
derpCloseFunc func()
729733

730734
metricsCache *metricscache.Cache
731735
workspaceAgentCache *wsconncache.Cache
@@ -739,6 +743,7 @@ type API struct {
739743
// Close waits for all WebSocket connections to drain before returning.
740744
func (api *API) Close() error {
741745
api.cancel()
746+
api.derpCloseFunc()
742747

743748
api.WebsocketWaitMutex.Lock()
744749
api.WebsocketWaitGroup.Wait()

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
4040

4141
// There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
4242
// https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
43-
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230210022917-446fc10e755a
43+
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230228204054-70dbfd7be018
4444

4545
// Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
4646
// See: https://github.com/coder/coder/issues/3371

‎go.sum

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,28 @@ github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHui
379379
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
380380
github.com/coder/tailscale v1.1.1-0.20230210022917-446fc10e755a h1:fTXoK+Yikz8Rl0v0nHxO+lTV0y8Q7wdmGFq1CqLgznE=
381381
github.com/coder/tailscale v1.1.1-0.20230210022917-446fc10e755a/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
382+
github.com/coder/tailscale v1.1.1-0.20230228183907-71829ca8456b h1:i8R1RwDqLihavxsaTovKCax45AJLy2dtXI0PcLm+BnM=
383+
github.com/coder/tailscale v1.1.1-0.20230228183907-71829ca8456b/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
384+
github.com/coder/tailscale v1.1.1-0.20230228184228-8e7815556b85 h1:tg89NZBDe91wWTeII4jFThHdB6USE3mBNXzK0/CWODM=
385+
github.com/coder/tailscale v1.1.1-0.20230228184228-8e7815556b85/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
386+
github.com/coder/tailscale v1.1.1-0.20230228184330-c2cd7d4f5a70 h1:gGevbQLqz4fWk5HytFodNe/Bl8rqgHb2+YsRoaZPf1g=
387+
github.com/coder/tailscale v1.1.1-0.20230228184330-c2cd7d4f5a70/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
388+
github.com/coder/tailscale v1.1.1-0.20230228184508-acb075f559ce h1:oAYRHRM8/rPrQhcPiXBkLw/YU/NyPhhN+HypRJWSq2k=
389+
github.com/coder/tailscale v1.1.1-0.20230228184508-acb075f559ce/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
390+
github.com/coder/tailscale v1.1.1-0.20230228184618-0e3e4965c416 h1:25uoFv5YNvve9r0BEGJAJHVWulyPmPPvXp6S3b0085o=
391+
github.com/coder/tailscale v1.1.1-0.20230228184618-0e3e4965c416/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
392+
github.com/coder/tailscale v1.1.1-0.20230228185337-873afd546fb2 h1:yrKQPYAadmtrj5J33Fs/Ldx3FgXCfd0gUbcFqjpsq+I=
393+
github.com/coder/tailscale v1.1.1-0.20230228185337-873afd546fb2/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
394+
github.com/coder/tailscale v1.1.1-0.20230228185607-b6042b55609a h1:aqp8CUtVBcCLDI369ORzbVt/pfOt0mTZBxIAejgZkgY=
395+
github.com/coder/tailscale v1.1.1-0.20230228185607-b6042b55609a/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
396+
github.com/coder/tailscale v1.1.1-0.20230228194335-31045cf53c85 h1:MHJDOehTwolaeT8FQrBQ5pNEdWXO+8pWTy5ODIYOZUU=
397+
github.com/coder/tailscale v1.1.1-0.20230228194335-31045cf53c85/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
398+
github.com/coder/tailscale v1.1.1-0.20230228194714-66eac1a43e5c h1:WylIWZ+8UpiW2NPTu6z2sgFHwqo9/XmT/QTNWT47qic=
399+
github.com/coder/tailscale v1.1.1-0.20230228194714-66eac1a43e5c/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
400+
github.com/coder/tailscale v1.1.1-0.20230228200853-ddedcafad3a1 h1:XgOpzrEiDR4GzoKE4dhqopZAg9MrG1cXPMc9/PXA6kk=
401+
github.com/coder/tailscale v1.1.1-0.20230228200853-ddedcafad3a1/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
402+
github.com/coder/tailscale v1.1.1-0.20230228204054-70dbfd7be018 h1:pv80tzqexcjrP/4CxFpalrk6bS2XJ2xOGA00u1WUy+g=
403+
github.com/coder/tailscale v1.1.1-0.20230228204054-70dbfd7be018/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
382404
github.com/coder/terraform-provider-coder v0.6.14 h1:NsJ1mo0MN1x/VyNLYmsgPUYn2JgzdVNZBqnj9OSIDgY=
383405
github.com/coder/terraform-provider-coder v0.6.14/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
384406
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=

‎tailnet/conn.go

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -192,19 +192,20 @@ func NewConn(options *Options) (conn *Conn, err error) {
192192
wireguardEngine.SetFilter(filter.New(netMap.PacketFilter, localIPs, logIPs, nil, Logger(options.Logger.Named("packet-filter"))))
193193
dialContext, dialCancel := context.WithCancel(context.Background())
194194
server := &Conn{
195-
blockEndpoints: options.BlockEndpoints,
196-
dialContext: dialContext,
197-
dialCancel: dialCancel,
198-
closed: make(chan struct{}),
199-
logger: options.Logger,
200-
magicConn: magicConn,
201-
dialer: dialer,
202-
listeners: map[listenKey]*listener{},
203-
peerMap: map[tailcfg.NodeID]*tailcfg.Node{},
204-
tunDevice: tunDevice,
205-
netMap: netMap,
206-
netStack: netStack,
207-
wireguardMonitor: wireguardMonitor,
195+
blockEndpoints: options.BlockEndpoints,
196+
dialContext: dialContext,
197+
dialCancel: dialCancel,
198+
closed: make(chan struct{}),
199+
logger: options.Logger,
200+
magicConn: magicConn,
201+
dialer: dialer,
202+
listeners: map[listenKey]*listener{},
203+
peerMap: map[tailcfg.NodeID]*tailcfg.Node{},
204+
lastDERPForcedWebsockets: map[int]string{},
205+
tunDevice: tunDevice,
206+
netMap: netMap,
207+
netStack: netStack,
208+
wireguardMonitor: wireguardMonitor,
208209
wireguardRouter: &router.Config{
209210
LocalAddrs: netMap.Addresses,
210211
},
@@ -247,6 +248,17 @@ func NewConn(options *Options) (conn *Conn, err error) {
247248
server.lastMutex.Unlock()
248249
server.sendNode()
249250
})
251+
magicConn.SetDERPForcedWebsocketCallback(func(region int, reason string) {
252+
server.logger.Debug(context.Background(), "derp forced websocket", slog.F("region", region), slog.F("reason", reason))
253+
server.lastMutex.Lock()
254+
if server.lastDERPForcedWebsockets[region] == reason {
255+
server.lastMutex.Unlock()
256+
return
257+
}
258+
server.lastDERPForcedWebsockets[region] = reason
259+
server.lastMutex.Unlock()
260+
server.sendNode()
261+
})
250262
netStack.ForwardTCPIn = server.forwardTCP
251263

252264
err = netStack.Start(nil)
@@ -299,10 +311,11 @@ type Conn struct {
299311
nodeChanged bool
300312
// It's only possible to store these values via status functions,
301313
// so the values must be stored for retrieval later on.
302-
lastStatus time.Time
303-
lastEndpoints []tailcfg.Endpoint
304-
lastNetInfo *tailcfg.NetInfo
305-
nodeCallback func(node *Node)
314+
lastStatus time.Time
315+
lastEndpoints []tailcfg.Endpoint
316+
lastDERPForcedWebsockets map[int]string
317+
lastNetInfo *tailcfg.NetInfo
318+
nodeCallback func(node *Node)
306319

307320
trafficStats *connstats.Statistics
308321
}
@@ -632,21 +645,24 @@ func (c *Conn) selfNode() *Node {
632645
}
633646
var preferredDERP int
634647
var derpLatency map[string]float64
648+
var derpForcedWebsocket map[int]string
635649
if c.lastNetInfo != nil {
636650
preferredDERP = c.lastNetInfo.PreferredDERP
637651
derpLatency = c.lastNetInfo.DERPLatency
652+
derpForcedWebsocket = c.lastDERPForcedWebsockets
638653
}
639654

640655
node := &Node{
641-
ID: c.netMap.SelfNode.ID,
642-
AsOf: database.Now(),
643-
Key: c.netMap.SelfNode.Key,
644-
Addresses: c.netMap.SelfNode.Addresses,
645-
AllowedIPs: c.netMap.SelfNode.AllowedIPs,
646-
DiscoKey: c.magicConn.DiscoPublicKey(),
647-
Endpoints: endpoints,
648-
PreferredDERP: preferredDERP,
649-
DERPLatency: derpLatency,
656+
ID: c.netMap.SelfNode.ID,
657+
AsOf: database.Now(),
658+
Key: c.netMap.SelfNode.Key,
659+
Addresses: c.netMap.SelfNode.Addresses,
660+
AllowedIPs: c.netMap.SelfNode.AllowedIPs,
661+
DiscoKey: c.magicConn.DiscoPublicKey(),
662+
Endpoints: endpoints,
663+
PreferredDERP: preferredDERP,
664+
DERPLatency: derpLatency,
665+
DERPForcedWebsocket: derpForcedWebsocket,
650666
}
651667
if c.blockEndpoints {
652668
node.Endpoints = nil

‎tailnet/conn_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,83 @@ func TestTailnet(t *testing.T) {
8181
_ = nc.Close()
8282
<-conn
8383

84+
nodes := make(chan *tailnet.Node, 1)
85+
w2.SetNodeCallback(func(node *tailnet.Node) {
86+
select {
87+
case nodes <- node:
88+
default:
89+
}
90+
})
91+
node := <-nodes
92+
// Ensure this connected over DERP!
93+
require.Len(t, node.DERPForcedWebsocket, 0)
94+
95+
w1.Close()
96+
w2.Close()
97+
})
98+
99+
t.Run("ForcesWebSockets", func(t *testing.T) {
100+
t.Parallel()
101+
w1IP := tailnet.IP()
102+
derpMap := tailnettest.RunDERPOnlyWebSockets(t)
103+
w1, err := tailnet.NewConn(&tailnet.Options{
104+
Addresses: []netip.Prefix{netip.PrefixFrom(w1IP, 128)},
105+
Logger: logger.Named("w1"),
106+
DERPMap: derpMap,
107+
BlockEndpoints: true,
108+
})
109+
require.NoError(t, err)
110+
111+
w2, err := tailnet.NewConn(&tailnet.Options{
112+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
113+
Logger: logger.Named("w2"),
114+
DERPMap: derpMap,
115+
BlockEndpoints: true,
116+
})
117+
require.NoError(t, err)
118+
t.Cleanup(func() {
119+
_ = w1.Close()
120+
_ = w2.Close()
121+
})
122+
w1.SetNodeCallback(func(node *tailnet.Node) {
123+
err := w2.UpdateNodes([]*tailnet.Node{node}, false)
124+
assert.NoError(t, err)
125+
})
126+
w2.SetNodeCallback(func(node *tailnet.Node) {
127+
err := w1.UpdateNodes([]*tailnet.Node{node}, false)
128+
assert.NoError(t, err)
129+
})
130+
require.True(t, w2.AwaitReachable(context.Background(), w1IP))
131+
conn := make(chan struct{})
132+
go func() {
133+
listener, err := w1.Listen("tcp", ":35565")
134+
assert.NoError(t, err)
135+
defer listener.Close()
136+
nc, err := listener.Accept()
137+
if !assert.NoError(t, err) {
138+
return
139+
}
140+
_ = nc.Close()
141+
conn <- struct{}{}
142+
}()
143+
144+
nc, err := w2.DialContextTCP(context.Background(), netip.AddrPortFrom(w1IP, 35565))
145+
require.NoError(t, err)
146+
_ = nc.Close()
147+
<-conn
148+
149+
nodes := make(chan *tailnet.Node, 1)
150+
w2.SetNodeCallback(func(node *tailnet.Node) {
151+
select {
152+
case nodes <- node:
153+
default:
154+
}
155+
})
156+
node := <-nodes
157+
require.Len(t, node.DERPForcedWebsocket, 1)
158+
// Ensure the reason is valid!
159+
require.Equal(t, "GET failed with status code 400: Invalid \"Upgrade\" header: DERP", node.DERPForcedWebsocket[derpMap.RegionIDs()[0]])
160+
84161
w1.Close()
85162
w2.Close()
86163
})

‎tailnet/coordinator.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ type Node struct {
5858
PreferredDERP int `json:"preferred_derp"`
5959
// DERPLatency is the latency in seconds to each DERP server.
6060
DERPLatency map[string]float64 `json:"derp_latency"`
61+
// DERPForcedWebsocket contains a mapping of DERP regions to
62+
// error messages that caused the connection to be forced to
63+
// use WebSockets. We don't use WebSockets by default because
64+
// they are less performant.
65+
DERPForcedWebsocket map[int]string `json:"derp_forced_websockets"`
6166
// Addresses are the IP address ranges this connection exposes.
6267
Addresses []netip.Prefix `json:"addresses"`
6368
// AllowedIPs specify what addresses can dial the connection.

‎tailnet/derp.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package tailnet
2+
3+
import (
4+
"bufio"
5+
"context"
6+
"log"
7+
"net/http"
8+
"strings"
9+
"sync"
10+
11+
"nhooyr.io/websocket"
12+
"tailscale.com/derp"
13+
"tailscale.com/net/wsconn"
14+
)
15+
16+
// DERPWebsocketSupport returns an http.Handler that upgrades
17+
// connections to the "derp" subprotocol to WebSockets and
18+
// passes them to the DERP server.
19+
// Taken from: https://github.com/tailscale/tailscale/blob/e3211ff88ba85435f70984cf67d9b353f3d650d8/cmd/derper/websocket.go#L21
20+
func AddWebsocketSupport(s *derp.Server, base http.Handler) (http.Handler, func()) {
21+
var mu sync.Mutex
22+
var waitGroup sync.WaitGroup
23+
ctx, cancelFunc := context.WithCancel(context.Background())
24+
25+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26+
up := strings.ToLower(r.Header.Get("Upgrade"))
27+
28+
// Very early versions of Tailscale set "Upgrade: WebSocket" but didn't actually
29+
// speak WebSockets (they still assumed DERP's binary framing). So to distinguish
30+
// clients that actually want WebSockets, look for an explicit "derp" subprotocol.
31+
if up != "websocket" || !strings.Contains(r.Header.Get("Sec-Websocket-Protocol"), "derp") {
32+
base.ServeHTTP(w, r)
33+
return
34+
}
35+
36+
mu.Lock()
37+
waitGroup.Add(1)
38+
mu.Unlock()
39+
defer waitGroup.Done()
40+
c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
41+
Subprotocols: []string{"derp"},
42+
OriginPatterns: []string{"*"},
43+
// Disable compression because we transmit WireGuard messages that
44+
// are not compressible.
45+
// Additionally, Safari has a broken implementation of compression
46+
// (see https://github.com/nhooyr/websocket/issues/218) that makes
47+
// enabling it actively harmful.
48+
CompressionMode: websocket.CompressionDisabled,
49+
})
50+
if err != nil {
51+
log.Printf("websocket.Accept: %v", err)
52+
return
53+
}
54+
defer c.Close(websocket.StatusInternalError, "closing")
55+
if c.Subprotocol() != "derp" {
56+
c.Close(websocket.StatusPolicyViolation, "client must speak the derp subprotocol")
57+
return
58+
}
59+
wc := wsconn.NetConn(ctx, c, websocket.MessageBinary)
60+
brw := bufio.NewReadWriter(bufio.NewReader(wc), bufio.NewWriter(wc))
61+
s.Accept(r.Context(), wc, brw, r.RemoteAddr)
62+
}), func() {
63+
cancelFunc()
64+
mu.Lock()
65+
waitGroup.Wait()
66+
mu.Unlock()
67+
}
68+
}

0 commit comments

Comments
 (0)
0