Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2d0b910

Browse files
authored
fix: change servertailnet to register the DERP dialer before setting DERP map (#12137)
I noticed a possible race where tailnet.Conn can try to dial the embedded region before we've set our custom dialer that send the DERP in-memory. This closes that race and adds a test case for servertailnet with no STUN and an embedded relay
1 parent1bb4aec commit2d0b910

File tree

5 files changed

+81
-34
lines changed

5 files changed

+81
-34
lines changed

‎coderd/tailnet.go

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,41 @@ func NewServerTailnet(
5252
traceProvider trace.TracerProvider,
5353
) (*ServerTailnet,error) {
5454
logger=logger.Named("servertailnet")
55-
originalDerpMap:=derpMapFn()
5655
conn,err:=tailnet.NewConn(&tailnet.Options{
5756
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(),128)},
58-
DERPMap:originalDerpMap,
5957
DERPForceWebSockets:derpForceWebSockets,
6058
Logger:logger,
6159
})
6260
iferr!=nil {
6361
returnnil,xerrors.Errorf("create tailnet conn: %w",err)
6462
}
65-
6663
serverCtx,cancel:=context.WithCancel(ctx)
64+
65+
// This is set to allow local DERP traffic to be proxied through memory
66+
// instead of needing to hit the external access URL. Don't use the ctx
67+
// given in this callback, it's only valid while connecting.
68+
ifderpServer!=nil {
69+
conn.SetDERPRegionDialer(func(_ context.Context,region*tailcfg.DERPRegion) net.Conn {
70+
if!region.EmbeddedRelay {
71+
returnnil
72+
}
73+
logger.Debug(ctx,"connecting to embedded DERP via in-memory pipe")
74+
left,right:=net.Pipe()
75+
gofunc() {
76+
deferleft.Close()
77+
deferright.Close()
78+
brw:=bufio.NewReadWriter(bufio.NewReader(right),bufio.NewWriter(right))
79+
derpServer.Accept(ctx,right,brw,"internal")
80+
}()
81+
returnleft
82+
})
83+
}
84+
6785
derpMapUpdaterClosed:=make(chanstruct{})
86+
originalDerpMap:=derpMapFn()
87+
// it's important to set the DERPRegionDialer above _before_ we set the DERP map so that if
88+
// there is an embedded relay, we use the local in-memory dialer.
89+
conn.SetDERPMap(originalDerpMap)
6890
gofunc() {
6991
deferclose(derpMapUpdaterClosed)
7092

@@ -139,25 +161,6 @@ func NewServerTailnet(
139161
// registering the callback also triggers send of the initial node
140162
tn.coordinatee.SetNodeCallback(tn.nodeCallback)
141163

142-
// This is set to allow local DERP traffic to be proxied through memory
143-
// instead of needing to hit the external access URL. Don't use the ctx
144-
// given in this callback, it's only valid while connecting.
145-
ifderpServer!=nil {
146-
conn.SetDERPRegionDialer(func(_ context.Context,region*tailcfg.DERPRegion) net.Conn {
147-
if!region.EmbeddedRelay {
148-
returnnil
149-
}
150-
left,right:=net.Pipe()
151-
gofunc() {
152-
deferleft.Close()
153-
deferright.Close()
154-
brw:=bufio.NewReadWriter(bufio.NewReader(right),bufio.NewWriter(right))
155-
derpServer.Accept(ctx,right,brw,"internal")
156-
}()
157-
returnleft
158-
})
159-
}
160-
161164
gotn.watchAgentUpdates()
162165
gotn.expireOldAgents()
163166
returntn,nil

‎coderd/tailnet_test.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,24 @@ func TestServerTailnet_AgentConn_OK(t *testing.T) {
5050
assert.True(t,conn.AwaitReachable(ctx))
5151
}
5252

53+
funcTestServerTailnet_AgentConn_NoSTUN(t*testing.T) {
54+
t.Parallel()
55+
56+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitMedium)
57+
defercancel()
58+
59+
// Connect through the ServerTailnet
60+
agents,serverTailnet:=setupServerTailnetAgent(t,1,
61+
tailnettest.DisableSTUN,tailnettest.DERPIsEmbedded)
62+
a:=agents[0]
63+
64+
conn,release,err:=serverTailnet.AgentConn(ctx,a.id)
65+
require.NoError(t,err)
66+
deferrelease()
67+
68+
assert.True(t,conn.AwaitReachable(ctx))
69+
}
70+
5371
funcTestServerTailnet_ReverseProxy(t*testing.T) {
5472
t.Parallel()
5573

@@ -311,9 +329,9 @@ type agentWithID struct {
311329
agent.Agent
312330
}
313331

314-
funcsetupServerTailnetAgent(t*testing.T,agentNumint) ([]agentWithID,*coderd.ServerTailnet) {
332+
funcsetupServerTailnetAgent(t*testing.T,agentNumint,opts...tailnettest.DERPAndStunOption) ([]agentWithID,*coderd.ServerTailnet) {
315333
logger:=slogtest.Make(t,nil).Leveled(slog.LevelDebug)
316-
derpMap,derpServer:=tailnettest.RunDERPAndSTUN(t)
334+
derpMap,derpServer:=tailnettest.RunDERPAndSTUN(t,opts...)
317335

318336
coord:=tailnet.NewCoordinator(logger)
319337
t.Cleanup(func() {

‎tailnet/configmaps.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ func (c *configMaps) netMapLocked() *netmap.NetworkMap {
204204
nm.Addresses=make([]netip.Prefix,len(c.addresses))
205205
copy(nm.Addresses,c.addresses)
206206

207-
nm.DERPMap=c.derpMap.Clone()
207+
// we don't need to set the DERPMap in the network map because we separately
208+
// send the DERPMap directly via SetDERPMap
208209
nm.Peers=c.peerConfigLocked()
209210
nm.SelfNode.Addresses=nm.Addresses
210211
nm.SelfNode.AllowedIPs=nm.Addresses

‎tailnet/conn.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,6 @@ func NewConn(options *Options) (conn *Conn, err error) {
116116
iflen(options.Addresses)==0 {
117117
returnnil,xerrors.New("At least one IP range must be provided")
118118
}
119-
ifoptions.DERPMap==nil {
120-
returnnil,xerrors.New("DERPMap must be provided")
121-
}
122119

123120
nodePrivateKey:=key.NewNode()
124121
varnodeID tailcfg.NodeID
@@ -219,7 +216,9 @@ func NewConn(options *Options) (conn *Conn, err error) {
219216
magicConn.DiscoPublicKey(),
220217
)
221218
cfgMaps.setAddresses(options.Addresses)
222-
cfgMaps.setDERPMap(options.DERPMap)
219+
ifoptions.DERPMap!=nil {
220+
cfgMaps.setDERPMap(options.DERPMap)
221+
}
223222
cfgMaps.setBlockEndpoints(options.BlockEndpoints)
224223

225224
nodeUp:=newNodeUpdater(

‎tailnet/tailnettest/tailnettest.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,47 @@ import (
3232
//go:generate mockgen -destination ./coordinatormock.go -package tailnettest github.com/coder/coder/v2/tailnet Coordinator
3333
//go:generate mockgen -destination ./coordinateemock.go -package tailnettest github.com/coder/coder/v2/tailnet Coordinatee
3434

35+
typederpAndSTUNCfgstruct {
36+
DisableSTUNbool
37+
DERPIsEmbeddedbool
38+
}
39+
40+
typeDERPAndStunOptionfunc(cfg*derpAndSTUNCfg)
41+
42+
funcDisableSTUN(cfg*derpAndSTUNCfg) {
43+
cfg.DisableSTUN=true
44+
}
45+
46+
funcDERPIsEmbedded(cfg*derpAndSTUNCfg) {
47+
cfg.DERPIsEmbedded=true
48+
}
49+
3550
// RunDERPAndSTUN creates a DERP mapping for tests.
36-
funcRunDERPAndSTUN(t*testing.T) (*tailcfg.DERPMap,*derp.Server) {
51+
funcRunDERPAndSTUN(t*testing.T,opts...DERPAndStunOption) (*tailcfg.DERPMap,*derp.Server) {
52+
cfg:=new(derpAndSTUNCfg)
53+
for_,o:=rangeopts {
54+
o(cfg)
55+
}
3756
logf:=tailnet.Logger(slogtest.Make(t,nil))
3857
d:=derp.NewServer(key.NewNode(),logf)
3958
server:=httptest.NewUnstartedServer(derphttp.Handler(d))
4059
server.Config.ErrorLog=tslogger.StdLogger(logf)
4160
server.Config.TLSNextProto=make(map[string]func(*http.Server,*tls.Conn, http.Handler))
4261
server.StartTLS()
43-
44-
stunAddr,stunCleanup:=stuntest.ServeWithPacketListener(t, nettype.Std{})
4562
t.Cleanup(func() {
4663
server.CloseClientConnections()
4764
server.Close()
4865
d.Close()
49-
stunCleanup()
5066
})
67+
68+
stunPort:=-1
69+
if!cfg.DisableSTUN {
70+
stunAddr,stunCleanup:=stuntest.ServeWithPacketListener(t, nettype.Std{})
71+
t.Cleanup(func() {
72+
stunCleanup()
73+
})
74+
stunPort=stunAddr.Port
75+
}
5176
tcpAddr,ok:=server.Listener.Addr().(*net.TCPAddr)
5277
if!ok {
5378
t.FailNow()
@@ -65,11 +90,12 @@ func RunDERPAndSTUN(t *testing.T) (*tailcfg.DERPMap, *derp.Server) {
6590
RegionID:1,
6691
IPv4:"127.0.0.1",
6792
IPv6:"none",
68-
STUNPort:stunAddr.Port,
93+
STUNPort:stunPort,
6994
DERPPort:tcpAddr.Port,
7095
InsecureForTests:true,
7196
},
7297
},
98+
EmbeddedRelay:cfg.DERPIsEmbedded,
7399
},
74100
},
75101
},d

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp