diff --git a/crates/replication/src/config.rs b/crates/replication/src/config.rs index c5d46d61c..42e4f3067 100644 --- a/crates/replication/src/config.rs +++ b/crates/replication/src/config.rs @@ -104,12 +104,21 @@ pub fn unsupported_replication_config_field(config: &ReplicationConfiguration) - if rule.destination.encryption_configuration.is_some() { return Some("Destination.EncryptionConfiguration"); } + if rule.destination.access_control_translation.is_some() { + return Some("Destination.AccessControlTranslation"); + } + if rule.destination.account.is_some() { + return Some("Destination.Account"); + } if rule.destination.metrics.is_some() { return Some("Destination.Metrics"); } if rule.destination.replication_time.is_some() { return Some("Destination.ReplicationTime"); } + if rule.destination.storage_class.is_some() { + return Some("Destination.StorageClass"); + } } None } @@ -422,6 +431,7 @@ mod tests { MetricsStatus, ReplicaModifications, ReplicationRule, ReplicationTime, ReplicationTimeStatus, ReplicationTimeValue, SourceSelectionCriteria, SseKmsEncryptedObjects, SseKmsEncryptedObjectsStatus, }; + use s3s::xml::{Deserializer, Serializer}; fn replication_rule(id: &str, arn: &str) -> ReplicationRule { ReplicationRule { @@ -817,6 +827,19 @@ mod tests { assert_eq!(unsupported_replication_config_field(&config), Some("Destination.EncryptionConfiguration")); config.rules[0].destination.encryption_configuration = None; + config.rules[0].destination.access_control_translation = Some(s3s::dto::AccessControlTranslation { + owner: s3s::dto::OwnerOverride::from_static(s3s::dto::OwnerOverride::DESTINATION), + }); + assert_eq!( + unsupported_replication_config_field(&config), + Some("Destination.AccessControlTranslation") + ); + + config.rules[0].destination.access_control_translation = None; + config.rules[0].destination.account = Some("123456789012".to_string()); + assert_eq!(unsupported_replication_config_field(&config), Some("Destination.Account")); + + config.rules[0].destination.account = None; config.rules[0].destination.metrics = Some(Metrics { event_threshold: None, status: MetricsStatus::from_static(MetricsStatus::ENABLED), @@ -829,6 +852,72 @@ mod tests { time: ReplicationTimeValue { minutes: Some(15) }, }); assert_eq!(unsupported_replication_config_field(&config), Some("Destination.ReplicationTime")); + + config.rules[0].destination.replication_time = None; + config.rules[0].destination.storage_class = + Some(s3s::dto::StorageClass::from_static(s3s::dto::StorageClass::STANDARD_IA)); + assert_eq!(unsupported_replication_config_field(&config), Some("Destination.StorageClass")); + } + + #[test] + fn historical_destination_fields_survive_the_s3_xml_round_trip() { + let xml = br#" + + + + historical + Enabled + + arn:aws:s3:::destination + 123456789012 + Destination + STANDARD_IA + + + + "#; + let mut deserializer = Deserializer::new(xml); + let config = ::deserialize(&mut deserializer) + .expect("historical config should parse"); + deserializer + .expect_eof() + .expect("historical config should consume the whole body"); + + let mut encoded = Vec::new(); + ::serialize(&config, &mut Serializer::new(&mut encoded)) + .expect("historical config should serialize"); + let encoded = String::from_utf8(encoded).expect("serialized XML should be UTF-8"); + for field in [ + "123456789012", + "Destination", + "STANDARD_IA", + ] { + assert!(encoded.contains(field), "historical field {field} was lost: {encoded}"); + } + } + + #[test] + fn s3_xml_parser_discards_unknown_replication_elements_before_validation() { + let xml = br#" + + + future + + unknown + Enabled + + arn:aws:s3:::destination + + + + "#; + let mut deserializer = Deserializer::new(xml); + let config = ::deserialize(&mut deserializer) + .expect("s3s should accept unknown elements"); + deserializer.expect_eof().expect("unknown elements should still be consumed"); + + assert!(config.rules[0].destination.encryption_configuration.is_none()); + assert_eq!(unsupported_replication_config_field(&config), None); } #[test] diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index b5ef28d6d..3ee20cca2 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -580,8 +580,6 @@ async fn validate_bucket_replication_update(bucket: &str, config: &ReplicationCo )); } - validate_replication_config_capabilities(config)?; - let targets = metadata_sys::get_bucket_targets_config(bucket) .await .map_err(|err| match err { @@ -2379,6 +2377,8 @@ impl DefaultBucketUsecase { } = req.input; info!(bucket = %bucket, "updating bucket replication config"); + validate_replication_config_capabilities(&replication_configuration)?; + let Some(store) = self.object_store() else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; @@ -4391,6 +4391,31 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::InternalError); } + #[tokio::test] + async fn execute_put_bucket_replication_rejects_unsupported_fields_before_store_or_metadata_write() { + let mut rule = replication_rule_for_target("arn:rustfs:replication:us-east-1:target:bucket"); + rule.destination.account = Some("123456789012".to_string()); + let input = PutBucketReplicationInput::builder() + .bucket("test-bucket".to_string()) + .replication_configuration(ReplicationConfiguration { + role: String::new(), + rules: vec![rule], + }) + .build() + .unwrap(); + + let err = DefaultBucketUsecase::without_context() + .execute_put_bucket_replication(build_request(input, Method::PUT)) + .await + .expect_err("unsupported fields must be rejected before store access"); + + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + assert_eq!( + err.message(), + Some("replication field Destination.Account is not supported by this RustFS version") + ); + } + #[tokio::test] async fn execute_put_bucket_encryption_returns_internal_error_when_store_uninitialized() { let input = PutBucketEncryptionInput::builder()