From cf1e386cbd5f5487704c1b8d7e3f3d617897bc3d Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 18 Feb 2020 21:03:22 -0800 Subject: [PATCH] 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 --- cmd/tailscale/tailscale.go | 4 ++-- ipn/backend.go | 3 --- ipn/e2e_test.go | 2 +- ipn/fake.go | 2 +- ipn/handle.go | 5 +---- ipn/ipnserver/server.go | 11 +---------- ipn/local.go | 2 +- ipn/message_test.go | 5 +++-- ipn/prefs.go | 9 +++++++++ ipn/prefs_test.go | 22 ++++++++++++++++++---- 10 files changed, 37 insertions(+), 28 deletions(-) diff --git a/cmd/tailscale/tailscale.go b/cmd/tailscale/tailscale.go index fe852cd55..f8ee24269 100644 --- a/cmd/tailscale/tailscale.go +++ b/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, - ServerURL: *server, + StateKey: globalStateKey, Notify: func(n ipn.Notify) { if n.ErrMessage != nil { log.Fatalf("backend error: %v\n", *n.ErrMessage) diff --git a/ipn/backend.go b/ipn/backend.go index 6742e3302..da5f17592 100644 --- a/ipn/backend.go +++ b/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, diff --git a/ipn/e2e_test.go b/ipn/e2e_test.go index 7b50a929c..1474802d7 100644 --- a/ipn/e2e_test.go +++ b/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, diff --git a/ipn/fake.go b/ipn/fake.go index 89a5d58fa..544993bd9 100644 --- a/ipn/fake.go +++ b/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") } diff --git a/ipn/handle.go b/ipn/handle.go index bd351f8be..397e08631 100644 --- a/ipn/handle.go +++ b/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() { diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index f3d9d858c..0809ad6fd 100644 --- a/ipn/ipnserver/server.go +++ b/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, }, }, }) diff --git a/ipn/local.go b/ipn/local.go index 3e7c919b8..065b24f81 100644 --- a/ipn/local.go +++ b/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 diff --git a/ipn/message_test.go b/ipn/message_test.go index b4f9cd6be..dfc4067d1 100644 --- a/ipn/message_test.go +++ b/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 }, diff --git a/ipn/prefs.go b/ipn/prefs.go index a72e5f6d1..cfc51137e 100644 --- a/ipn/prefs.go +++ b/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 diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index 7542e9e51..302526909 100644 --- a/ipn/prefs_test.go +++ b/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, - Persist: &c, + ControlURL: "https://login.tailscale.com", + CorpDNS: true, + Persist: &c, } checkPrefs(t, p) }