Skip to content

fix: use client preferred URL for the default DERP #18911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type Client interface {
ConnectRPC26(ctx context.Context) (
proto.DRPCAgentClient26, tailnetproto.DRPCTailnetClient26, error,
)
RewriteDERPMap(derpMap *tailcfg.DERPMap)
tailnet.DERPMapRewriter
}

type Agent interface {
Expand Down
2 changes: 1 addition & 1 deletion coderd/tailnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func NewServerTailnet(
controller := tailnet.NewController(logger, dialer)
// it's important to set the DERPRegionDialer above _before_ we set the DERP map so that if
// there is an embedded relay, we use the local in-memory dialer.
controller.DERPCtrl = tailnet.NewBasicDERPController(logger, conn)
controller.DERPCtrl = tailnet.NewBasicDERPController(logger, nil, conn)
coordCtrl := NewMultiAgentController(serverCtx, logger, tracer, conn)
controller.CoordCtrl = coordCtrl
// TODO: support controller.TelemetryCtrl
Expand Down
39 changes: 6 additions & 33 deletions codersdk/agentsdk/agentsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"net/http"
"net/http/cookiejar"
"net/url"
"strconv"
"time"

"cloud.google.com/go/compute/metadata"
Expand All @@ -27,6 +26,7 @@ import (
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/drpcsdk"
"github.com/coder/coder/v2/tailnet"
tailnetproto "github.com/coder/coder/v2/tailnet/proto"
)

Expand Down Expand Up @@ -126,40 +126,13 @@ type Script struct {
Script string `json:"script"`
}

// RewriteDERPMap rewrites the DERP map to use the access URL of the SDK as the
// "embedded relay" access URL. The passed derp map is modified in place.
// RewriteDERPMap rewrites the DERP map to use the configured access URL of the
// agent as the "embedded relay" access URL.
//
// Agents can provide an arbitrary access URL that may be different that the
// globally configured one. This breaks the built-in DERP, which would continue
// to reference the global access URL.
// See tailnet.RewriteDERPMapDefaultRelay for more details on why this is
// necessary.
func (c *Client) RewriteDERPMap(derpMap *tailcfg.DERPMap) {
accessingPort := c.SDK.URL.Port()
if accessingPort == "" {
accessingPort = "80"
if c.SDK.URL.Scheme == "https" {
accessingPort = "443"
}
}
accessPort, err := strconv.Atoi(accessingPort)
if err != nil {
// this should never happen because URL.Port() returns the empty string if the port is not
// valid.
c.SDK.Logger().Critical(context.Background(), "failed to parse URL port", slog.F("port", accessingPort))
}
for _, region := range derpMap.Regions {
if !region.EmbeddedRelay {
continue
}

for _, node := range region.Nodes {
if node.STUNOnly {
continue
}
node.HostName = c.SDK.URL.Hostname()
node.DERPPort = accessPort
node.ForceHTTP = c.SDK.URL.Scheme == "http"
}
}
tailnet.RewriteDERPMapDefaultRelay(context.Background(), c.SDK.Logger(), derpMap, c.SDK.URL)
}

// ConnectRPC20 returns a dRPC client to the Agent API v2.0. Notably, it is missing
Expand Down
30 changes: 30 additions & 0 deletions codersdk/agentsdk/agentsdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
"tailscale.com/tailcfg"

"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/v2/codersdk/agentsdk"
Expand Down Expand Up @@ -120,3 +122,31 @@ func TestStreamAgentReinitEvents(t *testing.T) {
require.ErrorIs(t, receiveErr, context.Canceled)
})
}

func TestRewriteDERPMap(t *testing.T) {
t.Parallel()
// This test ensures that RewriteDERPMap mutates built-in DERPs with the
// client access URL.
dm := &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{
1: {
EmbeddedRelay: true,
RegionID: 1,
Nodes: []*tailcfg.DERPNode{{
HostName: "bananas.org",
DERPPort: 1,
}},
},
},
}
parsed, err := url.Parse("https://coconuts.org:44558")
require.NoError(t, err)
client := agentsdk.New(parsed)
client.RewriteDERPMap(dm)
region := dm.Regions[1]
require.True(t, region.EmbeddedRelay)
require.Len(t, region.Nodes, 1)
node := region.Nodes[0]
require.Equal(t, "coconuts.org", node.HostName)
require.Equal(t, 44558, node.DERPPort)
}
13 changes: 12 additions & 1 deletion codersdk/workspacesdk/workspacesdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,15 @@ type DialAgentOptions struct {
EnableTelemetry bool
}

// RewriteDERPMap rewrites the DERP map to use the configured access URL of the
// client as the "embedded relay" access URL.
//
// See tailnet.RewriteDERPMapDefaultRelay for more details on why this is
// necessary.
func (c *Client) RewriteDERPMap(derpMap *tailcfg.DERPMap) {
tailnet.RewriteDERPMapDefaultRelay(context.Background(), c.client.Logger(), derpMap, c.client.URL)
}

func (c *Client) DialAgent(dialCtx context.Context, agentID uuid.UUID, options *DialAgentOptions) (agentConn *AgentConn, err error) {
if options == nil {
options = &DialAgentOptions{}
Expand Down Expand Up @@ -248,6 +257,8 @@ func (c *Client) DialAgent(dialCtx context.Context, agentID uuid.UUID, options *
telemetrySink = basicTel
controller.TelemetryCtrl = basicTel
}

c.RewriteDERPMap(connInfo.DERPMap)
conn, err := tailnet.NewConn(&tailnet.Options{
Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)},
DERPMap: connInfo.DERPMap,
Expand All @@ -270,7 +281,7 @@ func (c *Client) DialAgent(dialCtx context.Context, agentID uuid.UUID, options *
coordCtrl := tailnet.NewTunnelSrcCoordController(options.Logger, conn)
coordCtrl.AddDestination(agentID)
controller.CoordCtrl = coordCtrl
controller.DERPCtrl = tailnet.NewBasicDERPController(options.Logger, conn)
controller.DERPCtrl = tailnet.NewBasicDERPController(options.Logger, c, conn)
controller.Run(ctx)

options.Logger.Debug(ctx, "running tailnet API v2+ connector")
Expand Down
3 changes: 1 addition & 2 deletions codersdk/workspacesdk/workspacesdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (

"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/tailnet"
"github.com/coder/coder/v2/testutil"
Expand All @@ -43,7 +42,7 @@ func TestWorkspaceRewriteDERPMap(t *testing.T) {
}
parsed, err := url.Parse("https://coconuts.org:44558")
require.NoError(t, err)
client := agentsdk.New(parsed)
client := workspacesdk.New(codersdk.New(parsed))
client.RewriteDERPMap(dm)
region := dm.Regions[1]
require.True(t, region.EmbeddedRelay)
Expand Down
27 changes: 19 additions & 8 deletions tailnet/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,13 +568,15 @@ type DERPMapSetter interface {
}

type basicDERPController struct {
logger slog.Logger
setter DERPMapSetter
logger slog.Logger
rewriter DERPMapRewriter // optional
setter DERPMapSetter
}

func (b *basicDERPController) New(client DERPClient) CloserWaiter {
l := &derpSetLoop{
logger: b.logger,
rewriter: b.rewriter,
setter: b.setter,
client: client,
errChan: make(chan error, 1),
Expand All @@ -584,17 +586,23 @@ func (b *basicDERPController) New(client DERPClient) CloserWaiter {
return l
}

func NewBasicDERPController(logger slog.Logger, setter DERPMapSetter) DERPController {
// NewBasicDERPController creates a DERP controller that rewrites the DERP map
// with the provided rewriter before setting it on the provided setter.
//
// The rewriter is optional and can be nil.
func NewBasicDERPController(logger slog.Logger, rewriter DERPMapRewriter, setter DERPMapSetter) DERPController {
return &basicDERPController{
logger: logger,
setter: setter,
logger: logger,
rewriter: rewriter,
setter: setter,
}
}

type derpSetLoop struct {
logger slog.Logger
setter DERPMapSetter
client DERPClient
logger slog.Logger
rewriter DERPMapRewriter // optional
setter DERPMapSetter
client DERPClient

sync.Mutex
closed bool
Expand Down Expand Up @@ -640,6 +648,9 @@ func (l *derpSetLoop) recvLoop() {
return
}
l.logger.Debug(context.Background(), "got new DERP Map", slog.F("derp_map", dm))
if l.rewriter != nil {
l.rewriter.RewriteDERPMap(dm)
}
l.setter.SetDERPMap(dm)
}
}
Expand Down
94 changes: 91 additions & 3 deletions tailnet/controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ func TestNewBasicDERPController_Mainline(t *testing.T) {
t.Parallel()
fs := make(chan *tailcfg.DERPMap)
logger := testutil.Logger(t)
uut := tailnet.NewBasicDERPController(logger, fakeSetter(fs))
uut := tailnet.NewBasicDERPController(logger, nil, fakeSetter(fs))
fc := fakeDERPClient{
ch: make(chan *tailcfg.DERPMap),
}
Expand All @@ -609,7 +609,7 @@ func TestNewBasicDERPController_RecvErr(t *testing.T) {
t.Parallel()
fs := make(chan *tailcfg.DERPMap)
logger := testutil.Logger(t)
uut := tailnet.NewBasicDERPController(logger, fakeSetter(fs))
uut := tailnet.NewBasicDERPController(logger, nil, fakeSetter(fs))
expectedErr := xerrors.New("a bad thing happened")
fc := fakeDERPClient{
ch: make(chan *tailcfg.DERPMap),
Expand Down Expand Up @@ -1041,7 +1041,7 @@ func TestController_Disconnects(t *testing.T) {
// darwin can be slow sometimes.
tailnet.WithGracefulTimeout(5*time.Second))
uut.CoordCtrl = tailnet.NewAgentCoordinationController(logger.Named("coord_ctrl"), fConn)
uut.DERPCtrl = tailnet.NewBasicDERPController(logger.Named("derp_ctrl"), fConn)
uut.DERPCtrl = tailnet.NewBasicDERPController(logger.Named("derp_ctrl"), nil, fConn)
uut.Run(ctx)

call := testutil.TryReceive(testCtx, t, fCoord.CoordinateCalls)
Expand Down Expand Up @@ -1945,6 +1945,52 @@ func TestTunnelAllWorkspaceUpdatesController_HandleErrors(t *testing.T) {
}
}

func TestBasicDERPController_RewriteDERPMap(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)

testDERPMap := &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{
1: {
RegionID: 1,
},
},
}

// Ensure the fake rewriter works as expected.
rewriter := &fakeDERPMapRewriter{
ctx: ctx,
calls: make(chan rewriteDERPMapCall, 16),
}
rewriter.RewriteDERPMap(testDERPMap)
rewriteCall := testutil.TryReceive(ctx, t, rewriter.calls)
require.Same(t, testDERPMap, rewriteCall.derpMap)

derpClient := &fakeDERPClient{
ch: make(chan *tailcfg.DERPMap),
err: nil,
}
defer derpClient.Close()

derpSetter := &fakeDERPMapSetter{
ctx: ctx,
calls: make(chan *setDERPMapCall, 16),
}

derpCtrl := tailnet.NewBasicDERPController(logger, rewriter, derpSetter)
derpCtrl.New(derpClient)

// Simulate receiving a new DERP map from the server, which should be passed
// to the rewriter and setter.
testDERPMap = testDERPMap.Clone() // make a new pointer
derpClient.ch <- testDERPMap
rewriteCall = testutil.TryReceive(ctx, t, rewriter.calls)
require.Same(t, testDERPMap, rewriteCall.derpMap)
setterCall := testutil.TryReceive(ctx, t, derpSetter.calls)
require.Same(t, testDERPMap, setterCall.derpMap)
}

type fakeWorkspaceUpdatesController struct {
ctx context.Context
t testing.TB
Expand Down Expand Up @@ -2040,3 +2086,45 @@ type fakeCloser struct{}
func (fakeCloser) Close() error {
return nil
}

type fakeDERPMapRewriter struct {
ctx context.Context
calls chan rewriteDERPMapCall
}

var _ tailnet.DERPMapRewriter = &fakeDERPMapRewriter{}

type rewriteDERPMapCall struct {
derpMap *tailcfg.DERPMap
}

func (f *fakeDERPMapRewriter) RewriteDERPMap(derpMap *tailcfg.DERPMap) {
call := rewriteDERPMapCall{
derpMap: derpMap,
}
select {
case f.calls <- call:
case <-f.ctx.Done():
}
}

type fakeDERPMapSetter struct {
ctx context.Context
calls chan *setDERPMapCall
}

var _ tailnet.DERPMapSetter = &fakeDERPMapSetter{}

type setDERPMapCall struct {
derpMap *tailcfg.DERPMap
}

func (f *fakeDERPMapSetter) SetDERPMap(derpMap *tailcfg.DERPMap) {
call := &setDERPMapCall{
derpMap: derpMap,
}
select {
case <-f.ctx.Done():
case f.calls <- call:
}
}
Loading
Loading