Skip to content

Commit 5d5db89

Browse files
deansheatherdeansheather
committed
feat: add server flag to force DERP to use always websockets
1 parent 545a256 commit 5d5db89

22 files changed

+260
-40
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: 9 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: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ import (
66
"net/http"
77
"net/netip"
88
"strconv"
9+
"strings"
910
"sync"
11+
"sync/atomic"
1012
"testing"
1113

14+
"github.com/davecgh/go-spew/spew"
15+
"github.com/google/uuid"
1216
"github.com/stretchr/testify/assert"
1317
"github.com/stretchr/testify/require"
1418
"go.uber.org/goleak"
@@ -17,8 +21,13 @@ import (
1721
"cdr.dev/slog"
1822
"cdr.dev/slog/sloggers/slogtest"
1923

24+
"github.com/coder/coder/v2/agent"
2025
"github.com/coder/coder/v2/buildinfo"
26+
"github.com/coder/coder/v2/coderd"
2127
"github.com/coder/coder/v2/coderd/coderdtest"
28+
"github.com/coder/coder/v2/codersdk"
29+
"github.com/coder/coder/v2/codersdk/agentsdk"
30+
"github.com/coder/coder/v2/provisioner/echo"
2231
"github.com/coder/coder/v2/tailnet"
2332
"github.com/coder/coder/v2/testutil"
2433
)
@@ -115,6 +124,94 @@ func TestDERP(t *testing.T) {
115124
w2.Close()
116125
}
117126

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"encoding/base64"
99
"encoding/json"
1010
"encoding/pem"
11-
"fmt"
1211
"io"
1312
"net/http"
1413
"net/url"
@@ -243,7 +242,7 @@ func (src *jwtTokenSource) Token() (*oauth2.Token, error) {
243242
}
244243

245244
if unmarshalError != nil {
246-
return nil, fmt.Errorf("oauth2: cannot unmarshal token: %w", err)
245+
return nil, xerrors.Errorf("oauth2: cannot unmarshal token: %w", err)
247246
}
248247

249248
newToken := &oauth2.Token{
@@ -264,7 +263,7 @@ func (src *jwtTokenSource) Token() (*oauth2.Token, error) {
264263
// decode returned id token to get expiry
265264
claimSet, err := jws.Decode(v)
266265
if err != nil {
267-
return nil, fmt.Errorf("oauth2: error decoding JWT token: %w", err)
266+
return nil, xerrors.Errorf("oauth2: error decoding JWT token: %w", err)
268267
}
269268
newToken.Expiry = time.Unix(claimSet.Exp, 0)
270269
}

‎coderd/workspaceagents.go

Lines changed: 3 additions & 0 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,
@@ -831,6 +832,7 @@ func (api *API) workspaceAgentConnection(rw http.ResponseWriter, r *http.Request
831832

832833
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
833834
DERPMap: api.DERPMap(),
835+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
834836
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
835837
})
836838
}
@@ -851,6 +853,7 @@ func (api *API) workspaceAgentConnectionGeneric(rw http.ResponseWriter, r *http.
851853

852854
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
853855
DERPMap: api.DERPMap(),
856+
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
854857
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
855858
})
856859
}

‎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"`

‎codersdk/deployment.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,10 @@ type DERPServerConfig struct {
228228
}
229229

230230
type DERPConfig struct {
231-
BlockDirect clibase.Bool `json:"block_direct" typescript:",notnull"`
232-
URL clibase.String `json:"url" typescript:",notnull"`
233-
Path clibase.String `json:"path" typescript:",notnull"`
231+
BlockDirect clibase.Bool `json:"block_direct" typescript:",notnull"`
232+
ForceWebSockets clibase.Bool `json:"force_websockets" typescript:",notnull"`
233+
URL clibase.String `json:"url" typescript:",notnull"`
234+
Path clibase.String `json:"path" typescript:",notnull"`
234235
}
235236

236237
type PrometheusConfig struct {
@@ -796,6 +797,15 @@ when required by your organization's security policy.`,
796797
Group: &deploymentGroupNetworkingDERP,
797798
YAML: "blockDirect",
798799
},
800+
{
801+
Name: "DERP Force WebSockets",
802+
Description: "Force clients and agents to always use WebSocket to connect to DERP relay servers. By default, DERP uses `Upgrade: derp`, which may cause issues with some reverse proxies. Clients may automatically fallback to WebSocket if they detect an issue with `Upgrade: derp`, but this does not work in all situations.",
803+
Flag: "derp-force-websockets",
804+
Env: "CODER_DERP_FORCE_WEBSOCKETS",
805+
Value: &c.DERP.Config.ForceWebSockets,
806+
Group: &deploymentGroupNetworkingDERP,
807+
YAML: "forceWebSockets",
808+
},
799809
{
800810
Name: "DERP Config URL",
801811
Description: "URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/.",

‎codersdk/workspaceagents.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ type DERPRegion struct {
186186
// @typescript-ignore WorkspaceAgentConnectionInfo
187187
type WorkspaceAgentConnectionInfo struct {
188188
DERPMap *tailcfg.DERPMap `json:"derp_map"`
189+
DERPForceWebSockets bool `json:"derp_force_websockets"`
189190
DisableDirectConnections bool `json:"disable_direct_connections"`
190191
}
191192

@@ -247,11 +248,12 @@ func (c *Client) DialWorkspaceAgent(ctx context.Context, agentID uuid.UUID, opti
247248
header = headerTransport.Header()
248249
}
249250
conn, err := tailnet.NewConn(&tailnet.Options{
250-
Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)},
251-
DERPMap: connInfo.DERPMap,
252-
DERPHeader: &header,
253-
Logger: options.Logger,
254-
BlockEndpoints: c.DisableDirectConnections || options.BlockEndpoints,
251+
Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)},
252+
DERPMap: connInfo.DERPMap,
253+
DERPHeader: &header,
254+
DERPForceWebSockets: connInfo.DERPForceWebSockets,
255+
Logger: options.Logger,
256+
BlockEndpoints: c.DisableDirectConnections || options.BlockEndpoints,
255257
})
256258
if err != nil {
257259
return nil, xerrors.Errorf("create tailnet: %w", err)

0 commit comments

Comments
 (0)