Skip to content

Commit 62efacd

Browse files
kylecarbskylecarbs
committed
fix: Replace access URL for built-in DERP servers
Fixes #4195.
1 parent 4054a9c commit 62efacd

File tree

10 files changed

+177
-67
lines changed

10 files changed

+177
-67
lines changed

‎agent/agent_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -481,19 +481,6 @@ func TestAgent(t *testing.T) {
481481
}
482482
})
483483

484-
t.Run("Tailnet", func(t *testing.T) {
485-
t.Parallel()
486-
derpMap := tailnettest.RunDERPAndSTUN(t)
487-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{
488-
DERPMap: derpMap,
489-
}, 0)
490-
defer conn.Close()
491-
require.Eventually(t, func() bool {
492-
_, err := conn.Ping()
493-
return err == nil
494-
}, testutil.WaitLong, testutil.IntervalFast)
495-
})
496-
497484
t.Run("Speedtest", func(t *testing.T) {
498485
t.Parallel()
499486
if testing.Short() {

‎cli/server.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -327,18 +327,21 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
327327
validatedAutoImportTemplates[i] = v
328328
}
329329

330-
defaultRegion := &tailcfg.DERPRegion{
331-
RegionID: derpServerRegionID,
332-
RegionCode: derpServerRegionCode,
333-
RegionName: derpServerRegionName,
334-
Nodes: []*tailcfg.DERPNode{{
335-
Name: fmt.Sprintf("%db", derpServerRegionID),
336-
RegionID: derpServerRegionID,
337-
HostName: accessURLParsed.Hostname(),
338-
DERPPort: accessURLPort,
339-
STUNPort: -1,
340-
ForceHTTP: accessURLParsed.Scheme == "http",
341-
}},
330+
defaultRegion := &tailnet.DERPRegion{
331+
DERPRegion: &tailcfg.DERPRegion{
332+
RegionID: derpServerRegionID,
333+
RegionCode: derpServerRegionCode,
334+
RegionName: derpServerRegionName,
335+
Nodes: []*tailcfg.DERPNode{{
336+
Name: fmt.Sprintf("%db", derpServerRegionID),
337+
RegionID: derpServerRegionID,
338+
HostName: accessURLParsed.Hostname(),
339+
DERPPort: accessURLPort,
340+
STUNPort: -1,
341+
ForceHTTP: accessURLParsed.Scheme == "http",
342+
}},
343+
},
344+
EmbeddedRelay: true,
342345
}
343346
if !derpServerEnabled {
344347
defaultRegion = nil

‎coderd/coderd.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"google.golang.org/api/idtoken"
2121
"tailscale.com/derp"
2222
"tailscale.com/derp/derphttp"
23-
"tailscale.com/tailcfg"
2423
"tailscale.com/types/key"
2524

2625
"cdr.dev/slog"
@@ -75,7 +74,7 @@ type Options struct {
7574
AutoImportTemplates []AutoImportTemplate
7675

7776
TailnetCoordinator *tailnet.Coordinator
78-
DERPMap *tailcfg.DERPMap
77+
DERPMap *tailnet.DERPMap
7978

8079
MetricsCacheRefreshInterval time.Duration
8180
AgentStatsRefreshInterval time.Duration

‎coderd/workspaceagents.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ func convertApps(dbApps []database.WorkspaceApp) []codersdk.WorkspaceApp {
469469
return apps
470470
}
471471

472-
func convertWorkspaceAgent(derpMap *tailcfg.DERPMap, coordinator *tailnet.Coordinator, dbAgent database.WorkspaceAgent, apps []codersdk.WorkspaceApp, agentInactiveDisconnectTimeout time.Duration) (codersdk.WorkspaceAgent, error) {
472+
func convertWorkspaceAgent(derpMap *tailnet.DERPMap, coordinator *tailnet.Coordinator, dbAgent database.WorkspaceAgent, apps []codersdk.WorkspaceApp, agentInactiveDisconnectTimeout time.Duration) (codersdk.WorkspaceAgent, error) {
473473
var envs map[string]string
474474
if dbAgent.EnvironmentVariables.Valid {
475475
err := json.Unmarshal(dbAgent.EnvironmentVariables.RawMessage, &envs)
@@ -506,9 +506,11 @@ func convertWorkspaceAgent(derpMap *tailcfg.DERPMap, coordinator *tailnet.Coordi
506506
// It's possible that a workspace agent is using an old DERPMap
507507
// and reports regions that do not exist. If that's the case,
508508
// report the region as unknown!
509-
region = &tailcfg.DERPRegion{
510-
RegionID: regionID,
511-
RegionName: fmt.Sprintf("Unnamed %d", regionID),
509+
region = &tailnet.DERPRegion{
510+
DERPRegion: &tailcfg.DERPRegion{
511+
RegionID: regionID,
512+
RegionName: fmt.Sprintf("Unnamed %d", regionID),
513+
},
512514
}
513515
}
514516
workspaceAgent.DERPLatency[region.RegionName] = codersdk.DERPRegion{

‎codersdk/workspaceagents.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ import (
1010
"net/http"
1111
"net/http/cookiejar"
1212
"net/netip"
13+
"strconv"
1314
"time"
1415

1516
"cloud.google.com/go/compute/metadata"
1617
"github.com/google/uuid"
1718
"golang.org/x/xerrors"
1819
"nhooyr.io/websocket"
1920
"nhooyr.io/websocket/wsjson"
20-
"tailscale.com/tailcfg"
2121

2222
"cdr.dev/slog"
2323

@@ -53,7 +53,7 @@ type WorkspaceAgentAuthenticateResponse struct {
5353
// a connection with a workspace.
5454
// @typescript-ignore WorkspaceAgentConnectionInfo
5555
type WorkspaceAgentConnectionInfo struct {
56-
DERPMap *tailcfg.DERPMap `json:"derp_map"`
56+
DERPMap *tailnet.DERPMap `json:"derp_map"`
5757
}
5858

5959
// @typescript-ignore PostWorkspaceAgentVersionRequest
@@ -63,7 +63,7 @@ type PostWorkspaceAgentVersionRequest struct {
6363

6464
// @typescript-ignore WorkspaceAgentMetadata
6565
type WorkspaceAgentMetadata struct {
66-
DERPMap *tailcfg.DERPMap `json:"derpmap"`
66+
DERPMap *tailnet.DERPMap `json:"derpmap"`
6767
EnvironmentVariables map[string]string `json:"environment_variables"`
6868
StartupScript string `json:"startup_script"`
6969
Directory string `json:"directory"`
@@ -208,7 +208,42 @@ func (c *Client) WorkspaceAgentMetadata(ctx context.Context) (WorkspaceAgentMeta
208208
return WorkspaceAgentMetadata{}, readBodyAsError(res)
209209
}
210210
var agentMetadata WorkspaceAgentMetadata
211-
return agentMetadata, json.NewDecoder(res.Body).Decode(&agentMetadata)
211+
err = json.NewDecoder(res.Body).Decode(&agentMetadata)
212+
if err != nil {
213+
return WorkspaceAgentMetadata{}, err
214+
}
215+
accessingPort := c.URL.Port()
216+
if accessingPort == "" {
217+
accessingPort = "80"
218+
if c.URL.Scheme == "https" {
219+
accessingPort = "443"
220+
}
221+
}
222+
accessPort, err := strconv.Atoi(accessingPort)
223+
if err != nil {
224+
return WorkspaceAgentMetadata{}, xerrors.Errorf("convert accessing port %q: %w", accessingPort, err)
225+
}
226+
// Agents can provide an arbitrary access URL that may be different
227+
// that the globally configured one. This breaks the built-in DERP,
228+
// which would continue to reference the global access URL.
229+
//
230+
// This converts all built-in DERPs to use the access URL that the
231+
// metadata request was performed with.
232+
for _, region := range agentMetadata.DERPMap.Regions {
233+
if !region.EmbeddedRelay {
234+
continue
235+
}
236+
237+
for _, node := range region.Nodes {
238+
if node.STUNOnly {
239+
continue
240+
}
241+
node.HostName = c.URL.Hostname()
242+
node.DERPPort = accessPort
243+
node.ForceHTTP = c.URL.Scheme == "http"
244+
}
245+
}
246+
return agentMetadata, nil
212247
}
213248

214249
func (c *Client) ListenWorkspaceAgentTailnet(ctx context.Context) (net.Conn, error) {

‎codersdk/workspaceagents_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package codersdk_test
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
8+
"strconv"
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
"tailscale.com/tailcfg"
13+
14+
"github.com/coder/coder/coderd/httpapi"
15+
"github.com/coder/coder/codersdk"
16+
"github.com/coder/coder/tailnet"
17+
)
18+
19+
func TestWorkspaceAgentMetadata(t *testing.T) {
20+
t.Parallel()
21+
// This test ensures that the DERP map returned properly
22+
// mutates built-in DERPs with the client access URL.
23+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
24+
httpapi.Write(context.Background(), w, http.StatusOK, codersdk.WorkspaceAgentMetadata{
25+
DERPMap: &tailnet.DERPMap{
26+
Regions: map[int]*tailnet.DERPRegion{
27+
1: {
28+
EmbeddedRelay: true,
29+
DERPRegion: &tailcfg.DERPRegion{
30+
RegionID: 1,
31+
Nodes: []*tailcfg.DERPNode{{
32+
HostName: "bananas.org",
33+
DERPPort: 1,
34+
}},
35+
},
36+
},
37+
},
38+
},
39+
})
40+
}))
41+
parsed, err := url.Parse(srv.URL)
42+
require.NoError(t, err)
43+
client := codersdk.New(parsed)
44+
metadata, err := client.WorkspaceAgentMetadata(context.Background())
45+
require.NoError(t, err)
46+
region := metadata.DERPMap.Regions[1]
47+
require.True(t, region.EmbeddedRelay)
48+
require.Len(t, region.Nodes, 1)
49+
node := region.Nodes[0]
50+
require.Equal(t, parsed.Hostname(), node.HostName)
51+
require.Equal(t, parsed.Port(), strconv.Itoa(node.DERPPort))
52+
}

‎tailnet/conn.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func init() {
4646

4747
type Options struct {
4848
Addresses []netip.Prefix
49-
DERPMap *tailcfg.DERPMap
49+
DERPMap *DERPMap
5050

5151
Logger slog.Logger
5252
}
@@ -161,7 +161,7 @@ func NewConn(options *Options) (*Conn, error) {
161161
return nil, xerrors.Errorf("start netstack: %w", err)
162162
}
163163
wireguardEngine = wgengine.NewWatchdog(wireguardEngine)
164-
wireguardEngine.SetDERPMap(options.DERPMap)
164+
wireguardEngine.SetDERPMap(options.DERPMap.Convert())
165165
netMapCopy := *netMap
166166
wireguardEngine.SetNetworkMap(&netMapCopy)
167167

@@ -279,12 +279,12 @@ func (c *Conn) SetNodeCallback(callback func(node *Node)) {
279279
}
280280

281281
// SetDERPMap updates the DERPMap of a connection.
282-
func (c *Conn) SetDERPMap(derpMap *tailcfg.DERPMap) {
282+
func (c *Conn) SetDERPMap(derpMap *DERPMap) {
283283
c.mutex.Lock()
284284
defer c.mutex.Unlock()
285-
c.netMap.DERPMap = derpMap
285+
c.netMap.DERPMap = derpMap.Convert()
286286
c.logger.Debug(context.Background(), "updating derp map", slog.F("derp_map", derpMap))
287-
c.wireguardEngine.SetDERPMap(derpMap)
287+
c.wireguardEngine.SetDERPMap(derpMap.Convert())
288288
}
289289

290290
// UpdateNodes connects with a set of peers. This can be constantly updated,

‎tailnet/derpmap.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,31 @@ import (
1313
"tailscale.com/tailcfg"
1414
)
1515

16+
// DERPMap matches `*tailcfg.DERPMap` but with our region type.
17+
type DERPMap struct {
18+
Regions map[int]*DERPRegion
19+
}
20+
21+
func (d *DERPMap) Convert() *tailcfg.DERPMap {
22+
n := &tailcfg.DERPMap{
23+
Regions: map[int]*tailcfg.DERPRegion{},
24+
}
25+
for key, region := range d.Regions {
26+
n.Regions[key] = region.DERPRegion
27+
}
28+
return n
29+
}
30+
31+
// DERPRegion extends `*tailcfg.DERPRegion` to provide additional options.
32+
type DERPRegion struct {
33+
*tailcfg.DERPRegion
34+
// EmbeddedRelay specifies whether the region is built-in.
35+
EmbeddedRelay bool
36+
}
37+
1638
// NewDERPMap constructs a DERPMap from a set of STUN addresses and optionally a remote
1739
// URL to fetch a mapping from e.g. https://controlplane.tailscale.com/derpmap/default.
18-
func NewDERPMap(ctx context.Context, region *tailcfg.DERPRegion, stunAddrs []string, remoteURL, localPath string) (*tailcfg.DERPMap, error) {
40+
func NewDERPMap(ctx context.Context, region *DERPRegion, stunAddrs []string, remoteURL, localPath string) (*DERPMap, error) {
1941
if remoteURL != "" && localPath != "" {
2042
return nil, xerrors.New("a remote URL or local path must be specified, not both")
2143
}
@@ -39,8 +61,8 @@ func NewDERPMap(ctx context.Context, region *tailcfg.DERPRegion, stunAddrs []str
3961
}
4062
}
4163

42-
derpMap := &tailcfg.DERPMap{
43-
Regions: map[int]*tailcfg.DERPRegion{},
64+
derpMap := &DERPMap{
65+
Regions: map[int]*DERPRegion{},
4466
}
4567
if remoteURL != "" {
4668
req, err := http.NewRequestWithContext(ctx, http.MethodGet, remoteURL, nil)

‎tailnet/derpmap_test.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ func TestNewDERPMap(t *testing.T) {
1919
t.Parallel()
2020
t.Run("WithoutRemoteURL", func(t *testing.T) {
2121
t.Parallel()
22-
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailcfg.DERPRegion{
23-
RegionID: 1,
24-
Nodes: []*tailcfg.DERPNode{{}},
22+
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailnet.DERPRegion{
23+
DERPRegion: &tailcfg.DERPRegion{
24+
RegionID: 1,
25+
Nodes: []*tailcfg.DERPNode{{}},
26+
},
2527
}, []string{"stun.google.com:2345"}, "", "")
2628
require.NoError(t, err)
2729
require.Len(t, derpMap.Regions[1].Nodes, 2)
@@ -37,8 +39,10 @@ func TestNewDERPMap(t *testing.T) {
3739
_, _ = w.Write(data)
3840
}))
3941
t.Cleanup(server.Close)
40-
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailcfg.DERPRegion{
41-
RegionID: 2,
42+
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailnet.DERPRegion{
43+
DERPRegion: &tailcfg.DERPRegion{
44+
RegionID: 2,
45+
},
4246
}, []string{}, server.URL, "")
4347
require.NoError(t, err)
4448
require.Len(t, derpMap.Regions, 2)
@@ -54,8 +58,10 @@ func TestNewDERPMap(t *testing.T) {
5458
_, _ = w.Write(data)
5559
}))
5660
t.Cleanup(server.Close)
57-
_, err := tailnet.NewDERPMap(context.Background(), &tailcfg.DERPRegion{
58-
RegionID: 1,
61+
_, err := tailnet.NewDERPMap(context.Background(), &tailnet.DERPRegion{
62+
DERPRegion: &tailcfg.DERPRegion{
63+
RegionID: 1,
64+
},
5965
}, []string{}, server.URL, "")
6066
require.Error(t, err)
6167
})
@@ -70,8 +76,10 @@ func TestNewDERPMap(t *testing.T) {
7076
require.NoError(t, err)
7177
err = os.WriteFile(localPath, content, 0600)
7278
require.NoError(t, err)
73-
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailcfg.DERPRegion{
74-
RegionID: 2,
79+
derpMap, err := tailnet.NewDERPMap(context.Background(), &tailnet.DERPRegion{
80+
DERPRegion: &tailcfg.DERPRegion{
81+
RegionID: 2,
82+
},
7583
}, []string{}, "", localPath)
7684
require.NoError(t, err)
7785
require.Len(t, derpMap.Regions, 2)

0 commit comments

Comments
 (0)