ipn: move Options.ServerURL into Prefs.

We can't rely on a frontend to provide a control
server URL, so this naturally belongs in server-persisted
state.

Signed-off-by: David Anderson <dave@natulte.net>

cmd/tailscale/tailscale.go

@@ -78,6 +78,7 @@ func main() {
 	// TODO(apenwarr): fix different semantics between prefs and uflags
 	// TODO(apenwarr): allow setting/using CorpDNS
 	prefs := ipn.Prefs{
+		ControlURL:       *server,
 		WantRunning:      true,
 		RouteAll:         *routeall,
 		AllowSingleHosts: !*nuroutes,
@@ -106,8 +107,7 @@ func main() {
 	bc := ipn.NewBackendClient(log.Printf, clientToServer)
 	bc.SetPrefs(prefs)
 	opts := ipn.Options{
-		StateKey:  globalStateKey,
+		StateKey: globalStateKey,
-		ServerURL: *server,
 		Notify: func(n ipn.Notify) {
 			if n.ErrMessage != nil {
 				log.Fatalf("backend error: %v\n", *n.ErrMessage)

ipn/backend.go

@@ -72,9 +72,6 @@ type StateKey string
 type Options struct {
 	// FrontendLogID is the public logtail id used by the frontend.
 	FrontendLogID string
-	// ServerURL is the base URL of the tailcontrol server to talk
-	// to. Required.
-	ServerURL string
 	// StateKey and Prefs together define the state the backend should
 	// use:
 	//  - StateKey=="" && Prefs!=nil: use Prefs for internal state,

ipn/e2e_test.go

@@ -168,8 +168,8 @@ func newNode(t *testing.T, prefix string, https *httptest.Server) testNode {
 	}
 	n.Start(Options{
 		FrontendLogID: prefix + "-f",
-		ServerURL:     https.URL,
 		Prefs: &Prefs{
+			ControlURL:       https.URL,
 			RouteAll:         true,
 			AllowSingleHosts: true,
 			CorpDNS:          true,

ipn/fake.go

@@ -16,7 +16,7 @@ type FakeBackend struct {
 }
 
 func (b *FakeBackend) Start(opts Options) error {
-	b.serverURL = opts.ServerURL
+	b.serverURL = opts.Prefs.ControlURL
 	if opts.Notify == nil {
 		log.Fatalf("FakeBackend.Start: opts.Notify is nil\n")
 	}

ipn/handle.go

@@ -5,7 +5,6 @@
 package ipn
 
 import (
-	"strings"
 	"sync"
 	"time"
 
@@ -14,7 +13,6 @@ import (
 )
 
 type Handle struct {
-	serverURL     string
 	frontendLogID string
 	b             Backend
 	xnotify       func(n Notify)
@@ -43,7 +41,6 @@ func NewHandle(b Backend, logf logger.Logf, opts Options) (*Handle, error) {
 }
 
 func (h *Handle) Start(opts Options) error {
-	h.serverURL = strings.TrimRight(opts.ServerURL, "/")
 	h.frontendLogID = opts.FrontendLogID
 	h.xnotify = opts.Notify
 	h.netmapCache = nil
@@ -148,7 +145,7 @@ func (h *Handle) Expiry() time.Time {
 }
 
 func (h *Handle) AdminPageURL() string {
-	return h.serverURL + "/admin/machines"
+	return h.prefsCache.ControlURL + "/admin/machines"
 }
 
 func (h *Handle) StartLoginInteractive() {

ipn/ipnserver/server.go

@@ -29,14 +29,6 @@ import (
 	"tailscale.com/wgengine"
 )
 
-// defaultLoginServer is the login URL used by an auto-starting
-// server.
-//
-// TODO(danderson): the reason this is hardcoded is that the server
-// URL is currently not stored in state, but passed in by the
-// frontend. This needs to be fixed.
-const defaultLoginServer = "https://login.tailscale.com"
-
 // Options is the configuration of the Tailscale node agent.
 type Options struct {
 	// SocketPath, on unix systems, is the unix socket path to listen
@@ -122,8 +114,7 @@ func Run(rctx context.Context, logf logger.Logf, logid string, opts Options, e w
 			Version: version.LONG,
 			Start: &ipn.StartArgs{
 				Opts: ipn.Options{
-					ServerURL: defaultLoginServer,
+					StateKey: opts.AutostartStateKey,
-					StateKey:  opts.AutostartStateKey,
 				},
 			},
 		})

ipn/local.go

@@ -149,13 +149,13 @@ func (b *LocalBackend) Start(opts Options) error {
 	hi.Services = b.hiCache.Services // keep any previous session
 	b.hiCache = hi
 	b.state = NoState
-	b.serverURL = opts.ServerURL
 
 	if err := b.loadStateWithLock(opts.StateKey, opts.Prefs); err != nil {
 		b.mu.Unlock()
 		return fmt.Errorf("loading requested state: %v", err)
 	}
 
+	b.serverURL = b.prefs.ControlURL
 	hi.RoutableIPs = append(hi.RoutableIPs, b.prefs.AdvertiseRoutes...)
 
 	b.notify = opts.Notify

ipn/message_test.go

@@ -95,8 +95,9 @@ func TestClientServer(t *testing.T) {
 
 	ch := make(chan Notify, 256)
 	h, err := NewHandle(bc, clogf, Options{
-		ServerURL: "http://example.com/fake",
+		Prefs: &Prefs{
-		Prefs:     &Prefs{},
+			ControlURL: "http://example.com/fake",
+		},
 		Notify: func(n Notify) {
 			ch <- n
 		},

ipn/prefs.go

@@ -19,6 +19,8 @@ import (
 
 // Prefs are the user modifiable settings of the Tailscale node agent.
 type Prefs struct {
+	// ControlURL is the URL of the control server to use.
+	ControlURL string
 	// RouteAll specifies whether to accept subnet and default routes
 	// advertised by other nodes on the Tailscale network.
 	RouteAll bool
@@ -91,6 +93,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool {
 	}
 
 	return p != nil && p2 != nil &&
+		p.ControlURL == p2.ControlURL &&
 		p.RouteAll == p2.RouteAll &&
 		p.AllowSingleHosts == p2.AllowSingleHosts &&
 		p.CorpDNS == p2.CorpDNS &&
@@ -118,6 +121,7 @@ func NewPrefs() Prefs {
 		// Provide default values for options which are normally
 		// true, but might be missing from the json data for any
 		// reason. The json can still override them to false.
+		ControlURL:       "https://login.tailscale.com",
 		RouteAll:         true,
 		AllowSingleHosts: true,
 		CorpDNS:          true,
@@ -142,6 +146,11 @@ func PrefsFromBytes(b []byte, enforceDefaults bool) (Prefs, error) {
 			log.Printf("Prefs parse: %v: %v\n", err, b)
 		}
 	}
+	if p.ControlURL == "" {
+		// TODO(danderson): compat shim, remove after
+		// Options.ServerURL has been gone for a release.
+		p.ControlURL = "https://login.tailscale.com"
+	}
 	if enforceDefaults {
 		p.RouteAll = true
 		p.AllowSingleHosts = true

ipn/prefs_test.go

@@ -20,7 +20,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 }
 
 func TestPrefsEqual(t *testing.T) {
-	prefsHandles := []string{"RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "UsePacketFilter", "AdvertiseRoutes", "NotepadURLs", "Persist"}
+	prefsHandles := []string{"ControlURL", "RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "UsePacketFilter", "AdvertiseRoutes", "NotepadURLs", "Persist"}
 	if have := fieldsOf(reflect.TypeOf(Prefs{})); !reflect.DeepEqual(have, prefsHandles) {
 		t.Errorf("Prefs.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
 			have, prefsHandles)
@@ -56,6 +56,17 @@ func TestPrefsEqual(t *testing.T) {
 			true,
 		},
 
+		{
+			&Prefs{ControlURL: "https://login.tailscale.com"},
+			&Prefs{ControlURL: "https://login.private.co"},
+			false,
+		},
+		{
+			&Prefs{ControlURL: "https://login.tailscale.com"},
+			&Prefs{ControlURL: "https://login.tailscale.com"},
+			true,
+		},
+
 		{
 			&Prefs{RouteAll: true},
 			&Prefs{RouteAll: false},
@@ -209,7 +220,9 @@ func checkPrefs(t *testing.T, p Prefs) {
 }
 
 func TestBasicPrefs(t *testing.T) {
-	p := Prefs{}
+	p := Prefs{
+		ControlURL: "https://login.tailscale.com",
+	}
 	checkPrefs(t, p)
 }
 
@@ -218,8 +231,9 @@ func TestPrefsPersist(t *testing.T) {
 		LoginName: "test@example.com",
 	}
 	p := Prefs{
-		CorpDNS: true,
+		ControlURL: "https://login.tailscale.com",
-		Persist: &c,
+		CorpDNS:    true,
+		Persist:    &c,
 	}
 	checkPrefs(t, p)
 }