cmd/tailscale: make up respect explicitly empty --operator= value

Fixes #3808

Signed-off-by: soypete <miriah@tailscale.com>
This commit is contained in:
soypete 2022-06-21 16:51:33 -06:00 committed by Miriah Peterson
parent c434e47f2d
commit 412c4c55e2
2 changed files with 47 additions and 9 deletions

View File

@ -493,14 +493,16 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
if err != nil {
t.Fatal(err)
}
applyImplicitPrefs(newPrefs, tt.curPrefs, tt.curUser)
var got string
if err := checkForAccidentalSettingReverts(newPrefs, tt.curPrefs, upCheckEnv{
upEnv := upCheckEnv{
goos: goos,
flagSet: flagSet,
curExitNodeIP: tt.curExitNodeIP,
distro: tt.distro,
}); err != nil {
user: tt.curUser,
}
applyImplicitPrefs(newPrefs, tt.curPrefs, upEnv)
var got string
if err := checkForAccidentalSettingReverts(newPrefs, tt.curPrefs, upEnv); err != nil {
got = err.Error()
}
if strings.TrimSpace(got) != tt.want {
@ -784,6 +786,10 @@ func TestUpdatePrefs(t *testing.T) {
curPrefs *ipn.Prefs
env upCheckEnv // empty goos means "linux"
// checkUpdatePrefsMutations, if non-nil, is run with the new prefs after
// updatePrefs might've mutated them (from applyImplicitPrefs).
checkUpdatePrefsMutations func(t *testing.T, newPrefs *ipn.Prefs)
wantSimpleUp bool
wantJustEditMP *ipn.MaskedPrefs
wantErrSubtr string
@ -885,6 +891,28 @@ func TestUpdatePrefs(t *testing.T) {
},
env: upCheckEnv{backendState: "Running"},
},
{
// Issue 3808: explicitly empty --operator= should clear value.
name: "explicit_empty_operator",
flags: []string{"--operator="},
curPrefs: &ipn.Prefs{
ControlURL: "https://login.tailscale.com",
CorpDNS: true,
AllowSingleHosts: true,
NetfilterMode: preftype.NetfilterOn,
OperatorUser: "somebody",
},
env: upCheckEnv{user: "somebody", backendState: "Running"},
wantJustEditMP: &ipn.MaskedPrefs{
OperatorUserSet: true,
WantRunningSet: true,
},
checkUpdatePrefsMutations: func(t *testing.T, prefs *ipn.Prefs) {
if prefs.OperatorUser != "" {
t.Errorf("operator sent to backend should be empty; got %q", prefs.OperatorUser)
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -909,6 +937,9 @@ func TestUpdatePrefs(t *testing.T) {
}
t.Fatal(err)
}
if tt.checkUpdatePrefsMutations != nil {
tt.checkUpdatePrefsMutations(t, newPrefs)
}
if simpleUp != tt.wantSimpleUp {
t.Fatalf("simpleUp=%v, want %v", simpleUp, tt.wantSimpleUp)
}

View File

@ -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)
if err := checkForAccidentalSettingReverts(prefs, curPrefs, env); err != nil {
return false, nil, err
@ -881,13 +881,20 @@ type isBool interface {
return errors.New(sb.String())
}
// applyImplicitPrefs mutates prefs to add implicit preferences. Currently
// this is just the operator user, which only needs to be set if it doesn't
// applyImplicitPrefs mutates prefs to add implicit preferences for the user operator.
// If the operator flag is passed no action is taken, otherwise this only needs to be set if it doesn't
// match the current user.
//
// 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 {
func applyImplicitPrefs(prefs, oldPrefs *ipn.Prefs, env upCheckEnv) {
explicitOperator := false
env.flagSet.Visit(func(f *flag.Flag) {
if f.Name == "operator" {
explicitOperator = true
}
})
if prefs.OperatorUser == "" && oldPrefs.OperatorUser == env.user && !explicitOperator {
prefs.OperatorUser = oldPrefs.OperatorUser
}
}