fix(lifecycle): respect Filter.Prefix and safe delete marker expiry (#2185)

Signed-off-by: likewu <likewu@126.com>
Signed-off-by: houseme <housemecn@gmail.com>
Co-authored-by: likewu <likewu@126.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>
Co-authored-by: houseme <housemecn@gmail.com>

crates/ecstore/src/bucket/lifecycle/lifecycle.rs

@@ -20,8 +20,8 @@
 
 use rustfs_filemeta::{ReplicationStatusType, VersionPurgeStatusType};
 use s3s::dto::{
-    BucketLifecycleConfiguration, ExpirationStatus, LifecycleExpiration, LifecycleRule, NoncurrentVersionTransition,
+    BucketLifecycleConfiguration, ExpirationStatus, LifecycleExpiration, LifecycleRule, LifecycleRuleAndOperator,
-    ObjectLockConfiguration, ObjectLockEnabled, RestoreRequest, Transition, TransitionStorageClass,
+    NoncurrentVersionTransition, ObjectLockConfiguration, ObjectLockEnabled, RestoreRequest, Transition, TransitionStorageClass,
 };
 use std::cmp::Ordering;
 use std::collections::HashMap;
@@ -134,6 +134,25 @@ impl RuleValidate for LifecycleRule {
     }
 }
 
+fn lifecycle_rule_prefix(rule: &LifecycleRule) -> Option<&str> {
+    // Prefer a non-empty legacy prefix; treat an empty legacy prefix as if it were not set
+    if let Some(p) = rule.prefix.as_deref() {
+        if !p.is_empty() {
+            return Some(p);
+        }
+    }
+
+    let Some(filter) = rule.filter.as_ref() else {
+        return None;
+    };
+
+    if let Some(p) = filter.prefix.as_deref() {
+        return Some(p);
+    }
+
+    filter.and.as_ref().and_then(|and| and.prefix.as_deref())
+}
+
 #[async_trait::async_trait]
 pub trait Lifecycle {
     async fn has_transition(&self) -> bool;
@@ -177,8 +196,11 @@ impl Lifecycle for BucketLifecycleConfiguration {
                 continue;
             }
 
-            let rule_prefix = &rule.prefix.clone().unwrap_or_default();
+            let rule_prefix = lifecycle_rule_prefix(rule).unwrap_or("");
-            if prefix.len() > 0 && rule_prefix.len() > 0 && !prefix.starts_with(rule_prefix) && !rule_prefix.starts_with(&prefix)
+            if !prefix.is_empty()
+                && !rule_prefix.is_empty()
+                && !prefix.starts_with(rule_prefix)
+                && !rule_prefix.starts_with(prefix)
             {
                 continue;
             }
@@ -297,8 +319,8 @@ impl Lifecycle for BucketLifecycleConfiguration {
             if rule.status.as_str() == ExpirationStatus::DISABLED {
                 continue;
             }
-            if let Some(prefix) = rule.prefix.clone() {
+            if let Some(rule_prefix) = lifecycle_rule_prefix(rule) {
-                if !obj.name.starts_with(prefix.as_str()) {
+                if !obj.name.starts_with(rule_prefix) {
                     continue;
                 }
             }
@@ -414,55 +436,22 @@ impl Lifecycle for BucketLifecycleConfiguration {
 
         if let Some(ref lc_rules) = self.filter_rules(obj).await {
             for rule in lc_rules.iter() {
-                if obj.expired_object_deletemarker() {
+                if obj.is_latest && obj.expired_object_deletemarker() {
                     if let Some(expiration) = rule.expiration.as_ref() {
-                        if let Some(expired_object_delete_marker) = expiration.expired_object_delete_marker {
+                        if expiration.expired_object_delete_marker.is_some_and(|v| v) {
-                            events.push(Event {
+                            if let Some(due) = expiration.next_due(obj) {
-                                action: IlmAction::DeleteVersionAction,
+                                if now.unix_timestamp() >= due.unix_timestamp() {
-                                rule_id: rule.id.clone().unwrap_or_default(),
+                                    events.push(Event {
-                                due: Some(now),
+                                        action: IlmAction::DeleteVersionAction,
-                                noncurrent_days: 0,
+                                        rule_id: rule.id.clone().unwrap_or_default(),
-                                newer_noncurrent_versions: 0,
+                                        due: Some(due),
-                                storage_class: "".into(),
+                                        noncurrent_days: 0,
-                            });
+                                        newer_noncurrent_versions: 0,
-                            break;
+                                        storage_class: "".into(),
-                        }
+                                    });
-
+                                    // Stop after scheduling an expired delete-marker event.
-                        if let Some(days) = expiration.days {
+                                    break;
-                            let expected_expiry = expected_expiry_time(mod_time, days /*, date*/);
-                            if now.unix_timestamp() >= expected_expiry.unix_timestamp() {
-                                events.push(Event {
-                                    action: IlmAction::DeleteVersionAction,
-                                    rule_id: rule.id.clone().unwrap_or_default(),
-                                    due: Some(expected_expiry),
-                                    noncurrent_days: 0,
-                                    newer_noncurrent_versions: 0,
-                                    storage_class: "".into(),
-                                });
-                                break;
-                            }
-                        }
-                    }
-                }
-
-                if obj.is_latest {
-                    if let Some(ref expiration) = rule.expiration {
-                        if let Some(expired_object_delete_marker) = expiration.expired_object_delete_marker {
-                            if obj.delete_marker && expired_object_delete_marker {
-                                let due = expiration.next_due(obj);
-                                if let Some(due) = due {
-                                    if now.unix_timestamp() >= due.unix_timestamp() {
-                                        events.push(Event {
-                                            action: IlmAction::DelMarkerDeleteAllVersionsAction,
-                                            rule_id: rule.id.clone().unwrap_or_default(),
-                                            due: Some(due),
-                                            noncurrent_days: 0,
-                                            newer_noncurrent_versions: 0,
-                                            storage_class: "".into(),
-                                        });
-                                    }
                                 }
-                                continue;
                             }
                         }
                     }
@@ -694,8 +683,16 @@ impl LifecycleCalculate for LifecycleExpiration {
         if !obj.is_latest || !obj.delete_marker {
             return None;
         }
+        // Check date first (date-based expiration takes priority over days).
+        // A zero unix timestamp means "not set" (default value) and is skipped.
+        if let Some(ref date) = self.date {
+            let expiry_date = OffsetDateTime::from(date.clone());
+            if expiry_date.unix_timestamp() != 0 {
+                return Some(expiry_date);
+            }
+        }
         match self.days {
-            Some(days) => Some(expected_expiry_time(obj.mod_time.unwrap(), days)),
+            Some(days) => obj.mod_time.map(|mod_time| expected_expiry_time(mod_time, days)),
             None => None,
         }
     }
@@ -860,6 +857,7 @@ impl Default for TransitionOptions {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use s3s::dto::LifecycleRuleFilter;
 
     #[tokio::test]
     async fn validate_rejects_non_positive_expiration_days() {
@@ -1074,4 +1072,208 @@ mod tests {
 
         assert_eq!(err.to_string(), ERR_LIFECYCLE_INVALID_RULE_STATUS);
     }
+
+    #[tokio::test]
+    async fn filter_rules_respects_filter_prefix() {
+        let mut filter = LifecycleRuleFilter::default();
+        filter.prefix = Some("prefix".to_string());
+        let lc = BucketLifecycleConfiguration {
+            rules: vec![LifecycleRule {
+                status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
+                expiration: Some(LifecycleExpiration {
+                    days: Some(30),
+                    ..Default::default()
+                }),
+                abort_incomplete_multipart_upload: None,
+                filter: Some(filter),
+                id: Some("rule".to_string()),
+                noncurrent_version_expiration: None,
+                noncurrent_version_transitions: None,
+                prefix: None,
+                transitions: None,
+            }],
+        };
+
+        let match_obj = ObjectOpts {
+            name: "prefix/file".to_string(),
+            mod_time: Some(OffsetDateTime::from_unix_timestamp(1_000_000).unwrap()),
+            is_latest: true,
+            ..Default::default()
+        };
+        let matched = lc.filter_rules(&match_obj).await.unwrap();
+        assert_eq!(matched.len(), 1);
+
+        let non_match_obj = ObjectOpts {
+            name: "other/file".to_string(),
+            mod_time: Some(OffsetDateTime::from_unix_timestamp(1_000_000).unwrap()),
+            is_latest: true,
+            ..Default::default()
+        };
+        let not_matched = lc.filter_rules(&non_match_obj).await.unwrap();
+        assert_eq!(not_matched.len(), 0);
+    }
+
+    #[tokio::test]
+    async fn filter_rules_respects_filter_and_prefix() {
+        let mut filter = LifecycleRuleFilter::default();
+
+        let mut and = LifecycleRuleAndOperator::default();
+        and.prefix = Some("prefix".to_string());
+        filter.and = Some(and);
+
+        let lc = BucketLifecycleConfiguration {
+            rules: vec![LifecycleRule {
+                status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
+                expiration: Some(LifecycleExpiration {
+                    days: Some(30),
+                    ..Default::default()
+                }),
+                abort_incomplete_multipart_upload: None,
+                filter: Some(filter),
+                id: Some("rule-and-prefix".to_string()),
+                noncurrent_version_expiration: None,
+                noncurrent_version_transitions: None,
+                prefix: None,
+                transitions: None,
+            }],
+        };
+
+        let match_obj = ObjectOpts {
+            name: "prefix/file".to_string(),
+            mod_time: Some(OffsetDateTime::from_unix_timestamp(1_000_000).unwrap()),
+            is_latest: true,
+            ..Default::default()
+        };
+        let matched = lc.filter_rules(&match_obj).await.unwrap();
+        assert_eq!(matched.len(), 1);
+
+        let non_match_obj = ObjectOpts {
+            name: "other/file".to_string(),
+            mod_time: Some(OffsetDateTime::from_unix_timestamp(1_000_000).unwrap()),
+            is_latest: true,
+            ..Default::default()
+        };
+        let not_matched = lc.filter_rules(&non_match_obj).await.unwrap();
+        assert_eq!(not_matched.len(), 0);
+    }
+
+    #[tokio::test]
+    async fn expired_object_delete_marker_requires_single_version() {
+        let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap();
+        let lc = BucketLifecycleConfiguration {
+            rules: vec![LifecycleRule {
+                status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
+                expiration: Some(LifecycleExpiration {
+                    days: Some(1),
+                    expired_object_delete_marker: Some(true),
+                    ..Default::default()
+                }),
+                abort_incomplete_multipart_upload: None,
+                filter: None,
+                id: Some("rule-expired-del-marker".to_string()),
+                noncurrent_version_expiration: None,
+                noncurrent_version_transitions: None,
+                prefix: None,
+                transitions: None,
+            }],
+        };
+
+        let opts = ObjectOpts {
+            name: "obj".to_string(),
+            mod_time: Some(base_time),
+            is_latest: true,
+            delete_marker: true,
+            num_versions: 2,
+            version_id: Some(Uuid::new_v4()),
+            ..Default::default()
+        };
+
+        let now = base_time + Duration::days(2);
+        let event = lc.eval_inner(&opts, now, 0).await;
+        assert_eq!(event.action, IlmAction::NoneAction);
+    }
+
+    #[tokio::test]
+    async fn expired_object_delete_marker_deletes_only_delete_marker_after_due() {
+        let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap();
+        let lc = BucketLifecycleConfiguration {
+            rules: vec![LifecycleRule {
+                status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
+                expiration: Some(LifecycleExpiration {
+                    days: Some(1),
+                    expired_object_delete_marker: Some(true),
+                    ..Default::default()
+                }),
+                abort_incomplete_multipart_upload: None,
+                filter: None,
+                id: Some("rule-expired-del-marker".to_string()),
+                noncurrent_version_expiration: None,
+                noncurrent_version_transitions: None,
+                prefix: None,
+                transitions: None,
+            }],
+        };
+
+        let opts = ObjectOpts {
+            name: "obj".to_string(),
+            mod_time: Some(base_time),
+            is_latest: true,
+            delete_marker: true,
+            num_versions: 1,
+            version_id: Some(Uuid::new_v4()),
+            ..Default::default()
+        };
+
+        let now = base_time + Duration::days(2);
+        let event = lc.eval_inner(&opts, now, 0).await;
+
+        assert_eq!(event.action, IlmAction::DeleteVersionAction);
+        assert_eq!(event.due, Some(expected_expiry_time(base_time, 1)));
+    }
+
+    #[tokio::test]
+    async fn expired_object_delete_marker_date_based_not_yet_due() {
+        // A date-based rule that has not yet reached its expiry date must not
+        // trigger immediate deletion (unwrap_or(now) must not override the date).
+        let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap();
+        let future_date = base_time + Duration::days(10);
+        let lc = BucketLifecycleConfiguration {
+            rules: vec![LifecycleRule {
+                status: ExpirationStatus::from_static(ExpirationStatus::ENABLED),
+                expiration: Some(LifecycleExpiration {
+                    date: Some(future_date.into()),
+                    expired_object_delete_marker: Some(true),
+                    ..Default::default()
+                }),
+                abort_incomplete_multipart_upload: None,
+                filter: None,
+                id: Some("rule-date-del-marker".to_string()),
+                noncurrent_version_expiration: None,
+                noncurrent_version_transitions: None,
+                prefix: None,
+                transitions: None,
+            }],
+        };
+
+        let opts = ObjectOpts {
+            name: "obj".to_string(),
+            mod_time: Some(base_time),
+            is_latest: true,
+            delete_marker: true,
+            num_versions: 1,
+            version_id: Some(Uuid::new_v4()),
+            ..Default::default()
+        };
+
+        // now is before the configured date — must not schedule deletion
+        let now_before = base_time + Duration::days(5);
+        let event_before = lc.eval_inner(&opts, now_before, 0).await;
+        assert_eq!(event_before.action, IlmAction::NoneAction);
+
+        // now is after the configured date — must schedule deletion
+        let now_after = base_time + Duration::days(11);
+        let event_after = lc.eval_inner(&opts, now_after, 0).await;
+        assert_eq!(event_after.action, IlmAction::DeleteVersionAction);
+        assert_eq!(event_after.due, Some(future_date));
+    }
 }