feat(phase-2): fix admin service-account create defaults for rustfs

Author: overtrue

crates/cli/src/commands/admin/mod.rs

@@ -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) => {
@@ -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));
@@ -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("/"), "/");
+    }
 }

crates/cli/src/commands/admin/service_account.rs

@@ -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 {
@@ -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) => {
@@ -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(),
+    }
+}
+
 async fn execute_info(args: InfoArgs, formatter: &Formatter) -> ExitCode {
     let client = match get_admin_client(&args.alias, formatter) {
         Ok(c) => c,
@@ -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)
+        );
+    }
+
+    #[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}"));
+    }
 }

crates/core/src/admin/types.rs

@@ -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>,
 
@@ -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,
@@ -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}"
         );
-
-        let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
-        assert!(parsed.get("expiration").is_some());
-        assert!(parsed["expiration"].is_null());
     }
 
     #[test]