diff --git a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs index a969588c8..135c77927 100644 --- a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs +++ b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs @@ -4137,20 +4137,27 @@ pub async fn eval_action_from_lifecycle( ); let lock_enabled = if let Some(lr) = lr { lr.mode.is_some() } else { false }; + let object_locked = object_lock_boundary::is_object_locked_by_metadata(&oi.user_defined, oi.delete_marker); match event.action { - IlmAction::DeleteAllVersionsAction | IlmAction::DelMarkerDeleteAllVersionsAction if lock_enabled => { + IlmAction::DeleteAllVersionsAction | IlmAction::DelMarkerDeleteAllVersionsAction if lock_enabled || object_locked => { return lifecycle::Event::default(); } - IlmAction::DeleteVersionAction | IlmAction::DeleteRestoredVersionAction => { - if oi.version_id.is_none() { + IlmAction::DeleteAction + | IlmAction::DeleteRestoredAction + | IlmAction::DeleteVersionAction + | IlmAction::DeleteRestoredVersionAction => { + if matches!(event.action, IlmAction::DeleteVersionAction | IlmAction::DeleteRestoredVersionAction) + && oi.version_id.is_none() + { return lifecycle::Event::default(); } // Lifecycle operations should never bypass governance retention - if lock_enabled - && object_lock_boundary::check_object_lock_for_deletion(&oi.bucket, oi, false) - .await - .is_some() + if object_locked + || (lock_enabled + && object_lock_boundary::check_object_lock_for_deletion(&oi.bucket, oi, false) + .await + .is_some()) { //if serverDebugLog { if oi.version_id.is_some() { @@ -4599,6 +4606,7 @@ mod tests { BucketLifecycleConfiguration, ExpirationStatus, LifecycleExpiration, LifecycleRule, MetadataEntry, OutputLocation, RestoreRequest, RestoreRequestType, S3Location, Timestamp, Transition, TransitionStorageClass, }; + use s3s::header::{X_AMZ_OBJECT_LOCK_LEGAL_HOLD, X_AMZ_OBJECT_LOCK_MODE, X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE}; use serial_test::serial; use sha2::{Digest, Sha256}; use std::collections::HashMap; @@ -7136,6 +7144,16 @@ mod tests { } } + fn current_object_with_metadata( + replication_status: ReplicationStatusType, + user_defined: HashMap, + ) -> ObjectInfo { + ObjectInfo { + user_defined: Arc::new(user_defined), + ..current_object(replication_status) + } + } + #[test] fn lifecycle_replication_blocks_only_pending_failed_or_pending_purge() { assert!(lifecycle_replication_blocks_action(&delete_marker_object( @@ -7737,6 +7755,41 @@ mod tests { assert_eq!(event.action, IlmAction::DeleteAction); } + #[tokio::test] + async fn existing_object_lifecycle_skips_current_expiration_for_explicit_legal_hold() { + let lc = latest_expiration_lifecycle(); + let object = current_object_with_metadata( + ReplicationStatusType::Completed, + HashMap::from([(X_AMZ_OBJECT_LOCK_LEGAL_HOLD.as_str().to_string(), "ON".to_string())]), + ); + + let event = eval_action_from_lifecycle(&lc, None, &object).await; + + assert_eq!(event.action, IlmAction::NoneAction); + } + + #[tokio::test] + async fn existing_object_lifecycle_skips_current_expiration_for_explicit_retention() { + let lc = latest_expiration_lifecycle(); + let retain_until = (OffsetDateTime::now_utc() + time::Duration::days(30)) + .format(&time::format_description::well_known::Rfc3339) + .expect("future retain-until date should format"); + let object = current_object_with_metadata( + ReplicationStatusType::Completed, + HashMap::from([ + ( + X_AMZ_OBJECT_LOCK_MODE.as_str().to_string(), + s3s::dto::ObjectLockRetentionMode::COMPLIANCE.to_string(), + ), + (X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str().to_string(), retain_until), + ]), + ); + + let event = eval_action_from_lifecycle(&lc, None, &object).await; + + assert_eq!(event.action, IlmAction::NoneAction); + } + #[tokio::test] async fn existing_object_lifecycle_skips_transition_while_replication_pending() { let lc = latest_transition_lifecycle(); diff --git a/crates/lifecycle/src/evaluator.rs b/crates/lifecycle/src/evaluator.rs index 3f722fef2..03d8a3613 100644 --- a/crates/lifecycle/src/evaluator.rs +++ b/crates/lifecycle/src/evaluator.rs @@ -66,19 +66,8 @@ impl Evaluator { } /// IsObjectLocked checks if it is appropriate to remove an - /// object according to locking configuration when this is lifecycle/bucket quota asking. - /// Uses the common `is_object_locked_by_metadata` function for consistency. + /// object according to its persisted object-lock metadata. pub fn is_object_locked(&self, obj: &ObjectOpts) -> bool { - // First check if object lock is enabled for this bucket - if self.lock_retention.as_ref().is_none_or(|v| { - v.object_lock_enabled - .as_ref() - .is_none_or(|v| v.as_str() != ObjectLockEnabled::ENABLED) - }) { - return false; - } - - // Use the common function to check if the object is locked object_lock::is_object_locked_by_metadata(&obj.user_defined, obj.delete_marker) } @@ -102,7 +91,8 @@ impl Evaluator { v.object_lock_enabled .as_ref() .is_some_and(|v| v.as_str() == ObjectLockEnabled::ENABLED) - }) || self.any_version_has_pending_replication(objs) + }) || objs.iter().any(|obj| self.is_object_locked(obj)) + || self.any_version_has_pending_replication(objs) { event = Event::default(); } else { @@ -122,9 +112,14 @@ impl Evaluator { break 'top_loop; } } - IlmAction::DeleteVersionAction | IlmAction::DeleteRestoredVersionAction => { + IlmAction::DeleteAction + | IlmAction::DeleteRestoredAction + | IlmAction::DeleteVersionAction + | IlmAction::DeleteRestoredVersionAction => { // Defensive code, should never happen - if obj.version_id.is_none_or(|v| v.is_nil()) { + if matches!(event.action, IlmAction::DeleteVersionAction | IlmAction::DeleteRestoredVersionAction) + && obj.version_id.is_none_or(|v| v.is_nil()) + { event.action = IlmAction::NoneAction; } if self.is_object_locked(obj) { @@ -198,12 +193,15 @@ fn lifecycle_action_waits_for_replication(action: IlmAction) -> bool { #[cfg(test)] mod tests { + use std::collections::HashMap; use std::sync::Arc; use rustfs_common::metrics::IlmAction; use s3s::dto::{ - BucketLifecycleConfiguration, ExpirationStatus, LifecycleExpiration, LifecycleRule, Transition, TransitionStorageClass, + BucketLifecycleConfiguration, ExpirationStatus, LifecycleExpiration, LifecycleRule, ObjectLockConfiguration, + ObjectLockEnabled, Transition, TransitionStorageClass, }; + use s3s::header::{X_AMZ_OBJECT_LOCK_LEGAL_HOLD, X_AMZ_OBJECT_LOCK_MODE, X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE}; use time::OffsetDateTime; use uuid::Uuid; @@ -295,6 +293,13 @@ mod tests { }) } + fn lock_enabled_without_default_retention() -> Arc { + Arc::new(ObjectLockConfiguration { + object_lock_enabled: Some(ObjectLockEnabled::from_static(ObjectLockEnabled::ENABLED)), + rule: None, + }) + } + fn object_opts(replication_status: ReplicationStatusType, version_purge_status: VersionPurgeStatusType) -> ObjectOpts { ObjectOpts { name: "logs/object".to_string(), @@ -321,6 +326,16 @@ mod tests { } } + fn locked_current_object_opts(replication_status: ReplicationStatusType) -> ObjectOpts { + let mut user_defined = HashMap::new(); + user_defined.insert(X_AMZ_OBJECT_LOCK_LEGAL_HOLD.as_str().to_string(), "ON".to_string()); + + ObjectOpts { + user_defined, + ..current_object_opts(replication_status) + } + } + fn versioned_object_opts(replication_status: ReplicationStatusType, is_latest: bool) -> ObjectOpts { ObjectOpts { num_versions: 2, @@ -329,6 +344,23 @@ mod tests { } } + fn locked_versioned_object_opts(replication_status: ReplicationStatusType, is_latest: bool) -> ObjectOpts { + let retain_until = (OffsetDateTime::now_utc() + time::Duration::days(30)) + .format(&time::format_description::well_known::Rfc3339) + .expect("future retain-until date should format"); + 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(), retain_until); + + ObjectOpts { + user_defined, + ..versioned_object_opts(replication_status, is_latest) + } + } + #[tokio::test] async fn evaluator_allows_expired_delete_marker_after_replication_completed() { let evaluator = Evaluator::new(expired_marker_lifecycle()); @@ -414,6 +446,19 @@ mod tests { assert_eq!(events[0].action, IlmAction::DeleteAction); } + #[tokio::test] + async fn evaluator_skips_latest_expiration_for_explicit_legal_hold_without_default_retention() { + let evaluator = + Evaluator::new(latest_expiration_lifecycle()).with_lock_retention(Some(lock_enabled_without_default_retention())); + + let events = evaluator + .eval(&[locked_current_object_opts(ReplicationStatusType::Completed)]) + .await + .expect("explicit legal hold should still produce a lifecycle decision"); + + assert_eq!(events[0].action, IlmAction::NoneAction); + } + #[tokio::test] async fn evaluator_skips_transition_while_replication_pending() { let evaluator = Evaluator::new(latest_transition_lifecycle()); @@ -467,4 +512,20 @@ mod tests { assert_eq!(events[0].action, IlmAction::DeleteAllVersionsAction); assert_eq!(events[1].action, IlmAction::NoneAction); } + + #[tokio::test] + async fn evaluator_skips_delete_all_versions_for_explicit_retention_without_default_retention() { + let evaluator = Evaluator::new(all_versions_expiration_lifecycle()) + .with_lock_retention(Some(lock_enabled_without_default_retention())); + let latest = versioned_object_opts(ReplicationStatusType::Completed, true); + let noncurrent = locked_versioned_object_opts(ReplicationStatusType::Completed, false); + + let events = evaluator + .eval(&[latest, noncurrent]) + .await + .expect("explicit retention should still produce lifecycle decisions"); + + assert_eq!(events[0].action, IlmAction::NoneAction); + assert_eq!(events[1].action, IlmAction::NoneAction); + } }