From ed2078e025fea79a1ad9ed7c88d06842a4174451 Mon Sep 17 00:00:00 2001 From: houseme Date: Fri, 22 May 2026 09:39:53 +0800 Subject: [PATCH] fix(ecstore): allow expired delete markers on locked buckets (#3048) * fix: allow expired delete markers on locked buckets * fix: reject zero-day del marker expiration --- crates/ecstore/src/bucket/lifecycle/core.rs | 87 ++++++++-- .../src/app/lifecycle_transition_api_test.rs | 159 +++++++++++++++++- 2 files changed, 234 insertions(+), 12 deletions(-) diff --git a/crates/ecstore/src/bucket/lifecycle/core.rs b/crates/ecstore/src/bucket/lifecycle/core.rs index 1288e785e..9ebda1f1b 100644 --- a/crates/ecstore/src/bucket/lifecycle/core.rs +++ b/crates/ecstore/src/bucket/lifecycle/core.rs @@ -34,7 +34,8 @@ const ERR_LIFECYCLE_NO_RULE: &str = "Lifecycle configuration should have at leas const ERR_LIFECYCLE_DUPLICATE_ID: &str = "Rule ID must be unique. Found same ID for more than one rule"; const _ERR_XML_NOT_WELL_FORMED: &str = "The XML you provided was not well-formed or did not validate against our published schema"; -const ERR_LIFECYCLE_BUCKET_LOCKED: &str = "ExpiredObjectDeleteMarker is not allowed on a bucket with Object Lock enabled"; +const ERR_LIFECYCLE_BUCKET_LOCKED: &str = + "ExpiredObjectAllVersions element and DelMarkerExpiration action cannot be used on an object locked bucket"; const ERR_LIFECYCLE_TOO_MANY_RULES: &str = "Lifecycle configuration should have at most 1000 rules"; const ERR_LIFECYCLE_INVALID_EXPIRATION_DAYS: &str = "Lifecycle expiration days must not be negative"; const ERR_LIFECYCLE_INVALID_NONCURRENT_EXPIRATION_DAYS: &str = "Lifecycle noncurrent expiration days must not be negative"; @@ -320,14 +321,15 @@ impl Lifecycle for BucketLifecycleConfiguration { r.validate()?; if let Some(object_lock_enabled) = lr.object_lock_enabled.as_ref() && object_lock_enabled.as_str() == ObjectLockEnabled::ENABLED - && let Some(expiration) = r.expiration.as_ref() { - // Object Lock + ExpiredObjectDeleteMarker conflict - if expiration.expired_object_delete_marker.is_some_and(|v| v) { - return Err(std::io::Error::other(ERR_LIFECYCLE_BUCKET_LOCKED)); + if let Some(expiration) = r.expiration.as_ref() { + // Object Lock + ExpiredObjectAllVersions conflict (MinIO extension) + if expiration.expired_object_all_versions.is_some_and(|v| v) { + return Err(std::io::Error::other(ERR_LIFECYCLE_BUCKET_LOCKED)); + } } - // Object Lock + ExpiredObjectAllVersions conflict (MinIO extension) - if expiration.expired_object_all_versions.is_some_and(|v| v) { + // Object Lock + DelMarkerExpiration conflict + if r.del_marker_expiration.is_some() { return Err(std::io::Error::other(ERR_LIFECYCLE_BUCKET_LOCKED)); } } @@ -2060,11 +2062,11 @@ mod tests { assert_eq!(event_after.due, Some(future_date)); } - // --- TASK-002 tests: Object Lock + ExpiredObjectDeleteMarker conflict --- + // --- TASK-002 tests: Object Lock + ExpiredObjectDeleteMarker compatibility --- #[tokio::test] #[serial] - async fn validate_rejects_expired_object_delete_marker_on_locked_bucket() { + async fn validate_allows_expired_object_delete_marker_on_locked_bucket() { let lc = BucketLifecycleConfiguration { expiry_updated_at: None, rules: vec![LifecycleRule { @@ -2089,8 +2091,9 @@ mod tests { ..Default::default() }; - let err = lc.validate(&locked_config).await.unwrap_err(); - assert_eq!(err.to_string(), ERR_LIFECYCLE_BUCKET_LOCKED); + lc.validate(&locked_config) + .await + .expect("expected validation to pass for ExpiredObjectDeleteMarker on locked bucket"); } #[tokio::test] @@ -2154,6 +2157,68 @@ mod tests { .expect("expected days-based expiration to pass on locked bucket"); } + #[tokio::test] + #[serial] + async fn validate_rejects_del_marker_expiration_on_locked_bucket() { + let lc = BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + expiration: Some(LifecycleExpiration { + days: Some(30), + ..Default::default() + }), + abort_incomplete_multipart_upload: None, + del_marker_expiration: Some(s3s::dto::DelMarkerExpiration { days: Some(1) }), + filter: None, + id: Some("test-rule".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + }], + }; + + let locked_config = ObjectLockConfiguration { + object_lock_enabled: Some(ObjectLockEnabled::from_static(ObjectLockEnabled::ENABLED)), + ..Default::default() + }; + + let err = lc.validate(&locked_config).await.unwrap_err(); + assert_eq!(err.to_string(), ERR_LIFECYCLE_BUCKET_LOCKED); + } + + #[tokio::test] + #[serial] + async fn validate_rejects_zero_day_del_marker_expiration_on_locked_bucket() { + let lc = BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + expiration: Some(LifecycleExpiration { + days: Some(30), + ..Default::default() + }), + abort_incomplete_multipart_upload: None, + del_marker_expiration: Some(s3s::dto::DelMarkerExpiration { days: Some(0) }), + filter: None, + id: Some("test-rule".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + }], + }; + + let locked_config = ObjectLockConfiguration { + object_lock_enabled: Some(ObjectLockEnabled::from_static(ObjectLockEnabled::ENABLED)), + ..Default::default() + }; + + let err = lc.validate(&locked_config).await.unwrap_err(); + assert_eq!(err.to_string(), ERR_LIFECYCLE_BUCKET_LOCKED); + } + // --- TASK-003 tests: Round up to next UTC processing boundary --- #[test] diff --git a/rustfs/src/app/lifecycle_transition_api_test.rs b/rustfs/src/app/lifecycle_transition_api_test.rs index 09f660308..a02a22644 100644 --- a/rustfs/src/app/lifecycle_transition_api_test.rs +++ b/rustfs/src/app/lifecycle_transition_api_test.rs @@ -21,7 +21,7 @@ use futures::stream; use http::{Extensions, HeaderMap, Method, Uri}; use rustfs_config::ENV_TEST_FORCE_IMMEDIATE_TRANSITION_ENQUEUE_TIMEOUT; use rustfs_ecstore::{ - bucket::metadata::BUCKET_LIFECYCLE_CONFIG, + bucket::metadata::{BUCKET_LIFECYCLE_CONFIG, OBJECT_LOCK_CONFIG}, bucket::metadata_sys, client::object_api_utils::to_s3s_etag, client::transition_api::{ReadCloser, ReaderImpl}, @@ -185,6 +185,15 @@ async fn set_bucket_lifecycle_transition_with_tier( Ok(()) } +async fn set_bucket_object_lock_enabled(bucket_name: &str) -> Result<(), Box> { + let object_lock_xml = r#" + + Enabled +"#; + metadata_sys::update(bucket_name, OBJECT_LOCK_CONFIG, object_lock_xml.as_bytes().to_vec()).await?; + Ok(()) +} + fn expiration_lifecycle_configuration(prefix: &str) -> BucketLifecycleConfiguration { BucketLifecycleConfiguration { expiry_updated_at: None, @@ -219,6 +228,54 @@ fn expiration_lifecycle_configuration(prefix: &str) -> BucketLifecycleConfigurat } } +fn expired_object_delete_marker_lifecycle_configuration() -> BucketLifecycleConfiguration { + BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + abort_incomplete_multipart_upload: None, + del_marker_expiration: None, + expiration: Some(LifecycleExpiration { + expired_object_delete_marker: Some(true), + ..Default::default() + }), + filter: Some(LifecycleRuleFilter { + prefix: Some("test/".to_string()), + ..Default::default() + }), + id: Some("expired-delete-marker".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + }], + } +} + +fn del_marker_expiration_lifecycle_configuration(days: i32) -> BucketLifecycleConfiguration { + BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + abort_incomplete_multipart_upload: None, + del_marker_expiration: Some(DelMarkerExpiration { days: Some(days) }), + expiration: Some(LifecycleExpiration { + days: Some(30), + ..Default::default() + }), + filter: Some(LifecycleRuleFilter { + prefix: Some("test/".to_string()), + ..Default::default() + }), + id: Some("del-marker-expiration".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: None, + }], + } +} + #[derive(Clone, Default)] struct MockWarmBackend { objects: Arc>>>, @@ -1027,3 +1084,103 @@ async fn put_bucket_lifecycle_configuration_expires_existing_objects() { "existing object should be lifecycle-deleted after lifecycle update" ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial] +#[ignore = "requires isolated global object layer state"] +async fn put_bucket_lifecycle_configuration_allows_expired_delete_marker_on_object_lock_bucket() { + let (_disk_paths, ecstore) = setup_test_env().await; + let usecase = DefaultBucketUsecase::without_context(); + + let bucket = format!("test-lock-expired-del-marker-{}", &Uuid::new_v4().simple().to_string()[..8]); + create_test_bucket(&ecstore, bucket.as_str()).await; + set_bucket_object_lock_enabled(bucket.as_str()) + .await + .expect("Failed to enable object lock for bucket"); + + let req = build_request( + PutBucketLifecycleConfigurationInput::builder() + .bucket(bucket.clone()) + .lifecycle_configuration(Some(expired_object_delete_marker_lifecycle_configuration())) + .build() + .unwrap(), + Method::PUT, + ); + + usecase + .execute_put_bucket_lifecycle_configuration(req) + .await + .expect("ExpiredObjectDeleteMarker should be accepted on object-lock bucket"); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial] +#[ignore = "requires isolated global object layer state"] +async fn put_bucket_lifecycle_configuration_rejects_del_marker_expiration_on_object_lock_bucket() { + let (_disk_paths, ecstore) = setup_test_env().await; + let usecase = DefaultBucketUsecase::without_context(); + + let bucket = format!("test-lock-del-marker-exp-{}", &Uuid::new_v4().simple().to_string()[..8]); + create_test_bucket(&ecstore, bucket.as_str()).await; + set_bucket_object_lock_enabled(bucket.as_str()) + .await + .expect("Failed to enable object lock for bucket"); + + let req = build_request( + PutBucketLifecycleConfigurationInput::builder() + .bucket(bucket.clone()) + .lifecycle_configuration(Some(del_marker_expiration_lifecycle_configuration(1))) + .build() + .unwrap(), + Method::PUT, + ); + + let err = usecase + .execute_put_bucket_lifecycle_configuration(req) + .await + .expect_err("DelMarkerExpiration must be rejected on object-lock bucket"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidArgument); + let message = err.message().unwrap_or_default(); + assert!( + message.contains( + "ExpiredObjectAllVersions element and DelMarkerExpiration action cannot be used on an object locked bucket" + ), + "unexpected error message: {message}" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial] +#[ignore = "requires isolated global object layer state"] +async fn put_bucket_lifecycle_configuration_rejects_zero_day_del_marker_expiration_on_object_lock_bucket() { + let (_disk_paths, ecstore) = setup_test_env().await; + let usecase = DefaultBucketUsecase::without_context(); + + let bucket = format!("test-lock-del-marker-exp0-{}", &Uuid::new_v4().simple().to_string()[..8]); + create_test_bucket(&ecstore, bucket.as_str()).await; + set_bucket_object_lock_enabled(bucket.as_str()) + .await + .expect("Failed to enable object lock for bucket"); + + let req = build_request( + PutBucketLifecycleConfigurationInput::builder() + .bucket(bucket.clone()) + .lifecycle_configuration(Some(del_marker_expiration_lifecycle_configuration(0))) + .build() + .unwrap(), + Method::PUT, + ); + + let err = usecase + .execute_put_bucket_lifecycle_configuration(req) + .await + .expect_err("DelMarkerExpiration with zero days must be rejected on object-lock bucket"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidArgument); + let message = err.message().unwrap_or_default(); + assert!( + message.contains( + "ExpiredObjectAllVersions element and DelMarkerExpiration action cannot be used on an object locked bucket" + ), + "unexpected error message: {message}" + ); +}