coder · deansheather · Aug 10, 2023 · Aug 10, 2023
diff --git a/cli/netcheck_test.go b/cli/netcheck_test.go
@@ -31,7 +31,7 @@ func TestNetcheck(t *testing.T) {
 	require.NoError(t, json.Unmarshal(b, &report))
 
 	assert.True(t, report.Healthy)
-	require.Len(t, report.Regions, 1)
+	require.Len(t, report.Regions, 1+5) // 1 built-in region + 5 STUN regions by default
 	for _, v := range report.Regions {
 		require.Len(t, v.NodeReports, len(v.Region.Nodes))
 	}

diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -175,18 +175,15 @@ backed by Tailscale and WireGuard.
       --derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true)
           Whether to enable or disable the embedded DERP relay server.
 
-      --derp-server-region-code string, $CODER_DERP_SERVER_REGION_CODE (default: coder)
-          Region code to use for the embedded DERP server.
-
-      --derp-server-region-id int, $CODER_DERP_SERVER_REGION_ID (default: 999)
-          Region ID to use for the embedded DERP server.
-
       --derp-server-region-name string, $CODER_DERP_SERVER_REGION_NAME (default: Coder Embedded Relay)
           Region name that for the embedded DERP server.
 
-      --derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: stun.l.google.com:19302)
-          Addresses for STUN servers to establish P2P connections. Use special
-          value 'disable' to turn off STUN.
+      --derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: 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)
+          Addresses for STUN servers to establish P2P connections. It's
+          recommended to have at least two STUN servers to give users the best
+          chance of connecting P2P to workspaces. Each STUN server will get it's
+          own DERP region, with region IDs starting at `--derp-server-region-id
+          + 1`. Use special value 'disable' to turn off STUN completely.
 
 [1mNetworking / HTTP Options[0m 
       --disable-password-auth bool, $CODER_DISABLE_PASSWORD_AUTH

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -111,11 +111,20 @@ networking:
     # Region name that for the embedded DERP server.
     # (default: Coder Embedded Relay, type: string)
     regionName: Coder Embedded Relay
-    # Addresses for STUN servers to establish P2P connections. Use special value
-    # 'disable' to turn off STUN.
-    # (default: stun.l.google.com:19302, type: string-array)
+    # Addresses for STUN servers to establish P2P connections. It's recommended to
+    # have at least two STUN servers to give users the best chance of connecting P2P
+    # to workspaces. Each STUN server will get it's own DERP region, with region IDs
+    # starting at `--derp-server-region-id + 1`. Use special value 'disable' to turn
+    # off STUN completely.
+    # (default:
+    # 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,
+    # type: string-array)
     stunAddresses:
       - 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
     # An HTTP URL that is accessible by other replicas to relay DERP traffic. Required
     # for high availability.
     # (default: <unset>, type: url)

diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -733,6 +733,7 @@ when required by your organization's security policy.`,
 			Value:       &c.DERP.Server.RegionID,
 			Group:       &deploymentGroupNetworkingDERP,
 			YAML:        "regionID",
+			Hidden:      true,
 			// Does not apply to external proxies as this value is generated.
 		},
 		{
@@ -744,6 +745,7 @@ when required by your organization's security policy.`,
 			Value:       &c.DERP.Server.RegionCode,
 			Group:       &deploymentGroupNetworkingDERP,
 			YAML:        "regionCode",
+			Hidden:      true,
 			// Does not apply to external proxies as we use the proxy name.
 		},
 		{
@@ -759,10 +761,10 @@ when required by your organization's security policy.`,
 		},
 		{
 			Name:        "DERP Server STUN Addresses",
-			Description: "Addresses for STUN servers to establish P2P connections. Use special value 'disable' to turn off STUN.",
+			Description: "Addresses for STUN servers to establish P2P connections. It's recommended to have at least two STUN servers to give users the best chance of connecting P2P to workspaces. Each STUN server will get it's own DERP region, with region IDs starting at `--derp-server-region-id + 1`. Use special value 'disable' to turn off STUN completely.",
 			Flag:        "derp-server-stun-addresses",
 			Env:         "CODER_DERP_SERVER_STUN_ADDRESSES",
-			Default:     "stun.l.google.com:19302",
+			Default:     "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",
 			Value:       &c.DERP.Server.STUNAddresses,
 			Group:       &deploymentGroupNetworkingDERP,
 			YAML:        "stunAddresses",

diff --git a/docs/cli/server.md b/docs/cli/server.md
diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden
@@ -175,18 +175,15 @@ backed by Tailscale and WireGuard.
       --derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true)
           Whether to enable or disable the embedded DERP relay server.
 
-      --derp-server-region-code string, $CODER_DERP_SERVER_REGION_CODE (default: coder)
-          Region code to use for the embedded DERP server.
-
-      --derp-server-region-id int, $CODER_DERP_SERVER_REGION_ID (default: 999)
-          Region ID to use for the embedded DERP server.
-
       --derp-server-region-name string, $CODER_DERP_SERVER_REGION_NAME (default: Coder Embedded Relay)
           Region name that for the embedded DERP server.
 
-      --derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: stun.l.google.com:19302)
-          Addresses for STUN servers to establish P2P connections. Use special
-          value 'disable' to turn off STUN.
+      --derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: 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)
+          Addresses for STUN servers to establish P2P connections. It's
+          recommended to have at least two STUN servers to give users the best
+          chance of connecting P2P to workspaces. Each STUN server will get it's
+          own DERP region, with region IDs starting at `--derp-server-region-id
+          + 1`. Use special value 'disable' to turn off STUN completely.
 
 [1mNetworking / HTTP Options[0m 
       --disable-password-auth bool, $CODER_DISABLE_PASSWORD_AUTH

diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -594,7 +594,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.DeploymentValues, api.ProxyHealth)
+			fn := derpMapper(api.Logger, api.ProxyHealth)
 			api.AGPL.DERPMapper.Store(&fn)
 		} else {
 			api.AGPL.DERPMapper.Store(nil)
@@ -659,7 +659,7 @@ var (
 	lastDerpConflictLog   time.Time
 )
 
-func derpMapper(logger slog.Logger, _ *codersdk.DeploymentValues, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap {
+func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap {
 	return func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap {
 		derpMap = derpMap.Clone()
 
@@ -753,46 +753,22 @@ func derpMapper(logger slog.Logger, _ *codersdk.DeploymentValues, proxyHealth *p
 				}
 			}
 
-			var stunNodes []*tailcfg.DERPNode
-			// TODO(@dean): potentially re-enable this depending on impact
-			/*
-				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:    regionName,
-				Nodes:         nodes,
+				Nodes: []*tailcfg.DERPNode{
+					{
+						Name:      fmt.Sprintf("%da", regionID),
+						RegionID:  regionID,
+						HostName:  u.Hostname(),
+						DERPPort:  portInt,
+						STUNPort:  -1,
+						ForceHTTP: u.Scheme == "http",
+					},
+				},
 			}
 		}