Skip to content

Commit

Permalink
ipn,wgengine: remove vestigial Prefs.AllowSingleHosts
Browse files Browse the repository at this point in the history
It was requested by the first customer 4-5 years ago and only used
for a brief moment of time. We later added netmap visibility trimming
which removes the need for this.

It's been hidden by the CLI for quite some time and never documented
anywhere else.

This keeps the CLI flag, though, out of caution. It just returns an
error if it's set to anything but true (its default).

Fixes tailscale#12058

Change-Id: I7514ba572e7b82519b04ed603ff9f3bdbaecfda7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
  • Loading branch information
bradfitz authored and Mmx233 committed May 20, 2024
1 parent 5ba2d16 commit 14e9dc5
Show file tree
Hide file tree
Showing 11 changed files with 263 additions and 304 deletions.
370 changes: 164 additions & 206 deletions cmd/tailscale/cli/cli_test.go

Large diffs are not rendered by default.

21 changes: 14 additions & 7 deletions cmd/tailscale/cli/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func newUpFlagSet(goos string, upArgs *upArgsT, cmd string) *flag.FlagSet {
upf.StringVar(&upArgs.server, "login-server", ipn.DefaultControlURL, "base URL of control server")
upf.BoolVar(&upArgs.acceptRoutes, "accept-routes", acceptRouteDefault(goos), "accept routes advertised by other Tailscale nodes")
upf.BoolVar(&upArgs.acceptDNS, "accept-dns", true, "accept DNS configuration from the admin panel")
upf.BoolVar(&upArgs.singleRoutes, "host-routes", true, hidden+"install host routes to other Tailscale nodes")
upf.Var(notFalseVar{}, "host-routes", hidden+"install host routes to other Tailscale nodes (must be true as of Tailscale 1.67+)")
upf.StringVar(&upArgs.exitNodeIP, "exit-node", "", "Tailscale exit node (IP or base name) for internet traffic, or empty string to not use an exit node")
upf.BoolVar(&upArgs.exitNodeAllowLANAccess, "exit-node-allow-lan-access", false, "Allow direct access to the local network when routing traffic via an exit node")
upf.BoolVar(&upArgs.shieldsUp, "shields-up", false, "don't allow incoming connections")
Expand Down Expand Up @@ -143,6 +143,18 @@ func newUpFlagSet(goos string, upArgs *upArgsT, cmd string) *flag.FlagSet {
return upf
}

// notFalseVar is is a flag.Value that can only be "true", if set.
type notFalseVar struct{}

func (notFalseVar) IsBoolFlag() bool { return true }
func (notFalseVar) Set(v string) error {
if v != "true" {
return fmt.Errorf("unsupported value; only 'true' is allowed")
}
return nil
}
func (notFalseVar) String() string { return "true" }

func defaultNetfilterMode() string {
if distro.Get() == distro.Synology {
return "off"
Expand All @@ -156,7 +168,6 @@ type upArgsT struct {
server string
acceptRoutes bool
acceptDNS bool
singleRoutes bool
exitNodeIP string
exitNodeAllowLANAccess bool
shieldsUp bool
Expand Down Expand Up @@ -278,7 +289,6 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo

prefs.ExitNodeAllowLANAccess = upArgs.exitNodeAllowLANAccess
prefs.CorpDNS = upArgs.acceptDNS
prefs.AllowSingleHosts = upArgs.singleRoutes
prefs.ShieldsUp = upArgs.shieldsUp
prefs.RunSSH = upArgs.runSSH
prefs.RunWebClient = upArgs.runWebClient
Expand Down Expand Up @@ -740,7 +750,6 @@ func init() {
addPrefFlagMapping("accept-dns", "CorpDNS")
addPrefFlagMapping("accept-routes", "RouteAll")
addPrefFlagMapping("advertise-tags", "AdvertiseTags")
addPrefFlagMapping("host-routes", "AllowSingleHosts")
addPrefFlagMapping("hostname", "Hostname")
addPrefFlagMapping("login-server", "ControlURL")
addPrefFlagMapping("netfilter-mode", "NetfilterMode")
Expand Down Expand Up @@ -779,7 +788,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", "timeout", "accept-risk":
case "auth-key", "force-reauth", "reset", "qr", "json", "timeout", "accept-risk", "host-routes":
return true
}
return false
Expand Down Expand Up @@ -975,8 +984,6 @@ func prefsToFlags(env upCheckEnv, prefs *ipn.Prefs) (flagVal map[string]any) {
set(prefs.ControlURL)
case "accept-routes":
set(prefs.RouteAll)
case "host-routes":
set(prefs.AllowSingleHosts)
case "accept-dns":
set(prefs.CorpDNS)
case "shields-up":
Expand Down
9 changes: 4 additions & 5 deletions cmd/tsconnect/wasm/wasm_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,10 @@ func (i *jsIPN) run(jsCallbacks js.Value) {
go func() {
err := i.lb.Start(ipn.Options{
UpdatePrefs: &ipn.Prefs{
ControlURL: i.controlURL,
RouteAll: false,
AllowSingleHosts: true,
WantRunning: true,
Hostname: i.hostname,
ControlURL: i.controlURL,
RouteAll: false,
WantRunning: true,
Hostname: i.hostname,
},
AuthKey: i.authKey,
})
Expand Down
2 changes: 1 addition & 1 deletion ipn/ipn_clone.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions ipn/ipn_view.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions ipn/ipnlocal/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -3649,9 +3649,6 @@ func (b *LocalBackend) authReconfig() {
if prefs.RouteAll() {
flags |= netmap.AllowSubnetRoutes
}
if prefs.AllowSingleHosts() {
flags |= netmap.AllowSingleHosts
}
if hasPAC && disableSubnetsIfPAC {
if flags&netmap.AllowSubnetRoutes != 0 {
b.logf("authReconfig: have PAC; disabling subnet routes")
Expand Down
35 changes: 17 additions & 18 deletions ipn/prefs.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,6 @@ type Prefs struct {
// controlled by ExitNodeID/IP below.
RouteAll bool

// AllowSingleHosts specifies whether to install routes for each
// node IP on the tailscale network, in addition to a route for
// the whole network.
// This corresponds to the "tailscale up --host-routes" value,
// which defaults to true.
//
// TODO(danderson): why do we have this? It dumps a lot of stuff
// into the routing table, and a single network route _should_ be
// all that we need. But when I turn this off in my tailscaled,
// packets stop flowing. What's up with that?
AllowSingleHosts bool

// ExitNodeID and ExitNodeIP specify the node that should be used
// as an exit node for internet traffic. At most one of these
// should be non-zero.
Expand Down Expand Up @@ -252,6 +240,16 @@ type Prefs struct {
// by name.
DriveShares []*drive.Share

// AllowSingleHosts was a legacy field that was always true
// for the past 4.5 years. It controlled whether Tailscale
// peers got /32 or /127 routes for each other.
// As of 2024-05-17 we're starting to ignore it, but to let
// people still downgrade Tailscale versions and not break
// all peer-to-peer networking we still write it to disk (as JSON)
// so it can be loaded back by old versions.
// TODO(bradfitz): delete this in 2025 sometime. See #12058.
AllowSingleHosts marshalAsTrueInJSON

// The Persist field is named 'Config' in the file for backward
// compatibility with earlier versions.
// TODO(apenwarr): We should move this out of here, it's not a pref.
Expand Down Expand Up @@ -282,6 +280,13 @@ func (au1 AutoUpdatePrefs) Equals(au2 AutoUpdatePrefs) bool {
ok1 == ok2
}

type marshalAsTrueInJSON struct{}

var trueJSON = []byte("true")

func (marshalAsTrueInJSON) MarshalJSON() ([]byte, error) { return trueJSON, nil }
func (*marshalAsTrueInJSON) UnmarshalJSON([]byte) error { return nil }

// AppConnectorPrefs are the app connector settings for the node agent.
type AppConnectorPrefs struct {
// Advertise specifies whether the app connector subsystem is advertising
Expand All @@ -299,7 +304,6 @@ type MaskedPrefs struct {

ControlURLSet bool `json:",omitempty"`
RouteAllSet bool `json:",omitempty"`
AllowSingleHostsSet bool `json:",omitempty"`
ExitNodeIDSet bool `json:",omitempty"`
ExitNodeIPSet bool `json:",omitempty"`
InternalExitNodePriorSet bool `json:",omitempty"` // Internal; can't be set by LocalAPI clients
Expand Down Expand Up @@ -484,9 +488,6 @@ func (p *Prefs) pretty(goos string) string {
var sb strings.Builder
sb.WriteString("Prefs{")
fmt.Fprintf(&sb, "ra=%v ", p.RouteAll)
if !p.AllowSingleHosts {
sb.WriteString("mesh=false ")
}
fmt.Fprintf(&sb, "dns=%v want=%v ", p.CorpDNS, p.WantRunning)
if p.RunSSH {
sb.WriteString("ssh=true ")
Expand Down Expand Up @@ -579,7 +580,6 @@ func (p *Prefs) Equals(p2 *Prefs) bool {

return p.ControlURL == p2.ControlURL &&
p.RouteAll == p2.RouteAll &&
p.AllowSingleHosts == p2.AllowSingleHosts &&
p.ExitNodeID == p2.ExitNodeID &&
p.ExitNodeIP == p2.ExitNodeIP &&
p.InternalExitNodePrior == p2.InternalExitNodePrior &&
Expand Down Expand Up @@ -663,7 +663,6 @@ func NewPrefs() *Prefs {
ControlURL: "",

RouteAll: true,
AllowSingleHosts: true,
CorpDNS: true,
WantRunning: false,
NetfilterMode: preftype.NetfilterOn,
Expand Down

0 comments on commit 14e9dc5

Please sign in to comment.