Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 27 additions & 1 deletion crates/cli/src/commands/admin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,19 @@ pub async fn execute(cmd: AdminCommands, output_config: OutputConfig) -> ExitCod
}
}

fn normalize_admin_alias(alias_name: &str) -> &str {
let normalized_alias = alias_name.trim_end_matches('/');
if normalized_alias.is_empty() {
alias_name
} else {
normalized_alias
}
}

/// Helper to get AdminClient from an alias name
pub fn get_admin_client(alias_name: &str, formatter: &Formatter) -> Result<AdminClient, ExitCode> {
let alias_lookup_name = normalize_admin_alias(alias_name);

let alias_manager = match AliasManager::new() {
Ok(am) => am,
Err(e) => {
Expand All @@ -69,7 +80,7 @@ pub fn get_admin_client(alias_name: &str, formatter: &Formatter) -> Result<Admin
}
};

let alias = match alias_manager.get(alias_name) {
let alias = match alias_manager.get(alias_lookup_name) {
Ok(a) => a,
Err(rc_core::Error::AliasNotFound(_)) => {
formatter.error(&format!("Alias '{}' not found", alias_name));
Expand Down Expand Up @@ -146,4 +157,19 @@ mod tests {
_ => panic!("Unexpected command parsing result"),
}
}

#[test]
fn test_normalize_admin_alias_trailing_slash() {
assert_eq!(normalize_admin_alias("local/"), "local");
}

#[test]
fn test_normalize_admin_alias_without_trailing_slash() {
assert_eq!(normalize_admin_alias("local"), "local");
}

#[test]
fn test_normalize_admin_alias_only_slash() {
assert_eq!(normalize_admin_alias("/"), "/");
}
}
68 changes: 60 additions & 8 deletions crates/cli/src/commands/admin/service_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use crate::exit_code::ExitCode;
use crate::output::Formatter;
use rc_core::admin::{AdminApi, CreateServiceAccountRequest, ServiceAccount};

const DEFAULT_SERVICE_ACCOUNT_EXPIRY: &str = "9999-12-31T23:59:59Z";

/// Service account management subcommands
#[derive(Subcommand, Debug)]
pub enum ServiceAccountCommands {
Expand Down Expand Up @@ -205,14 +207,7 @@ async fn execute_create(args: CreateArgs, formatter: &Formatter) -> ExitCode {
None
};

let request = CreateServiceAccountRequest {
policy,
expiry: args.expiry,
name: args.name,
description: args.description,
access_key: args.access_key.clone(),
secret_key: args.secret_key.clone(),
};
let request = build_create_service_account_request(&args, policy);

match client.create_service_account(request).await {
Ok(sa) => {
Expand Down Expand Up @@ -244,6 +239,24 @@ async fn execute_create(args: CreateArgs, formatter: &Formatter) -> ExitCode {
}
}

fn build_create_service_account_request(
args: &CreateArgs,
policy: Option<String>,
) -> CreateServiceAccountRequest {
CreateServiceAccountRequest {
policy,
expiry: args
.expiry
.clone()
.or_else(|| Some(DEFAULT_SERVICE_ACCOUNT_EXPIRY.to_string())),
// Keep existing CLI UX where positional ACCESS_KEY is enough.
name: args.name.clone().or_else(|| Some(args.access_key.clone())),
description: args.description.clone(),
access_key: args.access_key.clone(),
secret_key: args.secret_key.clone(),
}
Comment thread
overtrue marked this conversation as resolved.
}

async fn execute_info(args: InfoArgs, formatter: &Formatter) -> ExitCode {
let client = match get_admin_client(&args.alias, formatter) {
Ok(c) => c,
Expand Down Expand Up @@ -346,4 +359,43 @@ mod tests {
assert!(info.secret_key.is_some());
assert_eq!(info.parent_user, Some("admin".to_string()));
}

#[test]
fn test_build_create_request_uses_access_key_as_default_name() {
let args = CreateArgs {
alias: "local".to_string(),
access_key: "AKIAIOSFODNN7EXAMPLE".to_string(),
secret_key: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY".to_string(),
name: None,
description: None,
policy: None,
expiry: None,
};

let request = build_create_service_account_request(&args, None);
assert_eq!(request.name.as_deref(), Some("AKIAIOSFODNN7EXAMPLE"));
assert_eq!(
request.expiry.as_deref(),
Some(DEFAULT_SERVICE_ACCOUNT_EXPIRY)
);
Comment thread
overtrue marked this conversation as resolved.
Outdated
}

#[test]
fn test_build_create_request_keeps_explicit_name() {
let args = CreateArgs {
alias: "local".to_string(),
access_key: "AKIAIOSFODNN7EXAMPLE".to_string(),
secret_key: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY".to_string(),
name: Some("manual-name".to_string()),
description: Some("demo".to_string()),
policy: None,
expiry: Some("2030-01-01T00:00:00Z".to_string()),
};

let request = build_create_service_account_request(&args, Some("{\"x\":1}".to_string()));
assert_eq!(request.name.as_deref(), Some("manual-name"));
assert_eq!(request.description.as_deref(), Some("demo"));
assert_eq!(request.expiry.as_deref(), Some("2030-01-01T00:00:00Z"));
assert_eq!(request.policy.as_deref(), Some("{\"x\":1}"));
}
}
16 changes: 7 additions & 9 deletions crates/core/src/admin/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,10 @@ pub struct CreateServiceAccountRequest {
#[serde(skip_serializing_if = "Option::is_none")]
pub policy: Option<String>,

/// Expiration time (ISO 8601). The `expiration` field must be present in the request body; use null when no expiration is set.
/// Expiration time (ISO 8601)
///
/// RustFS latest rejects `"expiration": null`, so omit this field when unset.
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(rename = "expiration")]
pub expiry: Option<String>,
Comment thread
overtrue marked this conversation as resolved.
Outdated

Expand Down Expand Up @@ -487,7 +490,7 @@ mod tests {
}

#[test]
fn test_create_service_account_request_includes_expiration() {
fn test_create_service_account_request_omits_expiration_when_none() {
let request = CreateServiceAccountRequest {
policy: None,
expiry: None,
Expand All @@ -498,15 +501,10 @@ mod tests {
};

let json = serde_json::to_string(&request).unwrap();
// expiration field must always be present even when None
assert!(
json.contains("\"expiration\""),
"JSON must contain expiration field, got: {json}"
!json.contains("\"expiration\""),
"JSON must not contain expiration field when unset, got: {json}"
);
Comment thread
overtrue marked this conversation as resolved.

let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
assert!(parsed.get("expiration").is_some());
assert!(parsed["expiration"].is_null());
}

#[test]
Expand Down