From 6caa02428ef8d84576c14965e9b84b52fa023650 Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Fri, 30 Apr 2021 15:36:52 -0400 Subject: [PATCH] cmd/tailscale/cli/up: "LoggedOut" pref is implicit. There's no need to warn that it was not provided on the command line after doing a sequence of up; logout; up --args. If you're asking for tailscale to be up, you always mean that you prefer LoggedOut to become false. Fixes #1828 Signed-off-by: Avery Pennarun --- cmd/tailscale/cli/cli_test.go | 19 +++++++++++++++++++ cmd/tailscale/cli/up.go | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index fbfa4cffc..9f43d2fad 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -348,6 +348,25 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) { }, want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice", }, + { + name: "loggedout_is_implicit", + flagSet: f("advertise-exit-node"), + curPrefs: &ipn.Prefs{ + ControlURL: ipn.DefaultControlURL, + LoggedOut: true, + }, + mp: &ipn.MaskedPrefs{ + Prefs: ipn.Prefs{ + ControlURL: ipn.DefaultControlURL, + AdvertiseRoutes: []netaddr.IPPrefix{ + netaddr.MustParseIPPrefix("0.0.0.0/0"), + }, + }, + AdvertiseRoutesSet: true, + }, + // not an error. LoggedOut is implicit. + want: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 787711052..c9556e192 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -552,9 +552,17 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre flagExplicitValue := map[string]interface{}{} // e.g. "accept-dns" => true (from flagSet) for i := 0; i < prefType.NumField(); i++ { prefName := prefType.Field(i).Name + // Persist is a legacy field used for storing keys, which + // probably should never have been part of Prefs. It's + // likely to migrate elsewhere eventually. if prefName == "Persist" { continue } + // LoggedOut is a preference, but running the "up" command + // always implies that the user now prefers LoggedOut->false. + if prefName == "LoggedOut" { + continue + } flagName, hasFlag := flagForPref[prefName] // Special case for advertise-exit-node; which is a