-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
acl: bootstrap master token from config file #7935
Conversation
35ef359
to
9216d1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the excellent PR @skarrok! From a code perspective it looks right on.
However, while we know our existing management token bootstrapping method makes automating cluster setup difficult, we're unsure about hardcoding a management token like Consul allows. A one-time (or limited-time) use token might ease bootstrapping while greatly diminish the attack surface of a master token ended in a config file.
I'm going to mark this as 🤔 for now. I don't want to close it outright as we may decide the risk is worth it since the bootstrapping is such a pain.
} | ||
if token == nil { | ||
token = &structs.ACLToken{ | ||
AccessorID: uuid.Generate(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to allow the AccessorID to be specified in the HCL as well? Consul doesn't for their master_token
, but they do allow it for subsequent tokens created via their API in order to ease automation.
Thank you for your review! Yes, current process is not ideal. We even choose to not use acl in nomad in favor of fully automated idempotent bootstrapping. I'm sure strong arguments for and against have been made already when this feature landed in consul, so I'm gonna wait for decision. |
@schmichael Hi, I have been thinking about your comment and about what would be an easier approach for automation without hard-coding the token in the config file. I only suggested that in #7777 because consul already does it and I like consistency. What do you think about altering the |
@schmichael what do you think of closing this PR in lieu of the work that @lhaig is doing in #12520? I think that's closer to the suggestion you and @apollo13 had here. |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #7777
Add support for acl
master_token
like in Consul