TUN-5616: Never fallback transport if user chooses it on purpose

Author: nmldiegues

connection/protocol.go

@@ -19,9 +19,7 @@ const (
 	edgeH2TLSServerName = "h2.cftunnel.com"
 	// edgeQUICServerName is the server name to establish quic connection with edge.
 	edgeQUICServerName = "quic.cftunnel.com"
-	// threshold to switch back to h2mux when the user intentionally pick --protocol http2
-	explicitHTTP2FallbackThreshold = -1
-	autoSelectFlag                 = "auto"
+	autoSelectFlag     = "auto"
 )
 
 var (
@@ -237,25 +235,23 @@ func selectNamedTunnelProtocols(
 	threshold int32,
 	protocol Protocol,
 ) (ProtocolSelector, error) {
-	if protocolFlag == H2mux.String() {
-		return &staticProtocolSelector{
-			current: H2mux,
-		}, nil
-	}
-
-	if protocolFlag == QUIC.String() {
-		return newAutoProtocolSelector(QUIC, []Protocol{QUIC, HTTP2, H2mux}, explicitHTTP2FallbackThreshold, fetchFunc, ttl, log), nil
+	// If the user picks a protocol, then we stick to it no matter what.
+	switch protocolFlag {
+	case H2mux.String():
+		return &staticProtocolSelector{current: H2mux}, nil
+	case QUIC.String():
+		return &staticProtocolSelector{current: QUIC}, nil
+	case HTTP2.String():
+		return &staticProtocolSelector{current: HTTP2}, nil
 	}
 
-	if protocolFlag == HTTP2.String() {
-		return newAutoProtocolSelector(HTTP2, []Protocol{HTTP2, H2mux}, explicitHTTP2FallbackThreshold, fetchFunc, ttl, log), nil
+	// If the user does not pick (hopefully the majority) then we use the one derived from the TXT DNS record and
+	// fallback on failures.
+	if protocolFlag == autoSelectFlag {
+		return newAutoProtocolSelector(protocol, []Protocol{QUIC, HTTP2, H2mux}, threshold, fetchFunc, ttl, log), nil
 	}
 
-	if protocolFlag != autoSelectFlag {
-		return nil, fmt.Errorf("Unknown protocol %s, %s", protocolFlag, AvailableProtocolFlagMessage)
-	}
-
-	return newAutoProtocolSelector(protocol, []Protocol{QUIC, HTTP2, H2mux}, threshold, fetchFunc, ttl, log), nil
+	return nil, fmt.Errorf("Unknown protocol %s, %s", protocolFlag, AvailableProtocolFlagMessage)
 }
 
 func selectWarpRoutingProtocols(
@@ -266,19 +262,21 @@ func selectWarpRoutingProtocols(
 	threshold int32,
 	protocol Protocol,
 ) (ProtocolSelector, error) {
-	if protocolFlag == QUIC.String() {
-		return newAutoProtocolSelector(QUICWarp, []Protocol{QUICWarp, HTTP2Warp}, explicitHTTP2FallbackThreshold, fetchFunc, ttl, log), nil
-	}
-
-	if protocolFlag == HTTP2.String() {
-		return newAutoProtocolSelector(HTTP2Warp, []Protocol{HTTP2Warp}, explicitHTTP2FallbackThreshold, fetchFunc, ttl, log), nil
+	// If the user picks a protocol, then we stick to it no matter what.
+	switch protocolFlag {
+	case QUIC.String():
+		return &staticProtocolSelector{current: QUICWarp}, nil
+	case HTTP2.String():
+		return &staticProtocolSelector{current: HTTP2Warp}, nil
 	}
 
-	if protocolFlag != autoSelectFlag {
-		return nil, fmt.Errorf("Unknown protocol %s, %s", protocolFlag, AvailableProtocolFlagMessage)
+	// If the user does not pick (hopefully the majority) then we use the one derived from the TXT DNS record and
+	// fallback on failures.
+	if protocolFlag == autoSelectFlag {
+		return newAutoProtocolSelector(protocol, []Protocol{QUICWarp, HTTP2Warp}, threshold, fetchFunc, ttl, log), nil
 	}
 
-	return newAutoProtocolSelector(protocol, []Protocol{QUICWarp, HTTP2Warp}, threshold, fetchFunc, ttl, log), nil
+	return nil, fmt.Errorf("Unknown protocol %s, %s", protocolFlag, AvailableProtocolFlagMessage)
 }
 
 func switchThreshold(accountTag string) int32 {

connection/protocol_test.go

@@ -72,21 +72,33 @@ func TestNewProtocolSelector(t *testing.T) {
 			name:              "named tunnel over http2",
 			protocol:          "http2",
 			expectedProtocol:  HTTP2,
-			hasFallback:       true,
-			expectedFallback:  H2mux,
 			fetchFunc:         mockFetcher(false, edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: 0}),
 			namedTunnelConfig: testNamedTunnelConfig,
 		},
 		{
-			name:              "named tunnel http2 disabled",
+			name:              "named tunnel http2 disabled still gets http2 because it is manually picked",
 			protocol:          "http2",
-			expectedProtocol:  H2mux,
+			expectedProtocol:  HTTP2,
 			fetchFunc:         mockFetcher(false, edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: -1}),
 			namedTunnelConfig: testNamedTunnelConfig,
 		},
+		{
+			name:              "named tunnel quic disabled still gets quic because it is manually picked",
+			protocol:          "quic",
+			expectedProtocol:  QUIC,
+			fetchFunc:         mockFetcher(false, edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: 100}, edgediscovery.ProtocolPercent{Protocol: "quic", Percentage: -1}),
+			namedTunnelConfig: testNamedTunnelConfig,
+		},
+		{
+			name:              "named tunnel quic and http2 disabled",
+			protocol:          "auto",
+			expectedProtocol:  H2mux,
+			fetchFunc:         mockFetcher(false, edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: -1}, edgediscovery.ProtocolPercent{Protocol: "quic", Percentage: -1}),
+			namedTunnelConfig: testNamedTunnelConfig,
+		},
 		{
 			name:             "named tunnel quic disabled",
-			protocol:         "quic",
+			protocol:         "auto",
 			expectedProtocol: HTTP2,
 			// Hasfallback true is because if http2 fails, then we further fallback to h2mux.
 			hasFallback:       true,
@@ -155,7 +167,7 @@ func TestNewProtocolSelector(t *testing.T) {
 		},
 		{
 			name:               "warp routing quic",
-			protocol:           "quic",
+			protocol:           "auto",
 			expectedProtocol:   QUICWarp,
 			hasFallback:        true,
 			expectedFallback:   HTTP2Warp,
@@ -254,6 +266,7 @@ func TestAutoProtocolSelectorRefresh(t *testing.T) {
 
 func TestHTTP2ProtocolSelectorRefresh(t *testing.T) {
 	fetcher := dynamicMockFetcher{}
+	// Since the user chooses http2 on purpose, we always stick to it.
 	selector, err := NewProtocolSelector("http2", noWarpRoutingEnabled, testNamedTunnelConfig, fetcher.fetch(), testNoTTL, &log)
 	assert.NoError(t, err)
 	assert.Equal(t, HTTP2, selector.Current())
@@ -269,7 +282,7 @@ func TestHTTP2ProtocolSelectorRefresh(t *testing.T) {
 
 	fetcher.protocolPercents = edgediscovery.ProtocolPercents{edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: -1}}
 	fetcher.err = nil
-	assert.Equal(t, H2mux, selector.Current())
+	assert.Equal(t, HTTP2, selector.Current())
 
 	fetcher.protocolPercents = edgediscovery.ProtocolPercents{edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: 0}}
 	assert.Equal(t, HTTP2, selector.Current())
@@ -278,7 +291,7 @@ func TestHTTP2ProtocolSelectorRefresh(t *testing.T) {
 	assert.Equal(t, HTTP2, selector.Current())
 
 	fetcher.protocolPercents = edgediscovery.ProtocolPercents{edgediscovery.ProtocolPercent{Protocol: "http2", Percentage: -1}}
-	assert.Equal(t, H2mux, selector.Current())
+	assert.Equal(t, HTTP2, selector.Current())
 }
 
 func TestProtocolSelectorRefreshTTL(t *testing.T) {

origin/tunnel_test.go

@@ -41,7 +41,7 @@ func TestWaitForBackoffFallback(t *testing.T) {
 	}
 	warpRoutingEnabled := false
 	protocolSelector, err := connection.NewProtocolSelector(
-		connection.HTTP2.String(),
+		"auto",
 		warpRoutingEnabled,
 		namedTunnel,
 		mockFetcher.fetch(),