Fix major version check for stateless environment

[vapopov]

In this PR local process storage replaced with the backend storage with proper resource serialization. For current self-hosted clusters preserved last known version must be migrated to backend storage.

Related: #49848
Changelog: Fixed major version check for stateless environment

[vapopov]

We would probably need to check inventory store to identify version sets for the auth servers, for instance if we have in cluster auth1:v17.0.0, auth2:v18.0.0 - auth1 should be restricted to be downgraded to v16, and auth2 to be upgraded to v19. Not clear if we able to retrieve full information from inventory stream in auth initialization step. tbd

[vapopov]

Also was trying in parallel PR to reuse current PresenceService which stores the Teleport version in auth_server spec (to not create additional resource), but LocalSupervisor seems like keep updating this info at every start even if auth service is failed to init.

[hugoShaka]

If I understand correctly, during the startup:

• the auth infos are listed on startup to discover the lowest and highest major versions
• teleport writes its own authinfo resource with a TTL of 24h

I see several issues with the current approach:

• the auth info write happens on startup, if no auth got restarted in the last 24 hours, all authinfo resources are expired and the check becomes useless as it cannot figure the previous auth version
• it is not possible to perform sequentially two major updates the same day

Note: there's a variant of the first issue where no auth were running in the last 24 hours, this often happens when people forget to clear their dynamo backend and a new cluster reuses the old backend. So making the authInfo writes periodic might not completely solve the issue.

I don't have another solution to suggest yet but I would expect the version check to have the following properties:

• consistently works even if the cluster was downscale for a week
• supports every officially supported version transitions (e.g. updating from v15 to v16 and v16 to v17 the same day, as long as no 2 auths are running concurrently)

[vapopov]

@hugoShaka I was mostly worried case when after downscale you wont be able to update other auth instances because of existing records of downscaled instances, this is why expiry is set. We might just make it simple as possible and always check per host id instead of range of versions (but this don't prevent the case when we have v17 and v16 instances running in cluster to downgrade v16 one to v15, there might be the issues with roles registration in storage for instance, like deprecating or adding new role in new major version).

Another option I thought about is to use inventory storage and check the version for only online nodes (this one requires time to propagate hello messages from inventory stream, so we need to delay auth initialization). Correct me if I wrong.

[hugoShaka]

We might just make it simple as possible and always check per host id instead of range of versions

This will not work on stateless deployments as hostIDs are not persisted.

[hugoShaka]

I wonder if a single non-expiring cluster-wide BackendInfo resource would work. Something like:

• auth starts
• auth gets BackendInfo
  • if BackendInfo version is too low we crash
  • if BackendInfo is lower than us, we update backend info
  • if BackendInfo is higher than us, we don't change anything
• auth continues to start and runs its migrations
• (optionally) auth updates BackendInfo again to note that it finished applied its migrations

With optimistic locking we should be protected against races. The resource would represent pretty closely the backend state.

Another variant would be to create one BackendInfo per Teleport version, on startup we would list all backend infos and look at the highest. This would be closer to a regular database migration mechanism and the resource would represent the history of the backend migrations, which might be useful.

cc @espadolini @rosstimothy and @fspmarshall

[espadolini]

You can't do conditional operations on key ranges (and even fetching key ranges is sort of wobbly and slightly inconsistent, time-wise), so the logic becomes a lot more awkward if we have to make decisions based on a range of items.

I'm +1 for recording "the version" of the cluster in a single item and conditionally updating it at auth start, with the understanding that this only acts as a lint and not as a hard compatibility guarantee (the only way to truly do that would be to make every read and write an atomic operation that also checks the version item, AFAICT). We could reuse --skip-version-check (which only currently works on proxies and peripheral agents) to bypass the check to allow for downgrading, too.

[hugoShaka]

This looks way better, thanks for taking the time to do all the changes again.

cc @espadolini for another review 🙏

[hugoShaka]

The unconditional put makes racing possible, someone might have updated the version in the meantime. This operation should use conditional updates based on revision. I think we have all the machinery available in the generic service struct.

[vapopov]

I'm changing this one, but not sure if it is good to fail auth init because when we do parallel scale of auth instances and spawning same auth versions in cluster, they have to conditionally write to the same key and theoretically if they launched and the same second one of the instances must fail to write and restart

[espadolini]

Every time you're doing a read-modify-write you should handle the possibility of having to retry - it would be really weird to have to retry more than a handful of times here, so maybe give it a completely unscientific 5 attempts before giving up?

[vapopov]

@espadolini @hugoShaka could you please have another look?

[espadolini]

If this is just going to write the version you can just use upsert, the point of conditional operations is that you read the current value, you make a decision based on the value and then you update the value as long as nothing else changed it. With this implementation, a cluster running 16.3.0 that happens to launch a 16.4.2 and a 17.3.3 auth at the same time might end up with a stored version of 16.4.2 if the write of the first auth ends up happening later than the write of the second.

This function should be called UpdateTeleportVersion or something along those lines, and it should take the revision of the previous auth_info resource.

[vapopov]

make sense, I've moved retry logic to version check function, where we request auth_info resource with revision and retry if its already created or already updated by another process

[vapopov]

@espadolini @hugoShaka hope I've addressed all your comments, if I miss something let me know

[backport-bot-workflows]

@vapopov See the table below for backport results.

Branch	Result
branch/v15	Failed
branch/v16	Failed
branch/v17	Failed