From 6c223e9dfb1e55dbaa513df951440e41a0b70e2e Mon Sep 17 00:00:00 2001 From: cxymds Date: Mon, 20 Jul 2026 19:37:18 +0800 Subject: [PATCH] feat(phase-2): support service account updates --- README.md | 5 +- .../cli/src/commands/admin/service_account.rs | 165 +++++++++++++++++- crates/cli/tests/admin_service_account.rs | 100 +++++++++++ crates/cli/tests/help_contract.rs | 12 ++ crates/core/src/admin/mod.rs | 9 +- crates/core/src/admin/types.rs | 73 ++++++++ crates/s3/src/admin.rs | 50 +++++- 7 files changed, 409 insertions(+), 5 deletions(-) create mode 100644 crates/cli/tests/admin_service_account.rs diff --git a/README.md b/README.md index 4e35e63..be1dad8 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,9 @@ rc admin service-account create local/ AKIAIOSFODNN7EXAMPLE wJalrXUtnFEMI/K7MDEN # Create a service account with inline policy file rc admin service-account create local/ SAKEY123 SASECRET123 --policy ./service-account-policy.json +# Update selected fields on an existing service account +rc admin service-account update local/ SAKEY123 --policy ./service-account-policy.json --description "Automation access" + # Inspect any access key and resolve whether it belongs to a user, service account, or STS credential rc admin access-key info local/ AKIAIOSFODNN7EXAMPLE rc admin access-key info local/ AKIAIOSFODNN7EXAMPLE --json @@ -345,7 +348,7 @@ For full command documentation, see the [`rc` command reference](docs/reference/ | `admin user` | Manage IAM users (add, remove, list, info, enable, disable) | | `admin policy` | Manage IAM policies (create, remove, list, info, attach) | | `admin group` | Manage IAM groups (add, remove, list, info, enable, disable, add-members, rm-members) | -| `admin service-account` | Manage service accounts (create, remove, list, info) | +| `admin service-account` | Manage service accounts (create, update, remove, list, info) | | `admin access-key` | Inspect access key identity and metadata (info) | | `admin info` | Display cluster information (cluster, server, disk) | | `admin heal` | Manage cluster healing operations (status, start, stop) | diff --git a/crates/cli/src/commands/admin/service_account.rs b/crates/cli/src/commands/admin/service_account.rs index 2a7f4f0..2c91baf 100644 --- a/crates/cli/src/commands/admin/service_account.rs +++ b/crates/cli/src/commands/admin/service_account.rs @@ -1,6 +1,6 @@ //! Service account management commands //! -//! Commands for managing service accounts: list, create, info, remove. +//! Commands for managing service accounts: list, create, update, info, remove. use clap::Subcommand; use serde::Serialize; @@ -8,7 +8,9 @@ use serde::Serialize; use super::get_admin_client; use crate::exit_code::ExitCode; use crate::output::Formatter; -use rc_core::admin::{AdminApi, CreateServiceAccountRequest, ServiceAccount}; +use rc_core::admin::{ + AdminApi, CreateServiceAccountRequest, ServiceAccount, UpdateServiceAccountRequest, +}; const DEFAULT_SERVICE_ACCOUNT_EXPIRY: &str = "9999-12-31T23:59:59Z"; @@ -22,6 +24,10 @@ pub enum ServiceAccountCommands { /// Create a new service account Create(CreateArgs), + /// Update an existing service account + #[command(aliases = ["edit", "set"])] + Update(UpdateArgs), + /// Get service account information Info(InfoArgs), @@ -68,6 +74,39 @@ pub struct CreateArgs { pub expiry: Option, } +#[derive(clap::Args, Debug)] +pub struct UpdateArgs { + /// Alias name of the server + pub alias: String, + + /// Access key of the service account + pub access_key: String, + + /// Replacement secret key + #[arg(long)] + pub secret_key: Option, + + /// Replacement name for the service account + #[arg(long)] + pub name: Option, + + /// Replacement description + #[arg(long)] + pub description: Option, + + /// Replacement policy document (JSON file path) + #[arg(long)] + pub policy: Option, + + /// Replacement expiration time (ISO 8601 format) + #[arg(long)] + pub expiry: Option, + + /// Replacement account status + #[arg(long, value_parser = ["enabled", "disabled"])] + pub status: Option, +} + #[derive(clap::Args, Debug)] pub struct InfoArgs { /// Alias name of the server @@ -142,6 +181,7 @@ pub async fn execute(cmd: ServiceAccountCommands, formatter: &Formatter) -> Exit match cmd { ServiceAccountCommands::List(args) => execute_list(args, formatter).await, ServiceAccountCommands::Create(args) => execute_create(args, formatter).await, + ServiceAccountCommands::Update(args) => execute_update(args, formatter).await, ServiceAccountCommands::Info(args) => execute_info(args, formatter).await, ServiceAccountCommands::Remove(args) => execute_remove(args, formatter).await, } @@ -272,6 +312,78 @@ fn should_retry_with_default_expiry(args: &CreateArgs, error: &rc_core::Error) - text.contains("missing field `expiration`") || text.contains("InvalidExpiration") } +async fn execute_update(args: UpdateArgs, formatter: &Formatter) -> ExitCode { + let policy = if let Some(policy_path) = &args.policy { + match std::fs::read_to_string(policy_path) { + Ok(content) => Some(content), + Err(e) => { + formatter.error(&format!( + "Failed to read policy file '{}': {e}", + policy_path + )); + return ExitCode::UsageError; + } + } + } else { + None + }; + + let request = build_update_service_account_request(&args, policy); + if request.is_empty() { + formatter.error("Service account update requires at least one field"); + return ExitCode::UsageError; + } + + let client = match get_admin_client(&args.alias, formatter) { + Ok(c) => c, + Err(code) => return code, + }; + + match client + .update_service_account(&args.access_key, request) + .await + { + Ok(()) => { + let output = ServiceAccountOperationOutput { + success: true, + access_key: args.access_key.clone(), + message: format!("Service account '{}' updated successfully", args.access_key), + }; + if formatter.is_json() { + formatter.json(&output); + } else { + let styled_key = formatter.style_name(&args.access_key); + formatter.success(&format!( + "Service account '{styled_key}' updated successfully." + )); + } + ExitCode::Success + } + Err(rc_core::Error::NotFound(_)) => { + formatter.error(&format!("Service account '{}' not found", args.access_key)); + ExitCode::NotFound + } + Err(e) => { + formatter.error(&format!("Failed to update service account: {e}")); + ExitCode::GeneralError + } + } +} + +fn build_update_service_account_request( + args: &UpdateArgs, + policy: Option, +) -> UpdateServiceAccountRequest { + UpdateServiceAccountRequest { + new_policy: policy, + new_secret_key: args.secret_key.clone(), + new_status: args.status.clone(), + new_name: args.name.clone(), + new_description: args.description.clone(), + new_expiration: args.expiry.clone(), + } +} + async fn execute_info(args: InfoArgs, formatter: &Formatter) -> ExitCode { let client = match get_admin_client(&args.alias, formatter) { Ok(c) => c, @@ -411,6 +523,55 @@ mod tests { assert_eq!(request.policy.as_deref(), Some("{\"x\":1}")); } + #[test] + fn test_build_update_request_keeps_only_explicit_fields() { + let args = UpdateArgs { + alias: "local".to_string(), + access_key: "AKIAIOSFODNN7EXAMPLE".to_string(), + secret_key: None, + name: Some("automation-key".to_string()), + description: Some("Updated description".to_string()), + policy: Some("policy.json".to_string()), + expiry: None, + status: Some("enabled".to_string()), + }; + + let request = build_update_service_account_request( + &args, + Some("{\"Version\":\"2012-10-17\"}".to_string()), + ); + assert_eq!( + request.new_policy.as_deref(), + Some("{\"Version\":\"2012-10-17\"}") + ); + assert!(request.new_secret_key.is_none()); + assert_eq!(request.new_name.as_deref(), Some("automation-key")); + assert_eq!( + request.new_description.as_deref(), + Some("Updated description") + ); + assert!(request.new_expiration.is_none()); + assert_eq!(request.new_status.as_deref(), Some("enabled")); + assert!(!request.is_empty()); + } + + #[test] + fn test_build_update_request_detects_empty_update() { + let args = UpdateArgs { + alias: "local".to_string(), + access_key: "AKIAIOSFODNN7EXAMPLE".to_string(), + secret_key: None, + name: None, + description: None, + policy: None, + expiry: None, + status: None, + }; + + let request = build_update_service_account_request(&args, None); + assert!(request.is_empty()); + } + #[test] fn test_should_retry_with_default_expiry_for_missing_field_error() { let args = CreateArgs { diff --git a/crates/cli/tests/admin_service_account.rs b/crates/cli/tests/admin_service_account.rs new file mode 100644 index 0000000..2faf259 --- /dev/null +++ b/crates/cli/tests/admin_service_account.rs @@ -0,0 +1,100 @@ +#![cfg(not(windows))] + +mod admin_support; + +use std::fs; +use std::process::Command; +use std::time::Duration; + +use admin_support::{rc_binary, rc_host_alias, start_admin_test_server}; + +#[test] +fn service_account_update_dispatches_to_update_endpoint() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let policy_dir = tempfile::tempdir().expect("create policy dir"); + let policy_path = policy_dir.path().join("policy.json"); + fs::write(&policy_path, r#"{"Version":"2012-10-17","Statement":[]}"#) + .expect("write policy file"); + let (endpoint, receiver, handle) = start_admin_test_server(""); + + let output = Command::new(rc_binary()) + .args([ + "--json", + "admin", + "service-account", + "update", + "myalias", + "service-key", + "--policy", + policy_path.to_str().expect("UTF-8 policy path"), + "--description", + "Updated description", + "--secret-key", + "replacement-secret", + ]) + .env("RC_CONFIG_DIR", config_dir.path()) + .env("RC_HOST_myalias", rc_host_alias(&endpoint)) + .output() + .expect("run rc command"); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8"); + assert!(!stdout.contains("replacement-secret")); + let payload: serde_json::Value = serde_json::from_str(&stdout).expect("JSON output"); + assert_eq!(payload["success"], true); + assert_eq!(payload["access_key"], "service-key"); + + let request = receiver + .recv_timeout(Duration::from_secs(5)) + .expect("captured admin request"); + assert_eq!(request.method, "POST"); + assert_eq!( + request.target, + "/rustfs/admin/v3/update-service-account?accessKey=service-key" + ); + handle.join().expect("admin test server finished"); +} + +#[test] +fn service_account_update_requires_at_least_one_field() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let output = Command::new(rc_binary()) + .args([ + "admin", + "service-account", + "update", + "myalias", + "service-key", + ]) + .env("RC_CONFIG_DIR", config_dir.path()) + .output() + .expect("run rc command"); + + assert_eq!(output.status.code(), Some(2)); + assert!(String::from_utf8_lossy(&output.stderr).contains("at least one field")); +} + +#[test] +fn service_account_update_rejects_missing_policy_file() { + let config_dir = tempfile::tempdir().expect("create config dir"); + let output = Command::new(rc_binary()) + .args([ + "admin", + "service-account", + "update", + "myalias", + "service-key", + "--policy", + "/definitely/not/a/policy.json", + ]) + .env("RC_CONFIG_DIR", config_dir.path()) + .output() + .expect("run rc command"); + + assert_eq!(output.status.code(), Some(2)); + assert!(String::from_utf8_lossy(&output.stderr).contains("Failed to read policy file")); +} diff --git a/crates/cli/tests/help_contract.rs b/crates/cli/tests/help_contract.rs index a96a949..7bb3394 100644 --- a/crates/cli/tests/help_contract.rs +++ b/crates/cli/tests/help_contract.rs @@ -759,6 +759,18 @@ fn nested_subcommand_help_contract() { usage: "Usage: rc admin service-account create [OPTIONS] ", expected_tokens: &["--name", "--description", "--policy", "--expiry"], }, + HelpCase { + args: &["admin", "service-account", "update"], + usage: "Usage: rc admin service-account update [OPTIONS] ", + expected_tokens: &[ + "--secret-key", + "--name", + "--description", + "--policy", + "--expiry", + "--status", + ], + }, HelpCase { args: &["admin", "service-account", "info"], usage: "Usage: rc admin service-account info [OPTIONS] ", diff --git a/crates/core/src/admin/mod.rs b/crates/core/src/admin/mod.rs index 969b232..115fb8c 100644 --- a/crates/core/src/admin/mod.rs +++ b/crates/core/src/admin/mod.rs @@ -25,7 +25,7 @@ pub use types::{ AccessKeyDetails, AccessKeyInfo, BucketQuota, CreateServiceAccountRequest, Group, GroupStatus, LdapAccessKeyInfo, OpenIdAccessKeyInfo, Policy, PolicyEntity, PolicyInfo, ServiceAccount, ServiceAccountCreateResponse, ServiceAccountCredentials, SetPolicyRequest, - UpdateGroupMembersRequest, User, UserStatus, + UpdateGroupMembersRequest, UpdateServiceAccountRequest, User, UserStatus, }; use async_trait::async_trait; @@ -170,6 +170,13 @@ pub trait AdminApi: Send + Sync { request: CreateServiceAccountRequest, ) -> Result; + /// Update an existing service account + async fn update_service_account( + &self, + access_key: &str, + request: UpdateServiceAccountRequest, + ) -> Result<()>; + /// Delete a service account async fn delete_service_account(&self, access_key: &str) -> Result<()>; diff --git a/crates/core/src/admin/types.rs b/crates/core/src/admin/types.rs index 7c4edbc..b25fbd1 100644 --- a/crates/core/src/admin/types.rs +++ b/crates/core/src/admin/types.rs @@ -428,6 +428,47 @@ pub struct CreateServiceAccountRequest { pub secret_key: String, } +/// Request to update an existing service account +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct UpdateServiceAccountRequest { + /// Replacement policy document (JSON string) + #[serde(skip_serializing_if = "Option::is_none")] + pub new_policy: Option, + + /// Replacement secret key + #[serde(skip_serializing_if = "Option::is_none")] + pub new_secret_key: Option, + + /// Replacement account status + #[serde(skip_serializing_if = "Option::is_none")] + pub new_status: Option, + + /// Replacement friendly name + #[serde(skip_serializing_if = "Option::is_none")] + pub new_name: Option, + + /// Replacement description + #[serde(skip_serializing_if = "Option::is_none")] + pub new_description: Option, + + /// Replacement expiration time (ISO 8601) + #[serde(skip_serializing_if = "Option::is_none")] + pub new_expiration: Option, +} + +impl UpdateServiceAccountRequest { + /// Return true when the request does not change any field. + pub fn is_empty(&self) -> bool { + self.new_policy.is_none() + && self.new_secret_key.is_none() + && self.new_status.is_none() + && self.new_name.is_none() + && self.new_description.is_none() + && self.new_expiration.is_none() + } +} + /// Bucket quota information returned by Admin API #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -620,6 +661,38 @@ mod tests { ); } + #[test] + fn test_update_service_account_request_serializes_provided_fields() { + let request = UpdateServiceAccountRequest { + new_policy: Some(r#"{"Version":"2012-10-17"}"#.to_string()), + new_secret_key: Some("new-secret-key".to_string()), + new_status: Some("enabled".to_string()), + new_name: Some("automation-key".to_string()), + new_description: Some("Used by automation".to_string()), + new_expiration: Some("2030-01-01T00:00:00Z".to_string()), + }; + + let value = serde_json::to_value(&request).expect("serialize update request"); + assert_eq!(value["newPolicy"], r#"{"Version":"2012-10-17"}"#); + assert_eq!(value["newSecretKey"], "new-secret-key"); + assert_eq!(value["newStatus"], "enabled"); + assert_eq!(value["newName"], "automation-key"); + assert_eq!(value["newDescription"], "Used by automation"); + assert_eq!(value["newExpiration"], "2030-01-01T00:00:00Z"); + } + + #[test] + fn test_update_service_account_request_omits_unset_fields() { + let request = UpdateServiceAccountRequest { + new_description: Some("Updated description".to_string()), + ..Default::default() + }; + + let value = serde_json::to_value(&request).expect("serialize update request"); + assert_eq!(value.as_object().expect("request object").len(), 1); + assert_eq!(value["newDescription"], "Updated description"); + } + #[test] fn test_access_key_info_deserializes_openid_server_shape() { let value = serde_json::json!({ diff --git a/crates/s3/src/admin.rs b/crates/s3/src/admin.rs index 426827a..1640064 100644 --- a/crates/s3/src/admin.rs +++ b/crates/s3/src/admin.rs @@ -15,7 +15,7 @@ use rc_core::admin::{ HealStartRequest, HealStatus, HealTaskRequest, PeerSiteSpec, Policy, PolicyEntity, PolicyInfo, PoolStatus, PoolTarget, RebalanceStartResult, RebalanceStatus, ServiceAccount, ServiceAccountCreateResponse, ServiceActionResult, SiteRemoveSpec, SiteStatusOptions, - UpdateGroupMembersRequest, User, UserStatus, + UpdateGroupMembersRequest, UpdateServiceAccountRequest, User, UserStatus, }; use rc_core::{Alias, Error, Result}; use reqwest::header::{CONTENT_TYPE, HOST, HeaderMap, HeaderName, HeaderValue}; @@ -1118,6 +1118,22 @@ impl AdminApi for AdminClient { }) } + async fn update_service_account( + &self, + access_key: &str, + request: UpdateServiceAccountRequest, + ) -> Result<()> { + let query = [("accessKey", access_key)]; + let body = serde_json::to_vec(&request).map_err(Error::Json)?; + self.request_no_response( + Method::POST, + "/update-service-account", + Some(&query), + Some(&body), + ) + .await + } + async fn delete_service_account(&self, access_key: &str) -> Result<()> { let query = [("accessKey", access_key)]; self.request_no_response( @@ -2424,6 +2440,38 @@ mod tests { handle.join().expect("server thread should finish"); } + #[tokio::test] + async fn test_update_service_account_posts_access_key_and_partial_body() { + let (endpoint, receiver, handle) = start_admin_test_server("204 No Content", ""); + let client = admin_client_for_endpoint(&endpoint); + + client + .update_service_account( + "service key/one", + rc_core::admin::UpdateServiceAccountRequest { + new_policy: Some(r#"{"Version":"2012-10-17"}"#.to_string()), + new_description: Some("Updated description".to_string()), + ..Default::default() + }, + ) + .await + .expect("update service account request"); + + let request = receiver.recv().expect("captured request"); + assert_eq!(request.method, "POST"); + assert_eq!( + request.target, + "/rustfs/admin/v3/update-service-account?accessKey=service%20key%2Fone" + ); + + let body: serde_json::Value = + serde_json::from_slice(&request.body).expect("update body should be JSON"); + assert_eq!(body["newPolicy"], r#"{"Version":"2012-10-17"}"#); + assert_eq!(body["newDescription"], "Updated description"); + assert_eq!(body.as_object().expect("request object").len(), 2); + handle.join().expect("server thread should finish"); + } + #[test] fn test_admin_client_invalid_ca_bundle_path_surfaces_error() { let mut alias = Alias::new("test", "https://localhost:9000", "access", "secret");