Remove the remaining known sources of flakiness in TestStateMachine and
TestStateMachineSeamless.
Updates tailscale/corp#36230
Updates #19377
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
TestStateMachine & TestStateMachineSeamless both flake a lot asserting the
"Shutdown" call on cc after a Logout. This is because Shutdown is called on
a goroutine to avoid a deadlock if it's called while holding the
LocalBackend lock (#18052).
This fixes that cause of flakes by waiting for LocalBackend's goroutine
tracker to have no goroutines running (so the goroutine that calls Shutdown
must have finished).
This does not make TestStateMachine non-flaky because it can flake later in
the test, too: the assertion on "unpause" after clearing the netmap between
"Start4" and "Start4 -> netmap" sometimes fails.
Updates tailscale/corp#36230
Updates #19377
Updates #18052
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Previously we only set this when it updated, which was fine for the first
call to Start(), but after that point future updates would be skipped if
nothing had changed. If Start() was called again, it would wipe the peer API
endpoints and they wouldn't get added back again, breaking exit nodes (and
anything else requiring peer API to be advertised).
Updates tailscale/corp#27173
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Before we introduced seamless, the "blocked" state was used to track:
* Whether a login was required for connectivity, and therefore we should
keep the engine deconfigured until that happened
* Whether authentication was in progress
"blocked" would stop authReconfig from running. We want this when a login is
required: if your key has expired we want to deconfigure the engine and keep
it down, so that you don't keep using exit nodes (which won't work because
your key has expired).
Taking the engine down while auth was in progress was undesirable, so we
don't do that with seamless renewal. However, not entering the "blocked"
state meant that we needed to change the logic for when to send
LoginFinished on the IPN bus after seeing StateAuthenticated from the
controlclient. Initially we changed the "if blocked" check to "if blocked or
seamless is enabled" which was correct in other places.
In this place however, it introduced a bug: we are sending LoginFinished
every time we see StateAuthenticated, which happens even on a down & up, or
a profile switch. This in turn made it harder for UI clients to track when
authentication is complete.
Instead we should only send it out if we were blocked (i.e. seamless is
disabled, or our key expired) or an auth was in progress.
Updates tailscale/corp#31476
Updates tailscale/corp#32645
Fixes#17363
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
If we received a wg engine status while processing an auth URL, there was a
race condition where the authURL could be reset to "" immediately after we
set it.
To fix this we need to check that we are moving from a non-Running state to
a Running state rather than always resetting the URL when we "move" into a
Running state even if that is the current state.
We also need to make sure that we do not return from stopEngineAndWait until
the engine is stopped: before, we would return as soon as we received any
engine status update, but that might have been an update already in-flight
before we asked the engine to stop. Now we wait until we see an update that
is indicative of a stopped engine, or we see that the engine is unblocked
again, which indicates that the engine stopped and then started again while
we were waiting before we checked the state.
Updates #17388
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Co-authored-by: Nick Khyl <nickk@tailscale.com>
The `put` callback runs on a different goroutine to the test, so calling
t.Fatalf in put had no effect. `drain` is always called when checking what
was put and is called from the test goroutine, so that's a good place to
fail the test if the channel was too full.
Updates #17363
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Ideally we would remove this warning entirely, as it is now possible to
reauthenticate without losing connectivty. However, it is still possible to
lose SSH connectivity if the user changes the ownership of the machine when
they do a force-reauth, and we have no way of knowing if they are going to
do that before they do it.
For now, let's just reduce the strength of the warning to warn them that
they "may" lose their connection, rather than they "will".
Updates tailscale/corp#32429
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
This is an integration test that covers all the code in Direct, Auto, and
LocalBackend that processes NetMaps and creates a Filter. The test uses
tsnet as a convenient proxy for setting up all the client pieces correctly,
but is not actually a test specific to tsnet.
Updates tailscale/corp#20514
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
Only send a stored raw map message in reply to a streaming map response.
Otherwise a non-streaming map response might pick it up first, and
potentially drop it. This guarantees that a map response sent via
AddRawMapResponse will be picked up by the main map response loop in the
client.
Fixes#15362
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
This will be used by clients to make better decisions on when to warn users
about impending key expiry.
Updates tailscale/corp#16016
Signed-off-by: James Sanderson <jsanderson@tailscale.com>
