feat(storage): refactor audit and notification with OperationHelper (#825)

* improve code for audit

* improve code ecfs.rs

* improve code

* improve code for ecfs.rs

* feat(storage): refactor audit and notification with OperationHelper

This commit introduces a significant refactoring of the audit logging and event notification mechanisms within `ecfs.rs`.

The core of this change is the new `OperationHelper` struct, which encapsulates and simplifies the logic for both concerns. It replaces the previous `AuditHelper` and manual event dispatching.

Key improvements include:

- **Unified Handling**: `OperationHelper` manages both audit and notification builders, providing a single, consistent entry point for S3 operations.
- **RAII for Automation**: By leveraging the `Drop` trait, the helper automatically dispatches logs and notifications when it goes out of scope. This simplifies S3 method implementations and ensures cleanup even on early returns.
- **Fluent API**: A builder-like pattern with methods such as `.object()`, `.version_id()`, and `.suppress_event()` makes the code more readable and expressive.
- **Context-Aware Logic**: The helper's `.complete()` method intelligently populates log details based on the operation's `S3Result` and only triggers notifications on success.
- **Modular Design**: All helper logic is now isolated in `rustfs/src/storage/helper.rs`, improving separation of concerns and making `ecfs.rs` cleaner.

This refactoring significantly enhances code clarity, reduces boilerplate, and improves the robustness of logging and notification handling across the storage layer.

* fix

* fix

* fix

* fix

* fix

* fix

* fix

* improve code for audit and notify

* fix

* fix

* fix
This commit is contained in:
houseme
2025-11-10 17:30:50 +08:00
committed by GitHub
parent b26aad4129
commit 98be7df0f5
25 changed files with 905 additions and 835 deletions
+175 -248
View File
@@ -13,18 +13,10 @@
// limitations under the License.
use chrono::{DateTime, Utc};
use hashbrown::HashMap;
use rustfs_targets::EventName;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::HashMap;
/// Trait for types that can be serialized to JSON and have a timestamp
pub trait LogRecord {
/// Serialize the record to a JSON string
fn to_json(&self) -> String;
/// Get the timestamp of the record
fn get_timestamp(&self) -> chrono::DateTime<chrono::Utc>;
}
/// ObjectVersion represents an object version with key and versionId
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
@@ -36,19 +28,12 @@ pub struct ObjectVersion {
}
impl ObjectVersion {
/// Set the object name (chainable)
pub fn set_object_name(&mut self, name: String) -> &mut Self {
self.object_name = name;
self
}
/// Set the version ID (chainable)
pub fn set_version_id(&mut self, version_id: Option<String>) -> &mut Self {
self.version_id = version_id;
self
pub fn new(object_name: String, version_id: Option<String>) -> Self {
Self { object_name, version_id }
}
}
/// ApiDetails contains API information for the audit entry
/// `ApiDetails` contains API information for the audit entry.
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct ApiDetails {
#[serde(skip_serializing_if = "Option::is_none")]
@@ -79,75 +64,86 @@ pub struct ApiDetails {
pub time_to_response_in_ns: Option<String>,
}
impl ApiDetails {
/// Set API name (chainable)
pub fn set_name(&mut self, name: Option<String>) -> &mut Self {
self.name = name;
/// Builder for `ApiDetails`.
#[derive(Default, Clone)]
pub struct ApiDetailsBuilder(pub ApiDetails);
impl ApiDetailsBuilder {
pub fn new() -> Self {
Self::default()
}
pub fn name(mut self, name: impl Into<String>) -> Self {
self.0.name = Some(name.into());
self
}
/// Set bucket name (chainable)
pub fn set_bucket(&mut self, bucket: Option<String>) -> &mut Self {
self.bucket = bucket;
pub fn bucket(mut self, bucket: impl Into<String>) -> Self {
self.0.bucket = Some(bucket.into());
self
}
/// Set object name (chainable)
pub fn set_object(&mut self, object: Option<String>) -> &mut Self {
self.object = object;
pub fn object(mut self, object: impl Into<String>) -> Self {
self.0.object = Some(object.into());
self
}
/// Set objects list (chainable)
pub fn set_objects(&mut self, objects: Option<Vec<ObjectVersion>>) -> &mut Self {
self.objects = objects;
pub fn objects(mut self, objects: Vec<ObjectVersion>) -> Self {
self.0.objects = Some(objects);
self
}
/// Set status (chainable)
pub fn set_status(&mut self, status: Option<String>) -> &mut Self {
self.status = status;
pub fn status(mut self, status: impl Into<String>) -> Self {
self.0.status = Some(status.into());
self
}
/// Set status code (chainable)
pub fn set_status_code(&mut self, code: Option<i32>) -> &mut Self {
self.status_code = code;
pub fn status_code(mut self, code: i32) -> Self {
self.0.status_code = Some(code);
self
}
/// Set input bytes (chainable)
pub fn set_input_bytes(&mut self, bytes: Option<i64>) -> &mut Self {
self.input_bytes = bytes;
pub fn input_bytes(mut self, bytes: i64) -> Self {
self.0.input_bytes = Some(bytes);
self
}
/// Set output bytes (chainable)
pub fn set_output_bytes(&mut self, bytes: Option<i64>) -> &mut Self {
self.output_bytes = bytes;
pub fn output_bytes(mut self, bytes: i64) -> Self {
self.0.output_bytes = Some(bytes);
self
}
/// Set header bytes (chainable)
pub fn set_header_bytes(&mut self, bytes: Option<i64>) -> &mut Self {
self.header_bytes = bytes;
pub fn header_bytes(mut self, bytes: i64) -> Self {
self.0.header_bytes = Some(bytes);
self
}
/// Set time to first byte (chainable)
pub fn set_time_to_first_byte(&mut self, t: Option<String>) -> &mut Self {
self.time_to_first_byte = t;
pub fn time_to_first_byte(mut self, t: impl Into<String>) -> Self {
self.0.time_to_first_byte = Some(t.into());
self
}
/// Set time to first byte in nanoseconds (chainable)
pub fn set_time_to_first_byte_in_ns(&mut self, t: Option<String>) -> &mut Self {
self.time_to_first_byte_in_ns = t;
pub fn time_to_first_byte_in_ns(mut self, t: impl Into<String>) -> Self {
self.0.time_to_first_byte_in_ns = Some(t.into());
self
}
/// Set time to response (chainable)
pub fn set_time_to_response(&mut self, t: Option<String>) -> &mut Self {
self.time_to_response = t;
pub fn time_to_response(mut self, t: impl Into<String>) -> Self {
self.0.time_to_response = Some(t.into());
self
}
/// Set time to response in nanoseconds (chainable)
pub fn set_time_to_response_in_ns(&mut self, t: Option<String>) -> &mut Self {
self.time_to_response_in_ns = t;
pub fn time_to_response_in_ns(mut self, t: impl Into<String>) -> Self {
self.0.time_to_response_in_ns = Some(t.into());
self
}
pub fn build(self) -> ApiDetails {
self.0
}
}
/// AuditEntry represents an audit log entry
/// `AuditEntry` represents an audit log entry.
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct AuditEntry {
pub version: String,
@@ -155,6 +151,7 @@ pub struct AuditEntry {
pub deployment_id: Option<String>,
#[serde(rename = "siteName", skip_serializing_if = "Option::is_none")]
pub site_name: Option<String>,
#[serde(with = "chrono::serde::ts_milliseconds")]
pub time: DateTime<Utc>,
pub event: EventName,
#[serde(rename = "type", skip_serializing_if = "Option::is_none")]
@@ -191,200 +188,130 @@ pub struct AuditEntry {
pub error: Option<String>,
}
impl AuditEntry {
/// Create a new AuditEntry with required fields
#[allow(clippy::too_many_arguments)]
pub fn new(
version: String,
deployment_id: Option<String>,
site_name: Option<String>,
time: DateTime<Utc>,
event: EventName,
entry_type: Option<String>,
trigger: String,
api: ApiDetails,
) -> Self {
AuditEntry {
version,
deployment_id,
site_name,
time,
/// Constructor for `AuditEntry`.
pub struct AuditEntryBuilder(AuditEntry);
impl AuditEntryBuilder {
/// Create a new builder with all required fields.
pub fn new(version: impl Into<String>, event: EventName, trigger: impl Into<String>, api: ApiDetails) -> Self {
Self(AuditEntry {
version: version.into(),
time: Utc::now(),
event,
entry_type,
trigger,
trigger: trigger.into(),
api,
remote_host: None,
request_id: None,
user_agent: None,
req_path: None,
req_host: None,
req_node: None,
req_claims: None,
req_query: None,
req_header: None,
resp_header: None,
tags: None,
access_key: None,
parent_user: None,
error: None,
}
..Default::default()
})
}
/// Set version (chainable)
pub fn set_version(&mut self, version: String) -> &mut Self {
self.version = version;
self
}
/// Set deployment ID (chainable)
pub fn set_deployment_id(&mut self, id: Option<String>) -> &mut Self {
self.deployment_id = id;
self
}
/// Set site name (chainable)
pub fn set_site_name(&mut self, name: Option<String>) -> &mut Self {
self.site_name = name;
self
}
/// Set time (chainable)
pub fn set_time(&mut self, time: DateTime<Utc>) -> &mut Self {
self.time = time;
self
}
/// Set event (chainable)
pub fn set_event(&mut self, event: EventName) -> &mut Self {
self.event = event;
self
}
/// Set entry type (chainable)
pub fn set_entry_type(&mut self, entry_type: Option<String>) -> &mut Self {
self.entry_type = entry_type;
self
}
/// Set trigger (chainable)
pub fn set_trigger(&mut self, trigger: String) -> &mut Self {
self.trigger = trigger;
self
}
/// Set API details (chainable)
pub fn set_api(&mut self, api: ApiDetails) -> &mut Self {
self.api = api;
self
}
/// Set remote host (chainable)
pub fn set_remote_host(&mut self, host: Option<String>) -> &mut Self {
self.remote_host = host;
self
}
/// Set request ID (chainable)
pub fn set_request_id(&mut self, id: Option<String>) -> &mut Self {
self.request_id = id;
self
}
/// Set user agent (chainable)
pub fn set_user_agent(&mut self, agent: Option<String>) -> &mut Self {
self.user_agent = agent;
self
}
/// Set request path (chainable)
pub fn set_req_path(&mut self, path: Option<String>) -> &mut Self {
self.req_path = path;
self
}
/// Set request host (chainable)
pub fn set_req_host(&mut self, host: Option<String>) -> &mut Self {
self.req_host = host;
self
}
/// Set request node (chainable)
pub fn set_req_node(&mut self, node: Option<String>) -> &mut Self {
self.req_node = node;
self
}
/// Set request claims (chainable)
pub fn set_req_claims(&mut self, claims: Option<HashMap<String, Value>>) -> &mut Self {
self.req_claims = claims;
self
}
/// Set request query (chainable)
pub fn set_req_query(&mut self, query: Option<HashMap<String, String>>) -> &mut Self {
self.req_query = query;
self
}
/// Set request header (chainable)
pub fn set_req_header(&mut self, header: Option<HashMap<String, String>>) -> &mut Self {
self.req_header = header;
self
}
/// Set response header (chainable)
pub fn set_resp_header(&mut self, header: Option<HashMap<String, String>>) -> &mut Self {
self.resp_header = header;
self
}
/// Set tags (chainable)
pub fn set_tags(&mut self, tags: Option<HashMap<String, Value>>) -> &mut Self {
self.tags = tags;
self
}
/// Set access key (chainable)
pub fn set_access_key(&mut self, key: Option<String>) -> &mut Self {
self.access_key = key;
self
}
/// Set parent user (chainable)
pub fn set_parent_user(&mut self, user: Option<String>) -> &mut Self {
self.parent_user = user;
self
}
/// Set error message (chainable)
pub fn set_error(&mut self, error: Option<String>) -> &mut Self {
self.error = error;
// event
pub fn version(mut self, version: impl Into<String>) -> Self {
self.0.version = version.into();
self
}
/// Build AuditEntry from context or parameters (example, can be extended)
pub fn from_context(
version: String,
deployment_id: Option<String>,
time: DateTime<Utc>,
event: EventName,
trigger: String,
api: ApiDetails,
tags: Option<HashMap<String, Value>>,
) -> Self {
AuditEntry {
version,
deployment_id,
site_name: None,
time,
event,
entry_type: None,
trigger,
api,
remote_host: None,
request_id: None,
user_agent: None,
req_path: None,
req_host: None,
req_node: None,
req_claims: None,
req_query: None,
req_header: None,
resp_header: None,
tags,
access_key: None,
parent_user: None,
error: None,
}
}
}
impl LogRecord for AuditEntry {
/// Serialize AuditEntry to JSON string
fn to_json(&self) -> String {
serde_json::to_string(self).unwrap_or_else(|_| String::from("{}"))
}
/// Get the timestamp of the audit entry
fn get_timestamp(&self) -> DateTime<Utc> {
self.time
pub fn event(mut self, event: EventName) -> Self {
self.0.event = event;
self
}
pub fn api(mut self, api_details: ApiDetails) -> Self {
self.0.api = api_details;
self
}
pub fn deployment_id(mut self, id: impl Into<String>) -> Self {
self.0.deployment_id = Some(id.into());
self
}
pub fn site_name(mut self, name: impl Into<String>) -> Self {
self.0.site_name = Some(name.into());
self
}
pub fn time(mut self, time: DateTime<Utc>) -> Self {
self.0.time = time;
self
}
pub fn entry_type(mut self, entry_type: impl Into<String>) -> Self {
self.0.entry_type = Some(entry_type.into());
self
}
pub fn remote_host(mut self, host: impl Into<String>) -> Self {
self.0.remote_host = Some(host.into());
self
}
pub fn request_id(mut self, id: impl Into<String>) -> Self {
self.0.request_id = Some(id.into());
self
}
pub fn user_agent(mut self, agent: impl Into<String>) -> Self {
self.0.user_agent = Some(agent.into());
self
}
pub fn req_path(mut self, path: impl Into<String>) -> Self {
self.0.req_path = Some(path.into());
self
}
pub fn req_host(mut self, host: impl Into<String>) -> Self {
self.0.req_host = Some(host.into());
self
}
pub fn req_node(mut self, node: impl Into<String>) -> Self {
self.0.req_node = Some(node.into());
self
}
pub fn req_claims(mut self, claims: HashMap<String, Value>) -> Self {
self.0.req_claims = Some(claims);
self
}
pub fn req_query(mut self, query: HashMap<String, String>) -> Self {
self.0.req_query = Some(query);
self
}
pub fn req_header(mut self, header: HashMap<String, String>) -> Self {
self.0.req_header = Some(header);
self
}
pub fn resp_header(mut self, header: HashMap<String, String>) -> Self {
self.0.resp_header = Some(header);
self
}
pub fn tags(mut self, tags: HashMap<String, Value>) -> Self {
self.0.tags = Some(tags);
self
}
pub fn access_key(mut self, key: impl Into<String>) -> Self {
self.0.access_key = Some(key.into());
self
}
pub fn parent_user(mut self, user: impl Into<String>) -> Self {
self.0.parent_user = Some(user.into());
self
}
pub fn error(mut self, error: impl Into<String>) -> Self {
self.0.error = Some(error.into());
self
}
/// Construct the final `AuditEntry`.
pub fn build(self) -> AuditEntry {
self.0
}
}
+6 -6
View File
@@ -21,7 +21,7 @@ pub type AuditResult<T> = Result<T, AuditError>;
#[derive(Error, Debug)]
pub enum AuditError {
#[error("Configuration error: {0}")]
Configuration(String),
Configuration(String, #[source] Option<Box<dyn std::error::Error + Send + Sync>>),
#[error("config not loaded")]
ConfigNotLoaded,
@@ -35,11 +35,14 @@ pub enum AuditError {
#[error("System already initialized")]
AlreadyInitialized,
#[error("Storage not available: {0}")]
StorageNotAvailable(String),
#[error("Failed to save configuration: {0}")]
SaveConfig(String),
SaveConfig(#[source] Box<dyn std::error::Error + Send + Sync>),
#[error("Failed to load configuration: {0}")]
LoadConfig(String),
LoadConfig(#[source] Box<dyn std::error::Error + Send + Sync>),
#[error("Serialization error: {0}")]
Serialization(#[from] serde_json::Error),
@@ -49,7 +52,4 @@ pub enum AuditError {
#[error("Join error: {0}")]
Join(#[from] tokio::task::JoinError),
#[error("Server storage not initialized: {0}")]
ServerNotInitialized(String),
}
+25 -27
View File
@@ -15,7 +15,7 @@
use crate::{AuditEntry, AuditResult, AuditSystem};
use rustfs_ecstore::config::Config;
use std::sync::{Arc, OnceLock};
use tracing::{error, warn};
use tracing::{error, trace, warn};
/// Global audit system instance
static AUDIT_SYSTEM: OnceLock<Arc<AuditSystem>> = OnceLock::new();
@@ -30,6 +30,19 @@ pub fn audit_system() -> Option<Arc<AuditSystem>> {
AUDIT_SYSTEM.get().cloned()
}
/// A helper macro for executing closures if the global audit system is initialized.
/// If not initialized, log a warning and return `Ok(())`.
macro_rules! with_audit_system {
($async_closure:expr) => {
if let Some(system) = audit_system() {
(async move { $async_closure(system).await }).await
} else {
warn!("Audit system not initialized, operation skipped.");
Ok(())
}
};
}
/// Start the global audit system with configuration
pub async fn start_audit_system(config: Config) -> AuditResult<()> {
let system = init_audit_system();
@@ -38,32 +51,17 @@ pub async fn start_audit_system(config: Config) -> AuditResult<()> {
/// Stop the global audit system
pub async fn stop_audit_system() -> AuditResult<()> {
if let Some(system) = audit_system() {
system.close().await
} else {
warn!("Audit system not initialized, cannot stop");
Ok(())
}
with_audit_system!(|system: Arc<AuditSystem>| async move { system.close().await })
}
/// Pause the global audit system
pub async fn pause_audit_system() -> AuditResult<()> {
if let Some(system) = audit_system() {
system.pause().await
} else {
warn!("Audit system not initialized, cannot pause");
Ok(())
}
with_audit_system!(|system: Arc<AuditSystem>| async move { system.pause().await })
}
/// Resume the global audit system
pub async fn resume_audit_system() -> AuditResult<()> {
if let Some(system) = audit_system() {
system.resume().await
} else {
warn!("Audit system not initialized, cannot resume");
Ok(())
}
with_audit_system!(|system: Arc<AuditSystem>| async move { system.resume().await })
}
/// Dispatch an audit log entry to all targets
@@ -72,23 +70,23 @@ pub async fn dispatch_audit_log(entry: Arc<AuditEntry>) -> AuditResult<()> {
if system.is_running().await {
system.dispatch(entry).await
} else {
// System not running, just drop the log entry without error
// The system is initialized but not running (for example, it is suspended). Silently discard log entries based on original logic.
// For debugging purposes, it can be useful to add a trace log here.
trace!("Audit system is not running, dropping audit entry.");
Ok(())
}
} else {
// System not initialized, just drop the log entry without error
// The system is not initialized at all. This is a more important state.
// It might be better to return an error or log a warning.
warn!("Audit system not initialized, dropping audit entry.");
// If this should be a hard failure, you can return Err(AuditError::NotInitialized("..."))
Ok(())
}
}
/// Reload the global audit system configuration
pub async fn reload_audit_config(config: Config) -> AuditResult<()> {
if let Some(system) = audit_system() {
system.reload_config(config).await
} else {
warn!("Audit system not initialized, cannot reload config");
Ok(())
}
with_audit_system!(|system: Arc<AuditSystem>| async move { system.reload_config(config).await })
}
/// Check if the global audit system is running
+1 -1
View File
@@ -25,7 +25,7 @@ pub mod observability;
pub mod registry;
pub mod system;
pub use entity::{ApiDetails, AuditEntry, LogRecord, ObjectVersion};
pub use entity::{ApiDetails, AuditEntry, ObjectVersion};
pub use error::{AuditError, AuditResult};
pub use global::*;
pub use observability::{AuditMetrics, AuditMetricsReport, PerformanceValidation};
+4 -4
View File
@@ -14,6 +14,7 @@
use crate::{AuditEntry, AuditError, AuditResult};
use futures::{StreamExt, stream::FuturesUnordered};
use hashbrown::{HashMap, HashSet};
use rustfs_config::{
DEFAULT_DELIMITER, ENABLE_KEY, ENV_PREFIX, MQTT_BROKER, MQTT_KEEP_ALIVE_INTERVAL, MQTT_PASSWORD, MQTT_QOS, MQTT_QUEUE_DIR,
MQTT_QUEUE_LIMIT, MQTT_RECONNECT_INTERVAL, MQTT_TOPIC, MQTT_USERNAME, WEBHOOK_AUTH_TOKEN, WEBHOOK_BATCH_SIZE,
@@ -25,7 +26,6 @@ use rustfs_targets::{
Target, TargetError,
target::{ChannelTargetType, TargetType, mqtt::MQTTArgs, webhook::WebhookArgs},
};
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use std::time::Duration;
use tracing::{debug, error, info, warn};
@@ -251,7 +251,7 @@ impl AuditRegistry {
sections.extend(successes_by_section.keys().cloned());
for section_name in sections {
let mut section_map: HashMap<String, KVS> = HashMap::new();
let mut section_map: std::collections::HashMap<String, KVS> = std::collections::HashMap::new();
// The default entry (if present) is written back to `_`
if let Some(default_cfg) = section_defaults.get(&section_name) {
@@ -277,7 +277,7 @@ impl AuditRegistry {
// 7. Save the new configuration to the system
let Some(store) = rustfs_ecstore::new_object_layer_fn() else {
return Err(AuditError::ServerNotInitialized(
return Err(AuditError::StorageNotAvailable(
"Failed to save target configuration: server storage not initialized".to_string(),
));
};
@@ -286,7 +286,7 @@ impl AuditRegistry {
Ok(_) => info!("New audit configuration saved to system successfully"),
Err(e) => {
error!(error = %e, "Failed to save new audit configuration");
return Err(AuditError::SaveConfig(e.to_string()));
return Err(AuditError::SaveConfig(Box::new(e)));
}
}
}
+5 -5
View File
@@ -146,7 +146,7 @@ impl AuditSystem {
warn!("Audit system is already paused");
Ok(())
}
_ => Err(AuditError::Configuration("Cannot pause audit system in current state".to_string())),
_ => Err(AuditError::Configuration("Cannot pause audit system in current state".to_string(), None)),
}
}
@@ -164,7 +164,7 @@ impl AuditSystem {
warn!("Audit system is already running");
Ok(())
}
_ => Err(AuditError::Configuration("Cannot resume audit system in current state".to_string())),
_ => Err(AuditError::Configuration("Cannot resume audit system in current state".to_string(), None)),
}
}
@@ -460,7 +460,7 @@ impl AuditSystem {
info!(target_id = %target_id, "Target enabled");
Ok(())
} else {
Err(AuditError::Configuration(format!("Target not found: {target_id}")))
Err(AuditError::Configuration(format!("Target not found: {target_id}"), None))
}
}
@@ -473,7 +473,7 @@ impl AuditSystem {
info!(target_id = %target_id, "Target disabled");
Ok(())
} else {
Err(AuditError::Configuration(format!("Target not found: {target_id}")))
Err(AuditError::Configuration(format!("Target not found: {target_id}"), None))
}
}
@@ -487,7 +487,7 @@ impl AuditSystem {
info!(target_id = %target_id, "Target removed");
Ok(())
} else {
Err(AuditError::Configuration(format!("Target not found: {target_id}")))
Err(AuditError::Configuration(format!("Target not found: {target_id}"), None))
}
}