From ddfcc4326c37672bac9181afb188eead0d4b956e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 5 Feb 2021 15:23:01 -0800 Subject: [PATCH] types/persist: split controlclient.Persist into a small leaf package This one alone doesn't modify the global dependency map much (depaware.txt if anything looks slightly worse), but it leave controlclient as only containing NetworkMap: bradfitz@tsdev:~/src/tailscale.com/ipn$ grep -F "controlclient." *.go backend.go: NetMap *controlclient.NetworkMap // new netmap received fake_test.go: b.notify(Notify{NetMap: &controlclient.NetworkMap{}}) fake_test.go: b.notify(Notify{NetMap: &controlclient.NetworkMap{}}) handle.go: netmapCache *controlclient.NetworkMap handle.go:func (h *Handle) NetMap() *controlclient.NetworkMap { Once that goes into a leaf package, then ipn doesn't depend on controlclient at all, and then the client gets smaller. Updates #1278 --- cmd/tailscale/depaware.txt | 3 +- cmd/tailscaled/depaware.txt | 1 + control/controlclient/auto.go | 5 +- control/controlclient/direct.go | 68 ++--------------- control/controlclient/direct_clone.go | 20 ----- ipn/ipnlocal/local.go | 9 ++- ipn/ipnlocal/loglines_test.go | 4 +- ipn/prefs.go | 6 +- ipn/prefs_clone.go | 6 +- ipn/prefs_test.go | 16 ++-- types/persist/persist.go | 73 +++++++++++++++++++ types/persist/persist_clone.go | 34 +++++++++ .../persist}/persist_test.go | 11 ++- 13 files changed, 149 insertions(+), 107 deletions(-) delete mode 100644 control/controlclient/direct_clone.go create mode 100644 types/persist/persist.go create mode 100644 types/persist/persist_clone.go rename {control/controlclient => types/persist}/persist_test.go (91%) diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 2113e09ed..ba092e496 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -46,10 +46,11 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/types/key from tailscale.com/derp+ tailscale.com/types/logger from tailscale.com/cmd/tailscale/cli+ tailscale.com/types/opt from tailscale.com/control/controlclient+ + tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/cmd/tailscale/cli+ tailscale.com/types/strbuilder from tailscale.com/net/packet tailscale.com/types/structs from tailscale.com/control/controlclient+ - tailscale.com/types/wgkey from tailscale.com/control/controlclient + tailscale.com/types/wgkey from tailscale.com/control/controlclient+ tailscale.com/util/dnsname from tailscale.com/cmd/tailscale/cli+ W tailscale.com/util/endian from tailscale.com/net/netns tailscale.com/util/lineread from tailscale.com/control/controlclient+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index e65e8bfa2..137a1d686 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -108,6 +108,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/types/logger from tailscale.com/cmd/tailscaled+ tailscale.com/types/nettype from tailscale.com/wgengine/magicsock tailscale.com/types/opt from tailscale.com/control/controlclient+ + tailscale.com/types/persist from tailscale.com/control/controlclient+ tailscale.com/types/preftype from tailscale.com/ipn+ tailscale.com/types/strbuilder from tailscale.com/net/packet tailscale.com/types/structs from tailscale.com/control/controlclient+ diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 48a02f1b0..a3a7b02c5 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -22,6 +22,7 @@ import ( "tailscale.com/tailcfg" "tailscale.com/types/empty" "tailscale.com/types/logger" + "tailscale.com/types/persist" "tailscale.com/types/structs" "tailscale.com/types/wgkey" ) @@ -68,7 +69,7 @@ type Status struct { LoginFinished *empty.Message Err string URL string - Persist *Persist // locally persisted configuration + Persist *persist.Persist // locally persisted configuration NetMap *NetworkMap // server-pushed configuration Hostinfo *tailcfg.Hostinfo // current Hostinfo data State State @@ -618,7 +619,7 @@ func (c *Client) sendStatus(who string, err error, url string, nm *NetworkMap) { c.logf("[v1] sendStatus: %s: %v", who, state) - var p *Persist + var p *persist.Persist var fin *empty.Message if state == StateAuthenticated { fin = new(empty.Message) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 157840216..5c603a892 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -4,8 +4,6 @@ package controlclient -//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=direct_clone.go - import ( "bytes" "context" @@ -42,69 +40,13 @@ import ( "tailscale.com/tailcfg" "tailscale.com/types/logger" "tailscale.com/types/opt" - "tailscale.com/types/structs" + "tailscale.com/types/persist" "tailscale.com/types/wgkey" "tailscale.com/util/systemd" "tailscale.com/version" "tailscale.com/wgengine/filter" ) -type Persist struct { - _ structs.Incomparable - - // LegacyFrontendPrivateMachineKey is here temporarily - // (starting 2020-09-28) during migration of Windows users' - // machine keys from frontend storage to the backend. On the - // first LocalBackend.Start call, the backend will initialize - // the real (backend-owned) machine key from the frontend's - // provided value (if non-zero), picking a new random one if - // needed. This field should be considered read-only from GUI - // frontends. The real value should not be written back in - // this field, lest the frontend persist it to disk. - LegacyFrontendPrivateMachineKey wgkey.Private `json:"PrivateMachineKey"` - - PrivateNodeKey wgkey.Private - OldPrivateNodeKey wgkey.Private // needed to request key rotation - Provider string - LoginName string -} - -func (p *Persist) Equals(p2 *Persist) bool { - if p == nil && p2 == nil { - return true - } - if p == nil || p2 == nil { - return false - } - - return p.LegacyFrontendPrivateMachineKey.Equal(p2.LegacyFrontendPrivateMachineKey) && - p.PrivateNodeKey.Equal(p2.PrivateNodeKey) && - p.OldPrivateNodeKey.Equal(p2.OldPrivateNodeKey) && - p.Provider == p2.Provider && - p.LoginName == p2.LoginName -} - -func (p *Persist) Pretty() string { - var mk, ok, nk wgkey.Key - if !p.LegacyFrontendPrivateMachineKey.IsZero() { - mk = p.LegacyFrontendPrivateMachineKey.Public() - } - if !p.OldPrivateNodeKey.IsZero() { - ok = p.OldPrivateNodeKey.Public() - } - if !p.PrivateNodeKey.IsZero() { - nk = p.PrivateNodeKey.Public() - } - ss := func(k wgkey.Key) string { - if k.IsZero() { - return "" - } - return k.ShortString() - } - return fmt.Sprintf("Persist{lm=%v, o=%v, n=%v u=%#v}", - ss(mk), ss(ok), ss(nk), p.LoginName) -} - // Direct is the client that connects to a tailcontrol server for a node. type Direct struct { httpc *http.Client // HTTP client used to talk to tailcontrol @@ -121,7 +63,7 @@ type Direct struct { mu sync.Mutex // mutex guards the following fields serverKey wgkey.Key - persist Persist + persist persist.Persist authKey string tryingNewKey wgkey.Private expiry *time.Time @@ -133,7 +75,7 @@ type Direct struct { } type Options struct { - Persist Persist // initial persistent data + Persist persist.Persist // initial persistent data MachinePrivateKey wgkey.Private // the machine key to use ServerURL string // URL of the tailcontrol server AuthKey string // optional node auth key for auto registration @@ -271,7 +213,7 @@ func (c *Direct) SetNetInfo(ni *tailcfg.NetInfo) bool { return true } -func (c *Direct) GetPersist() Persist { +func (c *Direct) GetPersist() persist.Persist { c.mu.Lock() defer c.mu.Unlock() return c.persist @@ -294,7 +236,7 @@ func (c *Direct) TryLogout(ctx context.Context) error { // immediately invalidated. //if !c.persist.PrivateNodeKey.IsZero() { //} - c.persist = Persist{} + c.persist = persist.Persist{} return nil } diff --git a/control/controlclient/direct_clone.go b/control/controlclient/direct_clone.go deleted file mode 100644 index 9254d82ea..000000000 --- a/control/controlclient/direct_clone.go +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) 2020 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. - -// Code generated by tailscale.com/cmd/cloner -type Persist; DO NOT EDIT. - -package controlclient - -import () - -// Clone makes a deep copy of Persist. -// The result aliases no memory with the original. -func (src *Persist) Clone() *Persist { - if src == nil { - return nil - } - dst := new(Persist) - *dst = *src - return dst -} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ffd37ce2a..523292def 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -29,6 +29,7 @@ import ( "tailscale.com/types/empty" "tailscale.com/types/key" "tailscale.com/types/logger" + "tailscale.com/types/persist" "tailscale.com/types/wgkey" "tailscale.com/util/systemd" "tailscale.com/version" @@ -512,7 +513,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { b.notify = opts.Notify b.setNetMapLocked(nil) - persist := b.prefs.Persist + persistv := b.prefs.Persist machinePrivKey := b.machinePrivKey b.mu.Unlock() @@ -545,14 +546,14 @@ func (b *LocalBackend) Start(opts ipn.Options) error { } var err error - if persist == nil { + if persistv == nil { // let controlclient initialize it - persist = &controlclient.Persist{} + persistv = &persist.Persist{} } cli, err := controlclient.New(controlclient.Options{ MachinePrivateKey: machinePrivKey, Logf: logger.WithPrefix(b.logf, "control: "), - Persist: *persist, + Persist: *persistv, ServerURL: b.serverURL, AuthKey: opts.AuthKey, Hostinfo: hostinfo, diff --git a/ipn/ipnlocal/loglines_test.go b/ipn/ipnlocal/loglines_test.go index 656758afe..83ae8a309 100644 --- a/ipn/ipnlocal/loglines_test.go +++ b/ipn/ipnlocal/loglines_test.go @@ -9,13 +9,13 @@ import ( "testing" "time" - "tailscale.com/control/controlclient" "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" "tailscale.com/logtail" "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/types/key" + "tailscale.com/types/persist" "tailscale.com/wgengine" ) @@ -67,7 +67,7 @@ func TestLocalLogLines(t *testing.T) { } // log prefs line - persist := &controlclient.Persist{} + persist := &persist.Persist{} prefs := ipn.NewPrefs() prefs.Persist = persist lb.SetPrefs(prefs) diff --git a/ipn/prefs.go b/ipn/prefs.go index c8841f5de..7b4a5562c 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -17,8 +17,8 @@ import ( "inet.af/netaddr" "tailscale.com/atomicfile" - "tailscale.com/control/controlclient" "tailscale.com/tailcfg" + "tailscale.com/types/persist" "tailscale.com/types/preftype" ) @@ -144,7 +144,7 @@ type Prefs struct { // TODO(apenwarr): We should move this out of here, it's not a pref. // We can maybe do that once we're sure which module should persist // it (backend or frontend?) - Persist *controlclient.Persist `json:"Config"` + Persist *persist.Persist `json:"Config"` } // IsEmpty reports whether p is nil or pointing to a Prefs zero value. @@ -275,7 +275,7 @@ func PrefsFromBytes(b []byte, enforceDefaults bool) (*Prefs, error) { if len(b) == 0 { return p, nil } - persist := &controlclient.Persist{} + persist := &persist.Persist{} err := json.Unmarshal(b, persist) if err == nil && (persist.Provider != "" || persist.LoginName != "") { // old-style relaynode config; import it diff --git a/ipn/prefs_clone.go b/ipn/prefs_clone.go index 0691fc4fc..9e426a4fa 100644 --- a/ipn/prefs_clone.go +++ b/ipn/prefs_clone.go @@ -8,8 +8,8 @@ package ipn import ( "inet.af/netaddr" - "tailscale.com/control/controlclient" "tailscale.com/tailcfg" + "tailscale.com/types/persist" "tailscale.com/types/preftype" ) @@ -24,7 +24,7 @@ func (src *Prefs) Clone() *Prefs { dst.AdvertiseTags = append(src.AdvertiseTags[:0:0], src.AdvertiseTags...) dst.AdvertiseRoutes = append(src.AdvertiseRoutes[:0:0], src.AdvertiseRoutes...) if dst.Persist != nil { - dst.Persist = new(controlclient.Persist) + dst.Persist = new(persist.Persist) *dst.Persist = *src.Persist } return dst @@ -50,5 +50,5 @@ var _PrefsNeedsRegeneration = Prefs(struct { AdvertiseRoutes []netaddr.IPPrefix NoSNAT bool NetfilterMode preftype.NetfilterMode - Persist *controlclient.Persist + Persist *persist.Persist }{}) diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index a227b05b3..ad8905b03 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -14,8 +14,8 @@ import ( "time" "inet.af/netaddr" - "tailscale.com/control/controlclient" "tailscale.com/tstest" + "tailscale.com/types/persist" "tailscale.com/types/preftype" "tailscale.com/types/wgkey" ) @@ -225,13 +225,13 @@ func TestPrefsEqual(t *testing.T) { }, { - &Prefs{Persist: &controlclient.Persist{}}, - &Prefs{Persist: &controlclient.Persist{LoginName: "dave"}}, + &Prefs{Persist: &persist.Persist{}}, + &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, false, }, { - &Prefs{Persist: &controlclient.Persist{LoginName: "dave"}}, - &Prefs{Persist: &controlclient.Persist{LoginName: "dave"}}, + &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, + &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, true, }, } @@ -296,7 +296,7 @@ func TestBasicPrefs(t *testing.T) { func TestPrefsPersist(t *testing.T) { tstest.PanicOnLog() - c := controlclient.Persist{ + c := persist.Persist{ LoginName: "test@example.com", } p := Prefs{ @@ -362,14 +362,14 @@ func TestPrefsPretty(t *testing.T) { }, { Prefs{ - Persist: &controlclient.Persist{}, + Persist: &persist.Persist{}, }, "linux", `Prefs{ra=false mesh=false dns=false want=false routes=[] nf=off Persist{lm=, o=, n= u=""}}`, }, { Prefs{ - Persist: &controlclient.Persist{ + Persist: &persist.Persist{ PrivateNodeKey: wgkey.Private{1: 1}, }, }, diff --git a/types/persist/persist.go b/types/persist/persist.go new file mode 100644 index 000000000..169288280 --- /dev/null +++ b/types/persist/persist.go @@ -0,0 +1,73 @@ +// 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 persist contains the Persist type. +package persist + +import ( + "fmt" + + "tailscale.com/types/structs" + "tailscale.com/types/wgkey" +) + +//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=persist_clone.go + +// Persist is the JSON type stored on disk on nodes to remember their +// settings between runs. +type Persist struct { + _ structs.Incomparable + + // LegacyFrontendPrivateMachineKey is here temporarily + // (starting 2020-09-28) during migration of Windows users' + // machine keys from frontend storage to the backend. On the + // first LocalBackend.Start call, the backend will initialize + // the real (backend-owned) machine key from the frontend's + // provided value (if non-zero), picking a new random one if + // needed. This field should be considered read-only from GUI + // frontends. The real value should not be written back in + // this field, lest the frontend persist it to disk. + LegacyFrontendPrivateMachineKey wgkey.Private `json:"PrivateMachineKey"` + + PrivateNodeKey wgkey.Private + OldPrivateNodeKey wgkey.Private // needed to request key rotation + Provider string + LoginName string +} + +func (p *Persist) Equals(p2 *Persist) bool { + if p == nil && p2 == nil { + return true + } + if p == nil || p2 == nil { + return false + } + + return p.LegacyFrontendPrivateMachineKey.Equal(p2.LegacyFrontendPrivateMachineKey) && + p.PrivateNodeKey.Equal(p2.PrivateNodeKey) && + p.OldPrivateNodeKey.Equal(p2.OldPrivateNodeKey) && + p.Provider == p2.Provider && + p.LoginName == p2.LoginName +} + +func (p *Persist) Pretty() string { + var mk, ok, nk wgkey.Key + if !p.LegacyFrontendPrivateMachineKey.IsZero() { + mk = p.LegacyFrontendPrivateMachineKey.Public() + } + if !p.OldPrivateNodeKey.IsZero() { + ok = p.OldPrivateNodeKey.Public() + } + if !p.PrivateNodeKey.IsZero() { + nk = p.PrivateNodeKey.Public() + } + ss := func(k wgkey.Key) string { + if k.IsZero() { + return "" + } + return k.ShortString() + } + return fmt.Sprintf("Persist{lm=%v, o=%v, n=%v u=%#v}", + ss(mk), ss(ok), ss(nk), p.LoginName) +} diff --git a/types/persist/persist_clone.go b/types/persist/persist_clone.go new file mode 100644 index 000000000..533e9294d --- /dev/null +++ b/types/persist/persist_clone.go @@ -0,0 +1,34 @@ +// Copyright (c) 2020 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. + +// Code generated by tailscale.com/cmd/cloner -type Persist; DO NOT EDIT. + +package persist + +import ( + "tailscale.com/types/structs" + "tailscale.com/types/wgkey" +) + +// Clone makes a deep copy of Persist. +// The result aliases no memory with the original. +func (src *Persist) Clone() *Persist { + if src == nil { + return nil + } + dst := new(Persist) + *dst = *src + return dst +} + +// A compilation failure here means this code must be regenerated, with command: +// tailscale.com/cmd/cloner -type Persist +var _PersistNeedsRegeneration = Persist(struct { + _ structs.Incomparable + LegacyFrontendPrivateMachineKey wgkey.Private + PrivateNodeKey wgkey.Private + OldPrivateNodeKey wgkey.Private + Provider string + LoginName string +}{}) diff --git a/control/controlclient/persist_test.go b/types/persist/persist_test.go similarity index 91% rename from control/controlclient/persist_test.go rename to types/persist/persist_test.go index efee06273..04fdb8bc3 100644 --- a/control/controlclient/persist_test.go +++ b/types/persist/persist_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package controlclient +package persist import ( "reflect" @@ -11,6 +11,15 @@ import ( "tailscale.com/types/wgkey" ) +func fieldsOf(t reflect.Type) (fields []string) { + for i := 0; i < t.NumField(); i++ { + if name := t.Field(i).Name; name != "_" { + fields = append(fields, name) + } + } + return +} + func TestPersistEqual(t *testing.T) { persistHandles := []string{"LegacyFrontendPrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "LoginName"} if have := fieldsOf(reflect.TypeOf(Persist{})); !reflect.DeepEqual(have, persistHandles) {