Adds a new /v1/acl/bootstrap API #3349
Conversation
| // snapshots would become incompatible with older agents if we added this on | ||
| // the fly, so we rely on the leader to determine a safe time to add this so | ||
| // we can start tracking whether bootstrap is enabled. This will return an | ||
| // error if bootstrap is already initialized. |
There was a problem hiding this comment.
What's the reason for having this init step as its own separate thing, rather than just checking for the nonce/existing management tokens and server versions when we get a request to bootstrap?
There was a problem hiding this comment.
Good question! It's not really safe to add this until we know all the servers are upgraded, and the state store doesn't know about that at all, so I wrote the state store side to do all the tracking or not do the tracking based on the existence of this record. Then we rely on the leader loop to create this and basically tell the state store that it's ok to start tracking.
My first cut at this just checked for everything when you try to bootstrap, but it felt dangerous to delay that too much. One case I was worried about is if they somehow accidentally deleted their last management token, then bootstrapping would suddenly become available. By proactively doing the check as early as possible and keeping the state we prevent cases like that.
| } | ||
|
|
||
| // See if this cluster has already been bootstrapped. | ||
| bs := *existing.(*structs.ACLBootstrap) |
There was a problem hiding this comment.
why does is not have the second boolean arg for the type cast, to check if that fails?
There was a problem hiding this comment.
We checked for nil above and this only holds these kinds of objects so it's basically a coding error if this fails, so in the state store we usually omit the check for these and just let it panic if it ever happens.
| if err := tx.Insert("acls-bootstrap", &bs); err != nil { | ||
| return false, fmt.Errorf("failed creating acl bootstrap: %v", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
in ACLBoostrap below after we bootstrap, we immediately call aclDisableBootstrapTxn to disable it forever. But here we only insert. I am still a bit lost on why we have two different methods that look similar.
There was a problem hiding this comment.
In this one we make sure the record doesn't already exist, and then determine if bootstrapping should be allowed, then insert it with that set to true/false. Down in bootstrap we want to mark this record such that bootstrapping can never occur again, so we set it to false.
There was a problem hiding this comment.
There's a different set of pre-checks that are done in each case, so we have these two different methods.
There was a problem hiding this comment.
Oh also "insert" is used to create records in memdb and to update an existing record by replacing it with a new one, that can be a little confusing.
This change complements #3324 by creating a new API endpoint for bootstrapping ACLs. If a master token isn't configured, and the cluster has never been bootstrapped before, then /v1/acl/bootstrap can be used to generate the initial management token for a cluster without ever having to place a token into a configuration file.