cli/ : update up command to reset on blank string operator

Signed-off-by: soypete <miriah@tailscale.com>

cmd/tailscale/cli/cli_test.go

@@ -9,6 +9,7 @@
 	"encoding/json"
 	"flag"
 	"fmt"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -784,9 +785,10 @@ func TestUpdatePrefs(t *testing.T) {
 		curPrefs *ipn.Prefs
 		env      upCheckEnv // empty goos means "linux"
 
-		wantSimpleUp   bool
+		wantSimpleUp       bool
-		wantJustEditMP *ipn.MaskedPrefs
+		wantJustEditMP     *ipn.MaskedPrefs
-		wantErrSubtr   string
+		wantErrSubtr       string
+		wantOperatorChange bool
 	}{
 		{
 			name:  "bare_up_means_up",
@@ -846,6 +848,7 @@ func TestUpdatePrefs(t *testing.T) {
 				ShieldsUpSet:              true,
 				WantRunningSet:            true,
 			},
+			wantOperatorChange: true,
 		},
 		{
 			name:  "control_synonym",
@@ -885,6 +888,34 @@ func TestUpdatePrefs(t *testing.T) {
 			},
 			env: upCheckEnv{backendState: "Running"},
 		},
+		{
+			name:  "operator_user_force_blank",
+			flags: []string{"--operator="},
+			curPrefs: &ipn.Prefs{
+				ControlURL:   ipn.DefaultControlURL,
+				Persist:      &persist.Persist{LoginName: "crawshaw.github"},
+				OperatorUser: os.Getenv("USER"),
+			},
+			env: upCheckEnv{backendState: "Running"},
+			wantJustEditMP: &ipn.MaskedPrefs{
+				AdvertiseRoutesSet:        true,
+				AdvertiseTagsSet:          true,
+				AllowSingleHostsSet:       true,
+				ControlURLSet:             true,
+				CorpDNSSet:                true,
+				ExitNodeAllowLANAccessSet: true,
+				ExitNodeIDSet:             true,
+				ExitNodeIPSet:             true,
+				HostnameSet:               true,
+				NetfilterModeSet:          true,
+				NoSNATSet:                 true,
+				OperatorUserSet:           true,
+				RouteAllSet:               true,
+				RunSSHSet:                 true,
+				ShieldsUpSet:              true,
+				WantRunningSet:            true,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -915,6 +946,11 @@ func TestUpdatePrefs(t *testing.T) {
 			var oldEditPrefs ipn.Prefs
 			if justEditMP != nil {
 				oldEditPrefs = justEditMP.Prefs
+				if tt.wantOperatorChange {
+					if tt.curPrefs.OperatorUser == justEditMP.Prefs.OperatorUser {
+						t.Logf("current Operator User: %s, changes operator user: %s", tt.curPrefs.OperatorUser, justEditMP.OperatorUser)
+					}
+				}
 				justEditMP.Prefs = ipn.Prefs{} // uninteresting
 			}
 			if !reflect.DeepEqual(justEditMP, tt.wantJustEditMP) {
@@ -925,6 +961,95 @@ func TestUpdatePrefs(t *testing.T) {
 	}
 }
 
+// func TestOperatorEnv (t *testing.T){
+// 	tests := []struct {
+// 		name     string
+// 		flags    []string // argv to be parsed into env.flagSet and env.upArgs
+// 		curPrefs *ipn.Prefs
+// 		env      upCheckEnv // empty goos means "linux"
+
+// 		wantJustEditMP *ipn.MaskedPrefs
+// 		wantErrSubtr   string
+// 	}{
+// 		{
+// 			name:  "operator_user_ank",
+// 			flags: []string{"--operator="},
+// 			curPrefs: &ipn.Prefs{
+// 				ControlURL: ipn.DefaultControlURL,
+// 				Persist:    &persist.Persist{LoginName: "crawshaw.github"},
+// 			},
+// 		},
+// 		{
+// 			name:  "operator_user_force_blank",
+// 			flags: []string{"--operator="},
+// 			curPrefs: &ipn.Prefs{
+// 				ControlURL: ipn.DefaultControlURL,
+// 				Persist:    &persist.Persist{LoginName: "crawshaw.github"},
+// 			},
+// 		},
+// 		{
+// 			name:  "operator_user_reset",
+// 			flags: []string{"--reset"},
+// 			curPrefs: &ipn.Prefs{
+// 				ControlURL: ipn.DefaultControlURL,
+// 				Persist:    &persist.Persist{LoginName: "crawshaw.github"},
+// 			},
+// 			wantJustEditMP: &ipn.MaskedPrefs{
+// 				AdvertiseRoutesSet:        true,
+// 				AdvertiseTagsSet:          true,
+// 				AllowSingleHostsSet:       true,
+// 				ControlURLSet:             true,
+// 				CorpDNSSet:                true,
+// 				ExitNodeAllowLANAccessSet: true,
+// 				ExitNodeIDSet:             true,
+// 				ExitNodeIPSet:             true,
+// 				HostnameSet:               true,
+// 				NetfilterModeSet:          true,
+// 				NoSNATSet:                 true,
+// 				OperatorUserSet:           true,
+// 				RouteAllSet:               true,
+// 				RunSSHSet:                 true,
+// 				ShieldsUpSet:              true,
+// 				WantRunningSet:            true,
+// 			},
+// 		},
+// 	}
+// 	for _, tt := range tests {
+// 		t.Run(tt.name, func(t *testing.T) {
+// 			if tt.env.goos == "" {
+// 				tt.env.goos = "linux"
+// 			}
+// 			tt.env.flagSet = newUpFlagSet(tt.env.goos, &tt.env.upArgs)
+// 			flags := CleanUpArgs(tt.flags)
+// 			tt.env.flagSet.Parse(flags)
+
+// 			newPrefs, err := prefsFromUpArgs(tt.env.upArgs, t.Logf, new(ipnstate.Status), tt.env.goos)
+// 			if err != nil {
+// 				t.Fatal(err)
+// 			}
+// 		_, justEditMP, err := updatePrefs(newPrefs, tt.curPrefs, tt.env)
+// 			if err != nil {
+// 				if tt.wantErrSubtr != "" {
+// 					if !strings.Contains(err.Error(), tt.wantErrSubtr) {
+// 						t.Fatalf("want error %q, got: %v", tt.wantErrSubtr, err)
+// 					}
+// 					return
+// 				}
+// 				t.Fatal(err)
+// 			}
+// 			var oldEditPrefs ipn.Prefs
+// 			if justEditMP != nil {
+// 				oldEditPrefs = justEditMP.Prefs
+// 				justEditMP.Prefs = ipn.Prefs{} // uninteresting
+// 			}
+// 			if !reflect.DeepEqual(justEditMP, tt.wantJustEditMP) {
+// 				t.Logf("justEditMP != wantJustEditMP; following diff omits the Prefs field, which was %+v", oldEditPrefs)
+// 				t.Fatalf("justEditMP: %v\n\n: ", cmp.Diff(justEditMP, tt.wantJustEditMP, cmpIP))
+// 			}
+// 		})
+// 	}
+// }
+
 var cmpIP = cmp.Comparer(func(a, b netaddr.IP) bool {
 	return a == b
 })

cmd/tailscale/cli/up.go

@@ -106,7 +106,7 @@ func newUpFlagSet(goos string, upArgs *upArgsT) *flag.FlagSet {
 	upf.StringVar(&upArgs.advertiseRoutes, "advertise-routes", "", "routes to advertise to other nodes (comma-separated, e.g. \"10.0.0.0/8,192.168.0.0/24\") or empty string to not advertise routes")
 	upf.BoolVar(&upArgs.advertiseDefaultRoute, "advertise-exit-node", false, "offer to be an exit node for internet traffic for the tailnet")
 	if safesocket.GOOSUsesPeerCreds(goos) {
-		upf.StringVar(&upArgs.opUser, "operator", "", "Unix username to allow to operate on tailscaled without sudo")
+		upf.StringVar(&upArgs.opUser, "operator", os.Getenv("USER"), "Unix username to allow to operate on tailscaled without sudo")
 	}
 	switch goos {
 	case "linux":
@@ -362,7 +362,7 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo
 // without changing any settings.
 func updatePrefs(prefs, curPrefs *ipn.Prefs, env upCheckEnv) (simpleUp bool, justEditMP *ipn.MaskedPrefs, err error) {
 	if !env.upArgs.reset {
-		applyImplicitPrefs(prefs, curPrefs, env.user)
+		// applyImplicitPrefs(prefs, curPrefs, env.user)
 
 		if err := checkForAccidentalSettingReverts(prefs, curPrefs, env); err != nil {
 			return false, nil, err
@@ -857,7 +857,7 @@ type isBool interface {
 // this is just the operator user, which only needs to be set if it doesn't
 // match the current user.
 //
-// curUser is os.Getenv("USER"). It's pulled out for testability.
+// curUser is os.getenv("user"). It's pulled out for testability.
 func applyImplicitPrefs(prefs, oldPrefs *ipn.Prefs, curUser string) {
 	if prefs.OperatorUser == "" && oldPrefs.OperatorUser == curUser {
 		prefs.OperatorUser = oldPrefs.OperatorUser