From 2626965cab6083a249cf9415621fa145476fb4a0 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 31 Jul 2023 01:30:31 +0000 Subject: [PATCH 1/5] fix: fix tailnet netcheck issues - Add STUN nodes to every Coder-managed DERP region (previously was only the primary region). - Avoid adding port to DERP URL in healthcheck if it's the default port for the URL scheme. - Rework tailnet connection accept mechanism so we can update tailscale dependency. --- coderd/healthcheck/derp.go | 4 +-- enterprise/coderd/coderd.go | 45 +++++++++++++++++++++++-------- scripts/coder-dev.sh | 5 +++- scripts/develop.sh | 4 +-- tailnet/conn.go | 53 +++++++++++++++++++------------------ tailnet/derpmap.go | 48 ++++++++++++++++++++++----------- 6 files changed, 101 insertions(+), 58 deletions(-) diff --git a/coderd/healthcheck/derp.go b/coderd/healthcheck/derp.go index 0b77c42076254..632929f1ec7ea 100644 --- a/coderd/healthcheck/derp.go +++ b/coderd/healthcheck/derp.go @@ -112,7 +112,7 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) { mu.Unlock() } nc := &netcheck.Client{ - PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil), + PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil, nil), Logf: tslogger.WithPrefix(ncLogf, "netcheck: "), } ncReport, netcheckErr := nc.GetReport(ctx, opts.DERPMap) @@ -172,7 +172,7 @@ func (r *DERPNodeReport) derpURL() *url.URL { if r.Node.HostName == "" { derpURL.Host = r.Node.IPv4 } - if r.Node.DERPPort != 0 { + if r.Node.DERPPort != 0 && !(r.Node.DERPPort == 443 && derpURL.Scheme == "https") && !(r.Node.DERPPort == 80 && derpURL.Scheme == "http") { derpURL.Host = fmt.Sprintf("%s:%d", derpURL.Host, r.Node.DERPPort) } diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 87c041b88d443..fcd9b5a6550d2 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -578,7 +578,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspaceProxy); shouldUpdate(initial, changed, enabled) { if enabled { - fn := derpMapper(api.Logger, api.ProxyHealth) + fn := derpMapper(api.Logger, api.DeploymentValues, api.ProxyHealth) api.AGPL.DERPMapper.Store(&fn) } else { api.AGPL.DERPMapper.Store(nil) @@ -643,7 +643,7 @@ var ( lastDerpConflictLog time.Time ) -func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap { +func derpMapper(logger slog.Logger, cfg *codersdk.DeploymentValues, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap { return func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap { derpMap = derpMap.Clone() @@ -718,7 +718,7 @@ func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(* lastDerpConflictMutex.Unlock() if shouldLog { logger.Warn(context.Background(), - "proxy region ID or code conflict, ignoring workspace proxy for DERP map. Please change the flags on the affected proxy to use a different region ID and code", + "proxy region ID or code conflict, ignoring workspace proxy for DERP map", slog.F("proxy_id", status.Proxy.ID), slog.F("proxy_name", status.Proxy.Name), slog.F("proxy_display_name", status.Proxy.DisplayName), @@ -733,20 +733,43 @@ func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(* } } + var stunNodes []*tailcfg.DERPNode + if !cfg.DERP.Config.BlockDirect.Value() { + stunNodes, err = agpltailnet.STUNNodes(regionID, cfg.DERP.Server.STUNAddresses) + if err != nil { + // Log a warning if we haven't logged one in the last + // minute. + lastDerpConflictMutex.Lock() + shouldLog := lastDerpConflictLog.IsZero() || time.Since(lastDerpConflictLog) > time.Minute + if shouldLog { + lastDerpConflictLog = time.Now() + } + lastDerpConflictMutex.Unlock() + if shouldLog { + logger.Error(context.Background(), "failed to calculate STUN nodes", slog.Error(err)) + } + + // No continue because we can keep going. + stunNodes = []*tailcfg.DERPNode{} + } + } + + nodes := append(stunNodes, &tailcfg.DERPNode{ + Name: fmt.Sprintf("%da", regionID), + RegionID: regionID, + HostName: u.Hostname(), + DERPPort: portInt, + STUNPort: -1, + ForceHTTP: u.Scheme == "http", + }) + derpMap.Regions[regionID] = &tailcfg.DERPRegion{ // EmbeddedRelay ONLY applies to the primary. EmbeddedRelay: false, RegionID: regionID, RegionCode: regionCode, RegionName: status.Proxy.Name, - Nodes: []*tailcfg.DERPNode{{ - Name: fmt.Sprintf("%da", regionID), - RegionID: regionID, - HostName: u.Hostname(), - DERPPort: portInt, - STUNPort: -1, - ForceHTTP: u.Scheme == "http", - }}, + Nodes: nodes, } } diff --git a/scripts/coder-dev.sh b/scripts/coder-dev.sh index 9d3d6e7f74233..29072a60f7903 100755 --- a/scripts/coder-dev.sh +++ b/scripts/coder-dev.sh @@ -11,7 +11,10 @@ source "${SCRIPT_DIR}/lib.sh" GOOS="$(go env GOOS)" GOARCH="$(go env GOARCH)" BINARY_TYPE=coder-slim -if [[ $1 == server ]]; then +if [[ ${1:-} == server ]]; then + BINARY_TYPE=coder +fi +if [[ ${1:-} == wsproxy ]] && [[ ${2:-} == server ]]; then BINARY_TYPE=coder fi RELATIVE_BINARY_PATH="build/${BINARY_TYPE}_${GOOS}_${GOARCH}" diff --git a/scripts/develop.sh b/scripts/develop.sh index 2d15196195f2b..aa1fd6554a8b2 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -131,7 +131,7 @@ fatal() { trap 'fatal "Script encountered an error"' ERR cdroot - start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "http://127.0.0.1:3000" --dangerous-allow-cors-requests=true --experiments "*" "$@" + start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "http://127.0.0.1:3000" --dangerous-allow-cors-requests=true --experiments "*,moons" "$@" echo '== Waiting for Coder to become ready' # Start the timeout in the background so interrupting this script @@ -181,7 +181,7 @@ fatal() { log "Using external workspace proxy" ( # Attempt to delete the proxy first, in case it already exists. - "${CODER_DEV_SHIM}" wsproxy delete local-proxy || true + "${CODER_DEV_SHIM}" wsproxy delete local-proxy --yes || true # Create the proxy proxy_session_token=$("${CODER_DEV_SHIM}" wsproxy create --name=local-proxy --display-name="Local Proxy" --icon="/emojis/1f4bb.png" --only-token) # Start the proxy diff --git a/tailnet/conn.go b/tailnet/conn.go index 089a83ff79ff9..09fa95a4a37c9 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -292,8 +292,7 @@ func NewConn(options *Options) (conn *Conn, err error) { server.sendNode() }) - netStack.ForwardTCPIn = server.forwardTCP - netStack.ForwardTCPSockOpts = server.forwardTCPSockOpts + netStack.GetTCPHandlerForFlow = server.getTCPHandlerForFlow err = netStack.Start(nil) if err != nil { @@ -798,29 +797,11 @@ func (c *Conn) DialContextUDP(ctx context.Context, ipp netip.AddrPort) (*gonet.U return c.netStack.DialContextUDP(ctx, ipp) } -func (c *Conn) forwardTCP(conn net.Conn, port uint16) { - c.mutex.Lock() - ln, ok := c.listeners[listenKey{"tcp", "", fmt.Sprint(port)}] - c.mutex.Unlock() - if !ok { - c.forwardTCPToLocal(conn, port) - return - } - - t := time.NewTimer(time.Second) - defer t.Stop() - select { - case ln.conn <- conn: - return - case <-ln.closed: - case <-c.closed: - case <-t.C: - } - _ = conn.Close() -} - -func (*Conn) forwardTCPSockOpts(port uint16) []tcpip.SettableSocketOption { - opts := []tcpip.SettableSocketOption{} +func (c *Conn) getTCPHandlerForFlow(_, dst netip.AddrPort) (func(net.Conn), []tcpip.SettableSocketOption, bool) { + var ( + port = dst.Port() + opts = []tcpip.SettableSocketOption{} + ) // See: https://github.com/tailscale/tailscale/blob/c7cea825aea39a00aca71ea02bab7266afc03e7c/wgengine/netstack/netstack.go#L888 if port == WorkspaceAgentSSHPort || port == 22 { @@ -828,7 +809,27 @@ func (*Conn) forwardTCPSockOpts(port uint16) []tcpip.SettableSocketOption { opts = append(opts, &opt) } - return opts + c.mutex.Lock() + ln, ok := c.listeners[listenKey{"tcp", "", fmt.Sprint(port)}] + c.mutex.Unlock() + if !ok { + return func(conn net.Conn) { + c.forwardTCPToLocal(conn, port) + }, opts, true + } + + return func(conn net.Conn) { + t := time.NewTimer(time.Second) + defer t.Stop() + select { + case ln.conn <- conn: + return + case <-ln.closed: + case <-c.closed: + case <-t.C: + } + _ = conn.Close() + }, opts, true } func (c *Conn) forwardTCPToLocal(conn net.Conn, port uint16) { diff --git a/tailnet/derpmap.go b/tailnet/derpmap.go index b4bbd17d5879b..aa79300801d71 100644 --- a/tailnet/derpmap.go +++ b/tailnet/derpmap.go @@ -13,6 +13,33 @@ import ( "tailscale.com/tailcfg" ) +func STUNNodes(regionID int, stunAddrs []string) ([]*tailcfg.DERPNode, error) { + nodes := []*tailcfg.DERPNode{} + for index, stunAddr := range stunAddrs { + if stunAddr == "disable" { + return []*tailcfg.DERPNode{}, nil + } + + host, rawPort, err := net.SplitHostPort(stunAddr) + if err != nil { + return nil, xerrors.Errorf("split host port for %q: %w", stunAddr, err) + } + port, err := strconv.Atoi(rawPort) + if err != nil { + return nil, xerrors.Errorf("parse port for %q: %w", stunAddr, err) + } + nodes = append([]*tailcfg.DERPNode{{ + Name: fmt.Sprintf("%dstun%d", regionID, index), + RegionID: regionID, + HostName: host, + STUNOnly: true, + STUNPort: port, + }}, nodes...) + } + + return nodes, nil +} + // NewDERPMap constructs a DERPMap from a set of STUN addresses and optionally a remote // URL to fetch a mapping from e.g. https://controlplane.tailscale.com/derpmap/default. // @@ -26,23 +53,12 @@ func NewDERPMap(ctx context.Context, region *tailcfg.DERPRegion, stunAddrs []str } if region != nil { - for index, stunAddr := range stunAddrs { - host, rawPort, err := net.SplitHostPort(stunAddr) - if err != nil { - return nil, xerrors.Errorf("split host port for %q: %w", stunAddr, err) - } - port, err := strconv.Atoi(rawPort) - if err != nil { - return nil, xerrors.Errorf("parse port for %q: %w", stunAddr, err) - } - region.Nodes = append([]*tailcfg.DERPNode{{ - Name: fmt.Sprintf("%dstun%d", region.RegionID, index), - RegionID: region.RegionID, - HostName: host, - STUNOnly: true, - STUNPort: port, - }}, region.Nodes...) + stunNodes, err := STUNNodes(region.RegionID, stunAddrs) + if err != nil { + return nil, xerrors.Errorf("construct stun nodes: %w", err) } + + region.Nodes = append(stunNodes, region.Nodes...) } derpMap := &tailcfg.DERPMap{ From 7c9468f018b5b756375309460f322d9a60102d46 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 31 Jul 2023 11:48:43 +0000 Subject: [PATCH 2/5] fixup! fix: fix tailnet netcheck issues --- coderd/coderdtest/coderdtest.go | 20 ++++- coderd/healthcheck/derp.go | 2 +- enterprise/wsproxy/wsproxy_test.go | 127 +++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 +- tailnet/conn.go | 53 ++++++------ 6 files changed, 173 insertions(+), 35 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 546bb60b1c1bd..ada8782514381 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -303,9 +303,21 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can accessURL = serverURL } - stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{}) - stunAddr.IP = net.ParseIP("127.0.0.1") - t.Cleanup(stunCleanup) + // If the STUNAddresses setting is empty or the default, start a STUN + // server. Otherwise, use the value as is. + var ( + stunAddresses []string + dvStunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value() + ) + if len(dvStunAddresses) == 0 || (len(dvStunAddresses) == 1 && dvStunAddresses[0] == "stun.l.google.com:19302") { + stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{}) + stunAddr.IP = net.ParseIP("127.0.0.1") + t.Cleanup(stunCleanup) + stunAddresses = []string{stunAddr.String()} + options.DeploymentValues.DERP.Server.STUNAddresses = stunAddresses + } else if dvStunAddresses[0] != "disable" { + stunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value() + } derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug))) derpServer.SetMeshKey("test-key") @@ -346,7 +358,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can if !options.DeploymentValues.DERP.Server.Enable.Value() { region = nil } - derpMap, err := tailnet.NewDERPMap(ctx, region, []string{stunAddr.String()}, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value()) + derpMap, err := tailnet.NewDERPMap(ctx, region, stunAddresses, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value()) require.NoError(t, err) return func(h http.Handler) { diff --git a/coderd/healthcheck/derp.go b/coderd/healthcheck/derp.go index 632929f1ec7ea..31c91da22318f 100644 --- a/coderd/healthcheck/derp.go +++ b/coderd/healthcheck/derp.go @@ -112,7 +112,7 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) { mu.Unlock() } nc := &netcheck.Client{ - PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil, nil), + PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil), Logf: tslogger.WithPrefix(ncLogf, "netcheck: "), } ncReport, netcheckErr := nc.GetReport(ctx, opts.DERPMap) diff --git a/enterprise/wsproxy/wsproxy_test.go b/enterprise/wsproxy/wsproxy_test.go index 26c6fe418eb43..334ac165dbc52 100644 --- a/enterprise/wsproxy/wsproxy_test.go +++ b/enterprise/wsproxy/wsproxy_test.go @@ -25,6 +25,7 @@ import ( "github.com/coder/coder/enterprise/coderd/coderdenttest" "github.com/coder/coder/enterprise/coderd/license" "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/tailnet" "github.com/coder/coder/testutil" ) @@ -239,6 +240,132 @@ resourceLoop: }) } +func TestDERPMapStunNodes(t *testing.T) { + t.Parallel() + + deploymentValues := coderdtest.DeploymentValues(t) + deploymentValues.Experiments = []string{ + string(codersdk.ExperimentMoons), + "*", + } + stunAddresses := []string{ + "stun.l.google.com:19302", + "stun1.l.google.com:19302", + "stun2.l.google.com:19302", + "stun3.l.google.com:19302", + "stun4.l.google.com:19302", + } + deploymentValues.DERP.Server.STUNAddresses = stunAddresses + + client, closer, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: deploymentValues, + AppHostname: "*.primary.test.coder.com", + IncludeProvisionerDaemon: true, + RealIPConfig: &httpmw.RealIPConfig{ + TrustedOrigins: []*net.IPNet{{ + IP: net.ParseIP("127.0.0.1"), + Mask: net.CIDRMask(8, 32), + }}, + TrustedHeaders: []string{ + "CF-Connecting-IP", + }, + }, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureWorkspaceProxy: 1, + }, + }, + }) + t.Cleanup(func() { + _ = closer.Close() + }) + + // Create a running external proxy. + _ = coderdenttest.NewWorkspaceProxy(t, api, client, &coderdenttest.ProxyOptions{ + Name: "cool-proxy", + }) + + // Wait for both running proxies to become healthy. + require.Eventually(t, func() bool { + healthCtx := testutil.Context(t, testutil.WaitLong) + err := api.ProxyHealth.ForceUpdate(healthCtx) + if !assert.NoError(t, err) { + return false + } + + regions, err := client.Regions(healthCtx) + if !assert.NoError(t, err) { + return false + } + if !assert.Len(t, regions, 2) { + return false + } + + // All regions should be healthy. + for _, r := range regions { + if !r.Healthy { + return false + } + } + return true + }, testutil.WaitLong, testutil.IntervalMedium) + + // Get the DERP map and ensure that the built-in region and the proxy region + // both have the STUN nodes. + ctx := testutil.Context(t, testutil.WaitLong) + connInfo, err := client.WorkspaceAgentConnectionInfoGeneric(ctx) + require.NoError(t, err) + + // There should be two DERP servers in the map: the primary and the + // proxy. + require.NotNil(t, connInfo.DERPMap) + require.Len(t, connInfo.DERPMap.Regions, 2) + + var ( + primaryRegion *tailcfg.DERPRegion + proxyRegion *tailcfg.DERPRegion + ) + for _, r := range connInfo.DERPMap.Regions { + if r.EmbeddedRelay { + primaryRegion = r + continue + } + if r.RegionName == "cool-proxy" { + proxyRegion = r + continue + } + + t.Fatalf("unexpected region: %+v", r) + } + + // The primary region: + require.Equal(t, "Coder Embedded Relay", primaryRegion.RegionName) + require.Equal(t, "coder", primaryRegion.RegionCode) + require.Equal(t, 999, primaryRegion.RegionID) + require.True(t, primaryRegion.EmbeddedRelay) + require.Len(t, primaryRegion.Nodes, len(stunAddresses)+1) + + // The proxy region: + require.Equal(t, "cool-proxy", proxyRegion.RegionName) + require.Equal(t, "coder_cool-proxy", proxyRegion.RegionCode) + require.Equal(t, 10001, proxyRegion.RegionID) + require.False(t, proxyRegion.EmbeddedRelay) + require.Len(t, proxyRegion.Nodes, len(stunAddresses)+1) + + for _, region := range []*tailcfg.DERPRegion{primaryRegion, proxyRegion} { + stunNodes, err := tailnet.STUNNodes(region.RegionID, stunAddresses) + require.NoError(t, err) + require.Len(t, stunNodes, len(stunAddresses)) + + require.Equal(t, stunNodes, region.Nodes[:len(stunNodes)]) + + // The last node should be the Coder server. + require.NotZero(t, region.Nodes[len(region.Nodes)-1].DERPPort) + } +} + func TestDERPEndToEnd(t *testing.T) { t.Parallel() diff --git a/go.mod b/go.mod index 9f8d043812df2..bad17af8d0414 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ replace github.com/dlclark/regexp2 => github.com/dlclark/regexp2 v1.7.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 v0.0.0-20230522123520-74712221d00f +replace tailscale.com => github.com/coder/tailscale v0.0.0-20230731105344-d1b7f8087191 // Use our tempfork of gvisor that includes a fix for TCP connection stalls: // https://github.com/coder/coder/issues/7388 diff --git a/go.sum b/go.sum index 22a36d44eb1d2..a56d7397f3cfd 100644 --- a/go.sum +++ b/go.sum @@ -194,8 +194,8 @@ github.com/coder/retry v1.4.0 h1:g0fojHFxcdgM3sBULqgjFDxw1UIvaCqk4ngUDu0EWag= github.com/coder/retry v1.4.0/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= github.com/coder/ssh v0.0.0-20230621095435-9a7e23486f1c h1:TI7TzdFI0UvQmwgyQhtI1HeyYNRxAQpr8Tw/rjT8VSA= github.com/coder/ssh v0.0.0-20230621095435-9a7e23486f1c/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= -github.com/coder/tailscale v0.0.0-20230522123520-74712221d00f h1:F0Xr1d8h8dAHn7tab1HXuzYFkcjmCydnEfdMbkOhlVk= -github.com/coder/tailscale v0.0.0-20230522123520-74712221d00f/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA= +github.com/coder/tailscale v0.0.0-20230731105344-d1b7f8087191 h1:FweUPlasdC67APP0jS8LTUdVlWcl49cX4NqTkr0ZL/4= +github.com/coder/tailscale v0.0.0-20230731105344-d1b7f8087191/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA= github.com/coder/terraform-provider-coder v0.11.1 h1:1sXcHfQrX8XhmLbtKxBED2lZ5jk3/ezBtaw6uVhpJZ4= github.com/coder/terraform-provider-coder v0.11.1/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY= github.com/coder/wgtunnel v0.1.5 h1:WP3sCj/3iJ34eKvpMQEp1oJHvm24RYh0NHbj1kfUKfs= diff --git a/tailnet/conn.go b/tailnet/conn.go index 09fa95a4a37c9..089a83ff79ff9 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -292,7 +292,8 @@ func NewConn(options *Options) (conn *Conn, err error) { server.sendNode() }) - netStack.GetTCPHandlerForFlow = server.getTCPHandlerForFlow + netStack.ForwardTCPIn = server.forwardTCP + netStack.ForwardTCPSockOpts = server.forwardTCPSockOpts err = netStack.Start(nil) if err != nil { @@ -797,11 +798,29 @@ func (c *Conn) DialContextUDP(ctx context.Context, ipp netip.AddrPort) (*gonet.U return c.netStack.DialContextUDP(ctx, ipp) } -func (c *Conn) getTCPHandlerForFlow(_, dst netip.AddrPort) (func(net.Conn), []tcpip.SettableSocketOption, bool) { - var ( - port = dst.Port() - opts = []tcpip.SettableSocketOption{} - ) +func (c *Conn) forwardTCP(conn net.Conn, port uint16) { + c.mutex.Lock() + ln, ok := c.listeners[listenKey{"tcp", "", fmt.Sprint(port)}] + c.mutex.Unlock() + if !ok { + c.forwardTCPToLocal(conn, port) + return + } + + t := time.NewTimer(time.Second) + defer t.Stop() + select { + case ln.conn <- conn: + return + case <-ln.closed: + case <-c.closed: + case <-t.C: + } + _ = conn.Close() +} + +func (*Conn) forwardTCPSockOpts(port uint16) []tcpip.SettableSocketOption { + opts := []tcpip.SettableSocketOption{} // See: https://github.com/tailscale/tailscale/blob/c7cea825aea39a00aca71ea02bab7266afc03e7c/wgengine/netstack/netstack.go#L888 if port == WorkspaceAgentSSHPort || port == 22 { @@ -809,27 +828,7 @@ func (c *Conn) getTCPHandlerForFlow(_, dst netip.AddrPort) (func(net.Conn), []tc opts = append(opts, &opt) } - c.mutex.Lock() - ln, ok := c.listeners[listenKey{"tcp", "", fmt.Sprint(port)}] - c.mutex.Unlock() - if !ok { - return func(conn net.Conn) { - c.forwardTCPToLocal(conn, port) - }, opts, true - } - - return func(conn net.Conn) { - t := time.NewTimer(time.Second) - defer t.Stop() - select { - case ln.conn <- conn: - return - case <-ln.closed: - case <-c.closed: - case <-t.C: - } - _ = conn.Close() - }, opts, true + return opts } func (c *Conn) forwardTCPToLocal(conn net.Conn, port uint16) { From 8580f7acfab774686bea5f020d7d482ba29e0288 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 31 Jul 2023 12:33:00 +0000 Subject: [PATCH 3/5] fixup! fix: fix tailnet netcheck issues --- agent/agent.go | 6 ++++++ coderd/workspaceagents_test.go | 9 ++++++++- enterprise/wsproxy/wsproxy_test.go | 24 ++++++++++++------------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index aafaa3849c109..863af7b59fbbb 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -85,6 +85,8 @@ type Client interface { type Agent interface { HTTPDebug() http.Handler + // TailnetConn may be nil. + TailnetConn() *tailnet.Conn io.Closer } @@ -200,6 +202,10 @@ type agent struct { metrics *agentMetrics } +func (a *agent) TailnetConn() *tailnet.Conn { + return a.network +} + func (a *agent) init(ctx context.Context) { sshSrv, err := agentssh.NewServer(ctx, a.logger.Named("ssh-server"), a.prometheusRegistry, a.filesystem, a.sshMaxTimeout, "") if err != nil { diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index c7d13b3393541..785d64fbba8e8 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1328,7 +1328,14 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) { currentDerpMap.Store(newDerpMap) // Wait for the agent's DERP map to be updated. - // TODO: this + require.Eventually(t, func() bool { + conn := agentCloser.TailnetConn() + if conn == nil { + return false + } + regionIDs := conn.DERPMap().RegionIDs() + return len(regionIDs) == 1 && regionIDs[0] == 2 + }, testutil.WaitLong, testutil.IntervalFast) // Wait for the DERP map to be updated on the existing client. require.Eventually(t, func() bool { diff --git a/enterprise/wsproxy/wsproxy_test.go b/enterprise/wsproxy/wsproxy_test.go index 334ac165dbc52..137b4326661bb 100644 --- a/enterprise/wsproxy/wsproxy_test.go +++ b/enterprise/wsproxy/wsproxy_test.go @@ -185,24 +185,24 @@ resourceLoop: require.Equal(t, "coder_best-proxy", proxy1Region.RegionCode) require.Equal(t, 10001, proxy1Region.RegionID) require.False(t, proxy1Region.EmbeddedRelay) - require.Len(t, proxy1Region.Nodes, 1) - require.Equal(t, "10001a", proxy1Region.Nodes[0].Name) - require.Equal(t, 10001, proxy1Region.Nodes[0].RegionID) - require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[0].HostName) - require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[0].DERPPort)) - require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[0].ForceHTTP) + require.Len(t, proxy1Region.Nodes, 2) // proxy + stun + require.Equal(t, "10001a", proxy1Region.Nodes[1].Name) + require.Equal(t, 10001, proxy1Region.Nodes[1].RegionID) + require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[1].HostName) + require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[1].DERPPort)) + require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[1].ForceHTTP) // The second proxy region: require.Equal(t, "worst-proxy", proxy2Region.RegionName) require.Equal(t, "coder_worst-proxy", proxy2Region.RegionCode) require.Equal(t, 10002, proxy2Region.RegionID) require.False(t, proxy2Region.EmbeddedRelay) - require.Len(t, proxy2Region.Nodes, 1) - require.Equal(t, "10002a", proxy2Region.Nodes[0].Name) - require.Equal(t, 10002, proxy2Region.Nodes[0].RegionID) - require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[0].HostName) - require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[0].DERPPort)) - require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[0].ForceHTTP) + require.Len(t, proxy2Region.Nodes, 2) // proxy + stun + require.Equal(t, "10002a", proxy2Region.Nodes[1].Name) + require.Equal(t, 10002, proxy2Region.Nodes[1].RegionID) + require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[1].HostName) + require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[1].DERPPort)) + require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[1].ForceHTTP) }) t.Run("ConnectDERP", func(t *testing.T) { From 24d094ee1717542556d0f05b6bd8ba31da4f6361 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 1 Aug 2023 13:44:46 +0000 Subject: [PATCH 4/5] fixup! fix: fix tailnet netcheck issues --- agent/agent_test.go | 9 +++++++++ coderd/workspaceagents.go | 3 +-- coderd/workspaceagents_test.go | 10 +++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index d897951496896..34637992536b7 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1818,6 +1818,15 @@ func TestAgent_UpdatedDERP(t *testing.T) { }) require.NoError(t, err) + require.Eventually(t, func() bool { + conn := closer.TailnetConn() + if conn == nil { + return false + } + regionIDs := conn.DERPMap().RegionIDs() + return len(regionIDs) == 1 && regionIDs[0] == 2 && conn.Node().PreferredDERP == 2 + }, testutil.WaitLong, testutil.IntervalFast) + // Connect from a second client and make sure it uses the new DERP map. conn2 := newClientConn(newDerpMap) require.Equal(t, []int{2}, conn2.DERPMap().RegionIDs()) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 4596b2f27f661..40ad2c72e4e20 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -866,6 +866,7 @@ func (api *API) derpMapUpdates(rw http.ResponseWriter, r *http.Request) { lastDERPMap = derpMap } + ticker.Reset(api.Options.DERPMapUpdateFrequency) select { case <-ctx.Done(): return @@ -873,8 +874,6 @@ func (api *API) derpMapUpdates(rw http.ResponseWriter, r *http.Request) { return case <-ticker.C: } - - ticker.Reset(api.Options.DERPMapUpdateFrequency) } } diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 785d64fbba8e8..12cb19efac012 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1270,11 +1270,9 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) { defer closer.Close() user := coderdtest.CreateFirstUser(t, client) - originalDerpMap := api.DERPMap() - require.NotNil(t, originalDerpMap) - // Change the DERP mapper to our custom one. var currentDerpMap atomic.Pointer[tailcfg.DERPMap] + originalDerpMap, _ := tailnettest.RunDERPAndSTUN(t) currentDerpMap.Store(originalDerpMap) derpMapFn := func(_ *tailcfg.DERPMap) *tailcfg.DERPMap { return currentDerpMap.Load().Clone() @@ -1304,10 +1302,8 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) { resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) agentID := resources[0].Agents[0].ID - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - // Connect from a client. + ctx := testutil.Context(t, testutil.WaitLong) conn1, err := client.DialWorkspaceAgent(ctx, agentID, &codersdk.DialWorkspaceAgentOptions{ Logger: logger.Named("client1"), }) @@ -1334,7 +1330,7 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) { return false } regionIDs := conn.DERPMap().RegionIDs() - return len(regionIDs) == 1 && regionIDs[0] == 2 + return len(regionIDs) == 1 && regionIDs[0] == 2 && conn.Node().PreferredDERP == 2 }, testutil.WaitLong, testutil.IntervalFast) // Wait for the DERP map to be updated on the existing client. From f3bc1caba971e7d72a3c90bd228cb16959eb7aba Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 1 Aug 2023 15:34:34 +0000 Subject: [PATCH 5/5] fixup! fix: fix tailnet netcheck issues --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index bad17af8d0414..f202fcc3ec856 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ replace github.com/golang/glog => github.com/coder/glog v1.0.1-0.20220322161911- replace github.com/fatedier/kcp-go => github.com/coder/kcp-go v2.0.4-0.20220409183554-83c0904cec69+incompatible // https://github.com/tcnksm/go-httpstat/pull/29 -replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0-20220831233600-c91452099472 +replace github.com/tcnksm/go-httpstat => github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322 // See https://github.com/dlclark/regexp2/issues/63 replace github.com/dlclark/regexp2 => github.com/dlclark/regexp2 v1.7.0 diff --git a/go.sum b/go.sum index a56d7397f3cfd..92a02c1961a39 100644 --- a/go.sum +++ b/go.sum @@ -186,6 +186,8 @@ github.com/coder/flog v1.1.0 h1:kbAes1ai8fIS5OeV+QAnKBQE22ty1jRF/mcAwHpLBa4= github.com/coder/flog v1.1.0/go.mod h1:UQlQvrkJBvnRGo69Le8E24Tcl5SJleAAR7gYEHzAmdQ= github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOul20JukUEpu4ip9u7biBL+ntgk= github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4= +github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322 h1:m0lPZjlQ7vdVpRBPKfYIFlmgevoTkBxB10wv6l2gOaU= +github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322/go.mod h1:rOLFDDVKVFiDqZFXoteXc97YXx7kFi9kYqR+2ETPkLQ= github.com/coder/go-scim/pkg/v2 v2.0.0-20230221055123-1d63c1222136 h1:0RgB61LcNs24WOxc3PBvygSNTQurm0PYPujJjLLOzs0= github.com/coder/go-scim/pkg/v2 v2.0.0-20230221055123-1d63c1222136/go.mod h1:VkD1P761nykiq75dz+4iFqIQIZka189tx1BQLOp0Skc= github.com/coder/gvisor v0.0.0-20230714132058-be2e4ac102c3 h1:gtuDFa+InmMVUYiurBV+XYu24AeMGv57qlZ23i6rmyE= @@ -576,8 +578,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylecarbs/embedded-postgres v1.17.1-0.20220615202325-461532cecd3a h1:uOnis+HNE6e6eR17YlqzKk51GDahd7E/FacnZxS8h8w= github.com/kylecarbs/embedded-postgres v1.17.1-0.20220615202325-461532cecd3a/go.mod h1:0B+3bPsMvcNgR9nN+bdM2x9YaNYDnf3ksUqYp1OAub0= -github.com/kylecarbs/go-httpstat v0.0.0-20220831233600-c91452099472 h1:KXbxoQY9tOxgacpw0vbHWfIb56Xuzgi0Oql5yr6RYaA= -github.com/kylecarbs/go-httpstat v0.0.0-20220831233600-c91452099472/go.mod h1:MdOqT7wdglCBuU45KzMIvO+xdKlCGHPUWwdTxytqHBU= github.com/kylecarbs/opencensus-go v0.23.1-0.20220307014935-4d0325a68f8b h1:1Y1X6aR78kMEQE1iCjQodB3lA7VO4jB88Wf8ZrzXSsA= github.com/kylecarbs/opencensus-go v0.23.1-0.20220307014935-4d0325a68f8b/go.mod h1:XItmlyltB5F7CS4xOC1DcqMoFqwtC6OG2xF7mCv7P7E= github.com/kylecarbs/readline v0.0.0-20220211054233-0d62993714c8/go.mod h1:n/KX1BZoN1m9EwoXkn/xAV4fd3k8c++gGBsgLONaPOY=