mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-29 01:29:00 +00:00
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
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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<dyn std::error::Error>> {
|
||||
let object_lock_xml = r#"<?xml version="1.0" encoding="UTF-8"?>
|
||||
<ObjectLockConfiguration>
|
||||
<ObjectLockEnabled>Enabled</ObjectLockEnabled>
|
||||
</ObjectLockConfiguration>"#;
|
||||
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<Mutex<HashMap<String, Vec<u8>>>>,
|
||||
@@ -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}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user