Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions influxdb3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ libc.workspace = true
num_cpus.workspace = true
parking_lot.workspace = true
rand.workspace = true
reqwest.workspace = true
rustls.workspace = true
secrecy.workspace = true
serde.workspace = true
Expand Down
8 changes: 7 additions & 1 deletion influxdb3/src/commands/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,13 @@ pub async fn command(config: Config) -> Result<(), Box<dyn Error>> {
println!("Trigger {} deleted successfully", trigger_name);
}
SubCommand::Token(TokenConfig { token_name, .. }) => {
if token_name == "_admin" {
println!(
"The operator token \"_admin\" is required and cannot be deleted. To regenerate an operator token, use: influxdb3 create token --admin --regenerate --token [TOKEN]"
);
Copy link
Copy Markdown
Contributor

@jdstrand jdstrand May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praveen-influx - the spec does not say whether the 'name' of a token is immutable or mutable. I recall thinking that since the actual operations under the hood would be on the immutable 'id', then names would be mutable.

If names are immutable, this if token_name == "_admin" is ok. If they are not, or we plan to change them in the future, this could be a problem. If they are (or we want them to become) mutable, we should:

  • not allow the operator token (named _admin) to be renamed
  • not allow duplicate token names (do we already enforce this?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not allow duplicate token names (do we already enforce this?)

Yes this is enforced.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are (or we want them to become) mutable, we should:

  • not allow the operator token (named _admin) to be renamed

When we allow changing the token names (i.e update feature issue), operator token will not be mutable

  • not allow duplicate token names (do we already enforce this?)

This is already enforced in the model (names are unique)

return Ok(());
}

println!(
"Are you sure you want to delete {:?}? Enter 'yes' to confirm",
token_name
Expand All @@ -303,7 +310,6 @@ pub async fn command(config: Config) -> Result<(), Box<dyn Error>> {
println!("Cannot delete token without confirmation");
} else {
client.api_v3_configure_token_delete(&token_name).await?;

println!("Token {:?} deleted successfully", &token_name);
}
}
Expand Down
18 changes: 12 additions & 6 deletions influxdb3/tests/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod api;

use crate::server::{ConfigProvider, TestServer, parse_token};
use assert_cmd::Command as AssertCmd;
use observability_deps::tracing::{debug, info};
use observability_deps::tracing::debug;
use pretty_assertions::assert_eq;
use serde_json::{Value, json};
use std::fs::File;
Expand Down Expand Up @@ -2991,7 +2991,7 @@ async fn test_create_admin_token_allowed_once() {
.unwrap();
assert_contains!(
&result,
"Failed to create token, error: ApiError { code: 500, message: \"token name already exists, _admin\" }"
"Failed to create token, error: ApiError { code: 409, message: \"token name already exists, _admin\" }"
);
}

Expand All @@ -3006,7 +3006,7 @@ async fn test_regenerate_admin_token() {
// already has admin token, so it cannot be created again
assert_contains!(
&result,
"Failed to create token, error: ApiError { code: 500, message: \"token name already exists, _admin\" }"
"Failed to create token, error: ApiError { code: 409, message: \"token name already exists, _admin\" }"
);

// regenerating token is allowed
Expand Down Expand Up @@ -3060,7 +3060,7 @@ async fn test_delete_token() {
let token = parse_token(result);

let result = server
.run_with_confirmation(
.run(
vec!["delete", "token"],
&[
"--token-name",
Expand All @@ -3072,7 +3072,10 @@ async fn test_delete_token() {
],
)
.unwrap();
info!(result, "test: deleted token using token name");
assert_contains!(
result,
"The operator token \"_admin\" is required and cannot be deleted. To regenerate an operator token, use: influxdb3 create token --admin --regenerate --token [TOKEN]"
);

// you should be able to create the token again
let result = server
Expand All @@ -3087,7 +3090,10 @@ async fn test_delete_token() {
args,
)
.unwrap();
assert_contains!(&result, "New token created successfully!");
assert_contains!(
&result,
"Failed to create token, error: ApiError { code: 409, message: \"token name already exists, _admin\" }"
);
}

#[test_log::test(tokio::test)]
Expand Down
28 changes: 3 additions & 25 deletions influxdb3/tests/server/configure.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use hyper::StatusCode;
use influxdb3_types::http::CreateTokenWithPermissionsResponse;
use observability_deps::tracing::{debug, info};
use pretty_assertions::assert_eq;
use serde_json::{Value, json};
Expand Down Expand Up @@ -1365,7 +1364,6 @@ async fn api_v3_configure_token_delete() {
let delete_url = format!("{base}/api/v3/configure/token", base = server.client_addr());

let admin_token = server.token().expect("admin token to be present");

let delete_result = client
.delete(&delete_url)
.bearer_auth(admin_token)
Expand All @@ -1374,32 +1372,12 @@ async fn api_v3_configure_token_delete() {
.await
.unwrap();
info!(?delete_result, "test: result running the token delete");
assert_eq!(delete_result.status(), StatusCode::METHOD_NOT_ALLOWED);

// create admin token again
let result = client.post(&create_url).send().await.unwrap();
info!(?result, "test: result running the create token");
assert_eq!(result.status(), StatusCode::CREATED);
let json: CreateTokenWithPermissionsResponse = result.json().await.unwrap();
info!(?json, "test: result running the token delete");
assert_eq!(json.id, 1);

// delete again
let delete_result = client
.delete(&delete_url)
.bearer_auth(&json.token)
.query(&[("token_name", token_name)])
.send()
.await
.unwrap();
info!(?delete_result, "test: result running the token delete");

// create admin token once again
// create admin token again - this will fail as operator token already exists
let result = client.post(&create_url).send().await.unwrap();
info!(?result, "test: result running the create token");
assert_eq!(result.status(), StatusCode::CREATED);
let json: CreateTokenWithPermissionsResponse = result.json().await.unwrap();
info!(?json, "test: result running the token delete");
assert_eq!(json.id, 2);
assert_eq!(result.status(), StatusCode::CONFLICT);
}

#[test_log::test(tokio::test)]
Expand Down
12 changes: 6 additions & 6 deletions influxdb3_catalog/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub const TIME_COLUMN_NAME: &str = "time";

pub const INTERNAL_DB_NAME: &str = "_internal";

const DEFAULT_ADMIN_TOKEN_NAME: &str = "_admin";
const DEFAULT_OPERATOR_TOKEN_NAME: &str = "_admin";

/// Limit for the number of tag columns on a table
pub(crate) const NUM_TAG_COLUMNS_LIMIT: usize = 250;
Expand Down Expand Up @@ -458,7 +458,7 @@ impl Catalog {
.read()
.tokens
.repo()
.get_by_name(DEFAULT_ADMIN_TOKEN_NAME);
.get_by_name(DEFAULT_OPERATOR_TOKEN_NAME);

if default_admin_token.is_none() {
return Err(CatalogError::MissingAdminTokenToUpdate);
Expand All @@ -482,10 +482,10 @@ impl Catalog {
.read()
.tokens
.repo()
.contains_name(DEFAULT_ADMIN_TOKEN_NAME)
.contains_name(DEFAULT_OPERATOR_TOKEN_NAME)
{
return Err(CatalogError::TokenNameAlreadyExists(
DEFAULT_ADMIN_TOKEN_NAME.to_owned(),
DEFAULT_OPERATOR_TOKEN_NAME.to_owned(),
));
}

Expand All @@ -501,7 +501,7 @@ impl Catalog {
time_ns: created_at,
ops: vec![TokenCatalogOp::CreateAdminToken(CreateAdminTokenDetails {
token_id,
name: Arc::from(DEFAULT_ADMIN_TOKEN_NAME),
name: Arc::from(DEFAULT_OPERATOR_TOKEN_NAME),
hash: hash.clone(),
created_at,
updated_at: None,
Expand All @@ -517,7 +517,7 @@ impl Catalog {
.read()
.tokens
.repo()
.get_by_name(DEFAULT_ADMIN_TOKEN_NAME)
.get_by_name(DEFAULT_OPERATOR_TOKEN_NAME)
.expect("token info must be present after token creation by name")
};

Expand Down
7 changes: 6 additions & 1 deletion influxdb3_catalog/src/catalog/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use super::{
};
use crate::{
CatalogError, Result,
catalog::NUM_TAG_COLUMNS_LIMIT,
catalog::{DEFAULT_OPERATOR_TOKEN_NAME, NUM_TAG_COLUMNS_LIMIT},
log::{
AddFieldsLog, CatalogBatch, CreateDatabaseLog, CreateTableLog, DatabaseCatalogOp,
DeleteDistinctCacheLog, DeleteLastCacheLog, DeleteTokenDetails, DeleteTriggerLog,
Expand Down Expand Up @@ -681,6 +681,11 @@ impl Catalog {

pub async fn delete_token(&self, token_name: &str) -> Result<OrderedCatalogBatch> {
info!(token_name, "delete token");

if token_name == DEFAULT_OPERATOR_TOKEN_NAME {
return Err(CatalogError::CannotDeleteOperatorToken);
}

self.catalog_update_with_retry(|| {
if !self.inner.read().tokens.repo().contains_name(token_name) {
// maybe deleted by another node or genuinely not present
Expand Down
3 changes: 3 additions & 0 deletions influxdb3_catalog/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ pub enum CatalogError {

#[error("tried to stop a node ({node_id}) that is already stopped")]
NodeAlreadyStopped { node_id: Arc<str> },

#[error("cannot delete operator token")]
CannotDeleteOperatorToken,
}

impl CatalogError {
Expand Down
8 changes: 8 additions & 0 deletions influxdb3_server/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,14 @@ impl IntoResponse for Error {
fn into_response(self) -> Response<Body> {
debug!(error = ?self, "API error");
match self {
Self::Catalog(err @ CatalogError::CannotDeleteOperatorToken) => Response::builder()
.status(StatusCode::METHOD_NOT_ALLOWED)
.body(Body::from(err.to_string()))
.unwrap(),
Self::Catalog(err @ CatalogError::TokenNameAlreadyExists { .. }) => Response::builder()
.status(StatusCode::CONFLICT)
.body(Body::from(err.to_string()))
.unwrap(),
Self::Catalog(err) | Self::WriteBuffer(WriteBufferError::CatalogUpdateError(err)) => {
err.into_response()
}
Expand Down