feat(cli): add aws check to ping p2p diagnostics (#14450)

2026-09-24 15:04:27 +08:00 · 2024-08-29 21:41:12 +10:00
parent e65eb0321c
commit 4b5c45d6df
7 changed files with 426 additions and 41 deletions
@@ -351,53 +351,100 @@ func PeerDiagnostics(w io.Writer, d tailnet.PeerDiagnostics) {
 }

 type ConnDiags struct {
-	ConnInfo        *workspacesdk.AgentConnectionInfo
+	ConnInfo        workspacesdk.AgentConnectionInfo
 	PingP2P         bool
 	DisableDirect   bool
 	LocalNetInfo    *tailcfg.NetInfo
 	LocalInterfaces *healthsdk.InterfacesReport
 	AgentNetcheck   *healthsdk.AgentNetcheckReport
+	ClientIPIsAWS   bool
+	AgentIPIsAWS    bool
+	Verbose         bool
 	// TODO: More diagnostics
 }

-func ConnDiagnostics(w io.Writer, d ConnDiags) {
+func (d ConnDiags) Write(w io.Writer) {
+	_, _ = fmt.Fprintln(w, "")
+	general, client, agent := d.splitDiagnostics()
+	for _, msg := range general {
+		_, _ = fmt.Fprintln(w, msg)
+	}
+	if len(client) > 0 {
+		_, _ = fmt.Fprint(w, "Possible client-side issues with direct connection:\n\n")
+		for _, msg := range client {
+			_, _ = fmt.Fprintf(w, " - %s\n\n", msg)
+		}
+	}
+	if len(agent) > 0 {
+		_, _ = fmt.Fprint(w, "Possible agent-side issues with direct connections:\n\n")
+		for _, msg := range agent {
+			_, _ = fmt.Fprintf(w, " - %s\n\n", msg)
+		}
+	}
+}
+
+func (d ConnDiags) splitDiagnostics() (general, client, agent []string) {
+	if d.PingP2P {
+		general = append(general, "✔ You are connected directly (p2p)")
+	} else {
+		general = append(general, "❗ You are connected via a DERP relay, not directly (p2p)")
+	}
+
 	if d.AgentNetcheck != nil {
 		for _, msg := range d.AgentNetcheck.Interfaces.Warnings {
-			_, _ = fmt.Fprintf(w, "❗ Agent: %s\n", msg.Message)
+			agent = append(agent, msg.Message)
 		}
 	}

 	if d.LocalInterfaces != nil {
 		for _, msg := range d.LocalInterfaces.Warnings {
-			_, _ = fmt.Fprintf(w, "❗ Client: %s\n", msg.Message)
+			client = append(client, msg.Message)
 		}
 	}

-	if d.PingP2P {
-		_, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n")
-		return
+	if d.PingP2P && !d.Verbose {
+		return general, client, agent
 	}
-	_, _ = fmt.Fprint(w, "❗ You are connected via a DERP relay, not directly (p2p)\n")

 	if d.DisableDirect {
-		_, _ = fmt.Fprint(w, "❗ Direct connections are disabled locally, by `--disable-direct` or `CODER_DISABLE_DIRECT`\n")
-		return
+		general = append(general, "❗ Direct connections are disabled locally, by `--disable-direct` or `CODER_DISABLE_DIRECT`")
+		if !d.Verbose {
+			return general, client, agent
+		}
 	}

-	if d.ConnInfo != nil && d.ConnInfo.DisableDirectConnections {
-		_, _ = fmt.Fprint(w, "❗ Your Coder administrator has blocked direct connections\n")
-		return
+	if d.ConnInfo.DisableDirectConnections {
+		general = append(general, "❗ Your Coder administrator has blocked direct connections")
+		if !d.Verbose {
+			return general, client, agent
+		}
 	}

-	if d.ConnInfo != nil && d.ConnInfo.DERPMap != nil && !d.ConnInfo.DERPMap.HasSTUN() {
-		_, _ = fmt.Fprint(w, "✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks\n")
+	if !d.ConnInfo.DERPMap.HasSTUN() {
+		general = append(general, "The DERP map is not configured to use STUN")
+	} else if d.LocalNetInfo != nil && !d.LocalNetInfo.UDP {
+		client = append(client, "Client could not connect to STUN over UDP")
 	}

 	if d.LocalNetInfo != nil && d.LocalNetInfo.MappingVariesByDestIP.EqualBool(true) {
-		_, _ = fmt.Fprint(w, "❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
+		client = append(client, "Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers")
 	}

-	if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil && d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
-		_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
+	if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil {
+		if d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
+			agent = append(agent, "Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers")
+		}
+		if !d.AgentNetcheck.NetInfo.UDP {
+			agent = append(agent, "Agent could not connect to STUN over UDP")
+		}
 	}
+
+	if d.ClientIPIsAWS {
+		client = append(client, "Client IP address is within an AWS range (AWS uses hard NAT)")
+	}
+
+	if d.AgentIPIsAWS {
+		agent = append(agent, "Agent IP address is within an AWS range (AWS uses hard NAT)")
+	}
+	return general, client, agent
 }
@@ -686,7 +686,9 @@ func TestConnDiagnostics(t *testing.T) {
 		{
 			name: "Direct",
 			diags: cliui.ConnDiags{
-				ConnInfo:     &workspacesdk.AgentConnectionInfo{},
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
 				PingP2P:      true,
 				LocalNetInfo: &tailcfg.NetInfo{},
 			},
@@ -697,7 +699,8 @@ func TestConnDiagnostics(t *testing.T) {
 		{
 			name: "DirectBlocked",
 			diags: cliui.ConnDiags{
-				ConnInfo: &workspacesdk.AgentConnectionInfo{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap:                  &tailcfg.DERPMap{},
 					DisableDirectConnections: true,
 				},
 			},
@@ -709,32 +712,89 @@ func TestConnDiagnostics(t *testing.T) {
 		{
 			name: "NoStun",
 			diags: cliui.ConnDiags{
-				ConnInfo: &workspacesdk.AgentConnectionInfo{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
 					DERPMap: &tailcfg.DERPMap{},
 				},
 				LocalNetInfo: &tailcfg.NetInfo{},
 			},
 			want: []string{
 				`❗ You are connected via a DERP relay, not directly (p2p)`,
-				`✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks`,
+				`The DERP map is not configured to use STUN`,
+			},
+		},
+		{
+			name: "ClientHasStunNoUDP",
+			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{
+						Regions: map[int]*tailcfg.DERPRegion{
+							999: {
+								Nodes: []*tailcfg.DERPNode{
+									{
+										STUNPort: 1337,
+									},
+								},
+							},
+						},
+					},
+				},
+				LocalNetInfo: &tailcfg.NetInfo{
+					UDP: false,
+				},
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`Client could not connect to STUN over UDP`,
+			},
+		},
+		{
+			name: "AgentHasStunNoUDP",
+			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{
+						Regions: map[int]*tailcfg.DERPRegion{
+							999: {
+								Nodes: []*tailcfg.DERPNode{
+									{
+										STUNPort: 1337,
+									},
+								},
+							},
+						},
+					},
+				},
+				AgentNetcheck: &healthsdk.AgentNetcheckReport{
+					NetInfo: &tailcfg.NetInfo{
+						UDP: false,
+					},
+				},
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`Agent could not connect to STUN over UDP`,
 			},
 		},
 		{
 			name: "ClientHardNat",
 			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
 				LocalNetInfo: &tailcfg.NetInfo{
 					MappingVariesByDestIP: "true",
 				},
 			},
 			want: []string{
 				`❗ You are connected via a DERP relay, not directly (p2p)`,
-				`❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
+				`Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
 			},
 		},
 		{
 			name: "AgentHardNat",
 			diags: cliui.ConnDiags{
-				ConnInfo:     &workspacesdk.AgentConnectionInfo{},
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
 				PingP2P:      false,
 				LocalNetInfo: &tailcfg.NetInfo{},
 				AgentNetcheck: &healthsdk.AgentNetcheckReport{
@@ -743,43 +803,77 @@ func TestConnDiagnostics(t *testing.T) {
 			},
 			want: []string{
 				`❗ You are connected via a DERP relay, not directly (p2p)`,
-				`❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
+				`Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
 			},
 		},
 		{
 			name: "AgentInterfaceWarnings",
 			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
 				PingP2P: true,
 				AgentNetcheck: &healthsdk.AgentNetcheckReport{
 					Interfaces: healthsdk.InterfacesReport{
 						BaseReport: healthsdk.BaseReport{
 							Warnings: []health.Message{
-								health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections"),
+								health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections"),
 							},
 						},
 					},
 				},
 			},
 			want: []string{
-				`❗ Agent: network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections`,
 				`✔ You are connected directly (p2p)`,
+				`Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections`,
 			},
 		},
 		{
 			name: "LocalInterfaceWarnings",
 			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
 				PingP2P: true,
 				LocalInterfaces: &healthsdk.InterfacesReport{
 					BaseReport: healthsdk.BaseReport{
 						Warnings: []health.Message{
-							health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections"),
+							health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections"),
 						},
 					},
 				},
 			},
 			want: []string{
-				`❗ Client: network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections`,
 				`✔ You are connected directly (p2p)`,
+				`Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections`,
+			},
+		},
+		{
+			name: "ClientAWSIP",
+			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
+				ClientIPIsAWS: true,
+				AgentIPIsAWS:  false,
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`Client IP address is within an AWS range (AWS uses hard NAT)`,
+			},
+		},
+		{
+			name: "AgentAWSIP",
+			diags: cliui.ConnDiags{
+				ConnInfo: workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{},
+				},
+				ClientIPIsAWS: false,
+				AgentIPIsAWS:  true,
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`Agent IP address is within an AWS range (AWS uses hard NAT)`,
 			},
 		},
 	}
@@ -790,7 +884,7 @@ func TestConnDiagnostics(t *testing.T) {
 			r, w := io.Pipe()
 			go func() {
 				defer w.Close()
-				cliui.ConnDiagnostics(w, tc.diags)
+				tc.diags.Write(w)
 			}()
 			bytes, err := io.ReadAll(r)
 			require.NoError(t, err)