Skip to content

Commit d2f22b0

Browse files
deansheatherdeansheather
authored
fix: move STUN servers into their own regions (#9030)
1 parent 25c6832 commit d2f22b0

File tree

10 files changed

+101
-243
lines changed

10 files changed

+101
-243
lines changed

‎cli/netcheck_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestNetcheck(t *testing.T) {
3131
require.NoError(t, json.Unmarshal(b, &report))
3232

3333
assert.True(t, report.Healthy)
34-
require.Len(t, report.Regions, 1)
34+
require.Len(t, report.Regions, 1+5) // 1 built-in region + 5 STUN regions by default
3535
for _, v := range report.Regions {
3636
require.Len(t, v.NodeReports, len(v.Region.Nodes))
3737
}

‎cli/testdata/coder_server_--help.golden

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -175,18 +175,15 @@ backed by Tailscale and WireGuard.
175175
--derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true)
176176
Whether to enable or disable the embedded DERP relay server.
177177

178-
--derp-server-region-code string, $CODER_DERP_SERVER_REGION_CODE (default: coder)
179-
Region code to use for the embedded DERP server.
180-
181-
--derp-server-region-id int, $CODER_DERP_SERVER_REGION_ID (default: 999)
182-
Region ID to use for the embedded DERP server.
183-
184178
--derp-server-region-name string, $CODER_DERP_SERVER_REGION_NAME (default: Coder Embedded Relay)
185179
Region name that for the embedded DERP server.
186180

187-
--derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: stun.l.google.com:19302)
188-
Addresses for STUN servers to establish P2P connections. Use special
189-
value 'disable' to turn off STUN.
181+
--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)
182+
Addresses for STUN servers to establish P2P connections. It's
183+
recommended to have at least two STUN servers to give users the best
184+
chance of connecting P2P to workspaces. Each STUN server will get it's
185+
own DERP region, with region IDs starting at `--derp-server-region-id
186+
+ 1`. Use special value 'disable' to turn off STUN completely.
190187

191188
Networking / HTTP Options
192189
--disable-password-auth bool, $CODER_DISABLE_PASSWORD_AUTH

‎cli/testdata/server-config.yaml.golden

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,20 @@ networking:
111111
# Region name that for the embedded DERP server.
112112
# (default: Coder Embedded Relay, type: string)
113113
regionName: Coder Embedded Relay
114-
# Addresses for STUN servers to establish P2P connections. Use special value
115-
# 'disable' to turn off STUN.
116-
# (default: stun.l.google.com:19302, type: string-array)
114+
# Addresses for STUN servers to establish P2P connections. It's recommended to
115+
# have at least two STUN servers to give users the best chance of connecting P2P
116+
# to workspaces. Each STUN server will get it's own DERP region, with region IDs
117+
# starting at `--derp-server-region-id + 1`. Use special value 'disable' to turn
118+
# off STUN completely.
119+
# (default:
120+
# 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,
121+
# type: string-array)
117122
stunAddresses:
118123
- stun.l.google.com:19302
124+
- stun1.l.google.com:19302
125+
- stun2.l.google.com:19302
126+
- stun3.l.google.com:19302
127+
- stun4.l.google.com:19302
119128
# An HTTP URL that is accessible by other replicas to relay DERP traffic. Required
120129
# for high availability.
121130
# (default: <unset>, type: url)

‎codersdk/deployment.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ when required by your organization's security policy.`,
733733
Value: &c.DERP.Server.RegionID,
734734
Group: &deploymentGroupNetworkingDERP,
735735
YAML: "regionID",
736+
Hidden: true,
736737
// Does not apply to external proxies as this value is generated.
737738
},
738739
{
@@ -744,6 +745,7 @@ when required by your organization's security policy.`,
744745
Value: &c.DERP.Server.RegionCode,
745746
Group: &deploymentGroupNetworkingDERP,
746747
YAML: "regionCode",
748+
Hidden: true,
747749
// Does not apply to external proxies as we use the proxy name.
748750
},
749751
{
@@ -759,10 +761,10 @@ when required by your organization's security policy.`,
759761
},
760762
{
761763
Name: "DERP Server STUN Addresses",
762-
Description: "Addresses for STUN servers to establish P2P connections. Use special value 'disable' to turn off STUN.",
764+
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.",
763765
Flag: "derp-server-stun-addresses",
764766
Env: "CODER_DERP_SERVER_STUN_ADDRESSES",
765-
Default: "stun.l.google.com:19302",
767+
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",
766768
Value: &c.DERP.Server.STUNAddresses,
767769
Group: &deploymentGroupNetworkingDERP,
768770
YAML: "stunAddresses",

‎docs/cli/server.md

Lines changed: 7 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎enterprise/cli/testdata/coder_server_--help.golden

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -175,18 +175,15 @@ backed by Tailscale and WireGuard.
175175
--derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true)
176176
Whether to enable or disable the embedded DERP relay server.
177177

178-
--derp-server-region-code string, $CODER_DERP_SERVER_REGION_CODE (default: coder)
179-
Region code to use for the embedded DERP server.
180-
181-
--derp-server-region-id int, $CODER_DERP_SERVER_REGION_ID (default: 999)
182-
Region ID to use for the embedded DERP server.
183-
184178
--derp-server-region-name string, $CODER_DERP_SERVER_REGION_NAME (default: Coder Embedded Relay)
185179
Region name that for the embedded DERP server.
186180

187-
--derp-server-stun-addresses string-array, $CODER_DERP_SERVER_STUN_ADDRESSES (default: stun.l.google.com:19302)
188-
Addresses for STUN servers to establish P2P connections. Use special
189-
value 'disable' to turn off STUN.
181+
--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)
182+
Addresses for STUN servers to establish P2P connections. It's
183+
recommended to have at least two STUN servers to give users the best
184+
chance of connecting P2P to workspaces. Each STUN server will get it's
185+
own DERP region, with region IDs starting at `--derp-server-region-id
186+
+ 1`. Use special value 'disable' to turn off STUN completely.
190187

191188
Networking / HTTP Options
192189
--disable-password-auth bool, $CODER_DISABLE_PASSWORD_AUTH

‎enterprise/coderd/coderd.go

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ func (api *API) updateEntitlements(ctx context.Context) error {
594594

595595
if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspaceProxy); shouldUpdate(initial, changed, enabled) {
596596
if enabled {
597-
fn := derpMapper(api.Logger, api.DeploymentValues, api.ProxyHealth)
597+
fn := derpMapper(api.Logger, api.ProxyHealth)
598598
api.AGPL.DERPMapper.Store(&fn)
599599
} else {
600600
api.AGPL.DERPMapper.Store(nil)
@@ -659,7 +659,7 @@ var (
659659
lastDerpConflictLog time.Time
660660
)
661661

662-
func derpMapper(logger slog.Logger, _ *codersdk.DeploymentValues, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap {
662+
func derpMapper(logger slog.Logger, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap {
663663
return func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap {
664664
derpMap = derpMap.Clone()
665665

@@ -753,46 +753,22 @@ func derpMapper(logger slog.Logger, _ *codersdk.DeploymentValues, proxyHealth *p
753753
}
754754
}
755755

756-
var stunNodes []*tailcfg.DERPNode
757-
// TODO(@dean): potentially re-enable this depending on impact
758-
/*
759-
if !cfg.DERP.Config.BlockDirect.Value() {
760-
stunNodes, err = agpltailnet.STUNNodes(regionID, cfg.DERP.Server.STUNAddresses)
761-
if err != nil {
762-
// Log a warning if we haven't logged one in the last
763-
// minute.
764-
lastDerpConflictMutex.Lock()
765-
shouldLog := lastDerpConflictLog.IsZero() || time.Since(lastDerpConflictLog) > time.Minute
766-
if shouldLog {
767-
lastDerpConflictLog = time.Now()
768-
}
769-
lastDerpConflictMutex.Unlock()
770-
if shouldLog {
771-
logger.Error(context.Background(), "failed to calculate STUN nodes", slog.Error(err))
772-
}
773-
774-
// No continue because we can keep going.
775-
stunNodes = []*tailcfg.DERPNode{}
776-
}
777-
}
778-
*/
779-
780-
nodes := append(stunNodes, &tailcfg.DERPNode{
781-
Name: fmt.Sprintf("%da", regionID),
782-
RegionID: regionID,
783-
HostName: u.Hostname(),
784-
DERPPort: portInt,
785-
STUNPort: -1,
786-
ForceHTTP: u.Scheme == "http",
787-
})
788-
789756
derpMap.Regions[regionID] = &tailcfg.DERPRegion{
790757
// EmbeddedRelay ONLY applies to the primary.
791758
EmbeddedRelay: false,
792759
RegionID: regionID,
793760
RegionCode: regionCode,
794761
RegionName: regionName,
795-
Nodes: nodes,
762+
Nodes: []*tailcfg.DERPNode{
763+
{
764+
Name: fmt.Sprintf("%da", regionID),
765+
RegionID: regionID,
766+
HostName: u.Hostname(),
767+
DERPPort: portInt,
768+
STUNPort: -1,
769+
ForceHTTP: u.Scheme == "http",
770+
},
771+
},
796772
}
797773
}
798774

0 commit comments

Comments
 (0)