From d5051cd088e8c0c2d98ca2738ce7ed96e4ea0f86 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Sun, 25 Sep 2022 17:52:08 +0000 Subject: [PATCH] fix: Replace access URL for built-in DERP servers Fixes #4195. --- agent/agent_test.go | 13 --------- cli/server.go | 7 +++-- coderd/coderdtest/coderdtest.go | 7 +++-- coderd/workspaceagents.go | 37 +++++++++++++++++++++++- codersdk/workspaceagents.go | 38 ++++++++++++++++++++++++- codersdk/workspaceagents_test.go | 49 ++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 +-- 8 files changed, 133 insertions(+), 24 deletions(-) create mode 100644 codersdk/workspaceagents_test.go diff --git a/agent/agent_test.go b/agent/agent_test.go index 4c96c3ec23531..06a33598b755f 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -481,19 +481,6 @@ func TestAgent(t *testing.T) { } }) - t.Run("Tailnet", func(t *testing.T) { - t.Parallel() - derpMap := tailnettest.RunDERPAndSTUN(t) - conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{ - DERPMap: derpMap, - }, 0) - defer conn.Close() - require.Eventually(t, func() bool { - _, err := conn.Ping() - return err == nil - }, testutil.WaitLong, testutil.IntervalFast) - }) - t.Run("Speedtest", func(t *testing.T) { t.Parallel() if testing.Short() { diff --git a/cli/server.go b/cli/server.go index 5c320a4e0ceaf..bedd70aa2728e 100644 --- a/cli/server.go +++ b/cli/server.go @@ -328,9 +328,10 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) } defaultRegion := &tailcfg.DERPRegion{ - RegionID: derpServerRegionID, - RegionCode: derpServerRegionCode, - RegionName: derpServerRegionName, + EmbeddedRelay: true, + RegionID: derpServerRegionID, + RegionCode: derpServerRegionCode, + RegionName: derpServerRegionName, Nodes: []*tailcfg.DERPNode{{ Name: fmt.Sprintf("%db", derpServerRegionID), RegionID: derpServerRegionID, diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 1f3589ad51b26..72d08255f3bd3 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -218,9 +218,10 @@ func NewOptions(t *testing.T, options *Options) (*httptest.Server, context.Cance DERPMap: &tailcfg.DERPMap{ Regions: map[int]*tailcfg.DERPRegion{ 1: { - RegionID: 1, - RegionCode: "coder", - RegionName: "Coder", + EmbeddedRelay: true, + RegionID: 1, + RegionCode: "coder", + RegionName: "Coder", Nodes: []*tailcfg.DERPNode{{ Name: "1a", RegionID: 1, diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 5f8df9a9de7ba..180de3654e94e 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -226,9 +226,44 @@ func (api *API) dialWorkspaceAgentTailnet(r *http.Request, agentID uuid.UUID) (* _ = serverConn.Close() }() + derpMap := api.DERPMap.Clone() + for _, region := range derpMap.Regions { + if !region.EmbeddedRelay { + continue + } + var node *tailcfg.DERPNode + for _, n := range region.Nodes { + if n.STUNOnly { + continue + } + node = n + break + } + if node == nil { + continue + } + // TODO: This should dial directly to execute the + // DERP server instead of contacting localhost. + // + // This requires modification of Tailscale internals + // to pipe through a proxy function per-region, so + // this is an easy and mostly reliable hack for now. + cloned := node.Clone() + // Add p for proxy. + // This first node supports TLS. + cloned.Name += "p" + cloned.IPv4 = "127.0.0.1" + cloned.InsecureForTests = true + region.Nodes = append(region.Nodes, cloned.Clone()) + // This second node forces HTTP. + cloned.Name += "-http" + cloned.ForceHTTP = true + region.Nodes = append(region.Nodes, cloned) + } + conn, err := tailnet.NewConn(&tailnet.Options{ Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, - DERPMap: api.DERPMap, + DERPMap: derpMap, Logger: api.Logger.Named("tailnet").Leveled(slog.LevelDebug), }) if err != nil { diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index b6039c7c2df9b..48c5743b7f894 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/cookiejar" "net/netip" + "strconv" "time" "cloud.google.com/go/compute/metadata" @@ -208,7 +209,42 @@ func (c *Client) WorkspaceAgentMetadata(ctx context.Context) (WorkspaceAgentMeta return WorkspaceAgentMetadata{}, readBodyAsError(res) } var agentMetadata WorkspaceAgentMetadata - return agentMetadata, json.NewDecoder(res.Body).Decode(&agentMetadata) + err = json.NewDecoder(res.Body).Decode(&agentMetadata) + if err != nil { + return WorkspaceAgentMetadata{}, err + } + accessingPort := c.URL.Port() + if accessingPort == "" { + accessingPort = "80" + if c.URL.Scheme == "https" { + accessingPort = "443" + } + } + accessPort, err := strconv.Atoi(accessingPort) + if err != nil { + return WorkspaceAgentMetadata{}, xerrors.Errorf("convert accessing port %q: %w", accessingPort, err) + } + // 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. + // + // This converts all built-in DERPs to use the access URL that the + // metadata request was performed with. + for _, region := range agentMetadata.DERPMap.Regions { + if !region.EmbeddedRelay { + continue + } + + for _, node := range region.Nodes { + if node.STUNOnly { + continue + } + node.HostName = c.URL.Hostname() + node.DERPPort = accessPort + node.ForceHTTP = c.URL.Scheme == "http" + } + } + return agentMetadata, nil } func (c *Client) ListenWorkspaceAgentTailnet(ctx context.Context) (net.Conn, error) { diff --git a/codersdk/workspaceagents_test.go b/codersdk/workspaceagents_test.go new file mode 100644 index 0000000000000..8027a1ce86c72 --- /dev/null +++ b/codersdk/workspaceagents_test.go @@ -0,0 +1,49 @@ +package codersdk_test + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "testing" + + "github.com/stretchr/testify/require" + "tailscale.com/tailcfg" + + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/codersdk" +) + +func TestWorkspaceAgentMetadata(t *testing.T) { + t.Parallel() + // This test ensures that the DERP map returned properly + // mutates built-in DERPs with the client access URL. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(context.Background(), w, http.StatusOK, codersdk.WorkspaceAgentMetadata{ + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: { + EmbeddedRelay: true, + RegionID: 1, + Nodes: []*tailcfg.DERPNode{{ + HostName: "bananas.org", + DERPPort: 1, + }}, + }, + }, + }, + }) + })) + parsed, err := url.Parse(srv.URL) + require.NoError(t, err) + client := codersdk.New(parsed) + metadata, err := client.WorkspaceAgentMetadata(context.Background()) + require.NoError(t, err) + region := metadata.DERPMap.Regions[1] + require.True(t, region.EmbeddedRelay) + require.Len(t, region.Nodes, 1) + node := region.Nodes[0] + require.Equal(t, parsed.Hostname(), node.HostName) + require.Equal(t, parsed.Port(), strconv.Itoa(node.DERPPort)) +} diff --git a/go.mod b/go.mod index 19571ad56c4f0..0870322f9c4d0 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0 // There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here: // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main -replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee +replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c // Switch to our fork that imports fixes from http://github.com/tailscale/ssh. // See: https://github.com/coder/coder/issues/3371 diff --git a/go.sum b/go.sum index 473bff729b212..a0cd57f00ff08 100644 --- a/go.sum +++ b/go.sum @@ -349,8 +349,8 @@ github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY= github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914= -github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee h1:77WUcIAL5FRQtd97/gOV66MJHLPhsPw+3vMxoEvcadI= -github.com/coder/tailscale v1.1.1-0.20220914175845-85b85d9a52ee/go.mod h1:5amxy08qijEa8bcTW2SeIy4MIqcmd7LMsuOxqOlj2Ak= +github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c h1:xa6lr5Pj87Is26tgpzwBsEGKL7aVz7/fRGgY9QIbf3E= +github.com/coder/tailscale v1.1.1-0.20220926024748-50f068456c6c/go.mod h1:5amxy08qijEa8bcTW2SeIy4MIqcmd7LMsuOxqOlj2Ak= github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE= github.com/containerd/aufs v0.0.0-20201003224125-76a6863f2989/go.mod h1:AkGGQs9NM2vtYHaUen+NljV0/baGCAPELGm2q9ZXpWU= github.com/containerd/aufs v0.0.0-20210316121734-20793ff83c97/go.mod h1:kL5kd6KM5TzQjR79jljyi4olc1Vrx6XBlcyj3gNv2PU=