other comments

coder · deansheather · Jul 26, 2023 · Apr 27, 2023 · May 1, 2023 · May 2, 2023
commit f0fa5784cf2772b3a9150b68827fc1e929682a51
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -327,7 +327,7 @@ func New(options *Options) *API {
 		options.HealthcheckFunc = func(ctx context.Context, apiKey string) *healthcheck.Report {
 			return healthcheck.Run(ctx, &healthcheck.ReportOptions{
 				AccessURL: options.AccessURL,
-				DERPMap:   api.DERPMap().Clone(),
+				DERPMap:   api.DERPMap(),
 				APIKey:    apiKey,
 			})
 		}

diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -476,22 +476,38 @@ func (api *API) updateEntitlements(ctx context.Context) error {
 //
 // Two ints are returned, the first is the starting region ID for proxies, and
 // the second is the maximum region ID that already exists in the DERP map.
-func getProxyDERPStartingRegionID(derpMap *tailcfg.DERPMap) (sID int, mID int) {
-	maxRegionID := 0
+func getProxyDERPStartingRegionID(derpMap *tailcfg.DERPMap) (sID int64, mID int64) {
+	var maxRegionID int64
 	for _, region := range derpMap.Regions {
-		if region.RegionID > maxRegionID {
-			maxRegionID = region.RegionID
+		rid := int64(region.RegionID)
+		if rid > maxRegionID {
+			maxRegionID = rid
 		}
 	}
 	if maxRegionID < 0 {
 		maxRegionID = 0
 	}
 
 	// Round to the nearest 10,000 with a sufficient buffer of at least 2,000.
+	// The buffer allows for future "fixed" regions to be added to the base DERP
+	// map without conflicting with proxy region IDs (standard DERP maps usually
+	// use incrementing IDs for new regions).
+	//
+	// Example:
+	//  maxRegionID = -2_000 -> startingRegionID = 10_000
+	//  maxRegionID = 8_000 -> startingRegionID = 10_000
+	//  maxRegionID = 8_500 -> startingRegionID = 20_000
+	//  maxRegionID = 12_000 -> startingRegionID = 20_000
+	//  maxRegionID = 20_000 -> startingRegionID = 30_000
 	const roundStartingRegionID = 10_000
 	const startingRegionIDBuffer = 2_000
+	// Add the buffer first.
 	startingRegionID := maxRegionID + startingRegionIDBuffer
-	startingRegionID = int(math.Ceil(float64(startingRegionID)/roundStartingRegionID) * roundStartingRegionID)
+	// Round UP to the nearest 10,000. Go's math.Ceil rounds up to the nearest
+	// integer, so we need to divide by 10,000 first and then multiply by
+	// 10,000.
+	startingRegionID = int64(math.Ceil(float64(startingRegionID)/roundStartingRegionID) * roundStartingRegionID)
+	// This should never be hit but it's here just in case.
 	if startingRegionID < roundStartingRegionID {
 		startingRegionID = roundStartingRegionID
 	}
@@ -565,7 +581,7 @@ func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(*
 			// This should be impossible to hit as the IDs are enforced to be
 			// unique by the database and the computed ID is greater than any
 			// existing ID in the DERP map.
-			regionID := startingRegionID + int(status.Proxy.RegionID)
+			regionID := int(startingRegionID) + int(status.Proxy.RegionID)
 			regionCode := fmt.Sprintf("coder_%s", strings.ToLower(status.Proxy.Name))
 			for _, r := range derpMap.Regions {
 				if r.RegionID == regionID || r.RegionCode == regionCode {

diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"database/sql"
+	"flag"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -531,7 +532,10 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if req.Version != buildinfo.Version() {
+	// Version check should be forced in non-dev builds and when running in
+	// tests.
+	shouldForceVersion := !buildinfo.IsDev() || flag.Lookup("test.v") != nil
+	if shouldForceVersion && req.Version != buildinfo.Version() {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: "Version mismatch.",
 			Detail:  fmt.Sprintf("Proxy version %q does not match primary server version %q", req.Version, buildinfo.Version()),

diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go
@@ -350,6 +350,7 @@ func (c *Client) RegisterWorkspaceProxyLoop(ctx context.Context, opts RegisterWo
 					failureFn(xerrors.Errorf("exceeded re-registration failure count of %d: last error: %w", opts.MaxFailureCount, err))
 					return
 				}
+				continue
 			}
 			failedAttempts = 0
-Original file line number
+Diff line change
@@ Expand Up @@
     					failureFn(xerrors.Errorf("exceeded re-registration failure count of %d: last error: %w", opts.MaxFailureCount, err))
     					return
     				}
+    				continue
     			}
     			failedAttempts = 0
@@ Expand Down @@