ACL Token Persistence and Reloading #5328
Conversation
mkeeler
added
theme/acls
| } | ||
|
|
||
| func (a *Agent) loadTokens(conf *config.RuntimeConfig) error { | ||
| persistedTokens, persistenceErr := a.getPersistedTokens() |
There was a problem hiding this comment.
Why not hard-fail if the file is unreadable or corrupt? It seems like an unlikely event that should warrant extraordinary operational attention.
There was a problem hiding this comment.
I guess it could be a hard fail. I modeled this off of other load* functions which did similarly.
There was a problem hiding this comment.
Should a corrupt file prevent the agent from starting at all? Thats what would happen with a hard fail.
There was a problem hiding this comment.
Should corrupt persisted services prevent the agent from starting up?. I think the answer is no and we should instead just log the warning.
There was a problem hiding this comment.
If we go for worst case scenario, the agent isn't going to be terribly useful on restart if the sole store of tokens is that file, it's corrupt, and the operator is has a full belt-and-suspenders ACL setup configured.
Sure the agent will start, but it won't have any acl tokens with which to operate. If you had a fleet of hundreds of agents how can you tell the difference between an operational one and one that starts up but has no valid tokens?
But then I guess this is nearly indistinguishable from an agent having all of its acl tokens be rotated into the bit bucket already today.
There was a problem hiding this comment.
Yes, but with the agent up you can remedy the situation by PUTing new tokens to the API. Which is better than needing to, remote in, blow away/fix the corrupt file and restart (maybe still need to repost up tokens depending on the corruption).
There was a problem hiding this comment.
Well if you were being secure about it, the API would only be bound to loopback anyway, so remoting in is still required.
There was a problem hiding this comment.
I disagree that the only secure way to enable the API is bound to loopback. If using HTTPs + ACLs I would allow it on the LAN.
Additionally, even if not able to directly connect to the API I would say that doing a consul acl set-agent-token ... is much easier/reliable then having to hand edit files (which is still always a viable alternative).
mkeeler
force-pushed
the
feature/acl-token-persistence
branch
2 times, most recently
from
ac1f9c5 to
755801c
Compare
Before it sounded to ACL specific (disregarding the fact that it is an ACL token). This token also gets used for connect replication. Also the agent API for setting tokens now recognizes simpler names in the URL for setting the tokens which match their names in the consul configuration. The API has a few new functions but left the others in place for backwards compatibility.
Co-Authored-By: mkeeler <mkeeler@users.noreply.github.com>
mkeeler
force-pushed
the
feature/acl-token-persistence
branch
from
d6a09a5 to
70b6dd2
Compare
agent/agent_endpoint.go
Outdated
|
|
||
| default: | ||
| resp.WriteHeader(http.StatusNotFound) | ||
| fmt.Fprintf(resp, "Token %q is unknown", target) | ||
| return nil, nil | ||
| } | ||
|
|
||
| if s.agent.config.ACLEnableTokenPersistence { | ||
| tokens := make(map[string]string) |
There was a problem hiding this comment.
The persistedTokens struct could be used here as well
When setting a token if a 404 is returned then we will fallback to the original names to make the request.
This PR adds two features which will be useful for operators when ACLs are in use.
acl.enable_token_persistenceis set totruein the configuration, tokens set via thev1/agent/tokenendpoint are now persisted to disk and loaded when the agent starts (or during configuration reload)Note that token persistence is opt-in so our users who do not want tokens on the local disk will see no change.
Some other secondary changes:
v1/agent/token/API. Instead of paths like:v1/agent/token/acl_replication_tokenthe path can now be justv1/agent/token/replication. The old paths remain to be valid.