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

kylecarbs · kylecarbs · commit 8afca6c44120 · 2022-09-01T03:37:26.000Z
This fixes scenarios where latency wasn't being reported if
a connection had UDP entirely blocked.
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -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) {
diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
@@ -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)
+}
diff --git a/coderd/coderdtest/authtest.go b/coderd/coderdtest/authtest.go
@@ -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{
diff --git a/go.mod b/go.mod
@@ -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
diff --git a/go.sum b/go.sum
@@ -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=

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