2021-04-10 21:56:18 -07:00
|
|
|
// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved.
|
|
|
|
// Use of this source code is governed by a BSD-style
|
|
|
|
// license that can be found in the LICENSE file.
|
|
|
|
|
|
|
|
package cli
|
|
|
|
|
|
|
|
import (
|
2021-04-17 20:35:20 -07:00
|
|
|
"bytes"
|
|
|
|
"encoding/json"
|
2021-04-10 21:56:18 -07:00
|
|
|
"flag"
|
2021-04-17 20:35:20 -07:00
|
|
|
"fmt"
|
2021-04-30 13:29:06 -07:00
|
|
|
"reflect"
|
2021-04-17 20:35:20 -07:00
|
|
|
"strings"
|
2021-04-10 21:56:18 -07:00
|
|
|
"testing"
|
|
|
|
|
2021-04-17 19:14:59 -07:00
|
|
|
"inet.af/netaddr"
|
2021-04-10 21:56:18 -07:00
|
|
|
"tailscale.com/ipn"
|
2021-04-17 20:35:20 -07:00
|
|
|
"tailscale.com/ipn/ipnstate"
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
"tailscale.com/types/logger"
|
2021-04-17 20:35:20 -07:00
|
|
|
"tailscale.com/types/preftype"
|
2021-04-10 21:56:18 -07:00
|
|
|
)
|
|
|
|
|
|
|
|
// Test that checkForAccidentalSettingReverts's updateMaskedPrefsFromUpFlag can handle
|
|
|
|
// all flags. This will panic if a new flag creeps in that's unhandled.
|
|
|
|
func TestUpdateMaskedPrefsFromUpFlag(t *testing.T) {
|
|
|
|
mp := new(ipn.MaskedPrefs)
|
|
|
|
upFlagSet.VisitAll(func(f *flag.Flag) {
|
|
|
|
updateMaskedPrefsFromUpFlag(mp, f.Name)
|
|
|
|
})
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestCheckForAccidentalSettingReverts(t *testing.T) {
|
|
|
|
f := func(flags ...string) map[string]bool {
|
|
|
|
m := make(map[string]bool)
|
|
|
|
for _, f := range flags {
|
|
|
|
m[f] = true
|
|
|
|
}
|
|
|
|
return m
|
|
|
|
}
|
|
|
|
tests := []struct {
|
2021-05-06 15:27:02 -07:00
|
|
|
name string
|
|
|
|
|
|
|
|
// flags, if non-nil populates flagSet and mp.
|
|
|
|
flags []string // if non-nil,
|
|
|
|
|
|
|
|
// flagSet and mp are required if flags is nil.
|
|
|
|
// flagSet is the set of flags that were explicitly set in flags.
|
|
|
|
// mp is the mutation to apply to server.
|
|
|
|
flagSet map[string]bool
|
|
|
|
mp *ipn.MaskedPrefs
|
|
|
|
|
2021-04-10 21:56:18 -07:00
|
|
|
curPrefs *ipn.Prefs
|
2021-04-21 15:47:59 -07:00
|
|
|
curUser string // os.Getenv("USER") on the client side
|
2021-05-02 15:35:55 -07:00
|
|
|
goos string // empty means "linux"
|
2021-04-10 21:56:18 -07:00
|
|
|
want string
|
|
|
|
}{
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "bare_up_means_up",
|
|
|
|
flags: []string{},
|
2021-04-10 21:56:18 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunning: false,
|
|
|
|
Hostname: "foo",
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "losing_hostname",
|
|
|
|
flags: []string{"--accept-dns"},
|
2021-04-10 21:56:18 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
WantRunning: false,
|
|
|
|
Hostname: "foo",
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
|
|
|
AllowSingleHosts: true,
|
2021-04-10 21:56:18 -07:00
|
|
|
},
|
2021-04-26 14:39:49 -07:00
|
|
|
want: accidentalUpPrefix + " --accept-dns --hostname=foo",
|
2021-04-10 21:56:18 -07:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "hostname_changing_explicitly",
|
|
|
|
flagSet: f("hostname"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunning: false,
|
|
|
|
Hostname: "foo",
|
|
|
|
},
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunning: true,
|
|
|
|
Hostname: "bar",
|
|
|
|
},
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURLSet: true,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunningSet: true,
|
|
|
|
HostnameSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "hostname_changing_empty_explicitly",
|
|
|
|
flagSet: f("hostname"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunning: false,
|
|
|
|
Hostname: "foo",
|
|
|
|
},
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunning: true,
|
|
|
|
Hostname: "",
|
|
|
|
},
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURLSet: true,
|
2021-04-10 21:56:18 -07:00
|
|
|
WantRunningSet: true,
|
|
|
|
HostnameSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
2021-04-17 19:14:59 -07:00
|
|
|
{
|
|
|
|
name: "empty_slice_equals_nil_slice",
|
|
|
|
flagSet: f("hostname"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-17 19:14:59 -07:00
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{},
|
|
|
|
},
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
2021-04-17 19:14:59 -07:00
|
|
|
AdvertiseRoutes: nil,
|
|
|
|
},
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
ControlURLSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
// Issue 1725: "tailscale up --authkey=..." (or other non-empty flags) works from
|
|
|
|
// a fresh server's initial prefs.
|
|
|
|
name: "up_with_default_prefs",
|
|
|
|
flagSet: f("authkey"),
|
|
|
|
curPrefs: ipn.NewPrefs(),
|
|
|
|
mp: &ipn.MaskedPrefs{
|
2021-05-03 09:23:01 -07:00
|
|
|
Prefs: *defaultPrefsFromUpArgs(t, "linux"),
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
WantRunningSet: true,
|
2021-04-17 19:14:59 -07:00
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
2021-04-21 15:47:59 -07:00
|
|
|
{
|
|
|
|
name: "implicit_operator_change",
|
|
|
|
flagSet: f("hostname"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
OperatorUser: "alice",
|
|
|
|
},
|
|
|
|
curUser: "eve",
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
},
|
|
|
|
ControlURLSet: true,
|
|
|
|
},
|
2021-04-26 14:39:49 -07:00
|
|
|
want: accidentalUpPrefix + " --hostname= --operator=alice",
|
2021-04-21 15:47:59 -07:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "implicit_operator_matches_shell_user",
|
|
|
|
flagSet: f("hostname"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
OperatorUser: "alice",
|
|
|
|
},
|
|
|
|
curUser: "alice",
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
},
|
|
|
|
ControlURLSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
2021-04-21 21:41:13 -07:00
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "error_advertised_routes_exit_node_removed",
|
|
|
|
flags: []string{"--advertise-routes=10.0.42.0/24"},
|
2021-04-21 21:41:13 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-04-21 21:41:13 -07:00
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("10.0.42.0/24"),
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
},
|
|
|
|
},
|
2021-04-26 14:39:49 -07:00
|
|
|
want: accidentalUpPrefix + " --advertise-routes=10.0.42.0/24 --advertise-exit-node",
|
2021-04-21 21:41:13 -07:00
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "advertised_routes_exit_node_removed",
|
|
|
|
flags: []string{"--advertise-routes=10.0.42.0/24", "--advertise-exit-node=false"},
|
2021-04-21 21:41:13 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-04-21 21:41:13 -07:00
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("10.0.42.0/24"),
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "advertised_routes_includes_the_0_routes", // but no --advertise-exit-node
|
|
|
|
flagSet: f("advertise-routes"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("10.0.42.0/24"),
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("11.1.43.0/24"),
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
AdvertiseRoutesSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "advertise_exit_node", // Issue 1859
|
|
|
|
flags: []string{"--advertise-exit-node"},
|
2021-05-05 20:17:23 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-05-05 20:17:23 -07:00
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "advertise_exit_node_over_existing_routes",
|
|
|
|
flags: []string{"--advertise-exit-node"},
|
2021-05-05 20:17:23 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-05-05 20:17:23 -07:00
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("1.2.0.0/16"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16",
|
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "advertise_exit_node_over_existing_routes_and_exit_node",
|
|
|
|
flags: []string{"--advertise-exit-node"},
|
2021-05-05 20:17:23 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
2021-05-06 15:27:02 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-05-05 20:17:23 -07:00
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
netaddr.MustParseIPPrefix("1.2.0.0/16"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: accidentalUpPrefix + " --advertise-exit-node --advertise-routes=1.2.0.0/16",
|
|
|
|
},
|
2021-04-23 10:56:08 -07:00
|
|
|
{
|
|
|
|
name: "exit_node_clearing", // Issue 1777
|
|
|
|
flagSet: f("exit-node"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
ExitNodeID: "fooID",
|
|
|
|
},
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
ExitNodeIP: netaddr.IP{},
|
|
|
|
},
|
|
|
|
ExitNodeIPSet: true,
|
|
|
|
},
|
|
|
|
want: "",
|
|
|
|
},
|
2021-04-26 14:39:49 -07:00
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "remove_all_implicit",
|
|
|
|
flags: []string{"--force-reauth"},
|
2021-04-26 14:39:49 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
WantRunning: true,
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
RouteAll: true,
|
|
|
|
AllowSingleHosts: false,
|
|
|
|
ExitNodeIP: netaddr.MustParseIP("100.64.5.6"),
|
2021-05-06 15:27:02 -07:00
|
|
|
CorpDNS: false,
|
2021-04-26 14:39:49 -07:00
|
|
|
ShieldsUp: true,
|
|
|
|
AdvertiseTags: []string{"tag:foo", "tag:bar"},
|
|
|
|
Hostname: "myhostname",
|
|
|
|
ForceDaemon: true,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("10.0.0.0/16"),
|
|
|
|
},
|
|
|
|
NetfilterMode: preftype.NetfilterNoDivert,
|
|
|
|
OperatorUser: "alice",
|
|
|
|
},
|
|
|
|
curUser: "eve",
|
2021-05-06 15:27:02 -07:00
|
|
|
want: accidentalUpPrefix + " --force-reauth --accept-routes --host-routes=false --exit-node=100.64.5.6 --accept-dns=false --shields-up --advertise-tags=tag:foo,tag:bar --hostname=myhostname --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
|
2021-04-26 14:39:49 -07:00
|
|
|
},
|
|
|
|
{
|
2021-05-06 15:27:02 -07:00
|
|
|
name: "remove_all_implicit_except_hostname",
|
|
|
|
flags: []string{"--hostname=newhostname"},
|
2021-04-26 14:39:49 -07:00
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
WantRunning: true,
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
RouteAll: true,
|
|
|
|
AllowSingleHosts: false,
|
|
|
|
ExitNodeIP: netaddr.MustParseIP("100.64.5.6"),
|
2021-05-06 15:27:02 -07:00
|
|
|
CorpDNS: false,
|
2021-04-26 14:39:49 -07:00
|
|
|
ShieldsUp: true,
|
|
|
|
AdvertiseTags: []string{"tag:foo", "tag:bar"},
|
|
|
|
Hostname: "myhostname",
|
|
|
|
ForceDaemon: true,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("10.0.0.0/16"),
|
|
|
|
},
|
|
|
|
NetfilterMode: preftype.NetfilterNoDivert,
|
|
|
|
OperatorUser: "alice",
|
|
|
|
},
|
|
|
|
curUser: "eve",
|
2021-05-06 15:27:02 -07:00
|
|
|
want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --host-routes=false --exit-node=100.64.5.6 --accept-dns=false --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
|
2021-04-26 14:39:49 -07:00
|
|
|
},
|
2021-04-30 15:36:52 -04:00
|
|
|
{
|
|
|
|
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: "",
|
|
|
|
},
|
2021-05-02 15:35:55 -07:00
|
|
|
{
|
|
|
|
// Test that a pre-1.8 version of Tailscale with bogus NoSNAT pref
|
|
|
|
// values is able to enable exit nodes without warnings.
|
|
|
|
name: "make_windows_exit_node",
|
|
|
|
flagSet: f("advertise-exit-node"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
NoSNAT: true, // assume this no-op accidental pre-1.8 value
|
|
|
|
},
|
|
|
|
goos: "windows",
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("192.168.0.0/16"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
AdvertiseRoutesSet: true,
|
|
|
|
},
|
|
|
|
want: "", // not an error
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "ignore_netfilter_change_non_linux",
|
|
|
|
flagSet: f("accept-dns"),
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
NetfilterMode: preftype.NetfilterNoDivert, // we never had this bug, but pretend it got set non-zero on Windows somehow
|
|
|
|
},
|
|
|
|
goos: "windows",
|
|
|
|
mp: &ipn.MaskedPrefs{
|
|
|
|
Prefs: ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
CorpDNS: false,
|
|
|
|
},
|
|
|
|
CorpDNSSet: true,
|
|
|
|
},
|
|
|
|
want: "", // not an error
|
|
|
|
},
|
2021-05-06 15:27:02 -07:00
|
|
|
{
|
|
|
|
name: "operator_losing_routes_step1", // https://twitter.com/EXPbits/status/1390418145047887877
|
|
|
|
flags: []string{"--operator=expbits"},
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
netaddr.MustParseIPPrefix("1.2.0.0/16"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: accidentalUpPrefix + " --operator=expbits --advertise-routes=1.2.0.0/16 --advertise-exit-node",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "operator_losing_routes_step2", // https://twitter.com/EXPbits/status/1390418145047887877
|
|
|
|
flags: []string{"--operator=expbits", "--advertise-routes=1.2.0.0/16"},
|
|
|
|
curPrefs: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
netaddr.MustParseIPPrefix("1.2.0.0/16"),
|
|
|
|
},
|
|
|
|
},
|
|
|
|
want: accidentalUpPrefix + " --advertise-routes=1.2.0.0/16 --operator=expbits --advertise-exit-node",
|
|
|
|
},
|
2021-04-10 21:56:18 -07:00
|
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
|
|
t.Run(tt.name, func(t *testing.T) {
|
2021-05-02 15:35:55 -07:00
|
|
|
goos := "linux"
|
|
|
|
if tt.goos != "" {
|
|
|
|
goos = tt.goos
|
|
|
|
}
|
2021-05-06 15:27:02 -07:00
|
|
|
flagSet := tt.flagSet
|
|
|
|
mp := tt.mp
|
|
|
|
if tt.flags != nil {
|
|
|
|
if tt.flagSet != nil || tt.mp != nil {
|
|
|
|
t.Fatal("flags and flagSet/mp are mutually exclusive")
|
|
|
|
}
|
|
|
|
var upArgs upArgsT
|
|
|
|
fs := newUpFlagSet(goos, &upArgs)
|
|
|
|
fs.Parse(tt.flags)
|
|
|
|
prefs, err := prefsFromUpArgs(upArgs, t.Logf, new(ipnstate.Status), goos)
|
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
|
|
|
flagSet, mp = flagSetAndMaskedPrefs(prefs, fs)
|
|
|
|
}
|
2021-04-10 21:56:18 -07:00
|
|
|
var got string
|
2021-05-06 15:27:02 -07:00
|
|
|
if err := checkForAccidentalSettingReverts(flagSet, tt.curPrefs, mp, goos, tt.curUser); err != nil {
|
2021-04-10 21:56:18 -07:00
|
|
|
got = err.Error()
|
|
|
|
}
|
2021-04-26 14:39:49 -07:00
|
|
|
if strings.TrimSpace(got) != tt.want {
|
2021-04-10 21:56:18 -07:00
|
|
|
t.Errorf("unexpected result\n got: %s\nwant: %s\n", got, tt.want)
|
|
|
|
}
|
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
2021-04-17 20:35:20 -07:00
|
|
|
|
2021-05-03 09:23:01 -07:00
|
|
|
func defaultPrefsFromUpArgs(t testing.TB, goos string) *ipn.Prefs {
|
2021-05-05 20:17:23 -07:00
|
|
|
upArgs := upArgsFromOSArgs(goos)
|
ipn{,/ipnlocal}, cmd/tailscale/cli: don't check pref reverts on initial up
The ipn.NewPrefs func returns a populated ipn.Prefs for historical
reasons. It's not used or as important as it once was, but it hasn't
yet been removed. Meanwhile, it contains some default values that are
used on some platforms. Notably, for this bug (#1725), Windows/Mac use
its Prefs.RouteAll true value (to accept subnets), but Linux users
have always gotten a "false" value for that, because that's what
cmd/tailscale's CLI default flag is _for all operating systems_. That
meant that "tailscale up" was rightfully reporting that the user was
changing an implicit setting: RouteAll was changing from true with
false with the user explicitly saying so.
An obvious fix might be to change ipn.NewPrefs to return
Prefs.RouteAll == false on some platforms, but the logic is
complicated by darwin: we want RouteAll true on windows, android, ios,
and the GUI mac app, but not the CLI tailscaled-on-macOS mode. But
even if we used build tags (e.g. the "redo" build tag) to determine
what the default is, that then means we have duplicated and differing
"defaults" between both the CLI up flags and ipn.NewPrefs. Furthering
that complication didn't seem like a good idea.
So, changing the NewPrefs defaults is too invasive at this stage of
the release, as is removing the NewPrefs func entirely.
Instead, tweak slightly the semantics of the ipn.Prefs.ControlURL
field. This now defines that a ControlURL of the empty string means
both "we're uninitialized" and also "just use the default".
Then, once we have the "empty-string-means-unintialized" semantics,
use that to suppress "tailscale up"'s recent implicit-setting-revert
checking safety net, if we've never initialized Tailscale yet.
And update/add tests.
Fixes #1725
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-04-17 22:50:58 -07:00
|
|
|
prefs, err := prefsFromUpArgs(upArgs, logger.Discard, new(ipnstate.Status), "linux")
|
|
|
|
if err != nil {
|
|
|
|
t.Fatalf("defaultPrefsFromUpArgs: %v", err)
|
|
|
|
}
|
|
|
|
prefs.WantRunning = true
|
|
|
|
return prefs
|
|
|
|
}
|
|
|
|
|
2021-05-05 20:17:23 -07:00
|
|
|
func upArgsFromOSArgs(goos string, flagArgs ...string) (args upArgsT) {
|
2021-05-03 09:23:01 -07:00
|
|
|
fs := newUpFlagSet(goos, &args)
|
2021-05-05 20:17:23 -07:00
|
|
|
fs.Parse(flagArgs) // populates args
|
2021-05-03 09:23:01 -07:00
|
|
|
return
|
|
|
|
}
|
|
|
|
|
2021-04-17 20:35:20 -07:00
|
|
|
func TestPrefsFromUpArgs(t *testing.T) {
|
|
|
|
tests := []struct {
|
|
|
|
name string
|
|
|
|
args upArgsT
|
|
|
|
goos string // runtime.GOOS; empty means linux
|
|
|
|
st *ipnstate.Status // or nil
|
|
|
|
want *ipn.Prefs
|
|
|
|
wantErr string
|
|
|
|
wantWarn string
|
|
|
|
}{
|
|
|
|
{
|
2021-05-03 09:23:01 -07:00
|
|
|
name: "default_linux",
|
|
|
|
goos: "linux",
|
2021-05-05 20:17:23 -07:00
|
|
|
args: upArgsFromOSArgs("linux"),
|
2021-05-03 09:23:01 -07:00
|
|
|
want: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
WantRunning: true,
|
|
|
|
NoSNAT: false,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
|
|
|
CorpDNS: true,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "default_windows",
|
2021-04-17 20:35:20 -07:00
|
|
|
goos: "windows",
|
2021-05-05 20:17:23 -07:00
|
|
|
args: upArgsFromOSArgs("windows"),
|
2021-04-17 20:35:20 -07:00
|
|
|
want: &ipn.Prefs{
|
2021-05-03 09:23:01 -07:00
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
WantRunning: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
2021-04-17 20:35:20 -07:00
|
|
|
},
|
|
|
|
},
|
2021-05-05 20:17:23 -07:00
|
|
|
{
|
|
|
|
name: "advertise_default_route",
|
|
|
|
args: upArgsFromOSArgs("linux", "--advertise-exit-node"),
|
|
|
|
want: &ipn.Prefs{
|
|
|
|
ControlURL: ipn.DefaultControlURL,
|
|
|
|
WantRunning: true,
|
|
|
|
AllowSingleHosts: true,
|
|
|
|
CorpDNS: true,
|
|
|
|
AdvertiseRoutes: []netaddr.IPPrefix{
|
|
|
|
netaddr.MustParseIPPrefix("0.0.0.0/0"),
|
|
|
|
netaddr.MustParseIPPrefix("::/0"),
|
|
|
|
},
|
|
|
|
NetfilterMode: preftype.NetfilterOn,
|
|
|
|
},
|
|
|
|
},
|
2021-04-17 20:35:20 -07:00
|
|
|
{
|
|
|
|
name: "error_advertise_route_invalid_ip",
|
|
|
|
args: upArgsT{
|
|
|
|
advertiseRoutes: "foo",
|
|
|
|
},
|
|
|
|
wantErr: `"foo" is not a valid IP address or CIDR prefix`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_advertise_route_unmasked_bits",
|
|
|
|
args: upArgsT{
|
|
|
|
advertiseRoutes: "1.2.3.4/16",
|
|
|
|
},
|
|
|
|
wantErr: `1.2.3.4/16 has non-address bits set; expected 1.2.0.0/16`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_exit_node_bad_ip",
|
|
|
|
args: upArgsT{
|
|
|
|
exitNodeIP: "foo",
|
|
|
|
},
|
|
|
|
wantErr: `invalid IP address "foo" for --exit-node: unable to parse IP`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_exit_node_allow_lan_without_exit_node",
|
|
|
|
args: upArgsT{
|
|
|
|
exitNodeAllowLANAccess: true,
|
|
|
|
},
|
|
|
|
wantErr: `--exit-node-allow-lan-access can only be used with --exit-node`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_tag_prefix",
|
|
|
|
args: upArgsT{
|
|
|
|
advertiseTags: "foo",
|
|
|
|
},
|
|
|
|
wantErr: `tag: "foo": tags must start with 'tag:'`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_long_hostname",
|
|
|
|
args: upArgsT{
|
|
|
|
hostname: strings.Repeat("a", 300),
|
|
|
|
},
|
|
|
|
wantErr: `hostname too long: 300 bytes (max 256)`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_linux_netfilter_empty",
|
|
|
|
args: upArgsT{
|
|
|
|
netfilterMode: "",
|
|
|
|
},
|
|
|
|
wantErr: `invalid value --netfilter-mode=""`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_linux_netfilter_bogus",
|
|
|
|
args: upArgsT{
|
|
|
|
netfilterMode: "bogus",
|
|
|
|
},
|
|
|
|
wantErr: `invalid value --netfilter-mode="bogus"`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "error_exit_node_ip_is_self_ip",
|
|
|
|
args: upArgsT{
|
|
|
|
exitNodeIP: "100.105.106.107",
|
|
|
|
},
|
|
|
|
st: &ipnstate.Status{
|
|
|
|
TailscaleIPs: []netaddr.IP{netaddr.MustParseIP("100.105.106.107")},
|
|
|
|
},
|
|
|
|
wantErr: `cannot use 100.105.106.107 as the exit node as it is a local IP address to this machine, did you mean --advertise-exit-node?`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "warn_linux_netfilter_nodivert",
|
|
|
|
goos: "linux",
|
|
|
|
args: upArgsT{
|
|
|
|
netfilterMode: "nodivert",
|
|
|
|
},
|
|
|
|
wantWarn: "netfilter=nodivert; add iptables calls to ts-* chains manually.",
|
|
|
|
want: &ipn.Prefs{
|
|
|
|
WantRunning: true,
|
|
|
|
NetfilterMode: preftype.NetfilterNoDivert,
|
|
|
|
NoSNAT: true,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "warn_linux_netfilter_off",
|
|
|
|
goos: "linux",
|
|
|
|
args: upArgsT{
|
|
|
|
netfilterMode: "off",
|
|
|
|
},
|
|
|
|
wantWarn: "netfilter=off; configure iptables yourself.",
|
|
|
|
want: &ipn.Prefs{
|
|
|
|
WantRunning: true,
|
|
|
|
NetfilterMode: preftype.NetfilterOff,
|
|
|
|
NoSNAT: true,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
|
|
var warnBuf bytes.Buffer
|
|
|
|
warnf := func(format string, a ...interface{}) {
|
|
|
|
fmt.Fprintf(&warnBuf, format, a...)
|
|
|
|
}
|
|
|
|
goos := tt.goos
|
|
|
|
if goos == "" {
|
|
|
|
goos = "linux"
|
|
|
|
}
|
|
|
|
st := tt.st
|
|
|
|
if st == nil {
|
|
|
|
st = new(ipnstate.Status)
|
|
|
|
}
|
|
|
|
got, err := prefsFromUpArgs(tt.args, warnf, st, goos)
|
|
|
|
gotErr := fmt.Sprint(err)
|
|
|
|
if tt.wantErr != "" {
|
|
|
|
if tt.wantErr != gotErr {
|
|
|
|
t.Errorf("wrong error.\n got error: %v\nwant error: %v\n", gotErr, tt.wantErr)
|
|
|
|
}
|
|
|
|
return
|
|
|
|
}
|
|
|
|
if err != nil {
|
|
|
|
t.Fatal(err)
|
|
|
|
}
|
|
|
|
if tt.want == nil {
|
|
|
|
t.Fatal("tt.want is nil")
|
|
|
|
}
|
|
|
|
if !got.Equals(tt.want) {
|
|
|
|
jgot, _ := json.MarshalIndent(got, "", "\t")
|
|
|
|
jwant, _ := json.MarshalIndent(tt.want, "", "\t")
|
|
|
|
if bytes.Equal(jgot, jwant) {
|
|
|
|
t.Logf("prefs differ only in non-JSON-visible ways (nil/non-nil zero-length arrays)")
|
|
|
|
}
|
|
|
|
t.Errorf("wrong prefs\n got: %s\nwant: %s\n\ngot: %s\nwant: %s\n",
|
|
|
|
got.Pretty(), tt.want.Pretty(),
|
|
|
|
jgot, jwant,
|
|
|
|
)
|
|
|
|
|
|
|
|
}
|
|
|
|
})
|
|
|
|
}
|
|
|
|
|
|
|
|
}
|
2021-04-30 13:29:06 -07:00
|
|
|
|
|
|
|
func TestPrefFlagMapping(t *testing.T) {
|
|
|
|
prefType := reflect.TypeOf(ipn.Prefs{})
|
|
|
|
for i := 0; i < prefType.NumField(); i++ {
|
|
|
|
prefName := prefType.Field(i).Name
|
|
|
|
if _, ok := flagForPref[prefName]; ok {
|
|
|
|
continue
|
|
|
|
}
|
|
|
|
switch prefName {
|
|
|
|
case "WantRunning", "Persist", "LoggedOut":
|
|
|
|
// All explicitly handled (ignored) by checkForAccidentalSettingReverts.
|
|
|
|
continue
|
|
|
|
case "OSVersion", "DeviceModel":
|
|
|
|
// Only used by Android, which doesn't have a CLI mode anyway, so
|
|
|
|
// fine to not map.
|
|
|
|
continue
|
|
|
|
case "NotepadURLs":
|
|
|
|
// TODO(bradfitz): https://github.com/tailscale/tailscale/issues/1830
|
|
|
|
continue
|
|
|
|
}
|
|
|
|
t.Errorf("unexpected new ipn.Pref field %q is not handled by up.go (see addPrefFlagMapping and checkForAccidentalSettingReverts)", prefName)
|
|
|
|
}
|
|
|
|
}
|