diff --git a/crates/ecstore/src/bucket/lifecycle/core.rs b/crates/ecstore/src/bucket/lifecycle/core.rs index 8287b7ef0..bfcf921bb 100644 --- a/crates/ecstore/src/bucket/lifecycle/core.rs +++ b/crates/ecstore/src/bucket/lifecycle/core.rs @@ -46,9 +46,13 @@ const ERR_LIFECYCLE_INVALID_NONCURRENT_EXPIRATION_DAYS: &str = "Lifecycle noncur const ERR_LIFECYCLE_INVALID_ABORT_INCOMPLETE_MPU_DAYS: &str = "DaysAfterInitiation must be 0 or greater when used with AbortIncompleteMultipartUpload"; const ERR_LIFECYCLE_INVALID_EXPIRATION_DATE_NOT_MIDNIGHT: &str = "Expiration.Date must be at midnight UTC"; +const ERR_LIFECYCLE_INVALID_EXPIRED_OBJECT_DELETE_MARKER: &str = + "ExpiredObjectDeleteMarker cannot be specified with Days or Date"; const ERR_LIFECYCLE_INVALID_RULE_ID_TOO_LONG: &str = "Rule ID must be at most 255 characters"; const ERR_LIFECYCLE_INVALID_RULE_STATUS: &str = "Rule status must be either Enabled or Disabled"; const ERR_LIFECYCLE_DEL_MARKER_WITH_TAGS: &str = "Rule with DelMarkerExpiration cannot have tags based filtering"; +const ERR_LIFECYCLE_EXPIRED_OBJECT_DELETE_MARKER_WITH_TAGS: &str = + "Rule with ExpiredObjectDeleteMarker cannot have tags based filtering"; const ERR_LIFECYCLE_RULE_MUST_HAVE_ACTION: &str = "Rule must have at least one of Expiration, Transition, NoncurrentVersionExpiration, NoncurrentVersionTransition, or DelMarkerExpiration"; const ERR_LIFECYCLE_PREFIX_FILTER_CONFLICT: &str = "Legacy Prefix and Filter cannot both be present in a lifecycle rule. Use Filter.Prefix instead of the top-level Prefix element."; @@ -135,6 +139,20 @@ impl RuleValidate for LifecycleRule { if has_tag_filter && self.del_marker_expiration.is_some() { return Err(std::io::Error::other(ERR_LIFECYCLE_DEL_MARKER_WITH_TAGS)); } + if has_tag_filter + && self + .expiration + .as_ref() + .is_some_and(|expiration| expiration.expired_object_delete_marker.is_some_and(|v| v)) + { + return Err(std::io::Error::other(ERR_LIFECYCLE_EXPIRED_OBJECT_DELETE_MARKER_WITH_TAGS)); + } + if let Some(expiration) = &self.expiration + && expiration.expired_object_delete_marker.is_some_and(|v| v) + && (expiration.days.is_some() || expiration.date.is_some()) + { + return Err(std::io::Error::other(ERR_LIFECYCLE_INVALID_EXPIRED_OBJECT_DELETE_MARKER)); + } // Rule must have at least one action let has_expiration = self.expiration.is_some(); let has_transition = self.transitions.as_ref().is_some_and(|t| !t.is_empty()); @@ -900,7 +918,7 @@ pub struct ObjectOpts { impl ObjectOpts { pub fn expired_object_deletemarker(&self) -> bool { - self.delete_marker && self.is_latest + self.delete_marker && self.is_latest && self.num_versions == 1 } pub fn from_object_info(oi: &ObjectInfo) -> Self { @@ -2200,14 +2218,13 @@ mod tests { #[tokio::test] #[serial] - async fn expired_object_delete_marker_applies_with_noncurrent_versions_present() { + async fn expired_object_delete_marker_ignores_marker_with_noncurrent_versions_present() { let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap(); let lc = BucketLifecycleConfiguration { expiry_updated_at: None, rules: vec![LifecycleRule { status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), expiration: Some(LifecycleExpiration { - days: Some(1), expired_object_delete_marker: Some(true), ..Default::default() }), @@ -2234,20 +2251,57 @@ mod tests { 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))); + assert_eq!(event.action, IlmAction::NoneAction); + assert_eq!(event.due, Some(OffsetDateTime::UNIX_EPOCH)); } #[tokio::test] - #[serial] - async fn expired_object_delete_marker_deletes_only_delete_marker_after_due() { + async fn expired_object_delete_marker_ignores_unknown_version_count() { + let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap(); + let lc = BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + expiration: Some(LifecycleExpiration { + 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, + del_marker_expiration: None, + }], + }; + + let opts = ObjectOpts { + name: "obj".to_string(), + mod_time: Some(base_time), + is_latest: true, + delete_marker: true, + num_versions: 0, + 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); + assert_eq!(event.due, Some(OffsetDateTime::UNIX_EPOCH)); + } + + #[tokio::test] + #[serial] + async fn expired_object_delete_marker_deletes_only_delete_marker_immediately() { let base_time = OffsetDateTime::from_unix_timestamp(1_000_000).unwrap(); let lc = BucketLifecycleConfiguration { expiry_updated_at: None, rules: vec![LifecycleRule { status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), expiration: Some(LifecycleExpiration { - days: Some(1), expired_object_delete_marker: Some(true), ..Default::default() }), @@ -2276,7 +2330,7 @@ mod tests { 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))); + assert_eq!(event.due, Some(now)); } #[tokio::test] @@ -2318,11 +2372,8 @@ mod tests { } #[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); + async fn validate_rejects_expired_object_delete_marker_with_date() { + let future_date = OffsetDateTime::from_unix_timestamp(86_400 * 10).expect("valid midnight UTC test timestamp"); let lc = BucketLifecycleConfiguration { expiry_updated_at: None, rules: vec![LifecycleRule { @@ -2343,26 +2394,68 @@ mod tests { }], }; - 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 err = lc.validate(&ObjectLockConfiguration::default()).await.unwrap_err(); + + assert_eq!(err.to_string(), ERR_LIFECYCLE_INVALID_EXPIRED_OBJECT_DELETE_MARKER); + } + + #[tokio::test] + async fn validate_rejects_expired_object_delete_marker_with_days() { + let lc = BucketLifecycleConfiguration { + expiry_updated_at: None, + 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-days-del-marker".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + del_marker_expiration: None, + }], }; - // 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); + let err = lc.validate(&ObjectLockConfiguration::default()).await.unwrap_err(); - // 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)); + assert_eq!(err.to_string(), ERR_LIFECYCLE_INVALID_EXPIRED_OBJECT_DELETE_MARKER); + } + + #[tokio::test] + async fn validate_rejects_expired_object_delete_marker_with_tag_filter() { + let lc = BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + expiration: Some(LifecycleExpiration { + expired_object_delete_marker: Some(true), + ..Default::default() + }), + abort_incomplete_multipart_upload: None, + filter: Some(LifecycleRuleFilter { + tag: Some(s3s::dto::Tag { + key: Some("env".to_string()), + value: Some("prod".to_string()), + }), + ..Default::default() + }), + id: Some("rule-tag-del-marker".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + del_marker_expiration: None, + }], + }; + + let err = lc.validate(&ObjectLockConfiguration::default()).await.unwrap_err(); + + assert_eq!(err.to_string(), ERR_LIFECYCLE_EXPIRED_OBJECT_DELETE_MARKER_WITH_TAGS); } // --- TASK-002 tests: Object Lock + ExpiredObjectDeleteMarker compatibility --- diff --git a/crates/ecstore/src/object_api/types.rs b/crates/ecstore/src/object_api/types.rs index 87e1f267c..a37fea71f 100644 --- a/crates/ecstore/src/object_api/types.rs +++ b/crates/ecstore/src/object_api/types.rs @@ -778,7 +778,7 @@ fn versions_after_marker(file_infos: &rustfs_filemeta::FileInfoVersions, marker: #[cfg(test)] mod tests { use super::*; - use rustfs_filemeta::ReplicationState; + use rustfs_filemeta::{FileInfo, FileMeta, MetaCacheEntry, ReplicationState, TRANSITION_COMPLETE}; #[test] fn versions_after_marker_handles_null_version_marker() { @@ -889,6 +889,67 @@ mod tests { assert_eq!(objects.len(), 5); } + #[tokio::test] + async fn versions_listing_excludes_tier_free_versions_from_delete_marker_count() { + let object_version_id = Uuid::new_v4(); + let remote_version_id = Uuid::new_v4(); + let free_version_id = Uuid::new_v4(); + let delete_marker_id = Uuid::new_v4(); + let base_time = OffsetDateTime::now_utc(); + let mut fm = FileMeta::new(); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + transition_status: TRANSITION_COMPLETE.to_string(), + transitioned_objname: "remote/object".to_string(), + transition_version_id: Some(remote_version_id), + transition_tier: "WARM".to_string(), + mod_time: Some(base_time), + ..Default::default() + }) + .expect("transitioned object version should be added"); + + let mut delete_fi = FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + mod_time: Some(base_time), + ..Default::default() + }; + delete_fi.set_tier_free_version_id(&free_version_id.to_string()); + fm.delete_version(&delete_fi) + .expect("transitioned delete should create a free-version record"); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(delete_marker_id), + deleted: true, + mod_time: Some(base_time + time::Duration::seconds(1)), + ..Default::default() + }) + .expect("delete marker should be added"); + + let entries = MetaCacheEntriesSorted { + o: rustfs_filemeta::MetaCacheEntries(vec![Some(MetaCacheEntry { + name: "object".to_string(), + metadata: fm.marshal_msg().expect("metadata should marshal"), + ..Default::default() + })]), + ..Default::default() + }; + + let objects = ObjectInfo::from_meta_cache_entries_sorted_versions(&entries, "bucket", "", None, None).await; + + assert_eq!(objects.len(), 1); + assert_eq!(objects[0].name, "object"); + assert!(objects[0].delete_marker); + assert!(objects[0].is_latest); + assert_eq!(objects[0].num_versions, 1); + } + #[test] fn get_actual_size_prefers_actual_size_field() { let info = ObjectInfo { diff --git a/crates/filemeta/src/filemeta.rs b/crates/filemeta/src/filemeta.rs index 17b244bdb..e486be27f 100644 --- a/crates/filemeta/src/filemeta.rs +++ b/crates/filemeta/src/filemeta.rs @@ -797,10 +797,13 @@ impl FileMeta { if !include_free_versions { versions.versions = versions_vec; - } - for fi in versions.free_versions.iter_mut() { - fi.num_versions = n; + for fi in versions.versions.iter_mut() { + fi.num_versions = n; + } + for fi in versions.free_versions.iter_mut() { + fi.num_versions = n; + } } Ok(versions) @@ -1714,6 +1717,68 @@ mod test { ); } + #[test] + fn get_file_info_versions_excludes_free_versions_from_num_versions() { + let object_version_id = Uuid::new_v4(); + let remote_version_id = Uuid::new_v4(); + let free_version_id = Uuid::new_v4(); + let delete_marker_id = Uuid::new_v4(); + let base_time = OffsetDateTime::now_utc(); + let mut fm = FileMeta::new(); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + transition_status: TRANSITION_COMPLETE.to_string(), + transitioned_objname: "remote/object".to_string(), + transition_version_id: Some(remote_version_id), + transition_tier: "WARM".to_string(), + mod_time: Some(base_time), + ..Default::default() + }) + .expect("transitioned object version should be added"); + + let mut delete_fi = FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + mod_time: Some(base_time), + ..Default::default() + }; + delete_fi.set_tier_free_version_id(&free_version_id.to_string()); + fm.delete_version(&delete_fi) + .expect("transitioned delete should create a free-version record"); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(delete_marker_id), + deleted: true, + mod_time: Some(base_time + time::Duration::seconds(1)), + ..Default::default() + }) + .expect("delete marker should be added"); + + let versions = fm + .get_file_info_versions("bucket", "object", false) + .expect("file info versions should parse"); + + assert_eq!(versions.versions.len(), 1); + assert_eq!(versions.free_versions.len(), 1); + assert!(versions.versions[0].deleted); + assert_eq!(versions.versions[0].num_versions, 1); + assert_eq!(versions.free_versions[0].num_versions, 1); + + let versions_with_free = fm + .get_file_info_versions("bucket", "object", true) + .expect("file info versions should preserve all versions"); + + assert_eq!(versions_with_free.versions.len(), 2); + assert!(versions_with_free.free_versions.is_empty()); + assert!(versions_with_free.versions.iter().all(|version| version.num_versions == 2)); + } + #[test] fn test_data_integrity_validation() { // Test data integrity checks diff --git a/crates/filemeta/src/metacache.rs b/crates/filemeta/src/metacache.rs index 5b3637dcc..42234f9b1 100644 --- a/crates/filemeta/src/metacache.rs +++ b/crates/filemeta/src/metacache.rs @@ -188,7 +188,9 @@ impl MetaCacheEntry { let mut fm = FileMeta::new(); fm.unmarshal_msg(&self.metadata)?; - fm.into_file_info_versions(bucket, self.name.as_str(), false) + let mut versions = fm.get_file_info_versions(bucket, self.name.as_str(), false)?; + versions.free_versions.clear(); + Ok(versions) } pub fn file_info_versions_with_free_versions(&self, bucket: &str) -> Result { @@ -1007,6 +1009,70 @@ mod tests { assert_eq!(with_free.free_versions[0].transition_version_id, Some(remote_version_id)); } + #[test] + fn file_info_versions_excludes_free_versions_from_visible_version_count() { + let object_version_id = Uuid::new_v4(); + let remote_version_id = Uuid::new_v4(); + let free_version_id = Uuid::new_v4(); + let delete_marker_id = Uuid::new_v4(); + let base_time = OffsetDateTime::now_utc(); + let mut fm = FileMeta::new(); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + transition_status: TRANSITION_COMPLETE.to_string(), + transitioned_objname: "remote/object".to_string(), + transition_version_id: Some(remote_version_id), + transition_tier: "WARM".to_string(), + mod_time: Some(base_time), + ..Default::default() + }) + .expect("transitioned object version should be added"); + + let mut delete_fi = FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(object_version_id), + mod_time: Some(base_time), + ..Default::default() + }; + delete_fi.set_tier_free_version_id(&free_version_id.to_string()); + fm.delete_version(&delete_fi) + .expect("transitioned delete should create a free-version record"); + + fm.add_version(FileInfo { + volume: "bucket".to_string(), + name: "object".to_string(), + version_id: Some(delete_marker_id), + deleted: true, + mod_time: Some(base_time + time::Duration::seconds(1)), + ..Default::default() + }) + .expect("delete marker should be added"); + + let entry = MetaCacheEntry { + name: "object".to_string(), + metadata: fm.marshal_msg().expect("metadata should marshal"), + ..Default::default() + }; + + let normal = entry.file_info_versions("bucket").expect("normal versions should parse"); + assert_eq!(normal.versions.len(), 1); + assert!(normal.free_versions.is_empty()); + assert!(normal.versions[0].deleted); + assert_eq!(normal.versions[0].num_versions, 1); + + let with_free = entry + .file_info_versions_with_free_versions("bucket") + .expect("versions with free versions should parse"); + assert_eq!(with_free.versions.len(), 1); + assert_eq!(with_free.free_versions.len(), 1); + assert_eq!(with_free.versions[0].num_versions, 1); + assert_eq!(with_free.free_versions[0].num_versions, 1); + } + #[test] fn transitioned_delete_persists_recoverable_free_version_after_metadata_roundtrip() { let version_id = Uuid::new_v4();