fix: Add latency-check for DERP over HTTP(s) (#3788)

* fix: Add latency-check for DERP over HTTP(s)

This fixes scenarios where latency wasn't being reported if
a connection had UDP entirely blocked.

* Add inactivity ping

* Improve coordinator error reporting consistency
This commit is contained in:
Kyle Carberry 2022-09-01 11:41:47 -05:00 committed by GitHub
parent f4c8bfdc18
commit 6826b976d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 49 additions and 32 deletions

View File

@ -165,7 +165,13 @@ func New(options *Options) *API {
// other applications might not as well.
r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
r.Get("/derp", derphttp.Handler(api.derpServer).ServeHTTP)
r.Route("/derp", func(r chi.Router) {
r.Get("/", derphttp.Handler(api.derpServer).ServeHTTP)
// This is used when UDP is blocked, and latency must be checked via HTTP(s).
r.Get("/latency-check", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
})
})
r.Route("/api/v2", func(r chi.Router) {
r.NotFound(func(rw http.ResponseWriter, r *http.Request) {

View File

@ -2,6 +2,7 @@ package coderd_test
import (
"context"
"net/http"
"net/netip"
"strconv"
"testing"
@ -114,3 +115,12 @@ func TestDERP(t *testing.T) {
w1.Close()
w2.Close()
}
func TestDERPLatencyCheck(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
res, err := client.Request(context.Background(), http.MethodGet, "/derp/latency-check", nil)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
}

View File

@ -168,6 +168,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
skipRoutes := map[string]string{
"POST:/api/v2/users/logout": "Logging out deletes the API Key for other routes",
"GET:/derp": "This requires a WebSocket upgrade!",
"GET:/derp/latency-check": "This always returns a 200!",
}
assertRoute := map[string]RouteCheck{

2
go.mod
View File

@ -49,7 +49,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.20220831012541-a77bda274fd6
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220901032724-2ed2977662a4
require (
cdr.dev/slog v1.4.2-0.20220525200111-18dce5c2cd5f

4
go.sum
View File

@ -352,8 +352,8 @@ github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOu
github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
github.com/coder/tailscale v1.1.1-0.20220831012541-a77bda274fd6 h1://ApBDDh58hFwMe0AzlgqJrGhzu6Rjk8fQXrR+mbhYE=
github.com/coder/tailscale v1.1.1-0.20220831012541-a77bda274fd6/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
github.com/coder/tailscale v1.1.1-0.20220901032724-2ed2977662a4 h1:TXtMXPt4ds1pM9QrLsfiDEJv7KE8q0yRQGCkSepeKZ8=
github.com/coder/tailscale v1.1.1-0.20220901032724-2ed2977662a4/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab h1:9yEvRWXXfyKzXu8AqywCi+tFZAoqCy4wVcsXwuvZNMc=
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab/go.mod h1:TCJ66NtXh3urJotTdoYQOHHkyE899vOQl5TuF+WLSes=
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=

View File

@ -161,21 +161,6 @@ func NewConn(options *Options) (*Conn, error) {
return nil, xerrors.Errorf("start netstack: %w", err)
}
wireguardEngine = wgengine.NewWatchdog(wireguardEngine)
// Update the wireguard configuration to allow traffic to flow.
wireguardConfig, err := nmcfg.WGCfg(netMap, Logger(options.Logger.Named("wgconfig")), netmap.AllowSingleHosts, "")
if err != nil {
return nil, xerrors.Errorf("create wgcfg: %w", err)
}
wireguardRouter := &router.Config{
LocalAddrs: wireguardConfig.Addresses,
}
err = wireguardEngine.Reconfig(wireguardConfig, wireguardRouter, &dns.Config{}, &tailcfg.Debug{})
if err != nil {
return nil, xerrors.Errorf("reconfig: %w", err)
}
wireguardEngine.SetDERPMap(options.DERPMap)
netMapCopy := *netMap
wireguardEngine.SetNetworkMap(&netMapCopy)
@ -198,8 +183,10 @@ func NewConn(options *Options) (*Conn, error) {
netMap: netMap,
netStack: netStack,
wireguardMonitor: wireguardMonitor,
wireguardRouter: wireguardRouter,
wireguardEngine: wireguardEngine,
wireguardRouter: &router.Config{
LocalAddrs: netMap.Addresses,
},
wireguardEngine: wireguardEngine,
}
netStack.ForwardTCPIn = server.forwardTCP
return server, nil
@ -261,7 +248,7 @@ func (c *Conn) SetNodeCallback(callback func(node *Node)) {
DERPLatency: c.lastDERPLatency,
}
}
c.magicConn.SetNetInfoCallback(func(ni *tailcfg.NetInfo) {
c.wireguardEngine.SetNetInfoCallback(func(ni *tailcfg.NetInfo) {
c.lastMutex.Lock()
c.lastPreferredDERP = ni.PreferredDERP
c.lastDERPLatency = ni.DERPLatency
@ -309,6 +296,7 @@ func (c *Conn) UpdateNodes(nodes []*Node) error {
peerMap[peer.ID] = peer
}
for _, node := range nodes {
peerStatus, ok := status.Peer[node.Key]
peerMap[node.ID] = &tailcfg.Node{
ID: node.ID,
Key: node.Key,
@ -318,12 +306,18 @@ func (c *Conn) UpdateNodes(nodes []*Node) error {
Endpoints: node.Endpoints,
DERP: fmt.Sprintf("%s:%d", tailcfg.DerpMagicIP, node.PreferredDERP),
Hostinfo: hostinfo.New().View(),
// Starting KeepAlive messages at the initialization
// of a connection cause it to hang for an unknown
// reason. TODO: @kylecarbs debug this!
KeepAlive: ok && peerStatus.Active,
}
}
c.netMap.Peers = make([]*tailcfg.Node, 0, len(peerMap))
for _, peer := range peerMap {
c.netMap.Peers = append(c.netMap.Peers, peer)
}
netMapCopy := *c.netMap
c.wireguardEngine.SetNetworkMap(&netMapCopy)
cfg, err := nmcfg.WGCfg(c.netMap, Logger(c.logger.Named("wgconfig")), netmap.AllowSingleHosts, "")
if err != nil {
return xerrors.Errorf("update wireguard config: %w", err)
@ -332,15 +326,13 @@ func (c *Conn) UpdateNodes(nodes []*Node) error {
if err != nil {
return xerrors.Errorf("reconfig: %w", err)
}
netMapCopy := *c.netMap
c.wireguardEngine.SetNetworkMap(&netMapCopy)
return nil
}
// Status returns the current ipnstate of a connection.
func (c *Conn) Status() *ipnstate.Status {
sb := &ipnstate.StatusBuilder{}
c.magicConn.UpdateStatus(sb)
c.wireguardEngine.UpdateStatus(sb)
return sb.Status()
}

View File

@ -55,10 +55,12 @@ func TestTailnet(t *testing.T) {
_ = w2.Close()
})
w1.SetNodeCallback(func(node *tailnet.Node) {
w2.UpdateNodes([]*tailnet.Node{node})
err := w2.UpdateNodes([]*tailnet.Node{node})
require.NoError(t, err)
})
w2.SetNodeCallback(func(node *tailnet.Node) {
w1.UpdateNodes([]*tailnet.Node{node})
err := w1.UpdateNodes([]*tailnet.Node{node})
require.NoError(t, err)
})
conn := make(chan struct{})

View File

@ -28,19 +28,25 @@ type Node struct {
// ServeCoordinator matches the RW structure of a coordinator to exchange node messages.
func ServeCoordinator(conn net.Conn, updateNodes func(node []*Node) error) (func(node *Node), <-chan error) {
errChan := make(chan error, 3)
errChan := make(chan error, 1)
sendErr := func(err error) {
select {
case errChan <- err:
default:
}
}
go func() {
decoder := json.NewDecoder(conn)
for {
var nodes []*Node
err := decoder.Decode(&nodes)
if err != nil {
errChan <- xerrors.Errorf("read: %w", err)
sendErr(xerrors.Errorf("read: %w", err))
return
}
err = updateNodes(nodes)
if err != nil {
errChan <- xerrors.Errorf("update nodes: %w", err)
sendErr(xerrors.Errorf("update nodes: %w", err))
}
}
}()
@ -48,12 +54,12 @@ func ServeCoordinator(conn net.Conn, updateNodes func(node []*Node) error) (func
return func(node *Node) {
data, err := json.Marshal(node)
if err != nil {
errChan <- xerrors.Errorf("marshal node: %w", err)
sendErr(xerrors.Errorf("marshal node: %w", err))
return
}
_, err = conn.Write(data)
if err != nil {
errChan <- xerrors.Errorf("write: %w", err)
sendErr(xerrors.Errorf("write: %w", err))
}
}, errChan
}