cmd/tailscale: format empty cities and countries as hyphens (#16495)

When running `tailscale exit-node list`, an empty city or country name
should be displayed as a hyphen "-". However, this only happened when
there was no location at all. If a node provides a Hostinfo.Location,
then the list would display exactly what was provided.

This patch changes the listing so that empty cities and countries will
either render the provided name or "-".

Fixes #16500

Signed-off-by: Simon Law <sfllaw@tailscale.com>

cmd/tailscale/cli/exitnode.go

@@ -131,7 +131,7 @@ func runExitNodeList(ctx context.Context, args []string) error {
 	for _, country := range filteredPeers.Countries {
 		for _, city := range country.Cities {
 			for _, peer := range city.Peers {
-				fmt.Fprintf(w, "\n %s\t%s\t%s\t%s\t%s\t", peer.TailscaleIPs[0], strings.Trim(peer.DNSName, "."), country.Name, city.Name, peerStatus(peer))
+				fmt.Fprintf(w, "\n %s\t%s\t%s\t%s\t%s\t", peer.TailscaleIPs[0], strings.Trim(peer.DNSName, "."), cmp.Or(country.Name, "-"), cmp.Or(city.Name, "-"), peerStatus(peer))
 			}
 		}
 	}
@@ -202,23 +202,16 @@ type filteredCity struct {
 	Peers []*ipnstate.PeerStatus
 }
 
-const noLocationData = "-"
-
-var noLocation = &tailcfg.Location{
-	Country:     noLocationData,
-	CountryCode: noLocationData,
-	City:        noLocationData,
-	CityCode:    noLocationData,
-}
-
 // filterFormatAndSortExitNodes filters and sorts exit nodes into
 // alphabetical order, by country, city and then by priority if
 // present.
+//
 // If an exit node has location data, and the country has more than
 // one city, an `Any` city is added to the country that contains the
 // highest priority exit node within that country.
+//
 // For exit nodes without location data, their country fields are
-// defined as '-' to indicate that the data is not available.
+// defined as the empty string to indicate that the data is not available.
 func filterFormatAndSortExitNodes(peers []*ipnstate.PeerStatus, filterBy string) filteredExitNodes {
 	// first get peers into some fixed order, as code below doesn't break ties
 	// and our input comes from a random range-over-map.
@@ -229,7 +222,10 @@ func filterFormatAndSortExitNodes(peers []*ipnstate.PeerStatus, filterBy string)
 	countries := make(map[string]*filteredCountry)
 	cities := make(map[string]*filteredCity)
 	for _, ps := range peers {
-		loc := cmp.Or(ps.Location, noLocation)
+		loc := ps.Location
+		if loc == nil {
+			loc = &tailcfg.Location{}
+		}
 
 		if filterBy != "" && !strings.EqualFold(loc.Country, filterBy) {
 			continue
@@ -259,7 +255,7 @@ func filterFormatAndSortExitNodes(peers []*ipnstate.PeerStatus, filterBy string)
 	}
 
 	for _, country := range filteredExitNodes.Countries {
-		if country.Name == noLocationData {
+		if country.Name == "" {
 			// Countries without location data should not
 			// be filtered further.
 			continue

cmd/tailscale/cli/exitnode_test.go

@@ -74,10 +74,10 @@ func TestFilterFormatAndSortExitNodes(t *testing.T) {
 		want := filteredExitNodes{
 			Countries: []*filteredCountry{
 				{
-					Name: noLocationData,
+					Name: "",
 					Cities: []*filteredCity{
 						{
-							Name: noLocationData,
+							Name: "",
 							Peers: []*ipnstate.PeerStatus{
 								ps[5],
 							},
@@ -273,14 +273,20 @@ func TestSortByCountryName(t *testing.T) {
 			Name: "Zimbabwe",
 		},
 		{
-			Name: noLocationData,
+			Name: "",
 		},
 	}
 
 	sortByCountryName(fc)
 
-	if fc[0].Name != noLocationData {
+	want := []string{"", "Albania", "Sweden", "Zimbabwe"}
-		t.Fatalf("sortByCountryName did not order countries by alphabetical order, got %v, want %v", fc[0].Name, noLocationData)
+	var got []string
+	for _, c := range fc {
+		got = append(got, c.Name)
+	}
+
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("sortByCountryName did not order countries by alphabetical order (-want +got):\n%s", diff)
 	}
 }
 
@@ -296,13 +302,19 @@ func TestSortByCityName(t *testing.T) {
 			Name: "Squamish",
 		},
 		{
-			Name: noLocationData,
+			Name: "",
 		},
 	}
 
 	sortByCityName(fc)
 
-	if fc[0].Name != noLocationData {
+	want := []string{"", "Goteborg", "Kingston", "Squamish"}
-		t.Fatalf("sortByCityName did not order cities by alphabetical order, got %v, want %v", fc[0].Name, noLocationData)
+	var got []string
+	for _, c := range fc {
+		got = append(got, c.Name)
+	}
+
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("sortByCityName did not order countries by alphabetical order (-want +got):\n%s", diff)
 	}
 }