Registration simplification #366
Conversation
This commit stores temporary registration data in cache, instead of memory allowing us to only have actually registered machines in the database.
This commit removes the field from the database and does a DB migration **removing** all unregistered machines from headscale. This means that from this version, all machines in the database is considered registered.
This commit removes the two extra caches (oidc, requested time) and uses the new central registration cache instead. The requested time is unified into the main machine object and the oidc key is just added to the same cache, as a string with the state as a key instead of machine key.
| google.protobuf.Timestamp last_seen = 10; | ||
| google.protobuf.Timestamp last_successful_update = 11; | ||
| google.protobuf.Timestamp expiry = 12; | ||
| google.protobuf.Timestamp last_seen = 8; |
There was a problem hiding this comment.
Is it ok to change theses keys in protobuf ? I thought that we couldn't change these without breaking compatibility
There was a problem hiding this comment.
Yes it will, I think we should change it still, so make sure we don't start accumulating tech debt before the api is stable.
There was a problem hiding this comment.
Of course having debt without a first version is nonsense 👍
| AuthKeyID uint | ||
| AuthKey *PreAuthKey | ||
|
|
||
| // TODO(kradalby): This seems like irrelevant information? |
There was a problem hiding this comment.
You can see which machine has been registered with what key
There was a problem hiding this comment.
Yes, but is it really relevant? I delete the keys quite often so not sure
machine.go
Outdated
| machine.NamespaceID = namespace.ID | ||
|
|
||
| // TODO(kradalby): This field is uneccessary metadata, | ||
| // move it to tags instead of having a column. |
There was a problem hiding this comment.
tags of the node? to be used in acls?
There was a problem hiding this comment.
That was the idea, but not sure if it makes sense. Might just keep it around for now.
|
@juanfont Please take another look, there was a flow breaker in the OpenID login. |
Resolves #294, #341, #365
This PR simplifies registration in a couple of ways:
DRY the registration logic into a function in
machine.goWe had quite a lot of duplicate logic as noted in Machine allocation logic exists in multiple locations. #294, and this reduces this and centers the logic around RegisterMachine.
Only write machines/nodes to the database when they are registered
In the current setup, we write the machine to the database when the registration process starts, and this means that machines that dont successfully register becomes ghost machines in the DB. They dont really have anything to do in persistent storage until successful, so this PR moves them to a cache like memory storage which is cleaned up occasionally.
Remove
Registeredcolumn, useExpiryas main machine filterSince we dont have machines that isnt "unregistered" anymore, we can simplify a lot of logic and only check if a machine exist, and if it is expired or not. This makes it easier to reason about if we should care for a machine in particular scenarios (no more
if registered and !expired).