fix: remove stun nodes from workspace proxy regions (#8990)

This commit is contained in:
Dean Sheather
2023-08-09 02:31:25 -07:00
committed by GitHub
parent 00a8221e51
commit 9941f49056
2 changed files with 35 additions and 30 deletions

View File

@ -659,7 +659,7 @@ var (
lastDerpConflictLog time.Time lastDerpConflictLog time.Time
) )
func derpMapper(logger slog.Logger, cfg *codersdk.DeploymentValues, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap { func derpMapper(logger slog.Logger, _ *codersdk.DeploymentValues, proxyHealth *proxyhealth.ProxyHealth) func(*tailcfg.DERPMap) *tailcfg.DERPMap {
return func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap { return func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap {
derpMap = derpMap.Clone() derpMap = derpMap.Clone()
@ -754,25 +754,28 @@ func derpMapper(logger slog.Logger, cfg *codersdk.DeploymentValues, proxyHealth
} }
var stunNodes []*tailcfg.DERPNode var stunNodes []*tailcfg.DERPNode
if !cfg.DERP.Config.BlockDirect.Value() { // TODO(@dean): potentially re-enable this depending on impact
stunNodes, err = agpltailnet.STUNNodes(regionID, cfg.DERP.Server.STUNAddresses) /*
if err != nil { if !cfg.DERP.Config.BlockDirect.Value() {
// Log a warning if we haven't logged one in the last stunNodes, err = agpltailnet.STUNNodes(regionID, cfg.DERP.Server.STUNAddresses)
// minute. if err != nil {
lastDerpConflictMutex.Lock() // Log a warning if we haven't logged one in the last
shouldLog := lastDerpConflictLog.IsZero() || time.Since(lastDerpConflictLog) > time.Minute // minute.
if shouldLog { lastDerpConflictMutex.Lock()
lastDerpConflictLog = time.Now() shouldLog := lastDerpConflictLog.IsZero() || time.Since(lastDerpConflictLog) > time.Minute
} if shouldLog {
lastDerpConflictMutex.Unlock() lastDerpConflictLog = time.Now()
if shouldLog { }
logger.Error(context.Background(), "failed to calculate STUN nodes", slog.Error(err)) lastDerpConflictMutex.Unlock()
} if shouldLog {
logger.Error(context.Background(), "failed to calculate STUN nodes", slog.Error(err))
}
// No continue because we can keep going. // No continue because we can keep going.
stunNodes = []*tailcfg.DERPNode{} stunNodes = []*tailcfg.DERPNode{}
}
} }
} */
nodes := append(stunNodes, &tailcfg.DERPNode{ nodes := append(stunNodes, &tailcfg.DERPNode{
Name: fmt.Sprintf("%da", regionID), Name: fmt.Sprintf("%da", regionID),

View File

@ -244,24 +244,24 @@ resourceLoop:
require.Equal(t, "coder_best-proxy", proxy1Region.RegionCode) require.Equal(t, "coder_best-proxy", proxy1Region.RegionCode)
require.Equal(t, 10001, proxy1Region.RegionID) require.Equal(t, 10001, proxy1Region.RegionID)
require.False(t, proxy1Region.EmbeddedRelay) require.False(t, proxy1Region.EmbeddedRelay)
require.Len(t, proxy1Region.Nodes, 2) // proxy + stun require.Len(t, proxy1Region.Nodes, 1)
require.Equal(t, "10001a", proxy1Region.Nodes[1].Name) require.Equal(t, "10001a", proxy1Region.Nodes[0].Name)
require.Equal(t, 10001, proxy1Region.Nodes[1].RegionID) require.Equal(t, 10001, proxy1Region.Nodes[0].RegionID)
require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[1].HostName) require.Equal(t, proxyAPI1.Options.AccessURL.Hostname(), proxy1Region.Nodes[0].HostName)
require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[1].DERPPort)) require.Equal(t, proxyAPI1.Options.AccessURL.Port(), fmt.Sprint(proxy1Region.Nodes[0].DERPPort))
require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[1].ForceHTTP) require.Equal(t, proxyAPI1.Options.AccessURL.Scheme == "http", proxy1Region.Nodes[0].ForceHTTP)
// The second proxy region: // The second proxy region:
require.Equal(t, "worst-proxy", proxy2Region.RegionName) require.Equal(t, "worst-proxy", proxy2Region.RegionName)
require.Equal(t, "coder_worst-proxy", proxy2Region.RegionCode) require.Equal(t, "coder_worst-proxy", proxy2Region.RegionCode)
require.Equal(t, 10002, proxy2Region.RegionID) require.Equal(t, 10002, proxy2Region.RegionID)
require.False(t, proxy2Region.EmbeddedRelay) require.False(t, proxy2Region.EmbeddedRelay)
require.Len(t, proxy2Region.Nodes, 2) // proxy + stun require.Len(t, proxy2Region.Nodes, 1)
require.Equal(t, "10002a", proxy2Region.Nodes[1].Name) require.Equal(t, "10002a", proxy2Region.Nodes[0].Name)
require.Equal(t, 10002, proxy2Region.Nodes[1].RegionID) require.Equal(t, 10002, proxy2Region.Nodes[0].RegionID)
require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[1].HostName) require.Equal(t, proxyAPI2.Options.AccessURL.Hostname(), proxy2Region.Nodes[0].HostName)
require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[1].DERPPort)) require.Equal(t, proxyAPI2.Options.AccessURL.Port(), fmt.Sprint(proxy2Region.Nodes[0].DERPPort))
require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[1].ForceHTTP) require.Equal(t, proxyAPI2.Options.AccessURL.Scheme == "http", proxy2Region.Nodes[0].ForceHTTP)
}) })
t.Run("ConnectDERP", func(t *testing.T) { t.Run("ConnectDERP", func(t *testing.T) {
@ -313,6 +313,8 @@ resourceLoop:
func TestDERPMapStunNodes(t *testing.T) { func TestDERPMapStunNodes(t *testing.T) {
t.Parallel() t.Parallel()
// See: enterprise/coderd/coderd.go
t.Skip("STUN nodes are removed from proxy regions in the DERP map for now")
deploymentValues := coderdtest.DeploymentValues(t) deploymentValues := coderdtest.DeploymentValues(t)
deploymentValues.Experiments = []string{ deploymentValues.Experiments = []string{