mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-29 09:38:59 +00:00
Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| fa1554be7f | |||
| f08b592c6f | |||
| 8add0126f5 | |||
| 09a83a8f56 | |||
| a0f1bb4ff0 | |||
| 3ac1d2ab0b |
@@ -19,6 +19,7 @@
|
||||
mod tests {
|
||||
use crate::common::{RustFSTestEnvironment, init_logging};
|
||||
use aws_sdk_s3::Client;
|
||||
use aws_sdk_s3::primitives::ByteStream;
|
||||
use aws_sdk_s3::types::{BucketVersioningStatus, VersioningConfiguration};
|
||||
use serial_test::serial;
|
||||
use tracing::info;
|
||||
@@ -179,4 +180,84 @@ mod tests {
|
||||
"Delete marker should no longer be latest after the second put"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_list_object_versions_prefix_with_marker_object_returns_children() {
|
||||
init_logging();
|
||||
info!("🧪 TEST: ListObjectVersions returns prefix children when a marker object also exists");
|
||||
|
||||
let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment");
|
||||
env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS");
|
||||
|
||||
let client = create_s3_client(&env);
|
||||
let bucket = "test-list-versions-prefix-marker";
|
||||
let marker_key = "data01";
|
||||
let child_keys = [
|
||||
"data01/meta/dump-2026-04-08-053205.json.gz",
|
||||
"data01/meta/dump-2026-04-08-063209.json.gz",
|
||||
];
|
||||
|
||||
client
|
||||
.create_bucket()
|
||||
.bucket(bucket)
|
||||
.send()
|
||||
.await
|
||||
.expect("Failed to create bucket");
|
||||
|
||||
client
|
||||
.put_bucket_versioning()
|
||||
.bucket(bucket)
|
||||
.versioning_configuration(
|
||||
VersioningConfiguration::builder()
|
||||
.status(BucketVersioningStatus::Suspended)
|
||||
.build(),
|
||||
)
|
||||
.send()
|
||||
.await
|
||||
.expect("Failed to suspend versioning");
|
||||
|
||||
client
|
||||
.put_object()
|
||||
.bucket(bucket)
|
||||
.key(marker_key)
|
||||
.body(ByteStream::from_static(b""))
|
||||
.send()
|
||||
.await
|
||||
.expect("Failed to put marker object");
|
||||
|
||||
for key in child_keys {
|
||||
client
|
||||
.put_object()
|
||||
.bucket(bucket)
|
||||
.key(key)
|
||||
.body(ByteStream::from_static(b"payload"))
|
||||
.send()
|
||||
.await
|
||||
.expect("Failed to put child object");
|
||||
}
|
||||
|
||||
let listing = client
|
||||
.list_object_versions()
|
||||
.bucket(bucket)
|
||||
.prefix("data01/")
|
||||
.send()
|
||||
.await
|
||||
.expect("Failed to list object versions by prefix");
|
||||
|
||||
let version_keys: Vec<_> = listing.versions().iter().filter_map(|version| version.key()).collect();
|
||||
|
||||
assert_eq!(
|
||||
version_keys.len(),
|
||||
child_keys.len(),
|
||||
"ListObjectVersions with a trailing slash prefix should include child objects even when the marker object exists"
|
||||
);
|
||||
|
||||
for key in child_keys {
|
||||
assert!(
|
||||
version_keys.contains(&key),
|
||||
"ListObjectVersions(prefix=data01/) should include child object {key}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,11 +120,6 @@ impl LockClient for GrpcLockClient {
|
||||
.map_err(|e| LockError::internal(e.to_string()))?
|
||||
.into_inner();
|
||||
|
||||
// Check for explicit error first
|
||||
if let Some(error_info) = resp.error_info {
|
||||
return Err(LockError::internal(error_info));
|
||||
}
|
||||
|
||||
// Check if the lock acquisition was successful
|
||||
if resp.success {
|
||||
Ok(LockResponse::success(
|
||||
@@ -134,7 +129,8 @@ impl LockClient for GrpcLockClient {
|
||||
} else {
|
||||
// Lock acquisition failed
|
||||
Ok(LockResponse::failure(
|
||||
"Lock acquisition failed on remote server".to_string(),
|
||||
resp.error_info
|
||||
.unwrap_or_else(|| "Lock acquisition failed on remote server".to_string()),
|
||||
std::time::Duration::ZERO,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -50,6 +50,18 @@ fn lock_result_from_error(error: impl Into<String>) -> GenerallyLockResult {
|
||||
}
|
||||
}
|
||||
|
||||
fn lock_result_from_release(lock_id: &rustfs_lock::LockId, success: bool) -> GenerallyLockResult {
|
||||
if success {
|
||||
GenerallyLockResult {
|
||||
success: true,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
}
|
||||
} else {
|
||||
lock_result_from_error(format!("lock not found for release: {lock_id}"))
|
||||
}
|
||||
}
|
||||
|
||||
/// Minimal NodeService implementation that only supports Lock RPCs
|
||||
/// Used for testing distributed lock scenarios with real gRPC
|
||||
#[derive(Debug)]
|
||||
@@ -102,7 +114,7 @@ impl NodeService for MinimalLockNodeService {
|
||||
let lock_info_json = result.lock_info.as_ref().and_then(|info| serde_json::to_string(info).ok());
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: None,
|
||||
error_info: result.error,
|
||||
lock_info: lock_info_json,
|
||||
}))
|
||||
}
|
||||
@@ -131,11 +143,14 @@ impl NodeService for MinimalLockNodeService {
|
||||
};
|
||||
|
||||
match self.lock_client.release(&args.lock_id).await {
|
||||
Ok(success) => Ok(Response::new(GenerallyLockResponse {
|
||||
success,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
})),
|
||||
Ok(success) => {
|
||||
let result = lock_result_from_release(&args.lock_id, success);
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: result.error_info,
|
||||
lock_info: None,
|
||||
}))
|
||||
}
|
||||
Err(err) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: false,
|
||||
error_info: Some(format!(
|
||||
@@ -161,11 +176,14 @@ impl NodeService for MinimalLockNodeService {
|
||||
};
|
||||
|
||||
match self.lock_client.force_release(&args.lock_id).await {
|
||||
Ok(success) => Ok(Response::new(GenerallyLockResponse {
|
||||
success,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
})),
|
||||
Ok(success) => {
|
||||
let result = lock_result_from_release(&args.lock_id, success);
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: result.error_info,
|
||||
lock_info: None,
|
||||
}))
|
||||
}
|
||||
Err(err) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: false,
|
||||
error_info: Some(format!(
|
||||
@@ -271,10 +289,9 @@ impl NodeService for MinimalLockNodeService {
|
||||
Ok(batch_results) => {
|
||||
for (result_idx, success) in batch_results.into_iter().enumerate() {
|
||||
if let Some(request_idx) = valid_indices.get(result_idx) {
|
||||
results[*request_idx] = GenerallyLockResult {
|
||||
success,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
results[*request_idx] = match lock_ids.get(result_idx) {
|
||||
Some(lock_id) => lock_result_from_release(lock_id, success),
|
||||
None => lock_result_from_error(format!("unlock response index out of range: {result_idx}")),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -275,6 +275,41 @@ async fn test_grpc_lock_client_batch_acquire_and_release() {
|
||||
handle.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_grpc_lock_client_uses_request_lock_id_and_reports_missing_unlock() {
|
||||
let manager = Arc::new(GlobalLockManager::new());
|
||||
let local_client: Arc<dyn rustfs_lock::LockClient> = Arc::new(LocalClient::with_manager(manager));
|
||||
|
||||
let (addr, handle) = spawn_lock_server(local_client).await.expect("Failed to spawn server");
|
||||
tokio::time::sleep(Duration::from_millis(100)).await;
|
||||
|
||||
let grpc_client = GrpcLockClient::new(addr);
|
||||
let request = LockRequest::new(test_resource(), LockType::Exclusive, "owner-a").with_acquire_timeout(Duration::from_secs(2));
|
||||
|
||||
let response = grpc_client.acquire_lock(&request).await.expect("gRPC acquire should succeed");
|
||||
let lock_info = response.lock_info.expect("gRPC acquire should include lock info");
|
||||
assert_eq!(lock_info.id, request.lock_id);
|
||||
|
||||
assert!(
|
||||
grpc_client
|
||||
.release(&request.lock_id)
|
||||
.await
|
||||
.expect("gRPC release should succeed"),
|
||||
"release should find the request lock id"
|
||||
);
|
||||
|
||||
let missing_release = grpc_client
|
||||
.release(&request.lock_id)
|
||||
.await
|
||||
.expect_err("second release should report missing lock");
|
||||
assert!(
|
||||
missing_release.to_string().contains("lock not found for release"),
|
||||
"missing release should preserve server error, got: {missing_release}"
|
||||
);
|
||||
|
||||
handle.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_distributed_lock_4_nodes_grpc_read_write_quorum_split_with_two_failed_nodes() {
|
||||
let manager1 = Arc::new(GlobalLockManager::new());
|
||||
|
||||
@@ -2072,6 +2072,7 @@ impl DiskAPI for LocalDisk {
|
||||
|
||||
let mut objs_returned = 0;
|
||||
|
||||
let mut skip_current_dir_object = false;
|
||||
if opts.base_dir.ends_with(SLASH_SEPARATOR) {
|
||||
if let Ok(data) = self
|
||||
.read_metadata(
|
||||
@@ -2098,7 +2099,7 @@ impl DiskAPI for LocalDisk {
|
||||
if let Ok(meta) = tokio::fs::metadata(fpath).await
|
||||
&& meta.is_file()
|
||||
{
|
||||
return Err(DiskError::FileNotFound);
|
||||
skip_current_dir_object = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2109,7 +2110,7 @@ impl DiskAPI for LocalDisk {
|
||||
&opts,
|
||||
&mut out,
|
||||
&mut objs_returned,
|
||||
false,
|
||||
skip_current_dir_object,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -190,11 +190,6 @@ impl LockClient for RemoteClient {
|
||||
Err(err) => return Ok(Self::rpc_failure_response(request, &err)),
|
||||
};
|
||||
|
||||
// Check for explicit error first
|
||||
if let Some(error_info) = resp.error_info {
|
||||
return Err(LockError::internal(error_info));
|
||||
}
|
||||
|
||||
// Check if the lock acquisition was successful
|
||||
if resp.success {
|
||||
Ok(LockResponse::success(
|
||||
@@ -204,7 +199,8 @@ impl LockClient for RemoteClient {
|
||||
} else {
|
||||
// Lock acquisition failed
|
||||
Ok(LockResponse::failure(
|
||||
"Lock acquisition failed on remote server".to_string(),
|
||||
resp.error_info
|
||||
.unwrap_or_else(|| "Lock acquisition failed on remote server".to_string()),
|
||||
std::time::Duration::ZERO,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
use crate::batch_processor::{AsyncBatchProcessor, get_global_processors};
|
||||
use crate::bitrot::{create_bitrot_reader, create_bitrot_writer};
|
||||
use crate::bucket::lifecycle::lifecycle::TRANSITION_COMPLETE;
|
||||
use crate::bucket::object_lock::objectlock_sys::check_retention_for_modification;
|
||||
use crate::bucket::replication::check_replicate_delete;
|
||||
use crate::bucket::versioning::VersioningApi;
|
||||
use crate::bucket::versioning_sys::BucketVersioningSys;
|
||||
@@ -1343,6 +1344,22 @@ impl BucketOperations for SetDisks {
|
||||
}
|
||||
}
|
||||
|
||||
fn check_object_lock_retention_update(bucket: &str, object: &str, obj_info: &ObjectInfo, opts: &ObjectOptions) -> Result<()> {
|
||||
if let Some(retention) = &opts.object_lock_retention
|
||||
&& check_retention_for_modification(
|
||||
&obj_info.user_defined,
|
||||
retention.mode.as_deref(),
|
||||
retention.retain_until,
|
||||
retention.bypass_governance,
|
||||
)
|
||||
.is_some()
|
||||
{
|
||||
return Err(StorageError::PrefixAccessDenied(bucket.to_string(), object.to_string()));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ObjectOperations for SetDisks {
|
||||
#[tracing::instrument(skip(self))]
|
||||
@@ -1989,6 +2006,8 @@ impl ObjectOperations for SetDisks {
|
||||
|
||||
let obj_info = ObjectInfo::from_file_info(&fi, bucket, object, opts.versioned || opts.version_suspended);
|
||||
|
||||
check_object_lock_retention_update(bucket, object, &obj_info, opts)?;
|
||||
|
||||
for (k, v) in obj_info.user_defined {
|
||||
fi.metadata.insert(k, v);
|
||||
}
|
||||
@@ -5483,6 +5502,74 @@ mod tests {
|
||||
assert!(rendered.contains("object"), "{rendered}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_object_lock_retention_update_blocks_compliance_shorten() {
|
||||
let now = OffsetDateTime::now_utc();
|
||||
let existing_until = now + Duration::from_secs(60 * 60 * 24 * 60);
|
||||
let requested_until = now + Duration::from_secs(60 * 60 * 24);
|
||||
|
||||
let mut user_defined = HashMap::new();
|
||||
user_defined.insert(
|
||||
X_AMZ_OBJECT_LOCK_MODE.as_str().to_string(),
|
||||
s3s::dto::ObjectLockRetentionMode::COMPLIANCE.to_string(),
|
||||
);
|
||||
user_defined.insert(
|
||||
X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str().to_string(),
|
||||
existing_until.format(&time::format_description::well_known::Rfc3339).unwrap(),
|
||||
);
|
||||
|
||||
let obj_info = ObjectInfo {
|
||||
user_defined,
|
||||
..Default::default()
|
||||
};
|
||||
let opts = ObjectOptions {
|
||||
object_lock_retention: Some(crate::store_api::ObjectLockRetentionOptions {
|
||||
mode: Some(s3s::dto::ObjectLockRetentionMode::COMPLIANCE.to_string()),
|
||||
retain_until: Some(requested_until),
|
||||
bypass_governance: true,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = check_object_lock_retention_update("bucket", "object", &obj_info, &opts)
|
||||
.expect_err("COMPLIANCE shortening must be blocked");
|
||||
|
||||
assert!(matches!(err, StorageError::PrefixAccessDenied(_, _)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_object_lock_retention_update_allows_governance_shorten_with_bypass() {
|
||||
let now = OffsetDateTime::now_utc();
|
||||
let existing_until = now + Duration::from_secs(60 * 60 * 24 * 60);
|
||||
let requested_until = now + Duration::from_secs(60 * 60 * 24);
|
||||
|
||||
let mut user_defined = HashMap::new();
|
||||
user_defined.insert(
|
||||
X_AMZ_OBJECT_LOCK_MODE.as_str().to_string(),
|
||||
s3s::dto::ObjectLockRetentionMode::GOVERNANCE.to_string(),
|
||||
);
|
||||
user_defined.insert(
|
||||
X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str().to_string(),
|
||||
existing_until.format(&time::format_description::well_known::Rfc3339).unwrap(),
|
||||
);
|
||||
|
||||
let obj_info = ObjectInfo {
|
||||
user_defined,
|
||||
..Default::default()
|
||||
};
|
||||
let opts = ObjectOptions {
|
||||
object_lock_retention: Some(crate::store_api::ObjectLockRetentionOptions {
|
||||
mode: Some(s3s::dto::ObjectLockRetentionMode::GOVERNANCE.to_string()),
|
||||
retain_until: Some(requested_until),
|
||||
bypass_governance: true,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
check_object_lock_retention_update("bucket", "object", &obj_info, &opts)
|
||||
.expect("GOVERNANCE shortening with bypass should remain allowed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_should_prevent_write() {
|
||||
let oi = ObjectInfo {
|
||||
|
||||
@@ -43,6 +43,13 @@ impl HTTPPreconditions {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct ObjectLockRetentionOptions {
|
||||
pub mode: Option<String>,
|
||||
pub retain_until: Option<OffsetDateTime>,
|
||||
pub bypass_governance: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct ObjectOptions {
|
||||
// Use the maximum parity (N/2), used when saving server configuration files
|
||||
@@ -79,6 +86,7 @@ pub struct ObjectOptions {
|
||||
pub lifecycle_audit_event: LcAuditEvent,
|
||||
|
||||
pub eval_metadata: Option<HashMap<String, String>>,
|
||||
pub object_lock_retention: Option<ObjectLockRetentionOptions>,
|
||||
|
||||
pub want_checksum: Option<Checksum>,
|
||||
pub skip_verify_bitrot: bool,
|
||||
|
||||
@@ -22,13 +22,15 @@ use futures::future::join_all;
|
||||
use rustfs_io_metrics::{
|
||||
record_read_lock_held_acquire, record_read_lock_held_release, record_write_lock_held_acquire, record_write_lock_held_release,
|
||||
};
|
||||
use std::sync::{Arc, LazyLock};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::task::JoinSet;
|
||||
use tracing::warn;
|
||||
use tracing::{debug, warn};
|
||||
use uuid::Uuid;
|
||||
|
||||
const UNLOCK_RETRY_ATTEMPTS: usize = 3;
|
||||
const UNLOCK_RETRY_BACKOFF: Duration = Duration::from_millis(100);
|
||||
|
||||
/// Generate a new aggregate lock ID for multiple client locks
|
||||
fn generate_aggregate_lock_id(resource: &ObjectKey) -> LockId {
|
||||
LockId {
|
||||
@@ -37,45 +39,6 @@ fn generate_aggregate_lock_id(resource: &ObjectKey) -> LockId {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct UnlockJob {
|
||||
/// Entries to release: each (LockId, client) pair will be released independently.
|
||||
entries: Vec<(LockId, Arc<dyn LockClient>)>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnlockRuntime {
|
||||
tx: mpsc::Sender<UnlockJob>,
|
||||
}
|
||||
|
||||
// Global unlock runtime with background worker
|
||||
static UNLOCK_RUNTIME: LazyLock<UnlockRuntime> = LazyLock::new(|| {
|
||||
// Larger buffer to reduce contention during bursts
|
||||
let (tx, mut rx) = mpsc::channel::<UnlockJob>(8192);
|
||||
|
||||
// Spawn background worker when first used; assumes a Tokio runtime is available
|
||||
tokio::spawn(async move {
|
||||
while let Some(job) = rx.recv().await {
|
||||
// Best-effort release across all (LockId, client) entries.
|
||||
let results = join_all(
|
||||
job.entries
|
||||
.into_iter()
|
||||
.map(|(lock_id, client)| async move { client.release(&lock_id).await.unwrap_or(false) }),
|
||||
)
|
||||
.await;
|
||||
let any_ok = results.into_iter().any(|released| released);
|
||||
|
||||
if !any_ok {
|
||||
tracing::warn!("DistributedLockGuard background release failed for one or more entries");
|
||||
} else {
|
||||
tracing::debug!("DistributedLockGuard background released one or more entries");
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
UnlockRuntime { tx }
|
||||
});
|
||||
|
||||
/// A RAII guard for distributed locks that releases the lock asynchronously when dropped.
|
||||
#[derive(Debug)]
|
||||
pub struct DistributedLockGuard {
|
||||
@@ -126,47 +89,22 @@ impl DistributedLockGuard {
|
||||
}
|
||||
|
||||
/// Manually release the lock early.
|
||||
/// This sends a release job to the background worker and then disarms the guard
|
||||
/// This spawns a background release task and then disarms the guard
|
||||
/// to prevent double-release on drop.
|
||||
/// Returns true if the lock was released (or was already released), false otherwise.
|
||||
/// Returns true if release was scheduled or the guard was already disarmed.
|
||||
pub fn release(&mut self) -> bool {
|
||||
if self.disarmed {
|
||||
// Lock was already released, return true to indicate lock is in released state
|
||||
return true;
|
||||
}
|
||||
|
||||
let job = UnlockJob {
|
||||
entries: self.entries.clone(),
|
||||
};
|
||||
|
||||
// Try a non-blocking send to avoid panics
|
||||
let success = if let Err(err) = UNLOCK_RUNTIME.tx.try_send(job) {
|
||||
// Channel full or closed; best-effort fallback: spawn a detached task
|
||||
let entries = self.entries.clone();
|
||||
tracing::warn!(
|
||||
"DistributedLockGuard channel send failed ({}), spawning fallback unlock task for {} entries",
|
||||
err,
|
||||
entries.len()
|
||||
);
|
||||
|
||||
// If runtime is not available, this will panic; but in RustFS we are inside Tokio contexts.
|
||||
let handle = tokio::spawn(async move {
|
||||
let futures_iter = entries
|
||||
.into_iter()
|
||||
.map(|(lock_id, client)| async move { client.release(&lock_id).await.unwrap_or(false) });
|
||||
let _ = join_all(futures_iter).await;
|
||||
});
|
||||
// Explicitly drop the JoinHandle to acknowledge detaching the task.
|
||||
drop(handle);
|
||||
true // Consider it successful even if we had to use fallback
|
||||
} else {
|
||||
true
|
||||
};
|
||||
let entries = self.entries.clone();
|
||||
DistributedLock::spawn_release_cleanup(entries, "distributed_lock_guard_release");
|
||||
|
||||
// Disarm to prevent double-release on drop
|
||||
self.disarmed = true;
|
||||
record_lock_held_release(self.lock_type);
|
||||
success
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -349,22 +287,64 @@ impl DistributedLock {
|
||||
pending
|
||||
}
|
||||
|
||||
async fn release_entries(entries: &[(LockId, Arc<dyn LockClient>)], context: &'static str) {
|
||||
let release_results = join_all(
|
||||
entries
|
||||
.iter()
|
||||
.map(|(lock_id, client)| async move { (lock_id, client.release(lock_id).await) }),
|
||||
)
|
||||
.await;
|
||||
async fn release_entries(entries: Vec<(LockId, Arc<dyn LockClient>)>, context: &'static str) {
|
||||
let mut pending = entries;
|
||||
|
||||
for (lock_id, result) in release_results {
|
||||
match result {
|
||||
Ok(true) | Ok(false) => {}
|
||||
Err(err) => {
|
||||
tracing::warn!("{context}: failed to release lock {} on client: {}", lock_id, err);
|
||||
for attempt in 1..=UNLOCK_RETRY_ATTEMPTS {
|
||||
let release_results = join_all(pending.into_iter().map(|(lock_id, client)| async move {
|
||||
match client.release(&lock_id).await {
|
||||
Ok(true) => None,
|
||||
Ok(false) => {
|
||||
warn!(%lock_id, attempt, context, "distributed unlock did not find lock on client");
|
||||
Some((lock_id, client))
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(%lock_id, attempt, context, "distributed unlock failed on client: {}", err);
|
||||
Some((lock_id, client))
|
||||
}
|
||||
}
|
||||
}))
|
||||
.await;
|
||||
|
||||
pending = release_results.into_iter().flatten().collect();
|
||||
if pending.is_empty() {
|
||||
debug!(attempt, context, "distributed unlock completed");
|
||||
return;
|
||||
}
|
||||
|
||||
if attempt < UNLOCK_RETRY_ATTEMPTS {
|
||||
tokio::time::sleep(UNLOCK_RETRY_BACKOFF * attempt as u32).await;
|
||||
}
|
||||
}
|
||||
|
||||
warn!(
|
||||
remaining = pending.len(),
|
||||
attempts = UNLOCK_RETRY_ATTEMPTS,
|
||||
context,
|
||||
"distributed unlock left unreleased entries after retry"
|
||||
);
|
||||
}
|
||||
|
||||
fn spawn_release_cleanup(entries: Vec<(LockId, Arc<dyn LockClient>)>, context: &'static str) {
|
||||
if entries.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Ok(handle) = tokio::runtime::Handle::try_current() {
|
||||
let join_handle = handle.spawn(async move {
|
||||
Self::release_entries(entries, context).await;
|
||||
});
|
||||
drop(join_handle);
|
||||
return;
|
||||
}
|
||||
|
||||
let join_handle = std::thread::spawn(move || match tokio::runtime::Builder::new_current_thread().enable_all().build() {
|
||||
Ok(runtime) => runtime.block_on(async move {
|
||||
Self::release_entries(entries, context).await;
|
||||
}),
|
||||
Err(err) => warn!(context, "failed to create fallback unlock runtime: {}", err),
|
||||
});
|
||||
drop(join_handle);
|
||||
}
|
||||
|
||||
fn spawn_pending_cleanup(
|
||||
@@ -387,9 +367,7 @@ impl DistributedLock {
|
||||
continue;
|
||||
};
|
||||
|
||||
if let Err(err) = client.release(&lock_id).await {
|
||||
tracing::warn!("{context}: failed to cleanup late lock {} on client {}: {}", lock_id, idx, err);
|
||||
}
|
||||
Self::release_entries(vec![(lock_id, client.clone())], context).await;
|
||||
}
|
||||
Ok((idx, Ok(resp))) => {
|
||||
tracing::debug!(
|
||||
@@ -506,7 +484,7 @@ impl DistributedLock {
|
||||
|
||||
if individual_locks.len() + pending.len() < required_quorum {
|
||||
let rollback_count = individual_locks.len();
|
||||
Self::release_entries(&individual_locks, "distributed_lock_quorum_rollback").await;
|
||||
Self::spawn_release_cleanup(individual_locks.clone(), "distributed_lock_quorum_rollback");
|
||||
if !pending.is_empty() {
|
||||
Self::spawn_pending_cleanup(
|
||||
pending,
|
||||
@@ -525,7 +503,7 @@ impl DistributedLock {
|
||||
}
|
||||
|
||||
let rollback_count = individual_locks.len();
|
||||
Self::release_entries(&individual_locks, "distributed_lock_quorum_rollback").await;
|
||||
Self::spawn_release_cleanup(individual_locks.clone(), "distributed_lock_quorum_rollback");
|
||||
let resp = LockResponse::failure(
|
||||
format!("Failed to acquire quorum: {rollback_count}/{required_quorum} required"),
|
||||
Duration::ZERO,
|
||||
|
||||
@@ -16,7 +16,10 @@ use super::*;
|
||||
use crate::client::{ClientFactory, local::LocalClient};
|
||||
use crate::types::LockType;
|
||||
use crate::{GlobalLockManager, LockError, LockInfo, LockResponse, LockStats};
|
||||
use std::sync::Arc;
|
||||
use std::sync::{
|
||||
Arc,
|
||||
atomic::{AtomicUsize, Ordering},
|
||||
};
|
||||
use std::time::Duration;
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
@@ -107,6 +110,75 @@ impl crate::client::LockClient for DelayedClient {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct FlakyReleaseClient {
|
||||
inner: LocalClient,
|
||||
failed_releases_remaining: AtomicUsize,
|
||||
release_attempts: AtomicUsize,
|
||||
}
|
||||
|
||||
impl FlakyReleaseClient {
|
||||
fn new(manager: Arc<GlobalLockManager>, failed_releases: usize) -> Self {
|
||||
Self {
|
||||
inner: LocalClient::with_manager(manager),
|
||||
failed_releases_remaining: AtomicUsize::new(failed_releases),
|
||||
release_attempts: AtomicUsize::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
fn release_attempts(&self) -> usize {
|
||||
self.release_attempts.load(Ordering::SeqCst)
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl crate::client::LockClient for FlakyReleaseClient {
|
||||
async fn acquire_lock(&self, request: &LockRequest) -> crate::Result<LockResponse> {
|
||||
self.inner.acquire_lock(request).await
|
||||
}
|
||||
|
||||
async fn release(&self, lock_id: &LockId) -> crate::Result<bool> {
|
||||
self.release_attempts.fetch_add(1, Ordering::SeqCst);
|
||||
if self
|
||||
.failed_releases_remaining
|
||||
.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |remaining| remaining.checked_sub(1))
|
||||
.is_ok()
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
self.inner.release(lock_id).await
|
||||
}
|
||||
|
||||
async fn refresh(&self, lock_id: &LockId) -> crate::Result<bool> {
|
||||
self.inner.refresh(lock_id).await
|
||||
}
|
||||
|
||||
async fn force_release(&self, lock_id: &LockId) -> crate::Result<bool> {
|
||||
self.inner.force_release(lock_id).await
|
||||
}
|
||||
|
||||
async fn check_status(&self, lock_id: &LockId) -> crate::Result<Option<LockInfo>> {
|
||||
self.inner.check_status(lock_id).await
|
||||
}
|
||||
|
||||
async fn get_stats(&self) -> crate::Result<LockStats> {
|
||||
self.inner.get_stats().await
|
||||
}
|
||||
|
||||
async fn close(&self) -> crate::Result<()> {
|
||||
self.inner.close().await
|
||||
}
|
||||
|
||||
async fn is_online(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
async fn is_local(&self) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
fn create_test_object_key(bucket: &str, object: &str) -> ObjectKey {
|
||||
ObjectKey {
|
||||
bucket: Arc::from(bucket),
|
||||
@@ -115,6 +187,41 @@ fn create_test_object_key(bucket: &str, object: &str) -> ObjectKey {
|
||||
}
|
||||
}
|
||||
|
||||
async fn wait_until_all_managers_can_write(managers: &[Arc<GlobalLockManager>], resource: ObjectKey) {
|
||||
let deadline = tokio::time::Instant::now() + Duration::from_secs(2);
|
||||
|
||||
loop {
|
||||
let mut guards = Vec::with_capacity(managers.len());
|
||||
let mut all_available = true;
|
||||
|
||||
for (idx, manager) in managers.iter().enumerate() {
|
||||
let local_lock = NamespaceLock::with_local_manager(format!("probe-node-{idx}"), manager.clone());
|
||||
match local_lock
|
||||
.get_write_lock(resource.clone(), "probe-owner", Duration::from_millis(20))
|
||||
.await
|
||||
{
|
||||
Ok(guard) => guards.push(guard),
|
||||
Err(_) => {
|
||||
all_available = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
drop(guards);
|
||||
|
||||
if all_available {
|
||||
return;
|
||||
}
|
||||
|
||||
assert!(
|
||||
tokio::time::Instant::now() < deadline,
|
||||
"distributed lock was not released on all simulated nodes"
|
||||
);
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_new() {
|
||||
let client = ClientFactory::create_local();
|
||||
@@ -174,6 +281,24 @@ async fn test_lock_client_default_batch_acquire_and_release() {
|
||||
assert_eq!(released, vec![true, true]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_local_client_uses_request_lock_id_for_release() {
|
||||
let manager = Arc::new(GlobalLockManager::new());
|
||||
let client = LocalClient::with_manager(manager);
|
||||
let resource = create_test_object_key("bucket", "object");
|
||||
let request = LockRequest::new(resource.clone(), LockType::Exclusive, "owner-a").with_acquire_timeout(Duration::from_secs(1));
|
||||
|
||||
let response = client.acquire_lock(&request).await.unwrap();
|
||||
let lock_info = response.lock_info.expect("successful acquire should return lock info");
|
||||
assert_eq!(lock_info.id, request.lock_id);
|
||||
|
||||
assert!(client.release(&request.lock_id).await.unwrap());
|
||||
|
||||
let second_request = LockRequest::new(resource, LockType::Exclusive, "owner-b").with_acquire_timeout(Duration::from_secs(1));
|
||||
let second_response = client.acquire_lock(&second_request).await.unwrap();
|
||||
assert!(second_response.success);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_get_resource_key() {
|
||||
let client = ClientFactory::create_local();
|
||||
@@ -488,6 +613,97 @@ async fn test_namespace_lock_distributed_with_clients_and_quorum() {
|
||||
drop(guard_b);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_distributed_eight_node_write_releases_all_nodes() {
|
||||
let managers = (0..8).map(|_| Arc::new(GlobalLockManager::new())).collect::<Vec<_>>();
|
||||
let clients = managers
|
||||
.iter()
|
||||
.map(|manager| Arc::new(LocalClient::with_manager(manager.clone())) as Arc<dyn LockClient>)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let lock = NamespaceLock::with_clients_and_quorum("eight-node".to_string(), clients, 5);
|
||||
let resource = create_test_object_key("bucket", "object-eight-node");
|
||||
|
||||
let mut guard = lock
|
||||
.get_write_lock(resource.clone(), "owner-a", Duration::from_secs(1))
|
||||
.await
|
||||
.expect("owner-a should acquire write lock across eight simulated nodes");
|
||||
|
||||
let err = lock
|
||||
.get_write_lock(resource.clone(), "owner-b", Duration::from_millis(100))
|
||||
.await
|
||||
.expect_err("owner-b should not acquire while owner-a holds all node locks");
|
||||
let err_str = err.to_string();
|
||||
assert!(
|
||||
err_str.contains("required 5") && err_str.contains("achieved"),
|
||||
"expected 8-node quorum failure below required write quorum, got: {err}"
|
||||
);
|
||||
|
||||
assert!(guard.release(), "distributed guard should enqueue release");
|
||||
wait_until_all_managers_can_write(&managers, resource).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_distributed_unlock_retries_release_false() {
|
||||
let managers = (0..3).map(|_| Arc::new(GlobalLockManager::new())).collect::<Vec<_>>();
|
||||
let flaky_clients = managers
|
||||
.iter()
|
||||
.map(|manager| Arc::new(FlakyReleaseClient::new(manager.clone(), 1)))
|
||||
.collect::<Vec<_>>();
|
||||
let clients = flaky_clients
|
||||
.iter()
|
||||
.map(|client| client.clone() as Arc<dyn LockClient>)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let lock = NamespaceLock::with_clients("flaky-release".to_string(), clients);
|
||||
let resource = create_test_object_key("bucket", "object-flaky-release");
|
||||
|
||||
let mut guard = lock
|
||||
.get_write_lock(resource.clone(), "owner-a", Duration::from_secs(1))
|
||||
.await
|
||||
.expect("owner-a should acquire write lock before flaky release");
|
||||
|
||||
assert!(guard.release(), "distributed guard should enqueue release");
|
||||
wait_until_all_managers_can_write(&managers, resource).await;
|
||||
|
||||
assert!(
|
||||
flaky_clients.iter().all(|client| client.release_attempts() >= 2),
|
||||
"each simulated node should be retried after an initial false release"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_namespace_lock_distributed_drop_without_runtime_does_not_panic() {
|
||||
let (manager, resource, guard) = {
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.expect("test runtime should be created");
|
||||
runtime.block_on(async {
|
||||
let manager = Arc::new(GlobalLockManager::new());
|
||||
let resource = create_test_object_key("bucket", "object-drop-no-runtime");
|
||||
let lock = NamespaceLock::with_clients(
|
||||
"drop-no-runtime".to_string(),
|
||||
vec![Arc::new(LocalClient::with_manager(manager.clone()))],
|
||||
);
|
||||
let guard = lock
|
||||
.get_write_lock(resource.clone(), "owner-a", Duration::from_secs(1))
|
||||
.await
|
||||
.expect("lock should be acquired");
|
||||
(manager, resource, guard)
|
||||
})
|
||||
};
|
||||
|
||||
let drop_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| drop(guard)));
|
||||
assert!(drop_result.is_ok(), "dropping distributed guard without runtime should not panic");
|
||||
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.expect("test runtime should be created");
|
||||
runtime.block_on(wait_until_all_managers_can_write(&[manager], resource));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_distributed_read_lock_succeeds_with_two_nodes_one_offline() {
|
||||
let manager = Arc::new(GlobalLockManager::new());
|
||||
@@ -568,6 +784,39 @@ async fn test_namespace_lock_distributed_quorum_failure_rolls_back_successful_no
|
||||
drop(guard2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_distributed_quorum_rollback_retries_release_false() {
|
||||
let managers = (0..2).map(|_| Arc::new(GlobalLockManager::new())).collect::<Vec<_>>();
|
||||
let flaky_clients = managers
|
||||
.iter()
|
||||
.map(|manager| Arc::new(FlakyReleaseClient::new(manager.clone(), 1)))
|
||||
.collect::<Vec<_>>();
|
||||
let clients = vec![
|
||||
flaky_clients[0].clone() as Arc<dyn LockClient>,
|
||||
flaky_clients[1].clone() as Arc<dyn LockClient>,
|
||||
Arc::new(FailingClient) as Arc<dyn LockClient>,
|
||||
];
|
||||
let resource = create_test_object_key("bucket", "object-rollback-retry");
|
||||
let lock = NamespaceLock::with_clients_and_quorum("rollback-retry".to_string(), clients, 3);
|
||||
|
||||
let err = lock
|
||||
.get_write_lock(resource.clone(), "owner-a", Duration::from_millis(100))
|
||||
.await
|
||||
.expect_err("write lock should fail when quorum requires the offline node");
|
||||
|
||||
let err_str = err.to_string().to_lowercase();
|
||||
assert!(
|
||||
err_str.contains("quorum") || err_str.contains("not reached"),
|
||||
"expected quorum error, got: {err}"
|
||||
);
|
||||
wait_until_all_managers_can_write(&managers, resource).await;
|
||||
|
||||
assert!(
|
||||
flaky_clients.iter().all(|client| client.release_attempts() >= 2),
|
||||
"rollback should retry node releases that initially returned false"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_namespace_lock_distributed_even_node_read_write_quorum_split() {
|
||||
let manager1 = Arc::new(GlobalLockManager::new());
|
||||
|
||||
@@ -15,7 +15,12 @@
|
||||
use crate::notification_system_subscriber::NotificationSystemSubscriberView;
|
||||
use crate::notifier::TargetList;
|
||||
use crate::{
|
||||
Event, error::NotificationError, notifier::EventNotifier, registry::TargetRegistry, rules::BucketNotificationConfig, stream,
|
||||
Event,
|
||||
error::NotificationError,
|
||||
notifier::EventNotifier,
|
||||
registry::TargetRegistry,
|
||||
rules::{BucketNotificationConfig, ParseConfigError},
|
||||
stream,
|
||||
};
|
||||
use hashbrown::HashMap;
|
||||
use rustfs_config::notify::{
|
||||
@@ -33,7 +38,7 @@ use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::time::{Duration, Instant};
|
||||
use tokio::sync::{RwLock, Semaphore, broadcast, mpsc};
|
||||
use tracing::{debug, error, info, warn};
|
||||
use tracing::{debug, info, warn};
|
||||
|
||||
const MAX_RECENT_LIVE_EVENTS: usize = 1024;
|
||||
|
||||
@@ -309,7 +314,10 @@ impl NotificationSystem {
|
||||
|
||||
let config = {
|
||||
let guard = self.config.read().await;
|
||||
debug!("Initializing notification system with config: {:?}", *guard);
|
||||
debug!(
|
||||
subsystem_count = guard.0.len(),
|
||||
"Initializing notification system with configuration summary"
|
||||
);
|
||||
guard.clone()
|
||||
};
|
||||
|
||||
@@ -517,7 +525,10 @@ impl NotificationSystem {
|
||||
if !changed {
|
||||
info!("Target {} of type {} not found, no changes made.", target_name, target_type);
|
||||
}
|
||||
debug!("Config after remove: {:?}", config);
|
||||
debug!(
|
||||
subsystem_count = config.0.len(),
|
||||
"Target config removal processed and configuration summary updated"
|
||||
);
|
||||
changed
|
||||
})
|
||||
.await;
|
||||
@@ -593,7 +604,6 @@ impl NotificationSystem {
|
||||
bucket: &str,
|
||||
cfg: &BucketNotificationConfig,
|
||||
) -> Result<(), NotificationError> {
|
||||
self.subscriber_view.apply_bucket_config(bucket, cfg);
|
||||
let arn_list = self.notifier.get_arn_list(&cfg.region).await;
|
||||
if arn_list.is_empty() {
|
||||
return Err(NotificationError::Configuration("No targets configured".to_string()));
|
||||
@@ -602,13 +612,18 @@ impl NotificationSystem {
|
||||
// Validate the configuration against the available ARNs
|
||||
if let Err(e) = cfg.validate(&cfg.region, &arn_list) {
|
||||
debug!("Bucket notification config validation region:{} failed: {}", &cfg.region, e);
|
||||
if !e.to_string().contains("ARN not found") {
|
||||
if !matches!(e, ParseConfigError::ArnNotFound(_)) {
|
||||
return Err(NotificationError::BucketNotification(e.to_string()));
|
||||
} else {
|
||||
error!("config validate failed, err: {}", e);
|
||||
}
|
||||
warn!(
|
||||
bucket = %bucket,
|
||||
region = %cfg.region,
|
||||
error = %e,
|
||||
"Bucket notification config references missing target ARN; keeping compatibility and loading remaining rules"
|
||||
);
|
||||
}
|
||||
|
||||
self.subscriber_view.apply_bucket_config(bucket, cfg);
|
||||
let rules_map = cfg.get_rules_map();
|
||||
self.notifier.add_rules_map(bucket, rules_map.clone()).await;
|
||||
info!("Loaded notification config for bucket: {}", bucket);
|
||||
|
||||
@@ -28,6 +28,33 @@ pub fn collect_target_configs(
|
||||
collect_target_configs_from_env(config, route_prefix, target_type, valid_fields, std::env::vars())
|
||||
}
|
||||
|
||||
fn is_sensitive_target_field(field_name: &str) -> bool {
|
||||
let field_name = field_name.to_ascii_lowercase();
|
||||
field_name.contains("password")
|
||||
|| field_name.contains("secret")
|
||||
|| field_name.contains("token")
|
||||
|| field_name.contains("credential")
|
||||
|| field_name.contains("private_key")
|
||||
|| field_name.contains("client_key")
|
||||
|| field_name.contains("access_key")
|
||||
|| field_name.contains("auth")
|
||||
}
|
||||
|
||||
fn redact_target_field_value(field_name: &str, value: &str) -> String {
|
||||
if is_sensitive_target_field(field_name) && !value.is_empty() {
|
||||
return "***redacted***".to_string();
|
||||
}
|
||||
value.to_string()
|
||||
}
|
||||
|
||||
fn redacted_target_config(config: &KVS) -> Vec<(String, String)> {
|
||||
config
|
||||
.0
|
||||
.iter()
|
||||
.map(|kv| (kv.key.clone(), redact_target_field_value(&kv.key, &kv.value)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn collect_env_target_instance_ids(route_prefix: &str, target_type: &str, valid_fields: &HashSet<String>) -> HashSet<String> {
|
||||
collect_env_target_instance_ids_from_env(route_prefix, target_type, valid_fields, std::env::vars())
|
||||
}
|
||||
@@ -104,7 +131,7 @@ where
|
||||
debug!(
|
||||
instance_id = %if instance_id == DEFAULT_DELIMITER { DEFAULT_DELIMITER } else { &instance_id },
|
||||
%field_name,
|
||||
%value,
|
||||
value = %redact_target_field_value(&field_name, value),
|
||||
"Parsed target environment override"
|
||||
);
|
||||
env_overrides
|
||||
@@ -137,7 +164,10 @@ where
|
||||
merged_config.extend(env_instance_cfg.clone());
|
||||
}
|
||||
|
||||
debug!(instance_id = %id, ?merged_config, "Merged target configuration");
|
||||
if tracing::enabled!(tracing::Level::DEBUG) {
|
||||
let redacted_config = redacted_target_config(&merged_config);
|
||||
debug!(instance_id = %id, ?redacted_config, "Merged target configuration");
|
||||
}
|
||||
if is_target_enabled(&merged_config) {
|
||||
merged_configs.push((id, merged_config));
|
||||
}
|
||||
@@ -148,7 +178,10 @@ where
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{collect_env_target_instance_ids_from_env, collect_target_configs_from_env};
|
||||
use super::{
|
||||
collect_env_target_instance_ids_from_env, collect_target_configs_from_env, redact_target_field_value,
|
||||
redacted_target_config,
|
||||
};
|
||||
use rustfs_config::notify::NOTIFY_ROUTE_PREFIX;
|
||||
use rustfs_config::{ENABLE_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_LIMIT};
|
||||
use rustfs_ecstore::config::{Config, KVS};
|
||||
@@ -236,4 +269,40 @@ mod tests {
|
||||
|
||||
assert_eq!(ids, HashSet::from(["primary".to_string()]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redact_target_field_value_redacts_sensitive_fields() {
|
||||
assert_eq!(redact_target_field_value("password", "secret"), "***redacted***");
|
||||
assert_eq!(redact_target_field_value("auth_token", "token"), "***redacted***");
|
||||
assert_eq!(redact_target_field_value("credentials_file", "/tmp/creds"), "***redacted***");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redact_target_field_value_keeps_non_sensitive_fields() {
|
||||
assert_eq!(redact_target_field_value("endpoint", "https://example.com"), "https://example.com");
|
||||
assert_eq!(redact_target_field_value("queue_limit", "1000"), "1000");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacted_target_config_masks_sensitive_values_without_mutating_shape() {
|
||||
let mut config = KVS::new();
|
||||
config.insert("endpoint".to_string(), "https://example.com/hook".to_string());
|
||||
config.insert("password".to_string(), "super-secret".to_string());
|
||||
config.insert("client_key".to_string(), "private-key".to_string());
|
||||
config.insert("auth_token".to_string(), "bearer-token".to_string());
|
||||
config.insert("empty_secret".to_string(), String::new());
|
||||
|
||||
let redacted = redacted_target_config(&config);
|
||||
|
||||
assert_eq!(
|
||||
redacted,
|
||||
vec![
|
||||
("endpoint".to_string(), "https://example.com/hook".to_string()),
|
||||
("password".to_string(), "***redacted***".to_string()),
|
||||
("client_key".to_string(), "***redacted***".to_string()),
|
||||
("auth_token".to_string(), "***redacted***".to_string()),
|
||||
("empty_secret".to_string(), String::new()),
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -672,7 +672,7 @@ where
|
||||
bucket = %meta.bucket_name,
|
||||
object = %meta.object_name,
|
||||
event = %meta.event_name,
|
||||
preview = %meta.best_effort_preview(&body, 256),
|
||||
payload_len = body.len(),
|
||||
"Sending MQTT payload"
|
||||
);
|
||||
|
||||
|
||||
@@ -288,7 +288,7 @@ where
|
||||
bucket = %meta.bucket_name,
|
||||
object = %meta.object_name,
|
||||
event = %meta.event_name,
|
||||
preview = %meta.best_effort_preview(&body, 256),
|
||||
payload_len = body.len(),
|
||||
"Sending webhook payload"
|
||||
);
|
||||
|
||||
|
||||
@@ -925,11 +925,13 @@ impl Operation for ListServiceAccount {
|
||||
};
|
||||
|
||||
let target_account = if query.user.as_ref().is_some_and(|v| v != &cred.access_key) {
|
||||
// Cross-user listing must be authorized by ListServiceAccounts, matching the
|
||||
// sibling InfoServiceAccount/InfoAccessKey/ListAccessKeysBulk handlers.
|
||||
if !iam_store
|
||||
.is_allowed(&Args {
|
||||
account: &cred.access_key,
|
||||
groups: &cred.groups,
|
||||
action: Action::AdminAction(AdminAction::UpdateServiceAccountAdminAction),
|
||||
action: Action::AdminAction(AdminAction::ListServiceAccountsAdminAction),
|
||||
bucket: "",
|
||||
conditions: &get_condition_values(
|
||||
&req.headers,
|
||||
|
||||
+23
-35
@@ -38,7 +38,7 @@ use rustfs_ecstore::{
|
||||
},
|
||||
error::{StorageError, is_err_bucket_not_found, is_err_object_not_found, is_err_version_not_found},
|
||||
new_object_layer_fn,
|
||||
store_api::{BucketOperations, BucketOptions, ObjectOperations, ObjectOptions},
|
||||
store_api::{BucketOperations, BucketOptions, ObjectLockRetentionOptions, ObjectOperations, ObjectOptions},
|
||||
};
|
||||
use rustfs_s3_common::{S3Operation, record_s3_op};
|
||||
use rustfs_targets::EventName;
|
||||
@@ -1284,44 +1284,27 @@ impl S3 for FS {
|
||||
.as_ref()
|
||||
.and_then(|r| r.retain_until_date.as_ref())
|
||||
.map(|d| OffsetDateTime::from(d.clone()));
|
||||
let new_mode = retention.as_ref().and_then(|r| r.mode.as_ref()).map(|mode| mode.as_str());
|
||||
let new_mode = retention
|
||||
.as_ref()
|
||||
.and_then(|r| r.mode.as_ref())
|
||||
.map(|mode| mode.as_str().to_string());
|
||||
|
||||
// TODO(security): Known TOCTOU race condition (fix in future PR).
|
||||
//
|
||||
// There is a time-of-check-time-of-use (TOCTOU) window between the retention
|
||||
// check below (using get_object_info + check_retention_for_modification) and
|
||||
// the actual update performed later in put_object_metadata.
|
||||
//
|
||||
// In theory:
|
||||
// * Thread A reads retention mode = GOVERNANCE and checks the bypass header.
|
||||
// * Thread B updates retention to COMPLIANCE mode.
|
||||
// * Thread A then proceeds to modify retention, still assuming GOVERNANCE,
|
||||
// and effectively bypasses what is now COMPLIANCE mode.
|
||||
//
|
||||
// This would violate the S3 spec, which states that COMPLIANCE-mode retention
|
||||
// cannot be modified even with a bypass header.
|
||||
//
|
||||
// Possible fixes (to be implemented in a future change):
|
||||
// 1. Pass the expected retention mode down to the storage layer and verify
|
||||
// it has not changed immediately before the update.
|
||||
// 2. Use optimistic concurrency (e.g., version/etag) so that the update
|
||||
// fails if the object changed between check and update.
|
||||
// 3. Perform the retention check inside the same lock/transaction scope as
|
||||
// the metadata update within the storage layer.
|
||||
//
|
||||
// Current mitigation: the storage layer provides a fast_lock_manager, which
|
||||
// offers some protection, but it does not fully eliminate this race.
|
||||
let bypass_governance = has_bypass_governance_header(&req.headers);
|
||||
// Keep the early check for existing response behavior; put_object_metadata
|
||||
// repeats the same check after taking the metadata write lock.
|
||||
let check_opts: ObjectOptions = get_opts(&bucket, &key, version_id.clone(), None, &req.headers)
|
||||
.await
|
||||
.map_err(ApiError::from)?;
|
||||
|
||||
if let Ok(existing_obj_info) = store.get_object_info(&bucket, &key, &check_opts).await {
|
||||
let bypass_governance = has_bypass_governance_header(&req.headers);
|
||||
if let Some(block_reason) =
|
||||
check_retention_for_modification(&existing_obj_info.user_defined, new_mode, new_retain_until, bypass_governance)
|
||||
{
|
||||
return Err(S3Error::with_message(S3ErrorCode::AccessDenied, block_reason.error_message()));
|
||||
}
|
||||
if let Ok(existing_obj_info) = store.get_object_info(&bucket, &key, &check_opts).await
|
||||
&& let Some(block_reason) = check_retention_for_modification(
|
||||
&existing_obj_info.user_defined,
|
||||
new_mode.as_deref(),
|
||||
new_retain_until,
|
||||
bypass_governance,
|
||||
)
|
||||
{
|
||||
return Err(S3Error::with_message(S3ErrorCode::AccessDenied, block_reason.error_message()));
|
||||
}
|
||||
|
||||
let eval_metadata = parse_object_lock_retention(retention)?;
|
||||
@@ -1330,10 +1313,15 @@ impl S3 for FS {
|
||||
.await
|
||||
.map_err(ApiError::from)?;
|
||||
opts.eval_metadata = Some(eval_metadata);
|
||||
opts.object_lock_retention = Some(ObjectLockRetentionOptions {
|
||||
mode: new_mode,
|
||||
retain_until: new_retain_until,
|
||||
bypass_governance,
|
||||
});
|
||||
|
||||
let object_info = store.put_object_metadata(&bucket, &key, &opts).await.map_err(|e| {
|
||||
error!("put_object_metadata failed, {}", e.to_string());
|
||||
s3_error!(InternalError, "{}", e.to_string())
|
||||
S3Error::from(ApiError::from(e))
|
||||
})?;
|
||||
|
||||
let output = PutObjectRetentionOutput {
|
||||
|
||||
@@ -30,6 +30,18 @@ fn lock_result_from_error(error: impl Into<String>) -> GenerallyLockResult {
|
||||
}
|
||||
}
|
||||
|
||||
fn lock_result_from_release(lock_id: &rustfs_lock::LockId, success: bool) -> GenerallyLockResult {
|
||||
if success {
|
||||
GenerallyLockResult {
|
||||
success: true,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
}
|
||||
} else {
|
||||
lock_result_from_error(format!("lock not found for release: {lock_id}"))
|
||||
}
|
||||
}
|
||||
|
||||
impl NodeService {
|
||||
pub(super) async fn handle_refresh(
|
||||
&self,
|
||||
@@ -71,12 +83,15 @@ impl NodeService {
|
||||
};
|
||||
|
||||
let lock_client = self.get_lock_client()?;
|
||||
match lock_client.release(&args.lock_id).await {
|
||||
Ok(_) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: true,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
})),
|
||||
match lock_client.force_release(&args.lock_id).await {
|
||||
Ok(success) => {
|
||||
let result = lock_result_from_release(&args.lock_id, success);
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: result.error_info,
|
||||
lock_info: None,
|
||||
}))
|
||||
}
|
||||
Err(err) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: false,
|
||||
error_info: Some(format!(
|
||||
@@ -106,11 +121,14 @@ impl NodeService {
|
||||
|
||||
let lock_client = self.get_lock_client()?;
|
||||
match lock_client.release(&args.lock_id).await {
|
||||
Ok(_) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: true,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
})),
|
||||
Ok(success) => {
|
||||
let result = lock_result_from_release(&args.lock_id, success);
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: result.error_info,
|
||||
lock_info: None,
|
||||
}))
|
||||
}
|
||||
Err(err) => Ok(Response::new(GenerallyLockResponse {
|
||||
success: false,
|
||||
error_info: Some(format!(
|
||||
@@ -146,7 +164,7 @@ impl NodeService {
|
||||
let lock_info_json = result.lock_info.as_ref().and_then(|info| serde_json::to_string(info).ok());
|
||||
Ok(Response::new(GenerallyLockResponse {
|
||||
success: result.success,
|
||||
error_info: None,
|
||||
error_info: result.error,
|
||||
lock_info: lock_info_json,
|
||||
}))
|
||||
}
|
||||
@@ -230,10 +248,9 @@ impl NodeService {
|
||||
Ok(batch_results) => {
|
||||
for (result_idx, success) in batch_results.into_iter().enumerate() {
|
||||
if let Some(request_idx) = valid_indices.get(result_idx) {
|
||||
results[*request_idx] = GenerallyLockResult {
|
||||
success,
|
||||
error_info: None,
|
||||
lock_info: None,
|
||||
results[*request_idx] = match lock_ids.get(result_idx) {
|
||||
Some(lock_id) => lock_result_from_release(lock_id, success),
|
||||
None => lock_result_from_error(format!("unlock response index out of range: {result_idx}")),
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -249,3 +266,38 @@ impl NodeService {
|
||||
Ok(Response::new(BatchGenerallyLockResponse { results }))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn test_lock_id() -> rustfs_lock::LockId {
|
||||
rustfs_lock::LockRequest::new(rustfs_lock::ObjectKey::new("bucket", "object"), rustfs_lock::LockType::Exclusive, "owner")
|
||||
.lock_id
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lock_result_from_release_reports_missing_lock() {
|
||||
let lock_id = test_lock_id();
|
||||
let result = lock_result_from_release(&lock_id, false);
|
||||
|
||||
assert!(!result.success);
|
||||
assert!(result.lock_info.is_none());
|
||||
assert!(
|
||||
result
|
||||
.error_info
|
||||
.expect("missing release should include error")
|
||||
.contains("lock not found for release")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lock_result_from_response_preserves_lock_failure_error() {
|
||||
let response = rustfs_lock::LockResponse::failure("lock conflict", std::time::Duration::ZERO);
|
||||
let result = lock_result_from_response(response);
|
||||
|
||||
assert!(!result.success);
|
||||
assert_eq!(result.error_info.as_deref(), Some("lock conflict"));
|
||||
assert!(result.lock_info.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,7 @@ SAMPLES=20000
|
||||
|
||||
# optional hooks
|
||||
APPLY_CMD=""
|
||||
APPLY_CMD_ARR=()
|
||||
APPLY_WAIT_SECS=20
|
||||
|
||||
EXTRA_ARGS=()
|
||||
@@ -74,7 +75,7 @@ s3bench options:
|
||||
|
||||
Hooks:
|
||||
--apply-cmd Optional command to apply/restart RustFS after profile env switch.
|
||||
Runs via: eval "$APPLY_CMD"
|
||||
Executed directly (no shell eval), e.g. "bash scripts/restart.sh"
|
||||
--apply-wait-secs Wait time after apply cmd (default: 20)
|
||||
|
||||
Extra:
|
||||
@@ -110,6 +111,23 @@ validate_positive_int() {
|
||||
fi
|
||||
}
|
||||
|
||||
parse_apply_cmd() {
|
||||
local raw="$1"
|
||||
|
||||
if [[ "$raw" == *';'* || "$raw" == *'&&'* || "$raw" == *'||'* || "$raw" == *'|'* || "$raw" == *'<'* || "$raw" == *'>'* || "$raw" == *'`'* || "$raw" == *'$'* ]]; then
|
||||
echo "ERROR: --apply-cmd does not allow shell operators or expansions; pass a plain command and args only" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
IFS=$' \t\n' read -r -a APPLY_CMD_ARR <<< "$raw"
|
||||
if [[ "${#APPLY_CMD_ARR[@]}" -eq 0 ]]; then
|
||||
echo "ERROR: --apply-cmd must not be empty" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
require_cmd "${APPLY_CMD_ARR[0]}"
|
||||
}
|
||||
|
||||
parse_args() {
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case "$1" in
|
||||
@@ -171,6 +189,9 @@ validate_args() {
|
||||
if [[ "$TOOL" == "s3bench" ]]; then
|
||||
validate_positive_int "$SAMPLES" "--samples"
|
||||
fi
|
||||
if [[ -n "$APPLY_CMD" ]]; then
|
||||
parse_apply_cmd "$APPLY_CMD"
|
||||
fi
|
||||
}
|
||||
|
||||
setup_out_root() {
|
||||
@@ -238,15 +259,17 @@ sizes_for_group() {
|
||||
|
||||
run_apply_hook_if_needed() {
|
||||
local group="$1"
|
||||
if [[ -z "$APPLY_CMD" ]]; then
|
||||
if [[ "${#APPLY_CMD_ARR[@]}" -eq 0 ]]; then
|
||||
return
|
||||
fi
|
||||
echo "[${group}] running apply command..."
|
||||
if [[ "$DRY_RUN" == "true" ]]; then
|
||||
echo "[DRY-RUN] eval \"$APPLY_CMD\""
|
||||
printf '[DRY-RUN] '
|
||||
printf '%q ' "${APPLY_CMD_ARR[@]}"
|
||||
printf '\n'
|
||||
echo "[DRY-RUN] sleep $APPLY_WAIT_SECS"
|
||||
else
|
||||
eval "$APPLY_CMD"
|
||||
"${APPLY_CMD_ARR[@]}"
|
||||
echo "[${group}] waiting ${APPLY_WAIT_SECS}s for service readiness..."
|
||||
sleep "$APPLY_WAIT_SECS"
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user