cmd/tailscale: surface authentication errors in status.Health (#4748)

Fixes #3713

Signed-off-by: Jordan Whited <jordan@tailscale.com>

cmd/tailscale/cli/status.go

@@ -128,12 +128,8 @@ func runStatus(ctx context.Context, args []string) error {
 		return err
 	}
 
-	description, ok := isRunningOrStarting(st)
-	if !ok {
-		outln(description)
-		os.Exit(1)
-	}
-
+	// print health check information prior to checking LocalBackend state as
+	// it may provide an explanation to the user if we choose to exit early
 	if len(st.Health) > 0 {
 		printf("# Health check:\n")
 		for _, m := range st.Health {
@@ -142,6 +138,12 @@ func runStatus(ctx context.Context, args []string) error {
 		outln()
 	}
 
+	description, ok := isRunningOrStarting(st)
+	if !ok {
+		outln(description)
+		os.Exit(1)
+	}
+
 	var buf bytes.Buffer
 	f := func(format string, a ...any) { fmt.Fprintf(&buf, format, a...) }
 	printPS := func(ps *ipnstate.PeerStatus) {

cmd/tailscale/cli/up.go

@@ -19,6 +19,7 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"time"
 
 	shellquote "github.com/kballard/go-shellquote"
 	"github.com/peterbourgon/ff/v3/ffcli"
@@ -114,7 +115,7 @@ func newUpFlagSet(goos string, upArgs *upArgsT) *flag.FlagSet {
 	case "windows":
 		upf.BoolVar(&upArgs.forceDaemon, "unattended", false, "run in \"Unattended Mode\" where Tailscale keeps running even after the current GUI user logs out (Windows-only)")
 	}
-
+	upf.DurationVar(&upArgs.timeout, "timeout", 0, "maximum amount of time to wait for tailscaled to enter a Running state; default (0s) blocks forever")
 	registerAcceptRiskFlag(upf)
 	return upf
 }
@@ -148,6 +149,7 @@ type upArgsT struct {
 	hostname               string
 	opUser                 string
 	json                   bool
+	timeout                time.Duration
 }
 
 func (a upArgsT) getAuthKey() (string, error) {
@@ -646,6 +648,12 @@ func runUp(ctx context.Context, args []string) error {
 	// need to prioritize reads from 'running' if it's
 	// readable; its send does happen before the pump mechanism
 	// shuts down. (Issue 2333)
+	var timeoutCh <-chan time.Time
+	if upArgs.timeout > 0 {
+		timeoutTimer := time.NewTimer(upArgs.timeout)
+		defer timeoutTimer.Stop()
+		timeoutCh = timeoutTimer.C
+	}
 	select {
 	case <-running:
 		return nil
@@ -663,6 +671,8 @@ func runUp(ctx context.Context, args []string) error {
 		default:
 		}
 		return err
+	case <-timeoutCh:
+		return errors.New(`timeout waiting for Tailscale service to enter a Running state; check health with "tailscale status"`)
 	}
 }
 
@@ -719,7 +729,7 @@ func addPrefFlagMapping(flagName string, prefNames ...string) {
 // correspond to an ipn.Pref.
 func preflessFlag(flagName string) bool {
 	switch flagName {
-	case "auth-key", "force-reauth", "reset", "qr", "json", "accept-risk":
+	case "auth-key", "force-reauth", "reset", "qr", "json", "timeout", "accept-risk":
 		return true
 	}
 	return false

control/controlclient/auto.go

@@ -289,6 +289,7 @@ func (c *Auto) authRoutine() {
 		}
 
 		if goal == nil {
+			health.SetAuthRoutineInError(nil)
 			// Wait for user to Login or Logout.
 			<-ctx.Done()
 			c.logf("[v1] authRoutine: context done.")
@@ -296,6 +297,7 @@ func (c *Auto) authRoutine() {
 		}
 
 		if !goal.wantLoggedIn {
+			health.SetAuthRoutineInError(nil)
 			err := c.direct.TryLogout(ctx)
 			goal.sendLogoutError(err)
 			if err != nil {
@@ -334,6 +336,7 @@ func (c *Auto) authRoutine() {
 				f = "TryLogin"
 			}
 			if err != nil {
+				health.SetAuthRoutineInError(err)
 				report(err, f)
 				bo.BackOff(ctx, err)
 				continue
@@ -358,6 +361,7 @@ func (c *Auto) authRoutine() {
 			}
 
 			// success
+			health.SetAuthRoutineInError(nil)
 			c.mu.Lock()
 			c.loggedIn = true
 			c.loginGoal = nil

health/health.go

@@ -45,6 +45,7 @@ var (
 	anyInterfaceUp          = true // until told otherwise
 	udp4Unbound             bool
 	controlHealth           []string
+	lastLoginErr            error
 )
 
 // Subsystem is the name of a subsystem whose health can be monitored.
@@ -287,6 +288,15 @@ func SetUDP4Unbound(unbound bool) {
 	selfCheckLocked()
 }
 
+// SetAuthRoutineInError records the latest error encountered as a result of a
+// login attempt. Providing a nil error indicates successful login, or that
+// being logged in w/coordination is not currently desired.
+func SetAuthRoutineInError(err error) {
+	mu.Lock()
+	defer mu.Unlock()
+	lastLoginErr = err
+}
+
 func timerSelfCheck() {
 	mu.Lock()
 	defer mu.Unlock()
@@ -321,9 +331,12 @@ func overallErrorLocked() error {
 	if !anyInterfaceUp {
 		return errors.New("network down")
 	}
-	if ipnState != "Running" || !ipnWantRunning {
+	if !ipnWantRunning {
 		return fmt.Errorf("state=%v, wantRunning=%v", ipnState, ipnWantRunning)
 	}
+	if lastLoginErr != nil {
+		return fmt.Errorf("not logged in, last login error=%v", lastLoginErr)
+	}
 	now := time.Now()
 	if !inMapPoll && (lastMapPollEndedAt.IsZero() || now.Sub(lastMapPollEndedAt) > 10*time.Second) {
 		return errors.New("not in map poll")

ipn/ipnlocal/local.go

@@ -2680,12 +2680,14 @@ func (b *LocalBackend) enterState(newState ipn.State) {
 	b.maybePauseControlClientLocked()
 	b.mu.Unlock()
 
+	// prefs may change irrespective of state; WantRunning should be explicitly
+	// set before potential early return even if the state is unchanged.
+	health.SetIPNState(newState.String(), prefs.WantRunning)
 	if oldState == newState {
 		return
 	}
 	b.logf("Switching ipn state %v -> %v (WantRunning=%v, nm=%v)",
 		oldState, newState, prefs.WantRunning, netMap != nil)
-	health.SetIPNState(newState.String(), prefs.WantRunning)
 	b.send(ipn.Notify{State: &newState})
 
 	switch newState {