Skip to content

Commit 71a6e59

Browse files
ethanndicksonethanndickson
committed
review p1
1 parent 79f824c commit 71a6e59

File tree

5 files changed

+96
-44
lines changed

5 files changed

+96
-44
lines changed

‎tailnet/controllers.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ type TunnelAllWorkspaceUpdatesController struct {
867867
ownerUsername string
868868
logger slog.Logger
869869

870-
sync.Mutex
870+
mu sync.Mutex
871871
updater *tunnelUpdater
872872
}
873873

@@ -911,8 +911,8 @@ type agent struct {
911911
}
912912

913913
func (t *TunnelAllWorkspaceUpdatesController) New(client WorkspaceUpdatesClient) CloserWaiter {
914-
t.Lock()
915-
defer t.Unlock()
914+
t.mu.Lock()
915+
defer t.mu.Unlock()
916916
updater := &tunnelUpdater{
917917
client: client,
918918
errChan: make(chan error, 1),
@@ -930,8 +930,8 @@ func (t *TunnelAllWorkspaceUpdatesController) New(client WorkspaceUpdatesClient)
930930
}
931931

932932
func (t *TunnelAllWorkspaceUpdatesController) CurrentState() *proto.WorkspaceUpdate {
933-
t.Lock()
934-
defer t.Unlock()
933+
t.mu.Lock()
934+
defer t.mu.Unlock()
935935
if t.updater == nil {
936936
return nil
937937
}

‎vpn/client.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ type vpnConn struct {
3434
updatesCtrl *tailnet.TunnelAllWorkspaceUpdatesController
3535
}
3636

37-
func (c vpnConn) CurrentWorkspaceState() *proto.WorkspaceUpdate {
37+
func (c *vpnConn) CurrentWorkspaceState() *proto.WorkspaceUpdate {
3838
return c.updatesCtrl.CurrentState()
3939
}
4040

41-
func (c vpnConn) Close() error {
41+
func (c *vpnConn) Close() error {
4242
c.cancelFn()
4343
<-c.controller.Closed()
4444
return c.Conn.Close()
@@ -89,8 +89,8 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string
8989
Header: headers,
9090
}
9191

92-
// New context, separate from dialCtx. We don't want to cancel the
93-
// connection if dialCtx is canceled.
92+
// New context, separate from initCtx. We don't want to cancel the
93+
// connection if initCtx is canceled.
9494
ctx, cancel := context.WithCancel(context.Background())
9595
defer func() {
9696
if err != nil {
@@ -171,7 +171,7 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string
171171
options.Logger.Debug(ctx, "connected to tailnet v2+ API")
172172
}
173173

174-
return vpnConn{
174+
return &vpnConn{
175175
Conn: conn,
176176
cancelFn: cancel,
177177
controller: controller,

‎vpn/dylib/lib.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const (
2222
)
2323

2424
// OpenTunnel creates a new VPN tunnel by `dup`ing the provided 'PIPE'
25-
// file descriptors for reading, writing, and logging.
25+
// file descriptors for reading and writing.
2626
//
2727
//export OpenTunnel
2828
func OpenTunnel(cReadFD, cWriteFD int32) int32 {
@@ -46,8 +46,10 @@ func OpenTunnel(cReadFD, cWriteFD int32) int32 {
4646
return ErrOpenPipe
4747
}
4848

49-
// Logs will be sent over the protocol
50-
_, err = vpn.NewTunnel(ctx, slog.Make(), conn, vpn.NewClient())
49+
_, err = vpn.NewTunnel(ctx, slog.Make(), conn, vpn.NewClient(),
50+
vpn.UseAsDNSConfig(),
51+
vpn.UseAsRouter(),
52+
)
5153
if err != nil {
5254
unix.Close(readFD)
5355
unix.Close(writeFD)

‎vpn/tunnel.go

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,38 @@ import (
1414
"unicode"
1515

1616
"golang.org/x/xerrors"
17+
"tailscale.com/net/dns"
18+
"tailscale.com/wgengine/router"
1719

20+
"cdr.dev/slog"
1821
"github.com/coder/coder/v2/coderd/util/ptr"
1922
"github.com/coder/coder/v2/tailnet/proto"
20-
21-
"cdr.dev/slog"
2223
)
2324

2425
type Tunnel struct {
2526
speaker[*TunnelMessage, *ManagerMessage, ManagerMessage]
2627
ctx context.Context
27-
logger slog.Logger
2828
requestLoopDone chan struct{}
2929

30+
logger slog.Logger
31+
router router.Router
32+
dnsConfigurator dns.OSConfigurator
33+
3034
logMu sync.Mutex
3135
logs []*TunnelMessage
3236

3337
client Client
3438
conn Conn
3539
}
3640

41+
type TunnelOption func(t *Tunnel)
42+
3743
func NewTunnel(
3844
ctx context.Context,
3945
logger slog.Logger,
4046
mgrConn io.ReadWriteCloser,
4147
client Client,
48+
opts ...TunnelOption,
4249
) (*Tunnel, error) {
4350
logger = logger.Named("vpn")
4451
s, err := newSpeaker[*TunnelMessage, *ManagerMessage](
@@ -54,6 +61,10 @@ func NewTunnel(
5461
requestLoopDone: make(chan struct{}),
5562
client: client,
5663
}
64+
65+
for _, opt := range opts {
66+
opt(t)
67+
}
5768
t.speaker.start()
5869
go t.requestLoop()
5970
return t, nil
@@ -106,7 +117,8 @@ func (t *Tunnel) handleRPC(req *ManagerMessage, msgID uint64) *TunnelMessage {
106117
}
107118
resp.Msg = &TunnelMessage_Start{
108119
Start: &StartResponse{
109-
Success: err == nil,
120+
Success: err == nil,
121+
ErrorMessage: err.Error(),
110122
},
111123
}
112124
return resp
@@ -120,7 +132,8 @@ func (t *Tunnel) handleRPC(req *ManagerMessage, msgID uint64) *TunnelMessage {
120132
}
121133
resp.Msg = &TunnelMessage_Stop{
122134
Stop: &StopResponse{
123-
Success: err == nil,
135+
Success: err == nil,
136+
ErrorMessage: err.Error(),
124137
},
125138
}
126139
return resp
@@ -130,6 +143,24 @@ func (t *Tunnel) handleRPC(req *ManagerMessage, msgID uint64) *TunnelMessage {
130143
}
131144
}
132145

146+
func UseAsRouter() TunnelOption {
147+
return func(t *Tunnel) {
148+
t.router = NewRouter(t)
149+
}
150+
}
151+
152+
func UseAsLogger() TunnelOption {
153+
return func(t *Tunnel) {
154+
t.logger = t.logger.AppendSinks(t)
155+
}
156+
}
157+
158+
func UseAsDNSConfig() TunnelOption {
159+
return func(t *Tunnel) {
160+
t.dnsConfigurator = NewDNSConfigurator(t)
161+
}
162+
}
163+
133164
// ApplyNetworkSettings sends a request to the manager to apply the given network settings
134165
func (t *Tunnel) ApplyNetworkSettings(ctx context.Context, ns *NetworkSettingsRequest) error {
135166
msg, err := t.speaker.unaryRPC(ctx, &TunnelMessage{
@@ -168,7 +199,7 @@ func (t *Tunnel) start(req *StartRequest) error {
168199
}
169200
svrURL, err := url.Parse(rawURL)
170201
if err != nil {
171-
return xerrors.Errorf("parse url: %w", err)
202+
return xerrors.Errorf("parse url %q: %w", rawURL, err)
172203
}
173204
apiToken := req.GetApiToken()
174205
if apiToken == "" {
@@ -179,19 +210,23 @@ func (t *Tunnel) start(req *StartRequest) error {
179210
header.Add(h.GetName(), h.GetValue())
180211
}
181212

182-
t.conn, err = t.client.NewConn(
183-
t.ctx,
184-
svrURL,
185-
apiToken,
186-
&Options{
187-
Headers: header,
188-
Logger: t.logger,
189-
DNSConfigurator: NewDNSConfigurator(t),
190-
Router: NewRouter(t),
191-
TUNFileDescriptor: ptr.Ref(int(req.GetTunnelFileDescriptor())),
192-
UpdateHandler: t,
193-
},
194-
)
213+
if t.conn == nil {
214+
t.conn, err = t.client.NewConn(
215+
t.ctx,
216+
svrURL,
217+
apiToken,
218+
&Options{
219+
Headers: header,
220+
Logger: t.logger,
221+
DNSConfigurator: t.dnsConfigurator,
222+
Router: t.router,
223+
TUNFileDescriptor: ptr.Ref(int(req.GetTunnelFileDescriptor())),
224+
UpdateHandler: t,
225+
},
226+
)
227+
} else {
228+
t.logger.Warn(t.ctx, "asked to start tunnel, but tunnel is already running")
229+
}
195230
return err
196231
}
197232

@@ -252,26 +287,32 @@ func convertWorkspaceUpdate(update *proto.WorkspaceUpdate) *PeerUpdate {
252287
}
253288
for i, ws := range update.UpsertedWorkspaces {
254289
out.UpsertedWorkspaces[i] = &Workspace{
255-
Id: ws.Id,
256-
Name: ws.Name,
290+
Id: ws.Id,
291+
Name: ws.Name,
292+
Status: Workspace_Status(ws.Status),
257293
}
258294
}
259295
for i, agent := range update.UpsertedAgents {
260296
out.UpsertedAgents[i] = &Agent{
261-
Id: agent.Id,
262-
Name: agent.Name,
297+
Id: agent.Id,
298+
Name: agent.Name,
299+
WorkspaceId: agent.WorkspaceId,
300+
Fqdn: "",
263301
}
264302
}
265303
for i, ws := range update.DeletedWorkspaces {
266304
out.DeletedWorkspaces[i] = &Workspace{
267-
Id: ws.Id,
268-
Name: ws.Name,
305+
Id: ws.Id,
306+
Name: ws.Name,
307+
Status: Workspace_Status(ws.Status),
269308
}
270309
}
271310
for i, agent := range update.DeletedAgents {
272311
out.DeletedAgents[i] = &Agent{
273-
Id: agent.Id,
274-
Name: agent.Name,
312+
Id: agent.Id,
313+
Name: agent.Name,
314+
WorkspaceId: []byte{},
315+
Fqdn: "",
275316
}
276317
}
277318
return out

‎vpn/tunnel_internal_test.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"net"
66
"net/url"
7+
"sync"
78
"testing"
89

910
"github.com/stretchr/testify/require"
@@ -29,7 +30,12 @@ type fakeClient struct {
2930
var _ Client = (*fakeClient)(nil)
3031

3132
func (f *fakeClient) NewConn(context.Context, *url.URL, string, *Options) (Conn, error) {
32-
return testutil.RequireRecvCtx(f.ctx, f.t, f.ch), nil
33+
select {
34+
case <-f.ctx.Done():
35+
return nil, f.ctx.Err()
36+
case conn := <-f.ch:
37+
return conn, nil
38+
}
3339
}
3440

3541
func newFakeConn(state *proto.WorkspaceUpdate) *fakeConn {
@@ -40,8 +46,9 @@ func newFakeConn(state *proto.WorkspaceUpdate) *fakeConn {
4046
}
4147

4248
type fakeConn struct {
43-
state *proto.WorkspaceUpdate
44-
closed chan struct{}
49+
state *proto.WorkspaceUpdate
50+
closed chan struct{}
51+
doClose sync.Once
4552
}
4653

4754
var _ Conn = (*fakeConn)(nil)
@@ -51,7 +58,9 @@ func (f *fakeConn) CurrentWorkspaceState() *proto.WorkspaceUpdate {
5158
}
5259

5360
func (f *fakeConn) Close() error {
54-
close(f.closed)
61+
f.doClose.Do(func() {
62+
close(f.closed)
63+
})
5564
return nil
5665
}
5766

0 commit comments

Comments
 (0)