diff --git a/README.md b/README.md index b084c74..acb6a87 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,9 @@ rc ilm rule add local/my-bucket --expiry-days 30 --prefix "logs/" # Add lifecycle rule: transition to remote tier after 90 days rc ilm rule add local/my-bucket --transition-days 90 --storage-class WARM +# Expire noncurrent versions after 1 day and clean up the remaining delete marker +rc ilm rule add local/my-bucket --noncurrent-expiry-days 1 --expired-object-delete-marker + # List lifecycle rules rc ilm rule list local/my-bucket diff --git a/crates/cli/src/commands/ilm/rule.rs b/crates/cli/src/commands/ilm/rule.rs index 7f6e88f..38872be 100644 --- a/crates/cli/src/commands/ilm/rule.rs +++ b/crates/cli/src/commands/ilm/rule.rs @@ -225,6 +225,15 @@ pub async fn execute(cmd: RuleCommands, output_config: OutputConfig) -> ExitCode async fn execute_add(args: AddRuleArgs, output_config: OutputConfig) -> ExitCode { let formatter = Formatter::new(output_config); + if let Err(error) = validate_expired_delete_marker_inputs( + args.expired_object_delete_marker, + args.expiry_days.is_some() || args.expiry_date.is_some(), + false, + ) { + formatter.error(&error); + return ExitCode::UsageError; + } + let (alias_name, bucket) = match parse_bucket_path(&args.path) { Ok(parts) => parts, Err(error) => { @@ -476,6 +485,11 @@ async fn execute_edit(args: EditRuleArgs, output_config: OutputConfig) -> ExitCo rule.expired_object_delete_marker = Some(val); } + if let Err(error) = validate_expired_delete_marker_rule(rule) { + formatter.error(&error); + return ExitCode::UsageError; + } + match client.set_bucket_lifecycle(&bucket, rules).await { Ok(()) => { if formatter.is_json() { @@ -702,6 +716,15 @@ async fn execute_import(args: ImportRuleArgs, output_config: OutputConfig) -> Ex } }; + if let Some(error) = config + .rules + .iter() + .find_map(|rule| validate_expired_delete_marker_rule(rule).err()) + { + formatter.error(&error); + return ExitCode::UsageError; + } + let client = match setup_client(&alias_name, &bucket, args.force, &formatter).await { Ok(client) => client, Err(code) => return code, @@ -732,6 +755,38 @@ async fn execute_import(args: ImportRuleArgs, output_config: OutputConfig) -> Ex // ==================== Helpers ==================== +fn validate_expired_delete_marker_inputs( + cleanup_enabled: bool, + has_current_expiration: bool, + has_tags: bool, +) -> std::result::Result<(), String> { + if !cleanup_enabled { + return Ok(()); + } + if has_current_expiration { + return Err( + "expired delete-marker cleanup cannot be combined with current expiration days or date" + .to_string(), + ); + } + if has_tags { + return Err( + "expired delete-marker cleanup cannot be combined with tag filters".to_string(), + ); + } + Ok(()) +} + +fn validate_expired_delete_marker_rule(rule: &LifecycleRule) -> std::result::Result<(), String> { + validate_expired_delete_marker_inputs( + rule.expired_object_delete_marker == Some(true), + rule.expiration + .as_ref() + .is_some_and(|expiration| expiration.days.is_some() || expiration.date.is_some()), + rule.tags.as_ref().is_some_and(|tags| !tags.is_empty()), + ) +} + async fn setup_client( alias_name: &str, bucket: &str, @@ -938,6 +993,35 @@ mod tests { assert_eq!(code, ExitCode::UsageError); } + #[tokio::test] + async fn test_execute_add_rejects_current_expiration_with_marker_cleanup() { + let args = AddRuleArgs { + path: "local/my-bucket".to_string(), + expiry_days: Some(30), + expiry_date: None, + transition_days: None, + transition_date: None, + storage_class: None, + noncurrent_expiry_days: None, + noncurrent_transition_days: None, + noncurrent_transition_storage_class: None, + prefix: None, + expired_object_delete_marker: true, + newer_noncurrent_versions: None, + disable: false, + force: false, + }; + + let code = execute_add(args, OutputConfig::default()).await; + assert_eq!(code, ExitCode::UsageError); + } + + #[test] + fn test_marker_cleanup_validation_rejects_tags() { + assert!(validate_expired_delete_marker_inputs(true, false, true).is_err()); + assert!(validate_expired_delete_marker_inputs(true, false, false).is_ok()); + } + #[tokio::test] async fn test_execute_remove_no_id_or_all_returns_usage_error() { let args = RemoveRuleArgs { diff --git a/crates/s3/src/client.rs b/crates/s3/src/client.rs index e37be8e..bf7844d 100644 --- a/crates/s3/src/client.rs +++ b/crates/s3/src/client.rs @@ -1463,6 +1463,32 @@ fn build_lifecycle_rule_filter( Ok(Some(filter)) } +fn validate_lifecycle_rule(rule: &LifecycleRule) -> Result<()> { + if rule.expired_object_delete_marker != Some(true) { + return Ok(()); + } + + if rule + .expiration + .as_ref() + .is_some_and(|expiration| expiration.days.is_some() || expiration.date.is_some()) + { + return Err(Error::InvalidPath(format!( + "lifecycle rule '{}' cannot combine current expiration days or date with expired delete-marker cleanup", + rule.id + ))); + } + + if rule.tags.as_ref().is_some_and(|tags| !tags.is_empty()) { + return Err(Error::InvalidPath(format!( + "lifecycle rule '{}' cannot combine tag filters with expired delete-marker cleanup", + rule.id + ))); + } + + Ok(()) +} + impl HttpConnector for ReqwestConnector { fn call(&self, mut request: HttpRequest) -> HttpConnectorFuture { let client = self.client.clone(); @@ -6374,6 +6400,7 @@ impl ObjectStore for S3Client { let mut sdk_rules = Vec::new(); for rule in rules { + validate_lifecycle_rule(&rule)?; let status = match rule.status { rc_core::LifecycleRuleStatus::Enabled => ExpirationStatus::Enabled, rc_core::LifecycleRuleStatus::Disabled => ExpirationStatus::Disabled, @@ -6381,24 +6408,34 @@ impl ObjectStore for S3Client { let filter = build_lifecycle_rule_filter(rule.prefix.as_deref(), rule.tags.as_ref())?; - let expiration = rule.expiration.map(|exp| { - let mut builder = SdkExpiration::builder(); - if let Some(days) = exp.days { - builder = builder.days(days); - } - if let Some(ref date_str) = exp.date - && let Ok(dt) = aws_smithy_types::DateTime::from_str( - date_str, - aws_smithy_types::date_time::Format::DateTime, - ) - { - builder = builder.date(dt); - } - if let Some(true) = rule.expired_object_delete_marker { - builder = builder.expired_object_delete_marker(true); - } - builder.build() - }); + let expiration = + if rule.expiration.is_some() || rule.expired_object_delete_marker == Some(true) { + let mut builder = SdkExpiration::builder(); + if let Some(days) = rule + .expiration + .as_ref() + .and_then(|expiration| expiration.days) + { + builder = builder.days(days); + } + if let Some(date_str) = rule + .expiration + .as_ref() + .and_then(|expiration| expiration.date.as_deref()) + && let Ok(dt) = aws_smithy_types::DateTime::from_str( + date_str, + aws_smithy_types::date_time::Format::DateTime, + ) + { + builder = builder.date(dt); + } + if let Some(true) = rule.expired_object_delete_marker { + builder = builder.expired_object_delete_marker(true); + } + Some(builder.build()) + } else { + None + }; let transitions = rule.transition.map(|t| { #[allow(deprecated)] @@ -7416,6 +7453,160 @@ mod tests { assert_eq!(parsed_tags.get("team").map(String::as_str), Some("core")); } + #[tokio::test] + async fn set_bucket_lifecycle_serializes_marker_only_expiration() { + let response = http::Response::builder() + .status(200) + .body(SdkBody::empty()) + .expect("build lifecycle response"); + let (client, request_receiver) = test_s3_client(Some(response)); + let rule = LifecycleRule { + id: "marker-only".to_string(), + status: rc_core::LifecycleRuleStatus::Enabled, + prefix: Some(String::new()), + tags: None, + expiration: None, + transition: None, + noncurrent_version_expiration: None, + noncurrent_version_transition: None, + abort_incomplete_multipart_upload_days: None, + expired_object_delete_marker: Some(true), + }; + + ObjectStore::set_bucket_lifecycle(&client, "bucket", vec![rule]) + .await + .expect("set marker-only lifecycle rule"); + + let request = request_receiver.expect_request(); + let body = request.body().bytes().expect("request body bytes"); + let body = std::str::from_utf8(body).expect("request body is utf8"); + assert!(body.contains( + "true" + )); + } + + #[tokio::test] + async fn set_bucket_lifecycle_serializes_noncurrent_expiration_with_marker_cleanup() { + let response = http::Response::builder() + .status(200) + .body(SdkBody::empty()) + .expect("build lifecycle response"); + let (client, request_receiver) = test_s3_client(Some(response)); + let rule = LifecycleRule { + id: "noncurrent-marker".to_string(), + status: rc_core::LifecycleRuleStatus::Enabled, + prefix: Some(String::new()), + tags: None, + expiration: None, + transition: None, + noncurrent_version_expiration: Some(rc_core::NoncurrentVersionExpiration { + noncurrent_days: 1, + newer_noncurrent_versions: None, + }), + noncurrent_version_transition: None, + abort_incomplete_multipart_upload_days: None, + expired_object_delete_marker: Some(true), + }; + + ObjectStore::set_bucket_lifecycle(&client, "bucket", vec![rule]) + .await + .expect("set noncurrent lifecycle rule with marker cleanup"); + + let request = request_receiver.expect_request(); + let body = request.body().bytes().expect("request body bytes"); + let body = std::str::from_utf8(body).expect("request body is utf8"); + assert!(body.contains( + "1" + )); + assert!(body.contains( + "true" + )); + assert!(!body.contains("")); + assert!(!body.contains("")); + } + + #[tokio::test] + async fn get_then_set_bucket_lifecycle_preserves_marker_cleanup() { + let get_response = http::Response::builder() + .status(200) + .body(SdkBody::from( + r#" + + + noncurrent-marker + Enabled + + 1 + true + +"#, + )) + .expect("build get lifecycle response"); + let (read_client, _read_request_receiver) = test_s3_client(Some(get_response)); + let rules = ObjectStore::get_bucket_lifecycle(&read_client, "bucket") + .await + .expect("get lifecycle rules"); + assert_eq!(rules[0].expired_object_delete_marker, Some(true)); + + let set_response = http::Response::builder() + .status(200) + .body(SdkBody::empty()) + .expect("build set lifecycle response"); + let (write_client, write_request_receiver) = test_s3_client(Some(set_response)); + ObjectStore::set_bucket_lifecycle(&write_client, "bucket", rules) + .await + .expect("write lifecycle rules back"); + + let request = write_request_receiver.expect_request(); + let body = request.body().bytes().expect("request body bytes"); + let body = std::str::from_utf8(body).expect("request body is utf8"); + assert!(body.contains( + "true" + )); + } + + #[tokio::test] + async fn set_bucket_lifecycle_rejects_invalid_marker_cleanup_combinations() { + let (client, request_receiver) = test_s3_client(None); + let mut tags = HashMap::new(); + tags.insert("env".to_string(), "prod".to_string()); + let rules = vec![ + LifecycleRule { + id: "days-marker".to_string(), + status: rc_core::LifecycleRuleStatus::Enabled, + prefix: None, + tags: None, + expiration: Some(rc_core::LifecycleExpiration { + days: Some(30), + date: None, + }), + transition: None, + noncurrent_version_expiration: None, + noncurrent_version_transition: None, + abort_incomplete_multipart_upload_days: None, + expired_object_delete_marker: Some(true), + }, + LifecycleRule { + id: "tags-marker".to_string(), + status: rc_core::LifecycleRuleStatus::Enabled, + prefix: None, + tags: Some(tags), + expiration: None, + transition: None, + noncurrent_version_expiration: None, + noncurrent_version_transition: None, + abort_incomplete_multipart_upload_days: None, + expired_object_delete_marker: Some(true), + }, + ]; + + for rule in rules { + let result = ObjectStore::set_bucket_lifecycle(&client, "bucket", vec![rule]).await; + assert!(matches!(result, Err(Error::InvalidPath(_)))); + } + request_receiver.expect_no_request(); + } + #[test] fn bucket_policy_error_kind_uses_error_code() { assert_eq!(