ipn/ipnlocal: plumb health.Tracker into profileManager constructor

Setting the field after-the-fact wasn't working because we could migrate
prefs on creation, which would set health status for auto updates.

Updates #11986

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I41d79ebd61d64829a3a9e70586ce56f62d24ccfd
This commit is contained in:
Andrew Dunham 2024-05-03 10:59:22 -04:00 committed by Brad Fitzpatrick
parent e42c4396cf
commit e9505e5432
8 changed files with 41 additions and 34 deletions

View File

@ -364,11 +364,10 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo
if loginFlags&controlclient.LocalBackendStartKeyOSNeutral != 0 {
goos = ""
}
pm, err := newProfileManagerWithGOOS(store, logf, goos)
pm, err := newProfileManagerWithGOOS(store, logf, sys.HealthTracker(), goos)
if err != nil {
return nil, err
}
pm.health = sys.HealthTracker()
if sds, ok := store.(ipn.StateStoreDialerSetter); ok {
sds.SetDialer(dialer.SystemDial)
}

View File

@ -29,6 +29,7 @@ import (
"tailscale.com/control/controlclient"
"tailscale.com/drive"
"tailscale.com/drive/driveimpl"
"tailscale.com/health"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/net/netcheck"
@ -1849,7 +1850,7 @@ func TestSetExitNodeIDPolicy(t *testing.T) {
if test.prefs == nil {
test.prefs = ipn.NewPrefs()
}
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
pm.prefs = test.prefs.View()
b.netMap = test.nm
b.pm = pm
@ -2131,7 +2132,7 @@ func TestApplySysPolicy(t *testing.T) {
wantPrefs.ControlURL = ipn.DefaultControlURL
}
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
pm.prefs = usePrefs.View()
b := newTestBackend(t)

View File

@ -17,6 +17,7 @@ import (
"github.com/google/go-cmp/cmp"
"tailscale.com/control/controlclient"
"tailscale.com/health"
"tailscale.com/hostinfo"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
@ -148,7 +149,7 @@ func TestTKAEnablementFlow(t *testing.T) {
temp := t.TempDir()
cc := fakeControlClient(t, client)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -188,7 +189,7 @@ func TestTKADisablementFlow(t *testing.T) {
nlPriv := key.NewNLPrivate()
key := tka.Key{Kind: tka.Key25519, Public: nlPriv.Public().Verifier(), Votes: 2}
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -380,7 +381,7 @@ func TestTKASync(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
nodePriv := key.NewNode()
nlPriv := key.NewNLPrivate()
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -602,7 +603,7 @@ func TestTKADisable(t *testing.T) {
disablementSecret := bytes.Repeat([]byte{0xa5}, 32)
nlPriv := key.NewNLPrivate()
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -693,7 +694,7 @@ func TestTKASign(t *testing.T) {
toSign := key.NewNode()
nlPriv := key.NewNLPrivate()
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -782,7 +783,7 @@ func TestTKAForceDisable(t *testing.T) {
nlPriv := key.NewNLPrivate()
key := tka.Key{Kind: tka.Key25519, Public: nlPriv.Public().Verifier(), Votes: 2}
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -877,7 +878,7 @@ func TestTKAAffectedSigs(t *testing.T) {
// toSign := key.NewNode()
nlPriv := key.NewNLPrivate()
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -1010,7 +1011,7 @@ func TestTKARecoverCompromisedKeyFlow(t *testing.T) {
cosignPriv := key.NewNLPrivate()
compromisedPriv := key.NewNLPrivate()
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,
@ -1101,7 +1102,7 @@ func TestTKARecoverCompromisedKeyFlow(t *testing.T) {
// Cosign using the cosigning key.
{
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
must.Do(pm.SetPrefs((&ipn.Prefs{
Persist: &persist.Persist{
PrivateNodeKey: nodePriv,

View File

@ -26,6 +26,7 @@ import (
"tailscale.com/appc"
"tailscale.com/appc/appctest"
"tailscale.com/client/tailscale/apitype"
"tailscale.com/health"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/tailcfg"
@ -642,7 +643,7 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
h.ps = &peerAPIServer{
b: &LocalBackend{
e: eng,
@ -692,7 +693,7 @@ func TestPeerAPIPrettyReplyCNAME(t *testing.T) {
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, &appc.RouteInfo{}, fakeStoreRoutes)
@ -764,7 +765,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
rc := &appctest.RouteCollector{}
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes)
@ -827,7 +828,7 @@ func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) {
rc := &appctest.RouteCollector{}
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes)

View File

@ -485,7 +485,8 @@ func (pm *profileManager) CurrentPrefs() ipn.PrefsView {
// ReadStartupPrefsForTest reads the startup prefs from disk. It is only used for testing.
func ReadStartupPrefsForTest(logf logger.Logf, store ipn.StateStore) (ipn.PrefsView, error) {
pm, err := newProfileManager(store, logf)
ht := new(health.Tracker) // in tests, don't care about the health status
pm, err := newProfileManager(store, logf, ht)
if err != nil {
return ipn.PrefsView{}, err
}
@ -494,8 +495,8 @@ func ReadStartupPrefsForTest(logf logger.Logf, store ipn.StateStore) (ipn.PrefsV
// newProfileManager creates a new ProfileManager using the provided StateStore.
// It also loads the list of known profiles from the StateStore.
func newProfileManager(store ipn.StateStore, logf logger.Logf) (*profileManager, error) {
return newProfileManagerWithGOOS(store, logf, envknob.GOOS())
func newProfileManager(store ipn.StateStore, logf logger.Logf, health *health.Tracker) (*profileManager, error) {
return newProfileManagerWithGOOS(store, logf, health, envknob.GOOS())
}
func readAutoStartKey(store ipn.StateStore, goos string) (ipn.StateKey, error) {
@ -528,7 +529,7 @@ func readKnownProfiles(store ipn.StateStore) (map[ipn.ProfileID]*ipn.LoginProfil
return knownProfiles, nil
}
func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, goos string) (*profileManager, error) {
func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, ht *health.Tracker, goos string) (*profileManager, error) {
logf = logger.WithPrefix(logf, "pm: ")
stateKey, err := readAutoStartKey(store, goos)
if err != nil {
@ -544,6 +545,7 @@ func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, goos stri
store: store,
knownProfiles: knownProfiles,
logf: logf,
health: ht,
}
if stateKey != "" {

View File

@ -12,6 +12,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"tailscale.com/clientupdate"
"tailscale.com/health"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/tailcfg"
@ -24,7 +25,7 @@ import (
func TestProfileCurrentUserSwitch(t *testing.T) {
store := new(mem.Store)
pm, err := newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -61,7 +62,7 @@ func TestProfileCurrentUserSwitch(t *testing.T) {
t.Fatalf("CurrentPrefs() = %v, want emptyPrefs", pm.CurrentPrefs().Pretty())
}
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -79,7 +80,7 @@ func TestProfileCurrentUserSwitch(t *testing.T) {
func TestProfileList(t *testing.T) {
store := new(mem.Store)
pm, err := newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -283,7 +284,7 @@ func TestProfileDupe(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
store := new(mem.Store)
pm, err := newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -316,7 +317,7 @@ func TestProfileDupe(t *testing.T) {
func TestProfileManagement(t *testing.T) {
store := new(mem.Store)
pm, err := newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -414,7 +415,7 @@ func TestProfileManagement(t *testing.T) {
t.Logf("Recreate profile manager from store")
// Recreate the profile manager to ensure that it can load the profiles
// from the store at startup.
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -430,7 +431,7 @@ func TestProfileManagement(t *testing.T) {
t.Logf("Recreate profile manager from store after deleting default profile")
// Recreate the profile manager to ensure that it can load the profiles
// from the store at startup.
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -472,7 +473,7 @@ func TestProfileManagement(t *testing.T) {
t.Fatal("SetPrefs failed to save auto-update setting")
}
// Re-load profiles to trigger migration for invalid auto-update value.
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "linux")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
if err != nil {
t.Fatal(err)
}
@ -494,7 +495,7 @@ func TestProfileManagementWindows(t *testing.T) {
store := new(mem.Store)
pm, err := newProfileManagerWithGOOS(store, logger.Discard, "windows")
pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "windows")
if err != nil {
t.Fatal(err)
}
@ -565,7 +566,7 @@ func TestProfileManagementWindows(t *testing.T) {
t.Logf("Recreate profile manager from store, should reset prefs")
// Recreate the profile manager to ensure that it can load the profiles
// from the store at startup.
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "windows")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "windows")
if err != nil {
t.Fatal(err)
}
@ -590,7 +591,7 @@ func TestProfileManagementWindows(t *testing.T) {
}
// Recreate the profile manager to ensure that it starts with test profile.
pm, err = newProfileManagerWithGOOS(store, logger.Discard, "windows")
pm, err = newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "windows")
if err != nil {
t.Fatal(err)
}

View File

@ -24,6 +24,7 @@ import (
"testing"
"time"
"tailscale.com/health"
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/tailcfg"
@ -686,7 +687,7 @@ func newTestBackend(t *testing.T) *LocalBackend {
dir := t.TempDir()
b.SetVarRoot(dir)
pm := must.Get(newProfileManager(new(mem.Store), logf))
pm := must.Get(newProfileManager(new(mem.Store), logf, new(health.Tracker)))
pm.currentProfile = &ipn.LoginProfile{ID: "id0"}
b.pm = pm

View File

@ -10,6 +10,7 @@ import (
"reflect"
"testing"
"tailscale.com/health"
"tailscale.com/ipn/store/mem"
"tailscale.com/tailcfg"
"tailscale.com/util/must"
@ -49,7 +50,7 @@ type fakeSSHServer struct {
}
func TestGetSSHUsernames(t *testing.T) {
pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
b := &LocalBackend{pm: pm, store: pm.Store()}
b.sshServer = fakeSSHServer{}
res, err := b.getSSHUsernames(new(tailcfg.C2NSSHUsernamesRequest))