Skip to content

Commit 4b5c45d

Browse files
ethanndicksonethanndickson
authored
feat(cli): add aws check to ping p2p diagnostics (coder#14450)
1 parent e65eb03 commit 4b5c45d

File tree

7 files changed

+426
-41
lines changed

7 files changed

+426
-41
lines changed

‎cli/cliui/agent.go

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -351,53 +351,100 @@ func PeerDiagnostics(w io.Writer, d tailnet.PeerDiagnostics) {
351351
}
352352

353353
type ConnDiags struct {
354-
ConnInfo *workspacesdk.AgentConnectionInfo
354+
ConnInfo workspacesdk.AgentConnectionInfo
355355
PingP2P bool
356356
DisableDirect bool
357357
LocalNetInfo *tailcfg.NetInfo
358358
LocalInterfaces *healthsdk.InterfacesReport
359359
AgentNetcheck *healthsdk.AgentNetcheckReport
360+
ClientIPIsAWS bool
361+
AgentIPIsAWS bool
362+
Verbose bool
360363
// TODO: More diagnostics
361364
}
362365

363-
func ConnDiagnostics(w io.Writer, d ConnDiags) {
366+
func (d ConnDiags) Write(w io.Writer) {
367+
_, _ = fmt.Fprintln(w, "")
368+
general, client, agent := d.splitDiagnostics()
369+
for _, msg := range general {
370+
_, _ = fmt.Fprintln(w, msg)
371+
}
372+
if len(client) > 0 {
373+
_, _ = fmt.Fprint(w, "Possible client-side issues with direct connection:\n\n")
374+
for _, msg := range client {
375+
_, _ = fmt.Fprintf(w, " - %s\n\n", msg)
376+
}
377+
}
378+
if len(agent) > 0 {
379+
_, _ = fmt.Fprint(w, "Possible agent-side issues with direct connections:\n\n")
380+
for _, msg := range agent {
381+
_, _ = fmt.Fprintf(w, " - %s\n\n", msg)
382+
}
383+
}
384+
}
385+
386+
func (d ConnDiags) splitDiagnostics() (general, client, agent []string) {
387+
if d.PingP2P {
388+
general = append(general, "✔ You are connected directly (p2p)")
389+
} else {
390+
general = append(general, "❗ You are connected via a DERP relay, not directly (p2p)")
391+
}
392+
364393
if d.AgentNetcheck != nil {
365394
for _, msg := range d.AgentNetcheck.Interfaces.Warnings {
366-
_, _ = fmt.Fprintf(w, "❗ Agent: %s\n", msg.Message)
395+
agent = append(agent, msg.Message)
367396
}
368397
}
369398

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

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

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

387-
if d.ConnInfo != nil && d.ConnInfo.DisableDirectConnections {
388-
_, _ = fmt.Fprint(w, "❗ Your Coder administrator has blocked direct connections\n")
389-
return
416+
if d.ConnInfo.DisableDirectConnections {
417+
general = append(general, "❗ Your Coder administrator has blocked direct connections")
418+
if !d.Verbose {
419+
return general, client, agent
420+
}
390421
}
391422

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

396429
if d.LocalNetInfo != nil && d.LocalNetInfo.MappingVariesByDestIP.EqualBool(true) {
397-
_, _ = fmt.Fprint(w, "❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
430+
client = append(client, "Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers")
431+
}
432+
433+
if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil {
434+
if d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
435+
agent = append(agent, "Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers")
436+
}
437+
if !d.AgentNetcheck.NetInfo.UDP {
438+
agent = append(agent, "Agent could not connect to STUN over UDP")
439+
}
440+
}
441+
442+
if d.ClientIPIsAWS {
443+
client = append(client, "Client IP address is within an AWS range (AWS uses hard NAT)")
398444
}
399445

400-
if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil && d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
401-
_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
446+
if d.AgentIPIsAWS {
447+
agent = append(agent, "Agent IP address is within an AWS range (AWS uses hard NAT)")
402448
}
449+
return general, client, agent
403450
}

‎cli/cliui/agent_test.go

Lines changed: 106 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,9 @@ func TestConnDiagnostics(t *testing.T) {
686686
{
687687
name: "Direct",
688688
diags: cliui.ConnDiags{
689-
ConnInfo: &workspacesdk.AgentConnectionInfo{},
689+
ConnInfo: workspacesdk.AgentConnectionInfo{
690+
DERPMap: &tailcfg.DERPMap{},
691+
},
690692
PingP2P: true,
691693
LocalNetInfo: &tailcfg.NetInfo{},
692694
},
@@ -697,7 +699,8 @@ func TestConnDiagnostics(t *testing.T) {
697699
{
698700
name: "DirectBlocked",
699701
diags: cliui.ConnDiags{
700-
ConnInfo: &workspacesdk.AgentConnectionInfo{
702+
ConnInfo: workspacesdk.AgentConnectionInfo{
703+
DERPMap: &tailcfg.DERPMap{},
701704
DisableDirectConnections: true,
702705
},
703706
},
@@ -709,32 +712,89 @@ func TestConnDiagnostics(t *testing.T) {
709712
{
710713
name: "NoStun",
711714
diags: cliui.ConnDiags{
712-
ConnInfo: &workspacesdk.AgentConnectionInfo{
715+
ConnInfo: workspacesdk.AgentConnectionInfo{
713716
DERPMap: &tailcfg.DERPMap{},
714717
},
715718
LocalNetInfo: &tailcfg.NetInfo{},
716719
},
717720
want: []string{
718721
`❗ You are connected via a DERP relay, not directly (p2p)`,
719-
`✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks`,
722+
`The DERP map is not configured to use STUN`,
723+
},
724+
},
725+
{
726+
name: "ClientHasStunNoUDP",
727+
diags: cliui.ConnDiags{
728+
ConnInfo: workspacesdk.AgentConnectionInfo{
729+
DERPMap: &tailcfg.DERPMap{
730+
Regions: map[int]*tailcfg.DERPRegion{
731+
999: {
732+
Nodes: []*tailcfg.DERPNode{
733+
{
734+
STUNPort: 1337,
735+
},
736+
},
737+
},
738+
},
739+
},
740+
},
741+
LocalNetInfo: &tailcfg.NetInfo{
742+
UDP: false,
743+
},
744+
},
745+
want: []string{
746+
`❗ You are connected via a DERP relay, not directly (p2p)`,
747+
`Client could not connect to STUN over UDP`,
748+
},
749+
},
750+
{
751+
name: "AgentHasStunNoUDP",
752+
diags: cliui.ConnDiags{
753+
ConnInfo: workspacesdk.AgentConnectionInfo{
754+
DERPMap: &tailcfg.DERPMap{
755+
Regions: map[int]*tailcfg.DERPRegion{
756+
999: {
757+
Nodes: []*tailcfg.DERPNode{
758+
{
759+
STUNPort: 1337,
760+
},
761+
},
762+
},
763+
},
764+
},
765+
},
766+
AgentNetcheck: &healthsdk.AgentNetcheckReport{
767+
NetInfo: &tailcfg.NetInfo{
768+
UDP: false,
769+
},
770+
},
771+
},
772+
want: []string{
773+
`❗ You are connected via a DERP relay, not directly (p2p)`,
774+
`Agent could not connect to STUN over UDP`,
720775
},
721776
},
722777
{
723778
name: "ClientHardNat",
724779
diags: cliui.ConnDiags{
780+
ConnInfo: workspacesdk.AgentConnectionInfo{
781+
DERPMap: &tailcfg.DERPMap{},
782+
},
725783
LocalNetInfo: &tailcfg.NetInfo{
726784
MappingVariesByDestIP: "true",
727785
},
728786
},
729787
want: []string{
730788
`❗ You are connected via a DERP relay, not directly (p2p)`,
731-
`❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
789+
`Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
732790
},
733791
},
734792
{
735793
name: "AgentHardNat",
736794
diags: cliui.ConnDiags{
737-
ConnInfo: &workspacesdk.AgentConnectionInfo{},
795+
ConnInfo: workspacesdk.AgentConnectionInfo{
796+
DERPMap: &tailcfg.DERPMap{},
797+
},
738798
PingP2P: false,
739799
LocalNetInfo: &tailcfg.NetInfo{},
740800
AgentNetcheck: &healthsdk.AgentNetcheckReport{
@@ -743,43 +803,77 @@ func TestConnDiagnostics(t *testing.T) {
743803
},
744804
want: []string{
745805
`❗ You are connected via a DERP relay, not directly (p2p)`,
746-
`❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
806+
`Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`,
747807
},
748808
},
749809
{
750810
name: "AgentInterfaceWarnings",
751811
diags: cliui.ConnDiags{
812+
ConnInfo: workspacesdk.AgentConnectionInfo{
813+
DERPMap: &tailcfg.DERPMap{},
814+
},
752815
PingP2P: true,
753816
AgentNetcheck: &healthsdk.AgentNetcheckReport{
754817
Interfaces: healthsdk.InterfacesReport{
755818
BaseReport: healthsdk.BaseReport{
756819
Warnings: []health.Message{
757-
health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections"),
820+
health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections"),
758821
},
759822
},
760823
},
761824
},
762825
},
763826
want: []string{
764-
`❗ Agent: network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections`,
765827
`✔ You are connected directly (p2p)`,
828+
`Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections`,
766829
},
767830
},
768831
{
769832
name: "LocalInterfaceWarnings",
770833
diags: cliui.ConnDiags{
834+
ConnInfo: workspacesdk.AgentConnectionInfo{
835+
DERPMap: &tailcfg.DERPMap{},
836+
},
771837
PingP2P: true,
772838
LocalInterfaces: &healthsdk.InterfacesReport{
773839
BaseReport: healthsdk.BaseReport{
774840
Warnings: []health.Message{
775-
health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections"),
841+
health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections"),
776842
},
777843
},
778844
},
779845
},
780846
want: []string{
781-
`❗ Client: network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections`,
782847
`✔ You are connected directly (p2p)`,
848+
`Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections`,
849+
},
850+
},
851+
{
852+
name: "ClientAWSIP",
853+
diags: cliui.ConnDiags{
854+
ConnInfo: workspacesdk.AgentConnectionInfo{
855+
DERPMap: &tailcfg.DERPMap{},
856+
},
857+
ClientIPIsAWS: true,
858+
AgentIPIsAWS: false,
859+
},
860+
want: []string{
861+
`❗ You are connected via a DERP relay, not directly (p2p)`,
862+
`Client IP address is within an AWS range (AWS uses hard NAT)`,
863+
},
864+
},
865+
{
866+
name: "AgentAWSIP",
867+
diags: cliui.ConnDiags{
868+
ConnInfo: workspacesdk.AgentConnectionInfo{
869+
DERPMap: &tailcfg.DERPMap{},
870+
},
871+
ClientIPIsAWS: false,
872+
AgentIPIsAWS: true,
873+
},
874+
want: []string{
875+
`❗ You are connected via a DERP relay, not directly (p2p)`,
876+
`Agent IP address is within an AWS range (AWS uses hard NAT)`,
783877
},
784878
},
785879
}
@@ -790,7 +884,7 @@ func TestConnDiagnostics(t *testing.T) {
790884
r, w := io.Pipe()
791885
go func() {
792886
defer w.Close()
793-
cliui.ConnDiagnostics(w, tc.diags)
887+
tc.diags.Write(w)
794888
}()
795889
bytes, err := io.ReadAll(r)
796890
require.NoError(t, err)

0 commit comments

Comments
 (0)