Skip to content

Commit 9941f49

Browse files
deansheatherdeansheather
authored
fix: remove stun nodes from workspace proxy regions (#8990)
1 parent 00a8221 commit 9941f49

File tree

2 files changed

+35
-30
lines changed

2 files changed

+35
-30
lines changed

‎enterprise/coderd/coderd.go

+21-18
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ var (
659659
lastDerpConflictLog time.Time
660660
)
661661

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

@@ -754,25 +754,28 @@ func derpMapper(logger slog.Logger, cfg *codersdk.DeploymentValues, proxyHealth
754754
}
755755

756756
var stunNodes []*tailcfg.DERPNode
757-
if !cfg.DERP.Config.BlockDirect.Value() {
758-
stunNodes, err = agpltailnet.STUNNodes(regionID, cfg.DERP.Server.STUNAddresses)
759-
if err != nil {
760-
// Log a warning if we haven't logged one in the last
761-
// minute.
762-
lastDerpConflictMutex.Lock()
763-
shouldLog := lastDerpConflictLog.IsZero() || time.Since(lastDerpConflictLog) > time.Minute
764-
if shouldLog {
765-
lastDerpConflictLog = time.Now()
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{}
766776
}
767-
lastDerpConflictMutex.Unlock()
768-
if shouldLog {
769-
logger.Error(context.Background(), "failed to calculate STUN nodes", slog.Error(err))
770-
}
771-
772-
// No continue because we can keep going.
773-
stunNodes = []*tailcfg.DERPNode{}
774777
}
775-
}
778+
*/
776779

777780
nodes := append(stunNodes, &tailcfg.DERPNode{
778781
Name: fmt.Sprintf("%da", regionID),

‎enterprise/wsproxy/wsproxy_test.go

+14-12
Original file line numberDiff line numberDiff line change
@@ -244,24 +244,24 @@ resourceLoop:
244244
require.Equal(t, "coder_best-proxy", proxy1Region.RegionCode)
245245
require.Equal(t, 10001, proxy1Region.RegionID)
246246
require.False(t, proxy1Region.EmbeddedRelay)
247-
require.Len(t, proxy1Region.Nodes, 2) // proxy + stun
248-
require.Equal(t, "10001a", proxy1Region.Nodes[1].Name)
249-
require.Equal(t, 10001, proxy1Region.Nodes[1].RegionID)
250-
require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[1].HostName)
251-
require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[1].DERPPort))
252-
require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[1].ForceHTTP)
247+
require.Len(t, proxy1Region.Nodes, 1)
248+
require.Equal(t, "10001a", proxy1Region.Nodes[0].Name)
249+
require.Equal(t, 10001, proxy1Region.Nodes[0].RegionID)
250+
require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[0].HostName)
251+
require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[0].DERPPort))
252+
require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[0].ForceHTTP)
253253

254254
// The second proxy region:
255255
require.Equal(t, "worst-proxy", proxy2Region.RegionName)
256256
require.Equal(t, "coder_worst-proxy", proxy2Region.RegionCode)
257257
require.Equal(t, 10002, proxy2Region.RegionID)
258258
require.False(t, proxy2Region.EmbeddedRelay)
259-
require.Len(t, proxy2Region.Nodes, 2) // proxy + stun
260-
require.Equal(t, "10002a", proxy2Region.Nodes[1].Name)
261-
require.Equal(t, 10002, proxy2Region.Nodes[1].RegionID)
262-
require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[1].HostName)
263-
require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[1].DERPPort))
264-
require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[1].ForceHTTP)
259+
require.Len(t, proxy2Region.Nodes, 1)
260+
require.Equal(t, "10002a", proxy2Region.Nodes[0].Name)
261+
require.Equal(t, 10002, proxy2Region.Nodes[0].RegionID)
262+
require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[0].HostName)
263+
require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[0].DERPPort))
264+
require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[0].ForceHTTP)
265265
})
266266

267267
t.Run("ConnectDERP", func(t *testing.T) {
@@ -313,6 +313,8 @@ resourceLoop:
313313

314314
func TestDERPMapStunNodes(t *testing.T) {
315315
t.Parallel()
316+
// See: enterprise/coderd/coderd.go
317+
t.Skip("STUN nodes are removed from proxy regions in the DERP map for now")
316318

317319
deploymentValues := coderdtest.DeploymentValues(t)
318320
deploymentValues.Experiments = []string{

0 commit comments

Comments
 (0)