Skip to content

Commit 8415022

Browse files
authored
fix(devtunnel): close http.Server before wireguard interface (coder#2263)
1 parent de6f86b commit 8415022

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

coderd/devtunnel/tunnel.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ type configExt struct {
5151

5252
// NewWithConfig calls New with the given config. For documentation, see New.
5353
func NewWithConfig(ctx context.Context, logger slog.Logger, cfg Config) (*Tunnel, <-chan error, error) {
54-
err := startUpdateRoutine(ctx, logger, cfg)
54+
routineEnd, err := startUpdateRoutine(ctx, logger, cfg)
5555
if err != nil {
5656
return nil, nil, xerrors.Errorf("start update routine: %w", err)
5757
}
@@ -101,6 +101,7 @@ allowed_ip=%s/128`,
101101
case <-ctx.Done():
102102
_ = wgListen.Close()
103103
dev.Close()
104+
<-routineEnd
104105
close(ch)
105106

106107
case <-dev.Wait():
@@ -128,21 +129,24 @@ func New(ctx context.Context, logger slog.Logger) (*Tunnel, <-chan error, error)
128129
return NewWithConfig(ctx, logger, cfg)
129130
}
130131

131-
func startUpdateRoutine(ctx context.Context, logger slog.Logger, cfg Config) error {
132+
func startUpdateRoutine(ctx context.Context, logger slog.Logger, cfg Config) (<-chan struct{}, error) {
132133
// Ensure we send the first config before spawning in the background.
133134
_, err := sendConfigToServer(ctx, cfg)
134135
if err != nil {
135-
return xerrors.Errorf("send config to server: %w", err)
136+
return nil, xerrors.Errorf("send config to server: %w", err)
136137
}
137138

139+
endCh := make(chan struct{})
138140
go func() {
141+
defer close(endCh)
139142
ticker := time.NewTicker(30 * time.Second)
140143
defer ticker.Stop()
141144

142145
for {
143146
select {
144147
case <-ctx.Done():
145-
break
148+
return
149+
146150
case <-ticker.C:
147151
}
148152

@@ -152,19 +156,25 @@ func startUpdateRoutine(ctx context.Context, logger slog.Logger, cfg Config) err
152156
}
153157
}
154158
}()
155-
return nil
159+
return endCh, nil
156160
}
157161

158-
func sendConfigToServer(_ context.Context, cfg Config) (created bool, err error) {
162+
func sendConfigToServer(ctx context.Context, cfg Config) (created bool, err error) {
159163
raw, err := json.Marshal(configExt(cfg))
160164
if err != nil {
161165
return false, xerrors.Errorf("marshal config: %w", err)
162166
}
163167

164-
res, err := http.Post("https://"+EndpointHTTPS+"/tun", "application/json", bytes.NewReader(raw))
168+
req, err := http.NewRequestWithContext(ctx, "POST", "https://"+EndpointHTTPS+"/tun", bytes.NewReader(raw))
169+
if err != nil {
170+
return false, xerrors.Errorf("new request: %w", err)
171+
}
172+
173+
res, err := http.DefaultClient.Do(req)
165174
if err != nil {
166-
return false, xerrors.Errorf("send request: %w", err)
175+
return false, xerrors.Errorf("do request: %w", err)
167176
}
177+
168178
_, _ = io.Copy(io.Discard, res.Body)
169179
_ = res.Body.Close()
170180

coderd/devtunnel/tunnel_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package devtunnel_test
22

33
import (
44
"context"
5+
"io"
56
"net"
67
"net/http"
78
"testing"
89
"time"
910

11+
"github.com/stretchr/testify/assert"
1012
"github.com/stretchr/testify/require"
1113

1214
"cdr.dev/slog/sloggers/slogtest"
@@ -47,17 +49,26 @@ func TestTunnel(t *testing.T) {
4749
go server.Serve(tun.Listener)
4850
defer tun.Listener.Close()
4951

52+
httpClient := &http.Client{
53+
Timeout: 10 * time.Second,
54+
}
55+
5056
require.Eventually(t, func() bool {
5157
req, err := http.NewRequestWithContext(ctx, "GET", tun.URL, nil)
5258
require.NoError(t, err)
5359

54-
res, err := http.DefaultClient.Do(req)
60+
res, err := httpClient.Do(req)
5561
require.NoError(t, err)
5662
defer res.Body.Close()
63+
_, _ = io.Copy(io.Discard, res.Body)
64+
5765
return res.StatusCode == http.StatusOK
5866
}, time.Minute, time.Second)
5967

68+
httpClient.CloseIdleConnections()
69+
assert.NoError(t, server.Close())
6070
cancelTun()
71+
6172
select {
6273
case <-errCh:
6374
case <-time.After(10 * time.Second):

0 commit comments

Comments
 (0)