cmd/tailscale, ipn/localapi: use localapi for status, not IPN acrobatics

Yay simpler code.

Tested on Linux, macOS and Windows.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

client/tailscale/tailscale.go

@@ -15,6 +15,7 @@
 	"net/url"
 	"strconv"
 
+	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/safesocket"
 	"tailscale.com/tailcfg"
 )
@@ -79,6 +80,7 @@ func WhoIs(ctx context.Context, remoteAddr string) (*tailcfg.WhoIsResponse, erro
 	return r, nil
 }
 
+// Goroutines returns a dump of the Tailscale daemon's current goroutines.
 func Goroutines(ctx context.Context) ([]byte, error) {
 	req, err := http.NewRequestWithContext(ctx, "GET", "http://local-tailscaled.sock/localapi/v0/goroutines", nil)
 	if err != nil {
@@ -98,3 +100,25 @@ func Goroutines(ctx context.Context) ([]byte, error) {
 	}
 	return body, nil
 }
+
+// Status returns the Tailscale daemon's status.
+func Status(ctx context.Context) (*ipnstate.Status, error) {
+	req, err := http.NewRequestWithContext(ctx, "GET", "http://local-tailscaled.sock/localapi/v0/status", nil)
+	if err != nil {
+		return nil, err
+	}
+	res, err := DoLocalRequest(req)
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+	if res.StatusCode != 200 {
+		body, _ := ioutil.ReadAll(res.Body)
+		return nil, fmt.Errorf("HTTP %s: %s", res.Status, body)
+	}
+	st := new(ipnstate.Status)
+	if err := json.NewDecoder(res.Body).Decode(st); err != nil {
+		return nil, err
+	}
+	return st, nil
+}

cmd/tailscale/cli/down.go

@@ -6,10 +6,12 @@
 
 import (
 	"context"
+	"fmt"
 	"log"
 	"time"
 
 	"github.com/peterbourgon/ff/v2/ffcli"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn"
 )
 
@@ -26,6 +28,16 @@ func runDown(ctx context.Context, args []string) error {
 		log.Fatalf("too many non-flag arguments: %q", args)
 	}
 
+	st, err := tailscale.Status(ctx)
+	if err != nil {
+		return fmt.Errorf("error fetching current status: %w", err)
+	}
+	if st.BackendState == "Stopped" {
+		log.Printf("already stopped")
+		return nil
+	}
+	log.Printf("was in state %q", st.BackendState)
+
 	c, bc, ctx, cancel := connect(ctx)
 	defer cancel()
 
@@ -38,17 +50,6 @@ func runDown(ctx context.Context, args []string) error {
 		if n.ErrMessage != nil {
 			log.Fatal(*n.ErrMessage)
 		}
-		if n.Status != nil {
-			cur := n.Status.BackendState
-			switch cur {
-			case "Stopped":
-				log.Printf("already stopped")
-				cancel()
-			default:
-				log.Printf("was in state %q", cur)
-			}
-			return
-		}
 		if n.State != nil {
 			log.Printf("now in state %q", *n.State)
 			if *n.State == ipn.Stopped {
@@ -58,7 +59,6 @@ func runDown(ctx context.Context, args []string) error {
 		}
 	})
 
-	bc.RequestStatus()
 	bc.SetWantRunning(false)
 	pump(ctx, bc, c)
 

cmd/tailscale/cli/ping.go

@@ -15,6 +15,7 @@
 	"time"
 
 	"github.com/peterbourgon/ff/v2/ffcli"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 )
@@ -69,7 +70,6 @@ func runPing(ctx context.Context, args []string) error {
 	}
 	var ip string
 	prc := make(chan *ipnstate.PingResult, 1)
-	stc := make(chan *ipnstate.Status, 1)
 	bc.SetNotifyCallback(func(n ipn.Notify) {
 		if n.ErrMessage != nil {
 			log.Fatal(*n.ErrMessage)
@@ -77,9 +77,6 @@ func runPing(ctx context.Context, args []string) error {
 		if pr := n.PingResult; pr != nil && pr.IP == ip {
 			prc <- pr
 		}
-		if n.Status != nil {
-			stc <- n.Status
-		}
 	})
 	go pump(ctx, bc, c)
 
@@ -92,18 +89,16 @@ func runPing(ctx context.Context, args []string) error {
 
 	// Otherwise, try to resolve it first from the network peer list.
 	if ip == "" {
-		bc.RequestStatus()
+		st, err := tailscale.Status(ctx)
-		select {
+		if err != nil {
-		case st := <-stc:
+			return err
+		}
 		for _, ps := range st.Peer {
 			if hostOrIP == dnsOrQuoteHostname(st, ps) || hostOrIP == ps.DNSName {
 				ip = ps.TailAddr
 				break
 			}
 		}
-		case <-ctx.Done():
-			return ctx.Err()
-		}
 	}
 
 	// Finally, use DNS.

cmd/tailscale/cli/status.go

@@ -10,7 +10,6 @@
 	"encoding/json"
 	"flag"
 	"fmt"
-	"log"
 	"net"
 	"net/http"
 	"os"
@@ -19,6 +18,7 @@
 
 	"github.com/peterbourgon/ff/v2/ffcli"
 	"github.com/toqueteos/webbrowser"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/interfaces"
@@ -53,47 +53,8 @@
 	peers   bool   // in CLI mode, show status of peer machines
 }
 
-func getStatusFromServer(ctx context.Context, c net.Conn, bc *ipn.BackendClient) func() (*ipnstate.Status, error) {
-	ch := make(chan *ipnstate.Status, 1)
-	bc.SetNotifyCallback(func(n ipn.Notify) {
-		if n.ErrMessage != nil {
-			log.Fatal(*n.ErrMessage)
-		}
-		if n.Status != nil {
-			select {
-			case ch <- n.Status:
-			default:
-				// A status update from somebody else's request.
-				// Ignoring this matters mostly for "tailscale status -web"
-				// mode, otherwise the channel send would block forever
-				// and pump would stop reading from tailscaled, which
-				// previously caused tailscaled to block (while holding
-				// a mutex), backing up unrelated clients.
-				// See https://github.com/tailscale/tailscale/issues/1234
-			}
-		}
-	})
-	go pump(ctx, bc, c)
-
-	return func() (*ipnstate.Status, error) {
-		bc.RequestStatus()
-		select {
-		case st := <-ch:
-			return st, nil
-		case <-ctx.Done():
-			return nil, ctx.Err()
-		}
-	}
-}
-
 func runStatus(ctx context.Context, args []string) error {
-	c, bc, ctx, cancel := connect(ctx)
+	st, err := tailscale.Status(ctx)
-	defer cancel()
-
-	bc.AllowVersionSkew = true
-
-	getStatus := getStatusFromServer(ctx, c, bc)
-	st, err := getStatus()
 	if err != nil {
 		return err
 	}
@@ -131,7 +92,7 @@ func runStatus(ctx context.Context, args []string) error {
 				http.NotFound(w, r)
 				return
 			}
-			st, err := getStatus()
+			st, err := tailscale.Status(ctx)
 			if err != nil {
 				http.Error(w, err.Error(), 500)
 				return

cmd/tailscale/cli/up.go

@@ -21,6 +21,7 @@
 
 	"github.com/peterbourgon/ff/v2/ffcli"
 	"inet.af/netaddr"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/preftype"
@@ -253,7 +254,7 @@ func runUp(ctx context.Context, args []string) error {
 	defer cancel()
 
 	if !prefs.ExitNodeIP.IsZero() {
-		st, err := getStatusFromServer(ctx, c, bc)()
+		st, err := tailscale.Status(ctx)
 		if err != nil {
 			fatalf("can't fetch status from tailscaled: %v", err)
 		}

cmd/tailscale/cli/version.go

@@ -53,14 +53,14 @@ func runVersion(ctx context.Context, args []string) error {
 		if n.ErrMessage != nil {
 			log.Fatal(*n.ErrMessage)
 		}
-		if n.Status != nil {
+		if n.Engine != nil {
 			fmt.Printf("Daemon: %s\n", n.Version)
 			close(done)
 		}
 	})
 	go pump(ctx, bc, c)
 
-	bc.RequestStatus()
+	bc.RequestEngineStatus()
 	select {
 	case <-done:
 		return nil

ipn/backend.go

@@ -65,7 +65,6 @@ type Notify struct {
 	Prefs         *Prefs             // preferences were changed
 	NetMap        *netmap.NetworkMap // new netmap received
 	Engine        *EngineStatus      // wireguard engine stats
-	Status        *ipnstate.Status   // full status
 	BrowseToURL   *string            // UI should open a browser right now
 	BackendLogID  *string            // public logtail id used by backend
 	PingResult    *ipnstate.PingResult
@@ -159,9 +158,6 @@ type Backend interface {
 	// counts. Connection events are emitted automatically without
 	// polling.
 	RequestEngineStatus()
-	// RequestStatus requests that a full Status update
-	// notification is sent.
-	RequestStatus()
 	// FakeExpireAfter pretends that the current key is going to
 	// expire after duration x. This is useful for testing GUIs to
 	// make sure they react properly with keys that are going to

ipn/fake_test.go

@@ -87,10 +87,6 @@ func (b *FakeBackend) RequestEngineStatus() {
 	b.notify(Notify{Engine: &EngineStatus{}})
 }
 
-func (b *FakeBackend) RequestStatus() {
-	b.notify(Notify{Status: &ipnstate.Status{}})
-}
-
 func (b *FakeBackend) FakeExpireAfter(x time.Duration) {
 	b.notify(Notify{NetMap: &netmap.NetworkMap{}})
 }

ipn/handle.go

@@ -167,10 +167,6 @@ func (h *Handle) RequestEngineStatus() {
 	h.b.RequestEngineStatus()
 }
 
-func (h *Handle) RequestStatus() {
-	h.b.RequestStatus()
-}
-
 func (h *Handle) FakeExpireAfter(x time.Duration) {
 	h.b.FakeExpireAfter(x)
 }

ipn/ipnlocal/local.go

@@ -1617,12 +1617,6 @@ func (b *LocalBackend) RequestEngineStatus() {
 	b.e.RequestStatus()
 }
 
-// RequestStatus implements Backend.
-func (b *LocalBackend) RequestStatus() {
-	st := b.Status()
-	b.send(ipn.Notify{Status: st})
-}
-
 // stateMachine updates the state machine state based on other things
 // that have happened. It is invoked from the various callbacks that
 // feed events into LocalBackend.

ipn/localapi/localapi.go

@@ -56,6 +56,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		h.serveWhoIs(w, r)
 	case "/localapi/v0/goroutines":
 		h.serveGoroutines(w, r)
+	case "/localapi/v0/status":
+		h.serveStatus(w, r)
 	default:
 		io.WriteString(w, "tailscaled\n")
 	}
@@ -109,3 +111,14 @@ func (h *Handler) serveGoroutines(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/plain")
 	w.Write(buf)
 }
+
+func (h *Handler) serveStatus(w http.ResponseWriter, r *http.Request) {
+	if !h.PermitRead {
+		http.Error(w, "status access denied", http.StatusForbidden)
+		return
+	}
+	w.Header().Set("Content-Type", "application/json")
+	e := json.NewEncoder(w)
+	e.SetIndent("", "\t")
+	e.Encode(h.b.Status())
+}

ipn/message.go

@@ -173,9 +173,6 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
 	if c := cmd.RequestEngineStatus; c != nil {
 		bs.b.RequestEngineStatus()
 		return nil
-	} else if c := cmd.RequestStatus; c != nil {
-		bs.b.RequestStatus()
-		return nil
 	} else if c := cmd.Ping; c != nil {
 		bs.b.Ping(c.IP)
 		return nil