Skip to content

Commit 64df076

Browse files
deansheatherdeansheather
authored
feat: add server flag to force DERP to use always websockets (#9238)
1 parent 9cb913f commit 64df076

28 files changed

+280
-68
lines changed

‎agent/agent.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ func (a *agent) run(ctx context.Context) error {
678678
network := a.network
679679
a.closeMutex.Unlock()
680680
if network == nil {
681-
network, err = a.createTailnet(ctx, manifest.AgentID, manifest.DERPMap, manifest.DisableDirectConnections)
681+
network, err = a.createTailnet(ctx, manifest.AgentID, manifest.DERPMap, manifest.DERPForceWebSockets, manifest.DisableDirectConnections)
682682
if err != nil {
683683
return xerrors.Errorf("create tailnet: %w", err)
684684
}
@@ -701,8 +701,10 @@ func (a *agent) run(ctx context.Context) error {
701701
if err != nil {
702702
a.logger.Error(ctx, "update tailnet addresses", slog.Error(err))
703703
}
704-
// Update the DERP map and allow/disallow direct connections.
704+
// Update the DERP map, force WebSocket setting and allow/disallow
705+
// direct connections.
705706
network.SetDERPMap(manifest.DERPMap)
707+
network.SetDERPForceWebSockets(manifest.DERPForceWebSockets)
706708
network.SetBlockEndpoints(manifest.DisableDirectConnections)
707709
}
708710

@@ -756,14 +758,15 @@ func (a *agent) trackConnGoroutine(fn func()) error {
756758
return nil
757759
}
758760

759-
func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *tailcfg.DERPMap, disableDirectConnections bool) (_ *tailnet.Conn, err error) {
761+
func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *tailcfg.DERPMap, derpForceWebSockets, disableDirectConnections bool) (_ *tailnet.Conn, err error) {
760762
network, err := tailnet.NewConn(&tailnet.Options{
761-
ID: agentID,
762-
Addresses: a.wireguardAddresses(agentID),
763-
DERPMap: derpMap,
764-
Logger: a.logger.Named("net.tailnet"),
765-
ListenPort: a.tailnetListenPort,
766-
BlockEndpoints: disableDirectConnections,
763+
ID: agentID,
764+
Addresses: a.wireguardAddresses(agentID),
765+
DERPMap: derpMap,
766+
DERPForceWebSockets: derpForceWebSockets,
767+
Logger: a.logger.Named("net.tailnet"),
768+
ListenPort: a.tailnetListenPort,
769+
BlockEndpoints: disableDirectConnections,
767770
})
768771
if err != nil {
769772
return nil, xerrors.Errorf("create tailnet: %w", err)

‎cli/netcheck_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestNetcheck(t *testing.T) {
3131
require.NoError(t, json.Unmarshal(b, &report))
3232

3333
assert.True(t, report.Healthy)
34-
require.Len(t, report.Regions, 1+5) // 1 built-in region + 5 STUN regions by default
34+
require.Len(t, report.Regions, 1+1) // 1 built-in region + 1 test-managed STUN region
3535
for _, v := range report.Regions {
3636
require.Len(t, v.NodeReports, len(v.Region.Nodes))
3737
}

‎cli/testdata/coder_server_--help.golden

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,13 @@ backed by Tailscale and WireGuard.
172172
URL to fetch a DERP mapping on startup. See:
173173
https://tailscale.com/kb/1118/custom-derp-servers/.
174174

175+
--derp-force-websockets bool, $CODER_DERP_FORCE_WEBSOCKETS
176+
Force clients and agents to always use WebSocket to connect to DERP
177+
relay servers. By default, DERP uses `Upgrade: derp`, which may cause
178+
issues with some reverse proxies. Clients may automatically fallback
179+
to WebSocket if they detect an issue with `Upgrade: derp`, but this
180+
does not work in all situations.
181+
175182
--derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true)
176183
Whether to enable or disable the embedded DERP relay server.
177184

‎cli/testdata/server-config.yaml.golden

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ networking:
136136
# this change has been made, but new connections will still be proxied regardless.
137137
# (default: <unset>, type: bool)
138138
blockDirect: false
139+
# Force clients and agents to always use WebSocket to connect to DERP relay
140+
# servers. By default, DERP uses `Upgrade: derp`, which may cause issues with some
141+
# reverse proxies. Clients may automatically fallback to WebSocket if they detect
142+
# an issue with `Upgrade: derp`, but this does not work in all situations.
143+
# (default: <unset>, type: bool)
144+
forceWebSockets: false
139145
# URL to fetch a DERP mapping on startup. See:
140146
# https://tailscale.com/kb/1118/custom-derp-servers/.
141147
# (default: <unset>, type: string)

‎coderd/apidoc/docs.go

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ func New(options *Options) *API {
405405
options.Logger,
406406
options.DERPServer,
407407
api.DERPMap,
408+
options.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
408409
func(context.Context) (tailnet.MultiAgentConn, error) {
409410
return (*api.TailnetCoordinator.Load()).ServeMultiAgent(uuid.New()), nil
410411
},

‎coderd/coderd_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@ import (
77
"net/http"
88
"net/netip"
99
"strconv"
10+
"strings"
1011
"sync"
12+
"sync/atomic"
1113
"testing"
1214

15+
"github.com/davecgh/go-spew/spew"
16+
"github.com/google/uuid"
1317
"github.com/stretchr/testify/assert"
1418
"github.com/stretchr/testify/require"
1519
"go.uber.org/goleak"
@@ -18,8 +22,13 @@ import (
1822
"cdr.dev/slog"
1923
"cdr.dev/slog/sloggers/slogtest"
2024

25+
"github.com/coder/coder/v2/agent"
2126
"github.com/coder/coder/v2/buildinfo"
27+
"github.com/coder/coder/v2/coderd"
2228
"github.com/coder/coder/v2/coderd/coderdtest"
29+
"github.com/coder/coder/v2/codersdk"
30+
"github.com/coder/coder/v2/codersdk/agentsdk"
31+
"github.com/coder/coder/v2/provisioner/echo"
2332
"github.com/coder/coder/v2/tailnet"
2433
"github.com/coder/coder/v2/testutil"
2534
)
@@ -119,6 +128,91 @@ func TestDERP(t *testing.T) {
119128
w2.Close()
120129
}
121130

131+
func TestDERPForceWebSockets(t *testing.T) {
132+
t.Parallel()
133+
134+
dv := coderdtest.DeploymentValues(t)
135+
dv.DERP.Config.ForceWebSockets = true
136+
dv.DERP.Config.BlockDirect = true // to ensure the test always uses DERP
137+
138+
// Manually create a server so we can influence the HTTP handler.
139+
options := &coderdtest.Options{
140+
DeploymentValues: dv,
141+
}
142+
setHandler, cancelFunc, serverURL, newOptions := coderdtest.NewOptions(t, options)
143+
coderAPI := coderd.New(newOptions)
144+
t.Cleanup(func() {
145+
cancelFunc()
146+
_ = coderAPI.Close()
147+
})
148+
149+
// Set the HTTP handler to a custom one that ensures all /derp calls are
150+
// WebSockets and not `Upgrade: derp`.
151+
var upgradeCount int64
152+
setHandler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
153+
if strings.HasPrefix(r.URL.Path, "/derp") {
154+
up := r.Header.Get("Upgrade")
155+
if up != "" && up != "websocket" {
156+
t.Errorf("expected Upgrade: websocket, got %q", up)
157+
} else {
158+
atomic.AddInt64(&upgradeCount, 1)
159+
}
160+
}
161+
162+
coderAPI.RootHandler.ServeHTTP(rw, r)
163+
}))
164+
165+
// Start a provisioner daemon.
166+
provisionerCloser := coderdtest.NewProvisionerDaemon(t, coderAPI)
167+
t.Cleanup(func() {
168+
_ = provisionerCloser.Close()
169+
})
170+
171+
client := codersdk.New(serverURL)
172+
t.Cleanup(func() {
173+
client.HTTPClient.CloseIdleConnections()
174+
})
175+
user := coderdtest.CreateFirstUser(t, client)
176+
177+
gen, err := client.WorkspaceAgentConnectionInfoGeneric(context.Background())
178+
require.NoError(t, err)
179+
t.Log(spew.Sdump(gen))
180+
181+
authToken := uuid.NewString()
182+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
183+
Parse: echo.ParseComplete,
184+
ProvisionPlan: echo.ProvisionComplete,
185+
ProvisionApply: echo.ProvisionApplyWithAgent(authToken),
186+
})
187+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
188+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
189+
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
190+
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
191+
192+
agentClient := agentsdk.New(client.URL)
193+
agentClient.SetSessionToken(authToken)
194+
agentCloser := agent.New(agent.Options{
195+
Client: agentClient,
196+
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
197+
})
198+
defer func() {
199+
_ = agentCloser.Close()
200+
}()
201+
202+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
203+
defer cancel()
204+
205+
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
206+
conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
207+
require.NoError(t, err)
208+
defer func() {
209+
_ = conn.Close()
210+
}()
211+
conn.AwaitReachable(ctx)
212+
213+
require.GreaterOrEqual(t, atomic.LoadInt64(&upgradeCount), int64(1), "expected at least one /derp call")
214+
}
215+
122216
func TestDERPLatencyCheck(t *testing.T) {
123217
t.Parallel()
124218
client := coderdtest.New(t, nil)

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
326326
stunAddresses []string
327327
dvStunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value()
328328
)
329-
if len(dvStunAddresses) == 0 || (len(dvStunAddresses) == 1 && dvStunAddresses[0] == "stun.l.google.com:19302") {
329+
if len(dvStunAddresses) == 0 || dvStunAddresses[0] == "stun.l.google.com:19302" {
330330
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
331331
stunAddr.IP = net.ParseIP("127.0.0.1")
332332
t.Cleanup(stunCleanup)

‎coderd/tailnet.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,18 @@ func NewServerTailnet(
4545
logger slog.Logger,
4646
derpServer *derp.Server,
4747
derpMapFn func() *tailcfg.DERPMap,
48+
derpForceWebSockets bool,
4849
getMultiAgent func(context.Context) (tailnet.MultiAgentConn, error),
4950
cache *wsconncache.Cache,
5051
traceProvider trace.TracerProvider,
5152
) (*ServerTailnet, error) {
5253
logger = logger.Named("servertailnet")
5354
originalDerpMap := derpMapFn()
5455
conn, err := tailnet.NewConn(&tailnet.Options{
55-
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
56-
DERPMap: originalDerpMap,
57-
Logger: logger,
56+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
57+
DERPMap: originalDerpMap,
58+
DERPForceWebSockets: derpForceWebSockets,
59+
Logger: logger,
5860
})
5961
if err != nil {
6062
return nil, xerrors.Errorf("create tailnet conn: %w", err)

‎coderd/tailnet_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ func setupAgent(t *testing.T, agentAddresses []netip.Prefix) (uuid.UUID, agent.A
232232
logger,
233233
derpServer,
234234
func() *tailcfg.DERPMap { return manifest.DERPMap },
235+
false,
235236
func(context.Context) (tailnet.MultiAgentConn, error) { return coord.ServeMultiAgent(uuid.New()), nil },
236237
cache,
237238
trace.NewNoopTracerProvider(),

‎coderd/workspaceagents.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
167167
AgentID: apiAgent.ID,
168168
Apps: convertApps(dbApps),
169169
DERPMap: api.DERPMap(),
170+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
170171
GitAuthConfigs: len(api.GitAuthConfigs),
171172
EnvironmentVariables: apiAgent.EnvironmentVariables,
172173
StartupScript: apiAgent.StartupScript,
@@ -733,10 +734,11 @@ func (api *API) _dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.Workspa
733734

734735
derpMap := api.DERPMap()
735736
conn, err := tailnet.NewConn(&tailnet.Options{
736-
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
737-
DERPMap: api.DERPMap(),
738-
Logger: api.Logger.Named("net.tailnet"),
739-
BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
737+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
738+
DERPMap: api.DERPMap(),
739+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
740+
Logger: api.Logger.Named("net.tailnet"),
741+
BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
740742
})
741743
if err != nil {
742744
_ = clientConn.Close()
@@ -831,6 +833,7 @@ func (api *API) workspaceAgentConnection(rw http.ResponseWriter, r *http.Request
831833

832834
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
833835
DERPMap: api.DERPMap(),
836+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
834837
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
835838
})
836839
}
@@ -851,6 +854,7 @@ func (api *API) workspaceAgentConnectionGeneric(rw http.ResponseWriter, r *http.
851854

852855
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
853856
DERPMap: api.DERPMap(),
857+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
854858
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
855859
})
856860
}

‎coderd/wsconncache/wsconncache_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,10 @@ func setupAgent(t *testing.T, manifest agentsdk.Manifest, ptyTimeout time.Durati
179179
_ = closer.Close()
180180
})
181181
conn, err := tailnet.NewConn(&tailnet.Options{
182-
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
183-
DERPMap: manifest.DERPMap,
184-
Logger: slogtest.Make(t, nil).Named("tailnet").Leveled(slog.LevelDebug),
182+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
183+
DERPMap: manifest.DERPMap,
184+
DERPForceWebSockets: manifest.DERPForceWebSockets,
185+
Logger: slogtest.Make(t, nil).Named("tailnet").Leveled(slog.LevelDebug),
185186
})
186187
require.NoError(t, err)
187188
clientConn, serverConn := net.Pipe()

‎codersdk/agentsdk/agentsdk.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ type Manifest struct {
8989
VSCodePortProxyURI string `json:"vscode_port_proxy_uri"`
9090
Apps []codersdk.WorkspaceApp `json:"apps"`
9191
DERPMap *tailcfg.DERPMap `json:"derpmap"`
92+
DERPForceWebSockets bool `json:"derp_force_websockets"`
9293
EnvironmentVariables map[string]string `json:"environment_variables"`
9394
StartupScript string `json:"startup_script"`
9495
StartupScriptTimeout time.Duration `json:"startup_script_timeout"`

0 commit comments

Comments
 (0)