From 454d856be853c713e5e916f13f75cf183de2c94e Mon Sep 17 00:00:00 2001
From: Percy Wegmann <percy@tailscale.com>
Date: Tue, 1 Jul 2025 09:03:54 -0500
Subject: [PATCH] drive,ipn/ipnlocal: calculate peer taildrive URLs on-demand

Instead of calculating the PeerAPI URL at the time that we add the peer,
we now calculate it on every access to the peer. This way, if we
initially did not have a shared address family with the peer, but
later do, this allows us to access the peer at that point. This
follows the pattern from other places where we access the peer API,
which also calculate the URL on an as-needed basis.

Additionally, we now show peers as not Available when we can't get
a peer API URL.

Lastly, this moves some of the more frequent verbose Taildrive logging
from [v1] to [v2] level.

Updates #29702

Signed-off-by: Percy Wegmann <percy@tailscale.com>
---
 drive/driveimpl/drive_test.go |  2 +-
 drive/driveimpl/local_impl.go |  2 +-
 drive/local.go                |  2 +-
 ipn/ipnlocal/drive.go         | 27 +++++++++++++++++++--------
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drive/driveimpl/drive_test.go b/drive/driveimpl/drive_test.go
index e7dd83291..cff55fbb2 100644
--- a/drive/driveimpl/drive_test.go
+++ b/drive/driveimpl/drive_test.go
@@ -524,7 +524,7 @@ func (s *system) addRemote(name string) string {
 	for name, r := range s.remotes {
 		remotes = append(remotes, &drive.Remote{
 			Name: name,
-			URL:  fmt.Sprintf("http://%s", r.l.Addr()),
+			URL:  func() string { return fmt.Sprintf("http://%s", r.l.Addr()) },
 		})
 	}
 	s.local.fs.SetRemotes(
diff --git a/drive/driveimpl/local_impl.go b/drive/driveimpl/local_impl.go
index 8cdf60179..871d03343 100644
--- a/drive/driveimpl/local_impl.go
+++ b/drive/driveimpl/local_impl.go
@@ -81,7 +81,7 @@ func (s *FileSystemForLocal) SetRemotes(domain string, remotes []*drive.Remote,
 				Name:      remote.Name,
 				Available: remote.Available,
 			},
-			BaseURL:   func() (string, error) { return remote.URL, nil },
+			BaseURL:   func() (string, error) { return remote.URL(), nil },
 			Transport: transport,
 		})
 	}
diff --git a/drive/local.go b/drive/local.go
index aff79a57b..052efb3f9 100644
--- a/drive/local.go
+++ b/drive/local.go
@@ -17,7 +17,7 @@ import (
 // Remote represents a remote Taildrive node.
 type Remote struct {
 	Name      string
-	URL       string
+	URL       func() string
 	Available func() bool
 }
 
diff --git a/ipn/ipnlocal/drive.go b/ipn/ipnlocal/drive.go
index 8c2f339bb..d77481903 100644
--- a/ipn/ipnlocal/drive.go
+++ b/ipn/ipnlocal/drive.go
@@ -309,20 +309,26 @@ func (b *LocalBackend) driveRemotesFromPeers(nm *netmap.NetworkMap) []*drive.Rem
 	b.logf("[v1] taildrive: setting up drive remotes from peers")
 	driveRemotes := make([]*drive.Remote, 0, len(nm.Peers))
 	for _, p := range nm.Peers {
-		peerID := p.ID()
-		url := fmt.Sprintf("%s/%s", peerAPIBase(nm, p), taildrivePrefix[1:])
-		b.logf("[v1] taildrive: appending remote for peer %d: %s", peerID, url)
+		peer := p
+		peerID := peer.ID()
+		peerKey := peer.Key().ShortString()
+		b.logf("[v1] taildrive: appending remote for peer %s", peerKey)
 		driveRemotes = append(driveRemotes, &drive.Remote{
 			Name: p.DisplayName(false),
-			URL:  url,
+			URL: func() string {
+				url := fmt.Sprintf("%s/%s", b.currentNode().PeerAPIBase(peer), taildrivePrefix[1:])
+				b.logf("[v2] taildrive: url for peer %s: %s", peerKey, url)
+				return url
+			},
 			Available: func() bool {
 				// Peers are available to Taildrive if:
 				// - They are online
+				// - Their PeerAPI is reachable
 				// - They are allowed to share at least one folder with us
 				cn := b.currentNode()
 				peer, ok := cn.NodeByID(peerID)
 				if !ok {
-					b.logf("[v1] taildrive: Available(): peer %d not found", peerID)
+					b.logf("[v2] taildrive: Available(): peer %s not found", peerKey)
 					return false
 				}
 
@@ -335,17 +341,22 @@ func (b *LocalBackend) driveRemotesFromPeers(nm *netmap.NetworkMap) []*drive.Rem
 				// The netmap.Peers slice is not updated in all cases.
 				// It should be fixed now that we use PeerByIDOk.
 				if !peer.Online().Get() {
-					b.logf("[v1] taildrive: Available(): peer %d offline", peerID)
+					b.logf("[v2] taildrive: Available(): peer %s offline", peerKey)
+					return false
+				}
+
+				if b.currentNode().PeerAPIBase(peer) == "" {
+					b.logf("[v2] taildrive: Available(): peer %s PeerAPI unreachable", peerKey)
 					return false
 				}
 
 				// Check that the peer is allowed to share with us.
 				if cn.PeerHasCap(peer, tailcfg.PeerCapabilityTaildriveSharer) {
-					b.logf("[v1] taildrive: Available(): peer %d available", peerID)
+					b.logf("[v2] taildrive: Available(): peer %s available", peerKey)
 					return true
 				}
 
-				b.logf("[v1] taildrive: Available(): peer %d not allowed to share", peerID)
+				b.logf("[v2] taildrive: Available(): peer %s not allowed to share", peerKey)
 				return false
 			},
 		})