From bad17a1bfaa0ac3e62e2ebc95fca7c5c5959055b Mon Sep 17 00:00:00 2001 From: Simon Law Date: Tue, 8 Jul 2025 22:14:18 -0700 Subject: [PATCH] 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 --- cmd/tailscale/cli/exitnode.go | 22 +++++++++------------- cmd/tailscale/cli/exitnode_test.go | 28 ++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/cmd/tailscale/cli/exitnode.go b/cmd/tailscale/cli/exitnode.go index ad7a8ccee..b153f096d 100644 --- a/cmd/tailscale/cli/exitnode.go +++ b/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 diff --git a/cmd/tailscale/cli/exitnode_test.go b/cmd/tailscale/cli/exitnode_test.go index 9d569a45a..cc38fd3a4 100644 --- a/cmd/tailscale/cli/exitnode_test.go +++ b/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 { - t.Fatalf("sortByCountryName did not order countries by alphabetical order, got %v, want %v", fc[0].Name, noLocationData) + want := []string{"", "Albania", "Sweden", "Zimbabwe"} + 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 { - t.Fatalf("sortByCityName did not order cities by alphabetical order, got %v, want %v", fc[0].Name, noLocationData) + want := []string{"", "Goteborg", "Kingston", "Squamish"} + 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) } }