-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat: Add offline generation for admin tokens #26734
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
Conversation
I only skimmed the code and didn't review this, but I wonder if |
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.
Two things I'm requesting:
- The
--permission-tokens-file
arg (see comment) can be removed (Enterprise only?) - The
influxdb3 serve --help-all
needs to be updated with the new--admin-token-file
arg
I think either of those may have implications on the PR in Enterprise, e.g., the --permission-tokens-file
could be moved to the influxdb3_enterprise/clap_blocks/src/serve.rs
module; and, the influxdb3 serve --help-all
would also need to be updated on the Enterprise PR as well.
I also left a comment re: atomic writes, but that was just a suggestion.
/// Write a file atomically by writing to a temporary file and moving it into place | ||
/// This ensures the file either exists with correct permissions or doesn't exist at all | ||
pub(crate) fn write_file_atomically(path: &PathBuf, content: &str) -> Result<(), Box<dyn Error>> { |
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.
It occurred to me that the object_store
crate was implemented to ensure atomicity of writes. An alternative to this method would be to spin up a LocalFileSystem
and do a PUT
. Under the hood, I believe the PUT
implementation on LocalFileSystem
is doing what is done here (write to temp file then use a move as a single atomic operation).
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.
Since this was already approved by security I'm a little hesitant to spin up the LocalFileSystem to do this. I don't doubt it, but getting these changes in to both core and enterprise I think we will want to hold off on it for now and consider doing this in a followup ticket.
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.
I agree - don't want to make this change now but I thought this was worth noting.
@jdstrand I think it could be useful for admin, but I think given time we can open an issue for it and punt it till a later release |
This commit adds the ability to generate authentication tokens offline that can then be loaded up by the database at runtime if the tokens do not already exist. This is perfect for automated deployments and containerized environments. This is the implementation for Core which only has admin tokens. The Enterprise counterpart to this change also includes permissions based tokens. CLI Usage ```bash # Generate offline admin token influxdb3 create token --admin --offline --output-file ./config/admin_token.txt ``` Server Configuration The server can load tokens from files on startup: - Admin token: `--admin-token-file ./config/admin_token.txt` Closes #26437
3a512e7
to
e3bf74f
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.
This commit adds the ability to generate authentication tokens offline that can then be loaded up by the database at runtime if the tokens do not already exist. This is perfect for automated deployments and containerized environments. This is the implementation for Core which only has admin tokens. The Enterprise counterpart to this change also includes permissions based tokens.
CLI Usage
# Generate offline admin token influxdb3 create token --admin --offline --output-file ./config/admin_token.txt
Server Configuration
The server can load tokens from files on startup:
--admin-token-file ./config/admin_token.txt
Closes #26437
Note the Enterprise PR is here: https://github.com/influxdata/influxdb_pro/pull/977