diff --git a/.config/nextest.toml b/.config/nextest.toml index c6eaf1908..bc57b8b8f 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -69,7 +69,7 @@ filter = 'package(rustfs-ecstore) & test(/^(bucket::lifecycle::bucket_lifecycle_ setup = 'ecstore-large-stack' [[profile.default.scripts]] -filter = 'package(rustfs-ecstore) | package(rustfs-s3select-api) | package(rustfs-scanner) | (package(rustfs) & test(/^(app::multipart_usecase::tests::concurrent_completions_share_durable_bucket_quota_reservations|app::object::delete::tests::compressed_delete_requests_update_observed_usage_without_releasing_quota_floor|app::object::internal_put::tests::internal_multipart_roundtrip_completes_and_abort_leaves_nothing|storage::access::tests::(delete_object_access_captures_authorized_bucket_incarnation|copy_operations_reject_recreated_source_bucket_after_authorization|request_slot_keeps_bucket_policy_bound_to_its_store))$/))' +filter = 'package(rustfs-ecstore) | package(rustfs-s3select-api) | package(rustfs-scanner) | (package(rustfs) & test(/^(app::multipart_usecase::tests::concurrent_completions_share_durable_bucket_quota_reservations|app::object::delete::tests::compressed_delete_requests_update_observed_usage_without_releasing_quota_floor|app::object::internal_put::tests::internal_multipart_roundtrip_completes_and_abort_leaves_nothing|app::object::restore::tests::execute_restore_object_maps_failures_to_typed_s3_errors|storage::access::tests::(delete_object_access_captures_authorized_bucket_incarnation|copy_operations_reject_recreated_source_bucket_after_authorization|request_slot_keeps_bucket_policy_bound_to_its_store))$/))' setup = 'ecstore-base-stack' [[profile.default.scripts]] @@ -210,7 +210,7 @@ filter = 'package(rustfs-ecstore) & test(/^(bucket::lifecycle::bucket_lifecycle_ setup = 'ecstore-large-stack' [[profile.ci.scripts]] -filter = 'package(rustfs-ecstore) | package(rustfs-s3select-api) | package(rustfs-scanner) | (package(rustfs) & test(/^(app::multipart_usecase::tests::concurrent_completions_share_durable_bucket_quota_reservations|app::object::delete::tests::compressed_delete_requests_update_observed_usage_without_releasing_quota_floor|app::object::internal_put::tests::internal_multipart_roundtrip_completes_and_abort_leaves_nothing|storage::access::tests::(delete_object_access_captures_authorized_bucket_incarnation|copy_operations_reject_recreated_source_bucket_after_authorization|request_slot_keeps_bucket_policy_bound_to_its_store))$/))' +filter = 'package(rustfs-ecstore) | package(rustfs-s3select-api) | package(rustfs-scanner) | (package(rustfs) & test(/^(app::multipart_usecase::tests::concurrent_completions_share_durable_bucket_quota_reservations|app::object::delete::tests::compressed_delete_requests_update_observed_usage_without_releasing_quota_floor|app::object::internal_put::tests::internal_multipart_roundtrip_completes_and_abort_leaves_nothing|app::object::restore::tests::execute_restore_object_maps_failures_to_typed_s3_errors|storage::access::tests::(delete_object_access_captures_authorized_bucket_incarnation|copy_operations_reject_recreated_source_bucket_after_authorization|request_slot_keeps_bucket_policy_bound_to_its_store))$/))' setup = 'ecstore-base-stack' [[profile.ci.scripts]] diff --git a/Cargo.lock b/Cargo.lock index 2aba26999..e9e7d9fe5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1328,9 +1328,9 @@ dependencies = [ [[package]] name = "aws-smithy-runtime-api" -version = "1.15.0" +version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "954c563ce84507722d2679f07a35d21b9c6466b3872d513020d0281fc8112ac9" +checksum = "9c054752dd9e4dc73d0b75748c99ac2d0feafbf2f25c7b0516f03a3534161223" dependencies = [ "aws-smithy-async", "aws-smithy-runtime-api-macros", @@ -1368,9 +1368,9 @@ dependencies = [ [[package]] name = "aws-smithy-types" -version = "1.6.2" +version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fce83ce9abbb198d25bc7131e468d0f9fe1257125e58c39f3f9fc9f5098c9647" +checksum = "8f94d16e797ec62cd999fc9d5942b48fa7050c3093ddadff48e4d7528d16fcb9" dependencies = [ "base64-simd", "bytes", @@ -5103,9 +5103,9 @@ dependencies = [ [[package]] name = "hickory-net" -version = "0.26.1" +version = "0.26.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2295ed2f9c31e471e1428a8f88a3f0e1f4b27c15049592138d1eebe9c35b183" +checksum = "084e7bd6a377435d568f652153e571b50970d7ccc1d1eeec0519f834632287e1" dependencies = [ "async-trait", "cfg-if", @@ -5127,9 +5127,9 @@ dependencies = [ [[package]] name = "hickory-proto" -version = "0.26.1" +version = "0.26.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bab31817bfb44672a252e97fe81cd0c18d1b2cf892108922f6818820df8c643" +checksum = "7e2da0694c15b44c6f68a6b05e0233617008c54080e31d6eb848d858a9c5b38d" dependencies = [ "data-encoding", "idna", @@ -5147,9 +5147,9 @@ dependencies = [ [[package]] name = "hickory-resolver" -version = "0.26.1" +version = "0.26.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0d58d28879ceecde6607729660c2667a081ccdc082e082675042793960f178c" +checksum = "0e4f9f4603319422d482738f3f6fe5aac03157fdbfed1cd85a3ff45adb09072f" dependencies = [ "cfg-if", "futures-util", @@ -5226,9 +5226,9 @@ dependencies = [ [[package]] name = "hotpath" -version = "0.24.0" +version = "0.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2645642a23d4061ec15a4a6e74f851a3145c3125356846cfc7772ff9c6f2737" +checksum = "2ec7782e005cabd5eaf350febde384cd799faa3a0e624587aa8759c240e0b592" dependencies = [ "arc-swap", "async-channel", @@ -5240,7 +5240,6 @@ dependencies = [ "futures-util", "hdrhistogram", "hotpath-macros", - "hotpath-meta", "http 1.5.0", "libc", "object 0.36.7", @@ -5260,30 +5259,15 @@ dependencies = [ [[package]] name = "hotpath-macros" -version = "0.24.0" +version = "0.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89a3d3cdf9b0d4d3d4f6d4a29798f3b9170401ba500eaa58dd8f890f926af0f1" +checksum = "929b2285d2cd21b2733a7fb6ebc843bb4f83dbd1db0122f5f9ebb9567b1e2613" dependencies = [ "proc-macro2", "quote", "syn 2.0.119", ] -[[package]] -name = "hotpath-macros-meta" -version = "0.24.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e84cd2417fa60938241cf1cd6c03e09953f5c821122dc5da9b8f27975d136c5b" - -[[package]] -name = "hotpath-meta" -version = "0.24.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4d2c145b67b1a4e7bcefa918995e212c30a49a85f05cc5962fe1f717878d560b" -dependencies = [ - "hotpath-macros-meta", -] - [[package]] name = "htmlescape" version = "0.3.1" @@ -5642,18 +5626,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "internal-russh-num-bigint" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae8e22120c32fb4d19ec55fba35015f57095cd95a2e3b732e44457f5915b2ee8" -dependencies = [ - "num-integer", - "num-traits", - "rand 0.10.2", - "rand_core 0.10.1", -] - [[package]] name = "io-uring" version = "0.7.14" @@ -5904,17 +5876,20 @@ dependencies = [ [[package]] name = "kafka-protocol" -version = "0.17.0" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66292444a1cd4d430d450d472c30cba839d0724229aba2d79affffcf901516e2" +checksum = "099d5c2f1b40cd830cbf18ca4d2a0805f2875b811ca18f0372b2433e68fe2dda" dependencies = [ "anyhow", "bytes", "crc", "crc32c", + "flate2", "indexmap 2.14.1", - "paste", + "lz4", + "snap", "uuid", + "zstd", ] [[package]] @@ -6923,6 +6898,8 @@ checksum = "93e7820bc0a80a0238e650327316f929ba18d5be054b647490a3a6a339f3e7c0" dependencies = [ "num-integer", "num-traits", + "rand 0.10.2", + "rand_core 0.10.1", ] [[package]] @@ -7515,9 +7492,9 @@ dependencies = [ [[package]] name = "pageant" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3adadc44070da6f464b0918655a12f5792c156e088d8c4082d13e27d94c3e791" +checksum = "6d8eab09a361a4afe0b1668be978cd97e4f052e927a92b0b608cf902965d49ce" dependencies = [ "base16ct 1.0.0", "byteorder", @@ -7606,12 +7583,6 @@ dependencies = [ "phc", ] -[[package]] -name = "paste" -version = "1.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" - [[package]] name = "path-absolutize" version = "4.0.1" @@ -9250,9 +9221,9 @@ dependencies = [ [[package]] name = "russh" -version = "0.63.1" +version = "0.63.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35bab1b87d915817d5d9cc352637cd40d5f0b298a48c6309af9156a4addc3031" +checksum = "8e134e2480f4e86f83e4aa45b4c0a9723f84beaffa694c54bdf056e74efdd7dd" dependencies = [ "aes 0.9.3", "aws-lc-rs", @@ -9281,13 +9252,12 @@ dependencies = [ "hex-literal", "hmac 0.13.0", "inout 0.2.2", - "internal-russh-num-bigint", "keccak", "log", "md5", "ml-kem", "module-lattice", - "num-bigint 0.4.8", + "num-bigint 0.5.1", "p256 0.14.0", "p384 0.14.0", "p521", @@ -9978,11 +9948,11 @@ dependencies = [ [[package]] name = "rustfs-kafka" -version = "1.2.0" +version = "1.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4eee0644a99743fb2f51db7fbae1a6ca2d85f064daf7595784eaa52834a68c96" +checksum = "759f8ddf709b497006e0f422a89890f11ac86c84c2c39df282891477c22f1a13" dependencies = [ - "base64 0.22.1", + "base64 0.23.1", "bytes", "fnv", "hmac 0.13.0", @@ -9992,22 +9962,22 @@ dependencies = [ "pbkdf2 0.13.0", "rand 0.10.2", "rustls", - "rustls-native-certs", "sha2 0.11.0", "socket2", "thiserror 2.0.20", "tracing", "twox-hash", + "uuid", "webpki-roots 1.0.9", ] [[package]] name = "rustfs-kafka-async" -version = "1.2.0" +version = "1.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7cd1997c3116cb94ede80d9a0b828f46dd27386cc93825fce141a92eb3aa9630" +checksum = "5aa574a13d91ef529e3a08f059a5c3ee8d4c3b16cf62f62c72595a6dc4f79f8f" dependencies = [ - "base64 0.22.1", + "base64 0.23.1", "bytes", "hmac 0.13.0", "kafka-protocol", @@ -10016,11 +9986,11 @@ dependencies = [ "rand 0.10.2", "rustfs-kafka", "rustls", - "rustls-native-certs", "sha2 0.11.0", "tokio", "tokio-rustls", "tracing", + "uuid", "webpki-roots 1.0.9", ] @@ -10194,18 +10164,18 @@ dependencies = [ [[package]] name = "rustfs-mimalloc" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85d1a75bd188260754c4fa8ace1bb1f6a9956ee36c1d330f98fed5d25e39fa15" +checksum = "46a7b69356718defa4060de3059e609c9da6619c35924fefef55d902ccd94958" dependencies = [ "rustfs-mimalloc-sys", ] [[package]] name = "rustfs-mimalloc-sys" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d3adbf24cbe37f040f856c04f8d4baf75626eacd4e8b17eb6b3d79de7fde692" +checksum = "1dd3ec9b7e7b9fed453acd8b4e32713ddc0e01e1aee31a3deec5b8025c0880c3" dependencies = [ "cc", ] @@ -12301,7 +12271,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", "windows-sys 0.61.2", @@ -12485,9 +12455,9 @@ dependencies = [ [[package]] name = "tinyvec" -version = "1.12.0" +version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb4ebadaa0af04fab11ae01eb5f9fdb5f9c5b875506e210e71c07873528baa7f" +checksum = "4cf0ded5c4e56918d8f8a339e1bb67d038d3bc6d144ac407904015ba2e4cde9b" dependencies = [ "tinyvec_macros", ] @@ -13652,13 +13622,14 @@ checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" [[package]] name = "wnaf" -version = "0.14.0" +version = "0.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab12e7090f27e2ffd9322651492942d50c2926094af30601e1964337db39daf1" +checksum = "795ca18b3fdb5e62bf982199278341ddcf7ebf7d32e25e212ad05d496e95f6fa" dependencies = [ "ff 0.14.0", "group 0.14.0", "hybrid-array", + "primefield", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 644e5b08a..05ddc1110 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -165,7 +165,7 @@ http-body = "1.1.0" http-body-util = "0.1.5" minlz = "1.2.3" reqwest = "0.13.4" -rustfs-kafka-async = { version = "1.2.0" } +rustfs-kafka-async = { version = "1.3.1" } socket2 = { version = "0.6.5" } tokio = { version = "1.53.1" } tokio-rustls = { default-features = false, version = "0.26.4" } @@ -244,8 +244,8 @@ aws-sdk-kms = { default-features = false, version = "1.117.0" } aws-sdk-s3 = { default-features = false, version = "1.144.0" } aws-sdk-sts = { default-features = false, version = "1.113.0" } aws-smithy-http-client = { default-features = false, version = "1.4.0" } -aws-smithy-runtime-api = { version = "1.15.0" } -aws-smithy-types = { version = "1.6.2" } +aws-smithy-runtime-api = { version = "1.16.0" } +aws-smithy-types = { version = "1.6.3" } base64-simd = "0.8.0" brotli = "9.0.0" clap = { version = "4.6.6" } @@ -359,15 +359,15 @@ libunftp = { version = "0.23.0" } unftp-core = "0.1.0" suppaftp = { version = "11.0.0" } rcgen = { version = "0.14.10", default-features = false, features = ["aws_lc_rs", "crypto", "pem"] } -russh = { version = "0.63.1" } +russh = { version = "0.63.2" } russh-sftp = "2.4.0" # WebDAV dav-server = "0.11.0" # Performance Analysis and Memory Profiling -rustfs-mimalloc = { version = "0.5.2" } -hotpath = { version = "0.24.0", default-features = false } +rustfs-mimalloc = { version = "0.5.3" } +hotpath = { version = "0.25.0", default-features = false } # Snapshot testing for output format regression detection insta = { version = "1.48" } diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index aa04369a6..7ce3523fd 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -252,6 +252,13 @@ pub mod bucket { }; } + pub mod sealed_credentials { + pub use crate::bucket::sealed_credentials::{ + CredentialSealer, SEALED_CREDENTIAL_VERSION, SealScope, SealedCredential, SealedCredentialError, + SealedCredentialStore, credential_sealer, install_credential_sealer, seal_secret, unseal_secret, + }; + } + pub mod replication { pub use crate::bucket::replication::replication_pool::{ DurableMrfBacklogSummary, DurableMrfBucketBacklog, DurableMrfTargetBacklog, MrfBacklogObservabilitySummary, diff --git a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs index 2fc6bb0fe..f52c9ea3f 100644 --- a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs +++ b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs @@ -87,16 +87,12 @@ use rustfs_filemeta::{ use rustfs_scanner_metrics::metrics::{ IlmAction, Metrics, ScannerLifecycleExpiryStateUpdate, ScannerLifecycleTransitionStateUpdate, global_metrics, }; -use rustfs_utils::{ - get_env_i64, get_env_usize, - path::encode_dir_object, - string::{parse_bool, strings_has_prefix_fold}, -}; +use rustfs_utils::{get_env_i64, get_env_usize, path::encode_dir_object, string::parse_bool}; use s3s::dto::{ BucketLifecycleConfiguration, ExpirationStatus, ObjectLockConfiguration, RestoreRequest, RestoreRequestType, RestoreStatus, Timestamp, }; -use s3s::header::{X_AMZ_RESTORE, X_AMZ_SERVER_SIDE_ENCRYPTION}; +use s3s::header::X_AMZ_RESTORE; use sha2::{Digest, Sha256}; use std::any::Any; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -165,7 +161,6 @@ pub const AMZ_TAG_COUNT: &str = "x-amz-tagging-count"; reason = "MinIO-parity tier/lifecycle entry point that this port never wired (backlog#1823)" )] pub const AMZ_TAG_DIRECTIVE: &str = "X-Amz-Tagging-Directive"; -pub const AMZ_ENCRYPTION_AES: &str = "AES256"; #[allow( dead_code, reason = "MinIO-parity tier/lifecycle entry point that this port never wired (backlog#1823)" @@ -5041,24 +5036,24 @@ fn attach_tier_operation_lease(mut reader: GetObjectReader, lease: TierOperation reader } -pub async fn post_restore_opts(version_id: &str, bucket: &str, object: &str) -> Result { +/// Resolve the RestoreObject request options. +/// +/// Returns the typed [`StorageError`]: flattening these into an opaque +/// `io::Error` string erased the identity the S3 layer needs to answer +/// InvalidArgument instead of a generic 500 (backlog#2205). +pub async fn post_restore_opts(version_id: &str, bucket: &str, object: &str) -> Result { let versioned = BucketVersioningSys::prefix_enabled(bucket, object).await; let version_suspended = BucketVersioningSys::prefix_suspended(bucket, object).await; let vid = version_id.trim(); if !vid.is_empty() && vid != NULL_VERSION_ID { if let Err(_err) = Uuid::parse_str(vid) { - return Err(std::io::Error::other( - StorageError::InvalidVersionID(bucket.to_string(), object.to_string(), vid.to_string()).to_string(), - )); + return Err(StorageError::InvalidVersionID(bucket.to_string(), object.to_string(), vid.to_string())); } if !versioned && !version_suspended { - return Err(std::io::Error::other( - StorageError::InvalidArgument( - bucket.to_string(), - object.to_string(), - format!("version-id specified {} but versioning is not enabled on {}", vid, bucket), - ) - .to_string(), + return Err(StorageError::InvalidArgument( + bucket.to_string(), + object.to_string(), + format!("version-id specified {vid} but versioning is not enabled on {bucket}"), )); } } @@ -5111,43 +5106,18 @@ pub async fn put_restore_opts( } meta.insert(X_AMZ_STORAGE_CLASS.as_str().to_lowercase(), sc);*/ - if let Some(type_) = &rreq.type_ - && type_.as_str() == RestoreRequestType::SELECT + // A SELECT restore must never reach the restore writer: the caller writes + // the retrieved bytes back to the source bucket/object, so building + // SELECT output options here produced a source overwrite carrying only + // the OutputLocation metadata instead of a write to `OutputLocation.S3` + // (backlog#1341). RestoreObject rejects SELECT at the API boundary; this + // is the fail-closed backstop for any other caller. + if rreq + .type_ + .as_ref() + .is_some_and(|type_| type_.as_str() == RestoreRequestType::SELECT) { - let Some(s3) = select_restore_s3_location(rreq)? else { - return Err(std::io::Error::other("OutputLocation.S3 required for SELECT requests")); - }; - if let Some(user_metadata) = s3.user_metadata.as_ref() { - for metadata in user_metadata { - let name = metadata - .name - .as_deref() - .ok_or_else(|| std::io::Error::other("SELECT restore metadata name is required"))?; - let value = metadata.value.clone().unwrap_or_default(); - if strings_has_prefix_fold(name, "x-amz-meta") { - meta.insert(name.to_string(), value); - } else { - meta.insert(format!("x-amz-meta-{name}"), value); - } - } - } - if let Some(tags) = &s3.tagging { - meta.insert( - AMZ_OBJECT_TAGGING.to_string(), - serde_urlencoded::to_string(tags.tag_set.clone()).unwrap_or_else(|_| "".to_string()), - ); - } - if let Some(encryption) = &s3.encryption - && encryption.encryption_type.as_str() != "" - { - meta.insert(X_AMZ_SERVER_SIDE_ENCRYPTION.as_str().to_string(), AMZ_ENCRYPTION_AES.to_string()); - } - return Ok(ObjectOptions { - versioned: BucketVersioningSys::prefix_enabled(bucket, object).await, - version_suspended: BucketVersioningSys::prefix_suspended(bucket, object).await, - user_defined: meta, - ..Default::default() - }); + return Err(std::io::Error::other("SELECT restore requests are not supported")); } for (k, v) in oi.user_defined.iter() { meta.insert(k.to_string(), v.clone()); diff --git a/crates/ecstore/src/bucket/mod.rs b/crates/ecstore/src/bucket/mod.rs index 48fce4e02..e93419cf2 100644 --- a/crates/ecstore/src/bucket/mod.rs +++ b/crates/ecstore/src/bucket/mod.rs @@ -31,6 +31,7 @@ pub mod policy_sys; pub mod quota; pub mod remote_s3_client; pub mod replication; +pub mod sealed_credentials; pub mod tagging; pub mod target; pub mod utils; diff --git a/crates/ecstore/src/bucket/sealed_credentials.rs b/crates/ecstore/src/bucket/sealed_credentials.rs new file mode 100644 index 000000000..392e40dd1 --- /dev/null +++ b/crates/ecstore/src/bucket/sealed_credentials.rs @@ -0,0 +1,351 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Sealed remote credentials shared by the three stores that persist one +//! (rustfs/backlog#2168): replication targets (`bucket-targets.json`), remote +//! tiers (`tier-config.bin`) and on-demand migration sources +//! (`on-demand-migration.json`). +//! +//! The design record is `docs/architecture/remote-credential-sealing-adr.md`. +//! What this module owns: the versioned envelope, the encryption context that +//! binds a ciphertext to the record owning it, the sealer registration point, +//! and the fail-closed error type. What it deliberately does not own: any KMS +//! call (ECStore does not depend on `rustfs-kms`; the binary installs a +//! sealer, exactly like `ON_DEMAND_MIGRATION_CONFIG_HOOK` and the event +//! dispatch hook in `crates/ecstore/src/services/event_notification.rs`), and +//! any decision about which stored field a consumer writes. + +use async_trait::async_trait; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::fmt; +use std::sync::{Arc, OnceLock}; + +/// Envelope format this build writes. A reader accepts only versions it +/// knows; an unknown version is a typed error, never a fallback. +pub const SEALED_CREDENTIAL_VERSION: u8 = 1; + +/// Which store a sealed value belongs to. Part of the encryption context, so +/// a ciphertext cannot be replayed into a different store. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SealedCredentialStore { + /// `bucket-targets.json` (replication and other bucket targets). + BucketTargets, + /// `tier-config.bin` (remote tiers). + TierConfig, + /// `on-demand-migration.json` (migration sources). + OnDemandMigration, +} + +impl SealedCredentialStore { + pub fn as_str(self) -> &'static str { + match self { + SealedCredentialStore::BucketTargets => "bucket-targets", + SealedCredentialStore::TierConfig => "tier-config", + SealedCredentialStore::OnDemandMigration => "on-demand-migration", + } + } +} + +/// Identity of the record a secret belongs to: the store, its owner (bucket +/// name, tier name, or target ARN) and the field name. Rendered into the KMS +/// encryption context so a ciphertext moved between buckets, tiers or fields +/// fails to decrypt instead of silently authorizing a different remote. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SealScope { + pub store: SealedCredentialStore, + pub owner: String, + pub field: &'static str, +} + +impl SealScope { + pub fn new(store: SealedCredentialStore, owner: impl Into, field: &'static str) -> Self { + Self { + store, + owner: owner.into(), + field, + } + } + + /// The encryption context handed to the sealer. Keys are stable: they are + /// part of the on-disk contract, because a ciphertext only decrypts under + /// the same context. + pub fn encryption_context(&self) -> HashMap { + HashMap::from([ + ("rustfs:store".to_string(), self.store.as_str().to_string()), + ("rustfs:owner".to_string(), self.owner.clone()), + ("rustfs:field".to_string(), self.field.to_string()), + ]) + } +} + +/// A sealed secret as persisted. `Debug` prints no ciphertext: a sealed value +/// is not a secret, but it is noise in a log line and an operator reading one +/// should see the key it is wrapped under, not the bytes. +#[derive(Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct SealedCredential { + /// Envelope version; see [`SEALED_CREDENTIAL_VERSION`]. + pub v: u8, + /// KMS master key id the data key is wrapped under. + pub key_id: String, + /// Master key version, when the backend reports one. Carried so the KMS + /// re-wrap job (`docs/architecture/kms-bulk-rekey-contract.md`) can tell + /// stale envelopes apart; nothing here rotates on its own. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub key_version: Option, + /// Algorithm label reported by the sealer, for forensics and migration. + pub alg: String, + /// Ciphertext blob as produced by the sealer, base64 (standard, padded) + /// in the JSON stores and raw inside the tier msgpack payload. + pub ct: String, +} + +impl fmt::Debug for SealedCredential { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SealedCredential") + .field("v", &self.v) + .field("key_id", &self.key_id) + .field("key_version", &self.key_version) + .field("alg", &self.alg) + .field("ct", &format_args!("<{} bytes sealed>", self.ct.len())) + .finish() + } +} + +impl SealedCredential { + /// Rejects an envelope this build cannot read. Called before every + /// unseal so an unknown version fails here rather than inside a backend. + pub fn check_version(&self) -> Result<(), SealedCredentialError> { + if self.v == SEALED_CREDENTIAL_VERSION { + Ok(()) + } else { + Err(SealedCredentialError::UnsupportedVersion(self.v)) + } + } +} + +/// Why a seal or unseal did not produce a usable value. Every variant is +/// terminal for the record that carried it: a caller reports the remote as +/// unusable, and never substitutes a default or empty credential. +#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] +pub enum SealedCredentialError { + /// No sealer is installed: KMS is not configured, or the process has not + /// finished startup. Reading a sealed record is impossible here. + #[error("no credential sealer is installed")] + NoSealer, + /// The stored envelope is from a newer (or otherwise unknown) format. + #[error("unsupported sealed credential envelope version {0}")] + UnsupportedVersion(u8), + /// The stored bytes are not a well-formed envelope. + #[error("malformed sealed credential: {0}")] + Malformed(String), + /// The sealer refused: wrong encryption context, missing key, revoked + /// access, or a failed authentication tag. + #[error("sealed credential could not be unsealed: {0}")] + Kms(String), +} + +/// The KMS-backed half, installed by the binary at startup. +#[async_trait] +pub trait CredentialSealer: Send + Sync + 'static { + /// Wraps `plaintext` under the scope's encryption context. + async fn seal(&self, plaintext: &str, scope: &SealScope) -> Result; + + /// Unwraps a stored envelope. Must fail when the envelope was sealed + /// under a different scope. + async fn unseal(&self, sealed: &SealedCredential, scope: &SealScope) -> Result; +} + +static CREDENTIAL_SEALER: OnceLock> = OnceLock::new(); + +/// Installs the process-wide sealer. Returns `false` when one is already +/// installed, matching the other ECStore hooks. +pub fn install_credential_sealer(sealer: Arc) -> bool { + CREDENTIAL_SEALER.set(sealer).is_ok() +} + +/// The installed sealer, or `None` when KMS is not wired. Callers that only +/// need to know whether sealing is possible use this; callers that must have +/// it use [`seal_secret`] / [`unseal_secret`] and get the typed error. +pub fn credential_sealer() -> Option> { + CREDENTIAL_SEALER.get().cloned() +} + +/// Seals one secret field. Fails closed: without a sealer the caller must +/// reject the write rather than persist the secret in clear text after the +/// operator asked for sealing. +pub async fn seal_secret(plaintext: &str, scope: &SealScope) -> Result { + let sealer = credential_sealer().ok_or(SealedCredentialError::NoSealer)?; + sealer.seal(plaintext, scope).await +} + +/// Unseals one secret field, rejecting an unknown envelope version first. +pub async fn unseal_secret(sealed: &SealedCredential, scope: &SealScope) -> Result { + sealed.check_version()?; + let sealer = credential_sealer().ok_or(SealedCredentialError::NoSealer)?; + sealer.unseal(sealed, scope).await +} + +#[cfg(test)] +mod tests { + use super::*; + use parking_lot::Mutex; + + /// Stands in for the KMS-backed sealer: records the context it was called + /// with, and refuses a ciphertext presented under a different one. + #[derive(Default)] + struct FakeSealer { + sealed_contexts: Mutex>>, + } + + #[async_trait] + impl CredentialSealer for FakeSealer { + async fn seal(&self, plaintext: &str, scope: &SealScope) -> Result { + let context = scope.encryption_context(); + self.sealed_contexts.lock().push(context.clone()); + let mut bound = serde_json::to_string(&context).expect("context serializes"); + bound.push('|'); + bound.push_str(plaintext); + Ok(SealedCredential { + v: SEALED_CREDENTIAL_VERSION, + key_id: "key-1".to_string(), + key_version: Some("3".to_string()), + alg: "AES-256-GCM".to_string(), + ct: base64_simd::STANDARD.encode_to_string(bound.as_bytes()), + }) + } + + async fn unseal(&self, sealed: &SealedCredential, scope: &SealScope) -> Result { + let raw = base64_simd::STANDARD + .decode_to_vec(sealed.ct.as_bytes()) + .map_err(|err| SealedCredentialError::Malformed(err.to_string()))?; + let bound = String::from_utf8(raw).map_err(|err| SealedCredentialError::Malformed(err.to_string()))?; + let expected = serde_json::to_string(&scope.encryption_context()).expect("context serializes"); + bound + .strip_prefix(&expected) + .and_then(|rest| rest.strip_prefix('|')) + .map(str::to_string) + .ok_or_else(|| SealedCredentialError::Kms("encryption context mismatch".to_string())) + } + } + + fn scope(owner: &str) -> SealScope { + SealScope::new(SealedCredentialStore::OnDemandMigration, owner, "secret_key") + } + + #[tokio::test] + async fn seal_round_trips_and_binds_the_scope() { + let sealer = Arc::new(FakeSealer::default()); + let sealed = sealer.seal("super-secret", &scope("photos")).await.expect("seal"); + assert_eq!(sealed.v, SEALED_CREDENTIAL_VERSION); + assert_eq!(sealed.key_version.as_deref(), Some("3")); + assert_eq!(sealer.unseal(&sealed, &scope("photos")).await.expect("unseal"), "super-secret"); + + // The same ciphertext under another bucket must not unseal. + let err = sealer + .unseal(&sealed, &scope("other-bucket")) + .await + .expect_err("a ciphertext must not move between owners"); + assert!(matches!(err, SealedCredentialError::Kms(_)), "{err}"); + + // Nor under another field of the same record. + let other_field = SealScope::new(SealedCredentialStore::OnDemandMigration, "photos", "session_token"); + let err = sealer + .unseal(&sealed, &other_field) + .await + .expect_err("a ciphertext must not move between fields"); + assert!(matches!(err, SealedCredentialError::Kms(_)), "{err}"); + + let contexts = sealer.sealed_contexts.lock(); + assert_eq!(contexts.len(), 1); + assert_eq!(contexts[0]["rustfs:store"], "on-demand-migration"); + assert_eq!(contexts[0]["rustfs:owner"], "photos"); + assert_eq!(contexts[0]["rustfs:field"], "secret_key"); + } + + #[tokio::test] + async fn an_unknown_envelope_version_is_rejected_before_the_sealer_is_asked() { + let sealed = SealedCredential { + v: SEALED_CREDENTIAL_VERSION + 1, + key_id: "key-1".to_string(), + key_version: None, + alg: "AES-256-GCM".to_string(), + ct: "Zm9v".to_string(), + }; + assert_eq!( + sealed.check_version().expect_err("a newer envelope must not be read"), + SealedCredentialError::UnsupportedVersion(SEALED_CREDENTIAL_VERSION + 1) + ); + // The global helper reports the version, not "no sealer", even in a + // process where none is installed. + assert_eq!( + unseal_secret(&sealed, &scope("photos")).await.expect_err("version first"), + SealedCredentialError::UnsupportedVersion(SEALED_CREDENTIAL_VERSION + 1) + ); + } + + #[tokio::test] + async fn without_a_sealer_both_directions_fail_closed() { + // This test binary installs no sealer, so the global helpers must + // report NoSealer rather than fall back to clear text. + assert!(credential_sealer().is_none(), "no sealer is installed in unit tests"); + assert_eq!( + seal_secret("super-secret", &scope("photos")).await.expect_err("seal"), + SealedCredentialError::NoSealer + ); + let sealed = SealedCredential { + v: SEALED_CREDENTIAL_VERSION, + key_id: "key-1".to_string(), + key_version: None, + alg: "AES-256-GCM".to_string(), + ct: "Zm9v".to_string(), + }; + assert_eq!( + unseal_secret(&sealed, &scope("photos")).await.expect_err("unseal"), + SealedCredentialError::NoSealer + ); + } + + #[test] + fn debug_and_serde_keep_the_on_disk_shape_stable() { + let sealed = SealedCredential { + v: 1, + key_id: "key-1".to_string(), + key_version: None, + alg: "AES-256-GCM".to_string(), + ct: "Zm9v".to_string(), + }; + // key_version is omitted when absent, so an envelope from a backend + // without version history stays compact. + assert_eq!( + serde_json::to_string(&sealed).expect("serialize"), + r#"{"v":1,"key_id":"key-1","alg":"AES-256-GCM","ct":"Zm9v"}"# + ); + let parsed: SealedCredential = serde_json::from_str(r#"{"v":1,"key_id":"key-1","alg":"AES-256-GCM","ct":"Zm9v"}"#) + .expect("an envelope without key_version parses"); + assert_eq!(parsed, sealed); + + let rendered = format!("{sealed:?}"); + assert!(rendered.contains("key-1"), "{rendered}"); + assert!(!rendered.contains("Zm9v"), "Debug must not print the ciphertext: {rendered}"); + } + + #[test] + fn a_malformed_envelope_is_a_typed_error() { + let err = serde_json::from_str::(r#"{"v":1,"key_id":"key-1"}"#) + .map_err(|err| SealedCredentialError::Malformed(err.to_string())) + .expect_err("a truncated envelope must not parse"); + assert!(matches!(err, SealedCredentialError::Malformed(_)), "{err}"); + } +} diff --git a/crates/ecstore/src/services/tier/tier.rs b/crates/ecstore/src/services/tier/tier.rs index 377f6ba06..6f48db424 100644 --- a/crates/ecstore/src/services/tier/tier.rs +++ b/crates/ecstore/src/services/tier/tier.rs @@ -50,9 +50,12 @@ use tracing::{debug, error, info, warn}; use crate::error::{Error, Result, StorageError, stable_io_error}; use crate::services::tier::{ tier_admin::TierCreds, - tier_config::{TierConfig, TierType, TierWasabi}, + tier_config::{TIER_CREDENTIAL_REDACTED, TierConfig, TierType, TierWasabi}, tier_handlers::{ERR_TIER_ALREADY_EXISTS, ERR_TIER_NAME_NOT_UPPERCASE, ERR_TIER_NOT_FOUND, ERR_TIER_RESERVED_NAME}, - warm_backend::{TransitionCandidateProbe, WarmBackend, check_warm_backend, new_warm_backend}, + warm_backend::{ + TransitionCandidateProbe, WARM_BACKEND_PROBE_TIMEOUT, WarmBackend, check_warm_backend, check_warm_backend_until, + new_warm_backend, + }, }; use crate::storage_api_contracts::{ bucket::BucketOperations, @@ -136,6 +139,14 @@ struct TierDriverBuildBarrier { #[cfg(test)] static TIER_DRIVER_BUILD_BARRIER: LazyLock>>> = LazyLock::new(|| Mutex::new(None)); +#[cfg(test)] +type TierDriverTestFactory = Arc std::result::Result + Send + Sync + 'static>; + +#[cfg(test)] +tokio::task_local! { + static TIER_DRIVER_TEST_FACTORY: TierDriverTestFactory; +} + #[cfg(test)] struct TierDriverBuildBarrierGuard; @@ -157,12 +168,18 @@ fn install_tier_driver_build_barrier(tier_name: &str) -> (Arc std::result::Result { +fn tier_validation_timeout(message: impl Into) -> AdminError { + let mut err = ERR_TIER_BACKEND_IN_USE.clone(); + err.message = message.into(); + err +} + +#[cfg(test)] +async fn wait_for_tier_driver_build_barrier(tier_name: &str) { #[cfg(test)] let test_barrier = { lock_unpoisoned(&TIER_DRIVER_BUILD_BARRIER).clone() }; - #[cfg(test)] if let Some(barrier) = test_barrier - && barrier.tier_name == tier.name + && barrier.tier_name == tier_name { barrier.arrived.notify_one(); barrier @@ -172,7 +189,52 @@ async fn build_warm_backend(tier: &TierConfig, probe: bool) -> std::result::Resu .expect("tier driver build test barrier should stay open") .forget(); } - new_warm_backend(tier, probe).await +} + +async fn construct_warm_backend(tier: &TierConfig) -> std::result::Result { + #[cfg(test)] + if let Ok(result) = TIER_DRIVER_TEST_FACTORY.try_with(|factory| factory(tier)) { + return result; + } + new_warm_backend(tier, false).await +} + +async fn build_warm_backend(tier: &TierConfig, probe: bool) -> std::result::Result { + build_warm_backend_with_deadline(tier, probe, None).await +} + +async fn build_warm_backend_with_deadline( + tier: &TierConfig, + probe: bool, + deadline: Option, +) -> std::result::Result { + #[cfg(test)] + { + let wait = wait_for_tier_driver_build_barrier(&tier.name); + if let Some(deadline) = deadline { + timeout_at(deadline, wait) + .await + .map_err(|_| tier_validation_timeout("Timed out preparing the remote tier backend"))?; + } else { + wait.await; + } + } + + let driver = if let Some(deadline) = deadline { + timeout_at(deadline, construct_warm_backend(tier)) + .await + .map_err(|_| tier_validation_timeout("Timed out preparing the remote tier backend"))?? + } else { + construct_warm_backend(tier).await? + }; + if probe { + if let Some(deadline) = deadline { + check_warm_backend_until(Some(&driver), deadline).await?; + } else { + check_warm_backend(Some(&driver)).await?; + } + } + Ok(driver) } const TIER_CONFIG_LEGACY_FILE: &str = "tier-config.json"; @@ -219,6 +281,189 @@ lazy_static! { }; } +fn tier_invalid_config(message: impl Into) -> AdminError { + let mut err = ERR_TIER_INVALID_CONFIG.clone(); + err.message = message.into(); + err +} + +fn normalize_add_tier_name_fields( + canonical_name: &mut String, + nested_name: &mut String, + provider: &str, +) -> std::result::Result<(), AdminError> { + if !canonical_name.is_empty() && !nested_name.is_empty() && canonical_name.as_str() != nested_name.as_str() { + return Err(tier_invalid_config(format!( + "TierConfig.Name conflicts with the legacy {provider}.name field" + ))); + } + let resolved_name = if canonical_name.is_empty() { + nested_name.clone() + } else { + canonical_name.clone() + }; + if resolved_name.is_empty() { + return Err(tier_invalid_config("Remote tier name is empty")); + } + canonical_name.clone_from(&resolved_name); + nested_name.clone_from(&resolved_name); + Ok(()) +} + +fn normalize_s3_gcs_add_tier_name(config: &mut TierConfig) -> std::result::Result<(), AdminError> { + match config.tier_type { + TierType::S3 => { + if let Some(s3) = config.s3.as_mut() { + normalize_add_tier_name_fields(&mut config.name, &mut s3.name, "S3")?; + } + } + TierType::GCS => { + if let Some(gcs) = config.gcs.as_mut() { + normalize_add_tier_name_fields(&mut config.name, &mut gcs.name, "GCS")?; + } + } + _ => {} + } + Ok(()) +} + +fn credential_is_redacted(value: &str) -> bool { + value.trim() == TIER_CREDENTIAL_REDACTED +} + +fn validate_static_tier_credentials(access_key: &str, secret_key: &str) -> std::result::Result<(), AdminError> { + if access_key.is_empty() || secret_key.is_empty() || credential_is_redacted(access_key) || credential_is_redacted(secret_key) + { + return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); + } + Ok(()) +} + +fn tier_creds_request_aws_role(creds: &TierCreds) -> bool { + creds.aws_role || !creds.aws_role_web_identity_token_file.is_empty() || !creds.aws_role_arn.is_empty() +} + +fn s3_config_requests_aws_role(config: &crate::services::tier::tier_config::TierS3) -> bool { + config.aws_role + || !config.aws_role_web_identity_token_file.is_empty() + || !config.aws_role_arn.is_empty() + || !config.aws_role_session_name.is_empty() + || config.aws_role_duration_seconds != 0 +} + +fn reject_unsupported_aws_role() -> AdminError { + tier_invalid_config("AWS role and web identity credentials are not supported for remote tiers") +} + +fn validate_gcs_credentials_json(credentials: &str) -> std::result::Result<(), AdminError> { + if credentials.is_empty() || credential_is_redacted(credentials) { + return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); + } + let value: serde_json::Value = serde_json::from_str(credentials) + .map_err(|_| tier_invalid_config("GCS credentials must be valid service account JSON"))?; + if value.get("type").and_then(serde_json::Value::as_str) != Some("service_account") { + return Err(tier_invalid_config("GCS credentials must describe a service account")); + } + Ok(()) +} + +fn validate_azure_supported_options( + azure: &crate::services::tier::tier_config::TierAzure, +) -> std::result::Result<(), AdminError> { + let sp_auth_set = + !azure.sp_auth.tenant_id.is_empty() || !azure.sp_auth.client_id.is_empty() || !azure.sp_auth.client_secret.is_empty(); + if !azure.storage_class.is_empty() || sp_auth_set { + return Err(tier_invalid_config( + "Azure remote tiers do not support storageClass or spAuth yet; leave both unset", + )); + } + Ok(()) +} + +fn validate_tier_config_credentials(config: &TierConfig) -> std::result::Result<(), AdminError> { + match config.tier_type { + TierType::S3 => { + if let Some(s3) = config.s3.as_ref() { + if s3_config_requests_aws_role(s3) { + return Err(reject_unsupported_aws_role()); + } + validate_static_tier_credentials(&s3.access_key, &s3.secret_key)?; + } + } + TierType::Wasabi => { + if let Some(backend) = config.wasabi.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::RustFS => { + if let Some(backend) = config.rustfs.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::MinIO => { + if let Some(backend) = config.minio.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::Aliyun => { + if let Some(backend) = config.aliyun.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::Tencent => { + if let Some(backend) = config.tencent.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::Huaweicloud => { + if let Some(backend) = config.huaweicloud.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::Azure => { + if let Some(backend) = config.azure.as_ref() { + validate_azure_supported_options(backend)?; + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::GCS => { + if let Some(gcs) = config.gcs.as_ref() { + validate_gcs_credentials_json(&gcs.creds)?; + } + } + TierType::R2 => { + if let Some(backend) = config.r2.as_ref() { + validate_static_tier_credentials(&backend.access_key, &backend.secret_key)?; + } + } + TierType::Unsupported => {} + } + Ok(()) +} + +fn merge_static_tier_credentials( + access_key: &mut String, + secret_key: &mut String, + creds: &TierCreds, +) -> std::result::Result<(), AdminError> { + if tier_creds_request_aws_role(creds) { + return Err(reject_unsupported_aws_role()); + } + if !creds.creds_json.is_empty() { + return Err(tier_invalid_config("GCS credentials cannot be used with this remote tier type")); + } + match (creds.access_key.is_empty(), creds.secret_key.is_empty()) { + (true, true) => {} + (false, false) => { + validate_static_tier_credentials(&creds.access_key, &creds.secret_key)?; + access_key.clone_from(&creds.access_key); + secret_key.clone_from(&creds.secret_key); + } + _ => return Err(ERR_TIER_MISSING_CREDENTIALS.clone()), + } + validate_static_tier_credentials(access_key, secret_key) +} + #[derive(Serialize, Deserialize)] pub struct TierConfigMgr { #[serde(skip)] @@ -323,7 +568,11 @@ struct PreparedTierDriver { impl TierPublishTransition { async fn wait_for_active_leases(&self) -> std::result::Result<(), AdminError> { - let deadline = Instant::now() + TIER_OPERATION_DRAIN_TIMEOUT; + self.wait_for_active_leases_until(Instant::now() + TIER_OPERATION_DRAIN_TIMEOUT) + .await + } + + async fn wait_for_active_leases_until(&self, deadline: Instant) -> std::result::Result<(), AdminError> { for generation in self.revoked.values() { if timeout_at(deadline, generation.wait_for_no_active_leases()).await.is_err() { let mut err = ERR_TIER_BACKEND_IN_USE.clone(); @@ -542,6 +791,18 @@ enum TierCandidateMutation { } impl TierCandidateMutation { + fn add(mut config: TierConfig, force: bool) -> std::result::Result { + normalize_s3_gcs_add_tier_name(&mut config)?; + Ok(Self::Add(config, force)) + } + + fn normalize_add_tier_name(&mut self) -> std::result::Result<(), AdminError> { + if let Self::Add(config, _) = self { + normalize_s3_gcs_add_tier_name(config)?; + } + Ok(()) + } + fn intent_kind(&self) -> TierMutationIntentKind { match self { Self::Add(_, _) => TierMutationIntentKind::Add, @@ -605,15 +866,19 @@ impl TierCandidateMutation { ) } - async fn apply(self, candidate: &mut TierConfigMgr) -> std::result::Result, AdminError> { + async fn apply( + self, + candidate: &mut TierConfigMgr, + deadline: Option, + ) -> std::result::Result, AdminError> { match self { Self::Add(config, force) => { let tier_name = config.name.clone(); - candidate.add(config, force).await?; + candidate.add_with_deadline(config, force, deadline).await?; Ok(Some(tier_name)) } Self::Edit(tier_name, credentials) => { - candidate.edit(&tier_name, credentials).await?; + candidate.edit_with_deadline(&tier_name, credentials, deadline).await?; Ok(Some(tier_name)) } Self::Remove(tier_name, force) => { @@ -802,7 +1067,7 @@ where { let targets = affected_targets .iter() - .filter(|target| target.old_backend_identity.is_some()) + .filter(|target| target.old_backend_identity.is_some() && target.old_backend_identity != target.new_backend_identity) .cloned() .collect::>(); if targets.is_empty() { @@ -1572,7 +1837,12 @@ async fn apply_tier_candidate_mutation( candidate: &mut TierConfigMgr, deadline: Instant, ) -> std::result::Result, AdminError> { - match timeout_at(deadline, mutation.apply(candidate)).await { + if matches!(&mutation, TierCandidateMutation::Add(_, _) | TierCandidateMutation::Edit(_, _)) { + // Add/Edit validation performs its own deadline-aware cleanup. Do not + // wrap it in an outer timeout that would cancel an uncertain probe. + return mutation.apply(candidate, Some(deadline)).await; + } + match timeout_at(deadline, mutation.apply(candidate, None)).await { Ok(result) => result, Err(_) => { let mut err = ERR_TIER_BACKEND_IN_USE.clone(); @@ -2157,6 +2427,22 @@ struct ExternalTierCompatible { region: String, } +fn decode_external_gcs_credentials(credentials: &str) -> io::Result { + if serde_json::from_str::(credentials).is_ok() { + return Ok(credentials.to_string()); + } + // MinIO config and madmin AddTier payloads carry URL-safe-base64 credentials, + // while the existing RustFS v2 disk format stores raw JSON for rolling upgrades. + let decoded = base64_simd::STANDARD + .decode_to_vec(credentials.as_bytes()) + .or_else(|_| base64_simd::STANDARD_NO_PAD.decode_to_vec(credentials.as_bytes())) + .or_else(|_| base64_simd::URL_SAFE.decode_to_vec(credentials.as_bytes())) + .or_else(|_| base64_simd::URL_SAFE_NO_PAD.decode_to_vec(credentials.as_bytes())) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "tier config contains invalid GCS credentials encoding"))?; + String::from_utf8(decoded) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "tier config contains non-UTF-8 GCS credentials JSON")) +} + fn tier_config_path(file: &str) -> String { format!("{}{}{}", CONFIG_PREFIX, SLASH_SEPARATOR, file) } @@ -2577,7 +2863,7 @@ fn from_external_tier_config(name: String, ext: ExternalTierConfig) -> io::Resul cfg.gcs = Some(crate::services::tier::tier_config::TierGCS { name: cfg.name.clone(), endpoint: gcs.endpoint.clone(), - creds: gcs.creds.clone(), + creds: decode_external_gcs_credentials(&gcs.creds)?, bucket: gcs.bucket.clone(), prefix: gcs.prefix.clone(), region: gcs.region.clone(), @@ -2765,7 +3051,17 @@ impl TierConfigMgr { (TierType::Unsupported, false) } - pub async fn add(&mut self, mut tier_config: TierConfig, force: bool) -> std::result::Result<(), AdminError> { + pub async fn add(&mut self, tier_config: TierConfig, force: bool) -> std::result::Result<(), AdminError> { + self.add_with_deadline(tier_config, force, None).await + } + + async fn add_with_deadline( + &mut self, + mut tier_config: TierConfig, + force: bool, + deadline: Option, + ) -> std::result::Result<(), AdminError> { + normalize_s3_gcs_add_tier_name(&mut tier_config)?; self.ensure_generation_is_idle(&tier_config.name)?; let tier_name = tier_config.name.clone(); if tier_name != tier_name.to_uppercase() { @@ -2797,28 +3093,28 @@ impl TierConfigMgr { })?; } - // The Azure warm backend goes through the same S3-compatible TransitionClient as every - // other provider (backlog#2055): it has no Azure Blob-native client and no Azure AD - // dependency, so `storage_class` and `sp_auth` cannot be honored today even though the - // config type carries them. Reject them explicitly here instead of silently accepting - // and then dropping them at the WarmBackendAzure construction boundary. - if matches!(&tier_config.tier_type, TierType::Azure) - && let Some(azure) = tier_config.azure.as_ref() + if matches!(&tier_config.tier_type, TierType::GCS) + && let Some(gcs) = tier_config.gcs.as_mut() { - let sp_auth_set = !azure.sp_auth.tenant_id.is_empty() - || !azure.sp_auth.client_id.is_empty() - || !azure.sp_auth.client_secret.is_empty(); - if !azure.storage_class.is_empty() || sp_auth_set { - let mut err = ERR_TIER_INVALID_CONFIG.clone(); - err.message = "Azure remote tiers do not support storageClass or spAuth yet; leave both unset".to_string(); - return Err(err); - } + gcs.creds = decode_external_gcs_credentials(&gcs.creds).map_err(|source| tier_invalid_config(source.to_string()))?; } - let d = new_warm_backend(&tier_config, true).await?; + validate_tier_config_credentials(&tier_config)?; + let d = match deadline { + Some(deadline) => build_warm_backend_with_deadline(&tier_config, true, Some(deadline)).await?, + None => build_warm_backend(&tier_config, true).await?, + }; if !force { - let in_use = d.in_use().await; + let in_use = match deadline { + Some(deadline) => timeout_at(deadline, d.in_use()).await, + None => tokio::time::timeout(WARM_BACKEND_PROBE_TIMEOUT, d.in_use()).await, + } + .map_err(|_| { + let mut err = ERR_TIER_BACKEND_IN_USE.clone(); + err.message = "Timed out checking whether the remote tier is in use".to_string(); + err + })?; match in_use { Ok(b) => { if b { @@ -2910,7 +3206,7 @@ impl TierConfigMgr { pub fn list_tiers(&self) -> Vec { let mut tier_cfgs = Vec::::new(); for (_, tier) in self.tiers.iter() { - let tier = tier.clone(); + let tier = tier.redacted(); tier_cfgs.push(tier); } tier_cfgs @@ -2919,120 +3215,108 @@ impl TierConfigMgr { pub fn get(&self, tier_name: &str) -> Option { for (tier_name2, tier) in self.tiers.iter() { if tier_name == tier_name2 { - return Some(tier.clone()); + return Some(tier.redacted()); } } None } pub async fn edit(&mut self, tier_name: &str, creds: TierCreds) -> std::result::Result<(), AdminError> { + self.edit_with_deadline(tier_name, creds, None).await + } + + async fn edit_with_deadline( + &mut self, + tier_name: &str, + creds: TierCreds, + deadline: Option, + ) -> std::result::Result<(), AdminError> { self.ensure_generation_is_idle(tier_name)?; let (tier_type, exists) = self.is_tier_name_in_use(tier_name); if !exists { return Err(ERR_TIER_NOT_FOUND.clone()); } + if !creds.azure_service_principal.is_empty() { + return Err(tier_invalid_config( + "Azure service principal credentials are not supported for remote tier edits", + )); + } - let mut tier_config = self.tiers[tier_name].clone(); + let mut tier_config = self.tiers[tier_name].clone_with_credentials(); match tier_type { TierType::S3 => { if let Some(s3) = tier_config.s3.as_mut() { - if creds.aws_role { - s3.aws_role = true - } - if creds.aws_role_web_identity_token_file != "" && creds.aws_role_arn != "" { - s3.aws_role_arn = creds.aws_role_arn; - s3.aws_role_web_identity_token_file = creds.aws_role_web_identity_token_file; - } - if creds.access_key != "" && creds.secret_key != "" { - s3.access_key = creds.access_key; - s3.secret_key = creds.secret_key; - } + merge_static_tier_credentials(&mut s3.access_key, &mut s3.secret_key, &creds)?; } } TierType::Wasabi => { if let Some(wasabi) = tier_config.wasabi.as_mut() { - if creds.access_key.is_empty() || creds.secret_key.is_empty() { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - wasabi.access_key = creds.access_key; - wasabi.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut wasabi.access_key, &mut wasabi.secret_key, &creds)?; } } TierType::RustFS => { if let Some(rustfs) = tier_config.rustfs.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - rustfs.access_key = creds.access_key; - rustfs.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut rustfs.access_key, &mut rustfs.secret_key, &creds)?; } } TierType::MinIO => { if let Some(compatible_backend) = tier_config.minio.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - compatible_backend.access_key = creds.access_key; - compatible_backend.secret_key = creds.secret_key; + merge_static_tier_credentials( + &mut compatible_backend.access_key, + &mut compatible_backend.secret_key, + &creds, + )?; } } TierType::Aliyun => { if let Some(aliyun) = tier_config.aliyun.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - aliyun.access_key = creds.access_key; - aliyun.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut aliyun.access_key, &mut aliyun.secret_key, &creds)?; } } TierType::Tencent => { if let Some(tencent) = tier_config.tencent.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - tencent.access_key = creds.access_key; - tencent.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut tencent.access_key, &mut tencent.secret_key, &creds)?; } } TierType::Huaweicloud => { if let Some(huaweicloud) = tier_config.huaweicloud.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - huaweicloud.access_key = creds.access_key; - huaweicloud.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut huaweicloud.access_key, &mut huaweicloud.secret_key, &creds)?; } } TierType::Azure => { if let Some(azure) = tier_config.azure.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - azure.access_key = creds.access_key; - azure.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut azure.access_key, &mut azure.secret_key, &creds)?; } } TierType::GCS => { if let Some(gcs) = tier_config.gcs.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); + if tier_creds_request_aws_role(&creds) { + return Err(reject_unsupported_aws_role()); + } + if !creds.access_key.is_empty() || !creds.secret_key.is_empty() { + return Err(tier_invalid_config("Static access and secret keys cannot be used with a GCS tier")); + } + if !creds.creds_json.is_empty() { + let credentials = std::str::from_utf8(&creds.creds_json) + .map_err(|_| tier_invalid_config("GCS credentials must be UTF-8 service account JSON"))?; + validate_gcs_credentials_json(credentials)?; + gcs.creds = credentials.to_string(); } - gcs.creds = creds.access_key; //creds.creds_json } } TierType::R2 => { if let Some(r2) = tier_config.r2.as_mut() { - if creds.access_key == "" || creds.secret_key == "" { - return Err(ERR_TIER_MISSING_CREDENTIALS.clone()); - } - r2.access_key = creds.access_key; - r2.secret_key = creds.secret_key; + merge_static_tier_credentials(&mut r2.access_key, &mut r2.secret_key, &creds)?; } } _ => (), } - let d = new_warm_backend(&tier_config, true).await?; + validate_tier_config_credentials(&tier_config)?; + let d = match deadline { + Some(deadline) => build_warm_backend_with_deadline(&tier_config, true, Some(deadline)).await?, + None => build_warm_backend(&tier_config, true).await?, + }; self.revoke_driver(tier_name); self.tiers.insert(tier_name.to_string(), tier_config); self.replace_driver(tier_name, d)?; @@ -3266,11 +3550,12 @@ impl TierConfigMgr { async fn update_candidate_with_config_lock( handle: &Arc>, api: Arc, - mutation: TierCandidateMutation, + mut mutation: TierCandidateMutation, ) -> std::result::Result<(), TierConfigUpdateError> where S: TierReferenceProofStore + NamespaceLocking + 'static, { + mutation.normalize_add_tier_name().map_err(TierConfigUpdateError::Mutation)?; let config_lock = Self::acquire_tier_config_write_lock(api.clone()).await?; Self::reject_pending_mutation_recovery_before_update(handle, api.clone()).await?; let update = Self::admin_update_lock(handle).await; @@ -3452,7 +3737,7 @@ impl TierConfigMgr { let exact_get_delete = tier_exact_get_delete(config); Some(PreparedTierDriver { tier_name: tier_name.to_string(), - tier_config: config.clone(), + tier_config: config.clone_with_credentials(), config_fingerprint, backend_identity, exact_get_delete, @@ -3493,7 +3778,7 @@ impl TierConfigMgr { })?; let entry = Arc::new(TierDriverGeneration { tier_name: Arc::from(prepared.tier_name.as_str()), - tier_config: prepared.tier_config.clone(), + tier_config: prepared.tier_config.clone_with_credentials(), generation, config_fingerprint: prepared.config_fingerprint, backend_identity: prepared.backend_identity, @@ -3596,6 +3881,8 @@ impl TierConfigMgr { let handle = handle.clone(); #[cfg(test)] let test_peers = TIER_MUTATION_TEST_PEERS.try_with(|peers| peers.clone()).ok(); + #[cfg(test)] + let test_driver_factory = TIER_DRIVER_TEST_FACTORY.try_with(|factory| factory.clone()).ok(); tokio::spawn(async move { let update_task = async move { match AssertUnwindSafe(async move { @@ -3609,6 +3896,7 @@ impl TierConfigMgr { } let explicit_tier_name = mutation.explicit_tier_name().map(str::to_string); let mutation_force = mutation.force(); + let validation_deadline = Instant::now() + TIER_REMOTE_VALIDATION_TIMEOUT; let current_for_targets = TierConfigMgr { driver_cache: HashMap::new(), tiers: candidate @@ -3625,13 +3913,12 @@ impl TierConfigMgr { .map_err(TierConfigUpdateError::Publish)? }; transition - .wait_for_active_leases() + .wait_for_active_leases_until(validation_deadline) .await .map_err(TierConfigUpdateError::Publish)?; - let driver_tier = - apply_tier_candidate_mutation(mutation, &mut candidate, Instant::now() + TIER_REMOTE_VALIDATION_TIMEOUT) - .await - .map_err(TierConfigUpdateError::Mutation)?; + let driver_tier = apply_tier_candidate_mutation(mutation, &mut candidate, validation_deadline) + .await + .map_err(TierConfigUpdateError::Mutation)?; let proof_targets = tier_mutation_proof_targets( mutation_kind, explicit_tier_name.as_deref(), @@ -3830,10 +4117,15 @@ impl TierConfigMgr { }; #[cfg(test)] { - if let Some(peers) = test_peers { - TIER_MUTATION_TEST_PEERS.scope(peers, update_task).await - } else { - update_task.await + match (test_peers, test_driver_factory) { + (Some(peers), Some(factory)) => { + TIER_DRIVER_TEST_FACTORY + .scope(factory, TIER_MUTATION_TEST_PEERS.scope(peers, update_task)) + .await + } + (Some(peers), None) => TIER_MUTATION_TEST_PEERS.scope(peers, update_task).await, + (None, Some(factory)) => TIER_DRIVER_TEST_FACTORY.scope(factory, update_task).await, + (None, None) => update_task.await, } } #[cfg(not(test))] @@ -4923,7 +5215,8 @@ impl TierConfigMgr { tier_config: TierConfig, force: bool, ) -> std::result::Result<(), TierConfigUpdateError> { - Self::update_candidate_with_config_lock(handle, api, TierCandidateMutation::Add(tier_config, force)).await + let mutation = TierCandidateMutation::add(tier_config, force).map_err(TierConfigUpdateError::Mutation)?; + Self::update_candidate_with_config_lock(handle, api, mutation).await } pub async fn edit_and_save( @@ -4936,6 +5229,32 @@ impl TierConfigMgr { .await } + #[cfg(test)] + async fn edit_and_save_with( + handle: &Arc>, + api: Arc, + tier_name: &str, + credentials: TierCreds, + ) -> std::result::Result<(), TierConfigUpdateError> + where + S: TierReferenceProofStore + 'static, + { + let update = Self::admin_update_lock(handle).await; + let (candidate, version) = load_tier_config_for_update(api.clone()) + .await + .map_err(TierConfigUpdateError::Load)?; + Self::update_candidate_owned( + handle, + api, + candidate, + version, + TierCandidateMutation::Edit(tier_name.to_string(), credentials), + update, + None, + ) + .await + } + pub async fn remove_and_save( handle: &Arc>, api: Arc, @@ -4999,8 +5318,14 @@ impl TierConfigMgr { let lease = Self::acquire_operation_lease(handle, tier_name) .await .map_err(io::Error::other)?; - let driver: WarmBackendImpl = Box::new(SharedWarmBackendProxy(lease.inner.driver.clone())); - check_warm_backend(Some(&driver)).await.map_err(io::Error::other) + tokio::spawn(async move { + let driver: WarmBackendImpl = Box::new(SharedWarmBackendProxy(lease.inner.driver.clone())); + let result = check_warm_backend(Some(&driver)).await.map_err(io::Error::other); + drop(lease); + result + }) + .await + .map_err(|_| io::Error::other("remote tier verification task failed"))? } pub(crate) async fn acquire_operation_lease_for_backend_identity( @@ -5060,7 +5385,7 @@ impl TierConfigMgr { let driver: SharedWarmBackend = Arc::from(driver); let entry = Arc::new(TierDriverGeneration { tier_name: Arc::from(tier_name), - tier_config: config.clone(), + tier_config: config.clone_with_credentials(), generation, config_fingerprint, backend_identity, @@ -6379,7 +6704,7 @@ mod tests { // // These tests must not reach a real remote tier. Two techniques keep them // hermetic: - // * error paths that return *before* `new_warm_backend` constructs a + // * error paths that return *before* `build_warm_backend` constructs a // client (name validation, duplicate detection, unsupported type, // missing backend payload, missing credentials); // * a `MockWarmBackend` injected directly into `driver_cache`, so @@ -6387,6 +6712,7 @@ mod tests { // lets us drive `remove`/`verify` through every branch. // --------------------------------------------------------------------- + use crate::services::tier::test_util::{MockWarmBackend as RecordingWarmBackend, MockWarmOp}; use crate::services::tier::warm_backend::{WarmBackend, WarmBackendGetOpts}; use rustfs_s3_client::transition_api::{ReadCloser, ReaderImpl}; @@ -6417,6 +6743,24 @@ mod tests { } } + fn build_gcs_tier(name: &str, credentials: &str) -> TierConfig { + TierConfig { + version: "v1".to_string(), + tier_type: TierType::GCS, + name: name.to_string(), + gcs: Some(crate::services::tier::tier_config::TierGCS { + name: name.to_string(), + endpoint: "https://storage.googleapis.com".to_string(), + creds: credentials.to_string(), + bucket: "bucket-gcs".to_string(), + prefix: "prefix-gcs".to_string(), + region: String::new(), + storage_class: String::new(), + }), + ..Default::default() + } + } + #[derive(Debug)] struct LockingTierConfigStore { locks: Mutex>, @@ -6766,6 +7110,51 @@ mod tests { in_use_value: Option, /// When false, put/get/remove all fail (drives `verify` error paths). healthy: bool, + probe_present: AtomicBool, + } + + #[derive(Default)] + struct HangingInUseBackend { + probe_present: AtomicBool, + } + + #[async_trait::async_trait] + impl WarmBackend for HangingInUseBackend { + async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> io::Result { + self.probe_present.store(true, Ordering::SeqCst); + Ok("probe-version".to_string()) + } + + async fn put_with_meta( + &self, + object: &str, + r: ReaderImpl, + length: i64, + _meta: HashMap, + ) -> io::Result { + self.put(object, r, length).await + } + + async fn get(&self, _object: &str, _rv: &str, _opts: WarmBackendGetOpts) -> io::Result { + Ok(BufReader::new(Cursor::new(b"RustFS".to_vec()))) + } + + async fn remove(&self, _object: &str, _rv: &str) -> io::Result<()> { + self.probe_present.store(false, Ordering::SeqCst); + Ok(()) + } + + async fn probe_transition_candidate(&self, _object: &str) -> io::Result { + if self.probe_present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent("probe-version".to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + + async fn in_use(&self) -> io::Result { + std::future::pending().await + } } #[async_trait::async_trait] @@ -6788,6 +7177,7 @@ mod tests { async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> std::result::Result { if self.healthy { + self.probe_present.store(true, Ordering::SeqCst); Ok("mock-version".to_string()) } else { Err(std::io::Error::other("mock put failed")) @@ -6819,6 +7209,7 @@ mod tests { async fn remove(&self, _object: &str, _rv: &str) -> std::result::Result<(), std::io::Error> { if self.healthy { + self.probe_present.store(false, Ordering::SeqCst); Ok(()) } else { Err(std::io::Error::other("mock remove failed")) @@ -6832,6 +7223,20 @@ mod tests { self.remove(object, rv).await } + async fn probe_transition_candidate( + &self, + _object: &str, + ) -> std::result::Result { + if !self.healthy { + return Err(std::io::Error::other("mock candidate probe failed")); + } + if self.probe_present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent("mock-version".to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + async fn in_use(&self) -> std::result::Result { match self.in_use_value { Some(b) => Ok(b), @@ -6845,8 +7250,37 @@ mod tests { mgr.driver_cache.insert(name.to_string(), Box::new(mock)); } + fn recording_driver_factory( + backend: RecordingWarmBackend, + observed_configs: Arc>>, + ) -> TierDriverTestFactory { + Arc::new(move |config| { + lock_unpoisoned(&observed_configs).push(config.clone_with_credentials()); + Ok(Box::new(backend.clone())) + }) + } + + fn healthy_driver_factory() -> TierDriverTestFactory { + recording_driver_factory(RecordingWarmBackend::new(), Arc::new(Mutex::new(Vec::new()))) + } + // ---- add ------------------------------------------------------------ + #[test] + fn test_add_name_normalization_uses_canonical_name_and_supports_legacy_nested_name() { + let mut canonical_s3 = build_s3_tier("COLD-CANONICAL"); + canonical_s3.s3.as_mut().expect("S3 payload should exist").name.clear(); + normalize_s3_gcs_add_tier_name(&mut canonical_s3).expect("canonical S3 name should normalize"); + assert_eq!(canonical_s3.name, "COLD-CANONICAL"); + assert_eq!(canonical_s3.s3.as_ref().expect("S3 payload should remain").name, "COLD-CANONICAL"); + + let mut legacy_gcs = build_gcs_tier("COLD-LEGACY", r#"{"type":"service_account"}"#); + legacy_gcs.name.clear(); + normalize_s3_gcs_add_tier_name(&mut legacy_gcs).expect("legacy nested GCS name should normalize"); + assert_eq!(legacy_gcs.name, "COLD-LEGACY"); + assert_eq!(legacy_gcs.gcs.as_ref().expect("GCS payload should remain").name, "COLD-LEGACY"); + } + #[tokio::test] async fn test_add_rejects_non_uppercase_name() { let mut mgr = empty_mgr(); @@ -6952,24 +7386,102 @@ mod tests { assert!(mgr.tiers.is_empty()); } + #[tokio::test] + async fn test_add_rejects_canonical_azure_sp_auth_wire_before_backend_setup() { + let tier: TierConfig = serde_json::from_value(serde_json::json!({ + "type": "azure", + "Name": "COLD-AZURE", + "azure": { + "name": "COLD-AZURE", + "endpoint": "https://azure.example.invalid", + "accessKey": "account", + "secretKey": "key", + "bucket": "archive", + "spAuth": { + "TenantID": "tenant" + } + } + })) + .expect("mixed RustFS/madmin Azure payload should decode"); + let backend_builds = Arc::new(AtomicUsize::new(0)); + let observed_builds = backend_builds.clone(); + let factory: TierDriverTestFactory = Arc::new(move |_| { + observed_builds.fetch_add(1, Ordering::SeqCst); + Ok(Box::new(RecordingWarmBackend::new())) + }); + let mut mgr = empty_mgr(); + + let err = TIER_DRIVER_TEST_FACTORY + .scope(factory, mgr.add(tier, true)) + .await + .expect_err("canonical Azure service-principal fields must fail closed"); + + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("spAuth"), "{}", err.message); + assert_eq!(backend_builds.load(Ordering::SeqCst), 0); + assert!(mgr.tiers.is_empty()); + } + #[tokio::test] async fn test_add_does_not_reject_azure_config_without_storage_class_or_sp_auth() { // A plain Azure config (the common case: static access/secret key, no - // storageClass, no spAuth) must sail past the new gate. `new_warm_backend` - // builds the S3-compatible client lazily (no eager DNS/connect), so with - // `force: true` (which also skips the `in_use` probe) this succeeds even - // against a fake endpoint — the point here is only that the gate itself - // does not fire. + // storageClass, no spAuth) must sail past the provider-specific gate. let mut mgr = empty_mgr(); let tier = build_azure_tier("account-a"); let tier_name = tier.name.clone(); - mgr.add(tier, true) + TIER_DRIVER_TEST_FACTORY + .scope(healthy_driver_factory(), mgr.add(tier, true)) .await .expect("a config with no storageClass/spAuth must not trip the new gate"); assert!(mgr.tiers.contains_key(&tier_name)); } + #[tokio::test] + async fn test_add_force_does_not_bypass_bad_credentials_or_unreachable_probe() { + for unreachable in [false, true] { + let mut mgr = empty_mgr(); + let backend = RecordingWarmBackend::new(); + if unreachable { + backend.set_unreachable(true).await; + } else { + backend.set_reject_credentials(true).await; + } + let observed = Arc::new(Mutex::new(Vec::new())); + + let err = TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend, observed.clone()), + mgr.add(build_s3_tier("COLD-A"), true), + ) + .await + .expect_err("force must not bypass the backend read/write/delete probe"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert_eq!(lock_unpoisoned(&observed).len(), 1); + assert!(mgr.tiers.is_empty()); + } + } + + #[tokio::test] + async fn test_add_rejects_unsupported_s3_role_before_backend_setup() { + let mut mgr = empty_mgr(); + let mut tier = build_s3_tier("COLD-A"); + let s3 = tier.s3.as_mut().expect("S3 payload should exist"); + s3.access_key.clear(); + s3.secret_key.clear(); + s3.aws_role = true; + + let err = mgr + .add(tier, true) + .await + .expect_err("unsupported role credentials must fail before backend setup"); + + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("not supported")); + assert!(mgr.tiers.is_empty()); + } + #[tokio::test] async fn test_add_rejects_reserved_names() { // Supersedes the former `test_add_does_not_reserve_standard_name_regression_anchor` @@ -6998,6 +7510,240 @@ mod tests { } } + #[tokio::test] + async fn test_add_rejects_canonical_s3_role_fields_even_with_static_credentials() { + let mut tier: TierConfig = serde_json::from_value(serde_json::json!({ + "Version": "v1", + "Type": "s3", + "Name": "COLD-A", + "S3": { + "Endpoint": "https://s3.example.invalid", + "AccessKey": "static-access", + "SecretKey": "static-secret", + "Bucket": "archive", + "Prefix": "objects", + "Region": "us-east-1", + "StorageClass": "STANDARD", + "AWSRole": true, + "AWSRoleWebIdentityTokenFile": "/var/run/private-token", + "AWSRoleARN": "arn:aws:iam::123456789012:role/archive", + "AWSRoleSessionName": "archive-session", + "AWSRoleDurationSeconds": 900 + } + })) + .expect("canonical madmin S3 role payload should decode"); + assert_eq!(tier.name, "COLD-A"); + let s3 = tier.s3.as_ref().expect("S3 payload should decode"); + assert!(s3.name.is_empty(), "canonical madmin S3 has no nested Name field"); + assert_eq!(s3.endpoint, "https://s3.example.invalid"); + assert!(!s3.access_key.is_empty()); + assert!(!s3.secret_key.is_empty()); + assert_eq!(s3.bucket, "archive"); + assert_eq!(s3.prefix, "objects"); + assert_eq!(s3.region, "us-east-1"); + assert_eq!(s3.storage_class, "STANDARD"); + assert!(s3.aws_role); + assert_eq!(s3.aws_role_web_identity_token_file, "/var/run/private-token"); + assert_eq!(s3.aws_role_duration_seconds, 900); + normalize_s3_gcs_add_tier_name(&mut tier).expect("canonical madmin Name should normalize before validation"); + assert_eq!(tier.name, "COLD-A"); + assert_eq!(tier.s3.as_ref().expect("S3 payload should remain").name, "COLD-A"); + let serialized = serde_json::to_value(&tier).expect("S3 config should serialize"); + assert_eq!(serialized["type"], "s3"); + assert!(serialized.get("Type").is_none()); + assert!(serialized.get("S3").is_none()); + assert!(serialized.get("Name").is_none()); + assert!(serialized["s3"].get("AccessKey").is_none()); + assert!(serialized["s3"].get("accessKey").is_some()); + assert!(serialized["s3"].get("AWSRole").is_none()); + assert!(serialized["s3"].get("AWSRoleWebIdentityTokenFile").is_none()); + + let mut mgr = empty_mgr(); + let backend_builds = Arc::new(AtomicUsize::new(0)); + let observed_backend_builds = backend_builds.clone(); + let factory: TierDriverTestFactory = Arc::new(move |_| { + observed_backend_builds.fetch_add(1, Ordering::SeqCst); + Ok(Box::new(RecordingWarmBackend::new())) + }); + let err = TIER_DRIVER_TEST_FACTORY + .scope(factory, mgr.add(tier, true)) + .await + .expect_err("unsupported role fields must be rejected before backend setup"); + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("not supported")); + assert_eq!(backend_builds.load(Ordering::SeqCst), 0); + assert!(mgr.tiers.is_empty()); + } + + #[tokio::test] + async fn test_add_rejects_conflicting_canonical_and_legacy_s3_gcs_names_before_backend_setup() { + let fixtures = [ + ( + "S3", + serde_json::json!({ + "Type": "S3", + "Name": "COLD-CANONICAL", + "S3": { + "name": "COLD-LEGACY", + "Endpoint": "https://s3.example.invalid", + "AccessKey": "access", + "SecretKey": "secret", + "Bucket": "archive" + } + }), + ), + ( + "GCS", + serde_json::json!({ + "Type": "GCS", + "Name": "COLD-CANONICAL", + "GCS": { + "name": "COLD-LEGACY", + "Endpoint": "https://storage.googleapis.com/", + "Creds": "e30=", + "Bucket": "archive" + } + }), + ), + ]; + + for (provider, fixture) in fixtures { + let tier: TierConfig = serde_json::from_value(fixture) + .unwrap_or_else(|err| panic!("{provider} aliases should decode before name validation: {err}")); + let manager = TierConfigMgr::new(); + let store = Arc::new(CasConfigStore::default()); + let backend_builds = Arc::new(AtomicUsize::new(0)); + let observed_backend_builds = backend_builds.clone(); + let factory: TierDriverTestFactory = Arc::new(move |_| { + observed_backend_builds.fetch_add(1, Ordering::SeqCst); + Ok(Box::new(RecordingWarmBackend::new())) + }); + let peer_calls = Arc::new(Mutex::new(Vec::new())); + + let err = TIER_DRIVER_TEST_FACTORY + .scope( + factory, + TIER_MUTATION_TEST_PEERS.scope( + vec![FakeTierMutationPeer::boxed( + "peer-a", + peer_calls.clone(), + Ok(PeerTierMutationState::Committed), + )], + TierConfigMgr::update_candidate_with_config_lock(&manager, store, TierCandidateMutation::Add(tier, true)), + ), + ) + .await + .expect_err("conflicting canonical and legacy tier names must fail closed"); + let TierConfigUpdateError::Mutation(err) = err else { + panic!("conflicting {provider} names should fail as a typed mutation error") + }; + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("conflicts"), "{}", err.message); + assert_eq!(backend_builds.load(Ordering::SeqCst), 0, "{provider} backend must not be prepared"); + assert!(lock_unpoisoned(&peer_calls).is_empty(), "{provider} conflict must not prepare peers"); + assert!(manager.read().await.tiers.is_empty()); + } + } + + #[tokio::test(start_paused = true)] + async fn test_add_times_out_a_hanging_in_use_check_after_probe_cleanup() { + let mut mgr = empty_mgr(); + let factory: TierDriverTestFactory = Arc::new(|_| Ok(Box::new(HangingInUseBackend::default()))); + let err = TIER_DRIVER_TEST_FACTORY + .scope(factory, mgr.add(build_rustfs_tier("COLD-A"), false)) + .await + .expect_err("a hanging in-use check must not retain the admin lock forever"); + + assert_eq!(err.code, ERR_TIER_BACKEND_IN_USE.code); + assert!(err.message.contains("Timed out checking")); + assert!(mgr.tiers.is_empty()); + } + + #[tokio::test(start_paused = true)] + async fn candidate_add_honors_the_caller_validation_deadline() { + let mut candidate = empty_mgr(); + let factory: TierDriverTestFactory = Arc::new(|_| Ok(Box::new(HangingInUseBackend::default()))); + let deadline = Instant::now() + Duration::from_secs(1); + let err = { + let add = TIER_DRIVER_TEST_FACTORY.scope( + factory, + apply_tier_candidate_mutation( + TierCandidateMutation::Add(build_rustfs_tier("COLD-DEADLINE"), false), + &mut candidate, + deadline, + ), + ); + tokio::pin!(add); + tokio::task::yield_now().await; + + let result = tokio::time::timeout(Duration::from_secs(2), &mut add); + tokio::pin!(result); + tokio::time::advance(Duration::from_secs(2) + Duration::from_millis(1)).await; + result + .await + .expect("the add mutation must finish within its caller deadline") + .expect_err("a hanging in-use check must fail the add mutation") + }; + + assert_eq!(err.code, ERR_TIER_BACKEND_IN_USE.code); + assert!(err.message.contains("Timed out checking")); + assert!(candidate.tiers.is_empty()); + } + + #[tokio::test(start_paused = true)] + async fn candidate_edit_honors_the_caller_validation_deadline() { + let mut candidate = empty_mgr(); + candidate + .tiers + .insert("COLD-DEADLINE".to_string(), build_rustfs_tier("COLD-DEADLINE")); + let backend = RecordingWarmBackend::new(); + let get_barrier = backend.arm_get_barrier().await; + let factory = recording_driver_factory(backend.clone(), Arc::new(Mutex::new(Vec::new()))); + let deadline = Instant::now() + Duration::from_secs(1); + let err = { + let edit = TIER_DRIVER_TEST_FACTORY.scope( + factory, + apply_tier_candidate_mutation( + TierCandidateMutation::Edit( + "COLD-DEADLINE".to_string(), + TierCreds { + access_key: "rotated-access".to_string(), + secret_key: "rotated-secret".to_string(), + ..Default::default() + }, + ), + &mut candidate, + deadline, + ), + ); + tokio::pin!(edit); + tokio::select! { + _ = get_barrier.wait_until_paused() => {} + result = &mut edit => panic!("edit completed before the probe deadline: {result:?}"), + } + + let result = tokio::time::timeout(Duration::from_secs(2), &mut edit); + tokio::pin!(result); + tokio::time::advance(Duration::from_secs(2) + Duration::from_millis(1)).await; + result + .await + .expect("the edit mutation must finish within its caller deadline") + .expect_err("a hanging in-use check must fail the edit mutation") + }; + + assert_eq!(err.code, ERR_TIER_BACKEND_IN_USE.code); + assert!(err.message.contains("Timed out validating")); + assert_eq!(backend.object_count().await, 0, "deadline-aware edit must clean up its probe object"); + assert_eq!( + candidate.tiers["COLD-DEADLINE"] + .rustfs + .as_ref() + .expect("tier payload") + .access_key, + "ak" + ); + } + // ---- edit ----------------------------------------------------------- #[tokio::test] @@ -7011,32 +7757,46 @@ mod tests { } #[tokio::test] - async fn test_edit_rejects_missing_credentials_for_rustfs() { + async fn test_edit_rejects_half_static_credentials_for_rustfs() { let mut mgr = empty_mgr(); mgr.tiers.insert("COLD-R".to_string(), build_rustfs_tier("COLD-R")); - // Empty access/secret keys => rejected before any driver rebuild. let err = mgr - .edit("COLD-R", TierCreds::default()) + .edit( + "COLD-R", + TierCreds { + access_key: "rotated-access".to_string(), + ..Default::default() + }, + ) .await - .expect_err("empty credentials must be rejected"); + .expect_err("a half-filled static credential pair must be rejected"); assert_eq!(err.code, ERR_TIER_MISSING_CREDENTIALS.code); + let rustfs = mgr.tiers["COLD-R"].rustfs.as_ref().expect("original payload should remain"); + assert_eq!(rustfs.access_key, "ak"); + assert_eq!(rustfs.secret_key, "sk"); } #[tokio::test] - async fn test_edit_rejects_missing_credentials_for_wasabi() { + async fn test_edit_rejects_half_static_credentials_for_wasabi() { let mut mgr = empty_mgr(); mgr.tiers.insert("COLD-WASABI".to_string(), build_wasabi_tier("COLD-WASABI")); let err = mgr - .edit("COLD-WASABI", TierCreds::default()) + .edit( + "COLD-WASABI", + TierCreds { + secret_key: "rotated-secret".to_string(), + ..Default::default() + }, + ) .await - .expect_err("empty Wasabi credentials must be rejected before backend setup"); + .expect_err("a half-filled Wasabi credential pair must be rejected before backend setup"); assert_eq!(err.code, ERR_TIER_MISSING_CREDENTIALS.code); } #[tokio::test] - async fn test_edit_rejects_missing_credentials_for_minio() { + async fn test_edit_rejects_half_static_credentials_for_minio() { let mut mgr = empty_mgr(); let tier = TierConfig { version: "v1".to_string(), @@ -7056,12 +7816,290 @@ mod tests { mgr.tiers.insert("COLD-M".to_string(), tier); let err = mgr - .edit("COLD-M", TierCreds::default()) + .edit( + "COLD-M", + TierCreds { + access_key: "rotated-access".to_string(), + ..Default::default() + }, + ) .await - .expect_err("empty credentials must be rejected"); + .expect_err("a half-filled static credential pair must be rejected"); assert_eq!(err.code, ERR_TIER_MISSING_CREDENTIALS.code); } + #[tokio::test] + async fn test_edit_with_omitted_secret_fields_preserves_real_credentials() { + let mut mgr = empty_mgr(); + mgr.tiers.insert("COLD-R".to_string(), build_rustfs_tier("COLD-R")); + let backend = RecordingWarmBackend::new(); + + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend.clone(), Arc::new(Mutex::new(Vec::new()))), + mgr.edit("COLD-R", TierCreds::default()), + ) + .await + .expect("an edit with omitted secret fields should validate the preserved credentials"); + + let rustfs = mgr.tiers["COLD-R"].rustfs.as_ref().expect("edited payload should remain"); + assert_eq!(rustfs.access_key, "ak"); + assert_eq!(rustfs.secret_key, "sk"); + let operations = backend.op_log().await; + assert_eq!(operations.len(), 5); + assert!(matches!(&operations[0], MockWarmOp::Put { .. })); + assert!(matches!(&operations[1], MockWarmOp::Probe { .. })); + assert!(matches!(&operations[2], MockWarmOp::Get { .. })); + assert!(matches!(&operations[3], MockWarmOp::Remove { .. })); + assert!(matches!(&operations[4], MockWarmOp::Probe { .. })); + } + + #[tokio::test] + async fn test_edit_rejects_redaction_placeholder() { + let mut mgr = empty_mgr(); + mgr.tiers.insert("COLD-R".to_string(), build_rustfs_tier("COLD-R")); + + let err = mgr + .edit( + "COLD-R", + TierCreds { + access_key: "rotated-access".to_string(), + secret_key: TIER_CREDENTIAL_REDACTED.to_string(), + ..Default::default() + }, + ) + .await + .expect_err("the API redaction placeholder must never become a real credential"); + + assert_eq!(err.code, ERR_TIER_MISSING_CREDENTIALS.code); + assert_eq!( + mgr.tiers["COLD-R"] + .rustfs + .as_ref() + .expect("original payload should remain") + .secret_key, + "sk" + ); + } + + #[tokio::test] + async fn test_edit_rejects_unsupported_s3_role_before_backend_setup() { + let mut mgr = empty_mgr(); + mgr.tiers.insert("COLD-A".to_string(), build_s3_tier("COLD-A")); + + let err = mgr + .edit( + "COLD-A", + TierCreds { + aws_role: true, + ..Default::default() + }, + ) + .await + .expect_err("unsupported role credentials must fail before backend setup"); + + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("not supported")); + } + + #[tokio::test] + async fn test_edit_gcs_rotates_from_creds_json_and_runs_backend_probe() { + const OLD_CREDS: &str = r#"{"type":"service_account","project_id":"old-project"}"#; + const ROTATED_CREDS: &str = r#"{"type":"service_account","project_id":"rotated-project"}"#; + let mut mgr = empty_mgr(); + mgr.tiers + .insert("COLD-GCS".to_string(), build_gcs_tier("COLD-GCS", OLD_CREDS)); + let backend = RecordingWarmBackend::new(); + let observed = Arc::new(Mutex::new(Vec::new())); + + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend.clone(), observed.clone()), + mgr.edit( + "COLD-GCS", + TierCreds { + creds_json: ROTATED_CREDS.as_bytes().to_vec(), + ..Default::default() + }, + ), + ) + .await + .expect("GCS service account rotation should validate through the fake backend"); + + assert_eq!( + mgr.tiers["COLD-GCS"].gcs.as_ref().expect("GCS payload should remain").creds, + ROTATED_CREDS + ); + assert_eq!( + lock_unpoisoned(&observed)[0] + .gcs + .as_ref() + .expect("observed GCS payload should exist") + .creds, + ROTATED_CREDS + ); + let operations = backend.op_log().await; + assert_eq!(operations.len(), 5); + assert!(matches!(&operations[0], MockWarmOp::Put { .. })); + assert!(matches!(&operations[1], MockWarmOp::Probe { .. })); + assert!(matches!(&operations[2], MockWarmOp::Get { .. })); + assert!(matches!(&operations[3], MockWarmOp::Remove { .. })); + assert!(matches!(&operations[4], MockWarmOp::Probe { .. })); + } + + #[tokio::test] + async fn test_gcs_madmin_add_wire_normalizes_url_safe_credentials_to_raw_v2_storage() { + const INITIAL_JSON: &str = r#"{"type":"service_account","project_id":"tier-௿"}"#; + const INITIAL_MINIO_URL_BASE64: &str = "eyJ0eXBlIjoic2VydmljZV9hY2NvdW50IiwicHJvamVjdF9pZCI6InRpZXIt4K-_In0="; + const ROTATED_JSON: &str = r#"{"type":"service_account","project_id":"rotated-project"}"#; + let madmin_wire_add: TierConfig = serde_json::from_value(serde_json::json!({ + "Version": "v1", + "Type": "gcs", + "Name": "COLD-GCS", + "GCS": { + "Endpoint": "https://storage.googleapis.com/", + "Creds": INITIAL_MINIO_URL_BASE64, + "Bucket": "bucket-gcs", + "Prefix": "prefix-gcs", + "Region": "", + "StorageClass": "" + } + })) + .expect("canonical madmin GCS AddTier payload should decode"); + assert_eq!(madmin_wire_add.name, "COLD-GCS"); + assert!( + madmin_wire_add + .gcs + .as_ref() + .expect("the decoded madmin payload should contain GCS configuration") + .name + .is_empty(), + "canonical madmin GCS has no nested Name field" + ); + assert_eq!( + madmin_wire_add + .gcs + .as_ref() + .expect("the decoded madmin payload should contain GCS configuration") + .creds, + INITIAL_MINIO_URL_BASE64 + ); + let gcs = madmin_wire_add + .gcs + .as_ref() + .expect("the decoded madmin payload should contain GCS configuration"); + assert_eq!(gcs.endpoint, "https://storage.googleapis.com/"); + assert_eq!(gcs.bucket, "bucket-gcs"); + assert_eq!(gcs.prefix, "prefix-gcs"); + assert!(gcs.region.is_empty()); + assert!(gcs.storage_class.is_empty()); + let rustfs_output = serde_json::to_value(&madmin_wire_add).expect("GCS config should serialize"); + assert_eq!(rustfs_output["type"], "gcs"); + assert!(rustfs_output.get("Type").is_none()); + assert!(rustfs_output.get("GCS").is_none()); + assert!(rustfs_output.get("Name").is_none()); + assert!(rustfs_output["gcs"].get("Creds").is_none()); + assert_eq!(rustfs_output["gcs"]["creds"], INITIAL_MINIO_URL_BASE64); + let mut mgr = empty_mgr(); + let backend = RecordingWarmBackend::new(); + let observed = Arc::new(Mutex::new(Vec::new())); + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend.clone(), observed.clone()), + mgr.add(madmin_wire_add, true), + ) + .await + .expect("GCS add should normalize madmin URL-safe credentials before validation"); + + assert_eq!( + lock_unpoisoned(&observed)[0] + .gcs + .as_ref() + .expect("the backend should receive a GCS payload") + .creds, + INITIAL_JSON + ); + assert_eq!( + mgr.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("the added GCS payload should exist") + .creds, + INITIAL_JSON + ); + assert_eq!(mgr.tiers["COLD-GCS"].name, "COLD-GCS"); + assert_eq!( + mgr.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("the added GCS payload should exist") + .name, + "COLD-GCS" + ); + + let blob = encode_external_tiering_config_blob(&mgr).expect("GCS add should persist externally"); + let external: ExternalTierConfigMgr = rmp_serde::from_slice(&blob[4..]).expect("external GCS payload should decode"); + assert_eq!( + external.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("external GCS payload should exist") + .creds, + INITIAL_JSON + ); + let mut loaded = decode_external_tiering_config_blob(&blob).expect("persisted GCS tier should load"); + assert_eq!( + loaded.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("loaded GCS payload should exist") + .creds, + INITIAL_JSON + ); + + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend, Arc::new(Mutex::new(Vec::new()))), + loaded.edit( + "COLD-GCS", + TierCreds { + creds_json: ROTATED_JSON.as_bytes().to_vec(), + ..Default::default() + }, + ), + ) + .await + .expect("GCS edit should accept raw madmin credential bytes"); + assert_eq!( + loaded.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("edited GCS payload should exist") + .creds, + ROTATED_JSON + ); + let rotated_blob = encode_external_tiering_config_blob(&loaded).expect("edited GCS tier should persist externally"); + assert_eq!( + rmp_serde::from_slice::(&rotated_blob[4..]) + .expect("rotated external tier should exist") + .tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("rotated external GCS payload should exist") + .creds, + ROTATED_JSON + ); + let reloaded = decode_external_tiering_config_blob(&rotated_blob).expect("edited GCS tier should reload"); + assert_eq!( + reloaded.tiers["COLD-GCS"] + .gcs + .as_ref() + .expect("reloaded GCS payload should exist") + .creds, + ROTATED_JSON + ); + } + // ---- remove --------------------------------------------------------- #[tokio::test] @@ -7081,6 +8119,7 @@ mod tests { MockWarmBackend { in_use_value: Some(true), healthy: true, + probe_present: AtomicBool::new(false), }, ); @@ -7103,6 +8142,7 @@ mod tests { MockWarmBackend { in_use_value: Some(false), healthy: true, + probe_present: AtomicBool::new(false), }, ); @@ -7122,6 +8162,7 @@ mod tests { MockWarmBackend { in_use_value: None, healthy: true, + probe_present: AtomicBool::new(false), }, ); @@ -7140,11 +8181,12 @@ mod tests { MockWarmBackend { in_use_value: None, healthy: true, + probe_present: AtomicBool::new(false), }, ); let mutation = TierCandidateMutation::Remove("COLD-A".to_string(), true); mutation - .apply(&mut candidate) + .apply(&mut candidate, None) .await .expect("force remove must skip in-use probing"); assert!(!candidate.tiers.contains_key("COLD-A")); @@ -7160,10 +8202,11 @@ mod tests { MockWarmBackend { in_use_value: Some(true), healthy: true, + probe_present: AtomicBool::new(false), }, ); let err = TierCandidateMutation::Remove("COLD-A".to_string(), false) - .apply(&mut candidate) + .apply(&mut candidate, None) .await .expect_err("non-force remove must reject an in-use backend"); assert_eq!(err.code, ERR_TIER_BACKEND_NOT_EMPTY.code); @@ -7180,6 +8223,7 @@ mod tests { MockWarmBackend { in_use_value: None, healthy: true, + probe_present: AtomicBool::new(false), }, ); forced.clear_tier(true).await.expect("force clear must skip in-use probing"); @@ -7193,6 +8237,7 @@ mod tests { MockWarmBackend { in_use_value: Some(true), healthy: true, + probe_present: AtomicBool::new(false), }, ); let err = guarded @@ -7213,6 +8258,7 @@ mod tests { MockWarmBackend { in_use_value: None, healthy: true, + probe_present: AtomicBool::new(false), }, ); @@ -7243,6 +8289,7 @@ mod tests { MockWarmBackend { in_use_value: Some(false), healthy: true, + probe_present: AtomicBool::new(false), }, ); @@ -7259,6 +8306,7 @@ mod tests { MockWarmBackend { in_use_value: Some(false), healthy: false, + probe_present: AtomicBool::new(false), }, ); @@ -7272,6 +8320,7 @@ mod tests { let unhealthy: SharedWarmBackend = Arc::new(MockWarmBackend { in_use_value: Some(false), healthy: false, + probe_present: AtomicBool::new(false), }); let proxy = SharedWarmBackendProxy(unhealthy); let err = proxy.validate().await.expect_err("proxy must forward backend validation"); @@ -7280,6 +8329,7 @@ mod tests { let healthy: SharedWarmBackend = Arc::new(MockWarmBackend { in_use_value: Some(false), healthy: true, + probe_present: AtomicBool::new(false), }); let proxy = SharedWarmBackendProxy(healthy); let err = proxy @@ -7329,7 +8379,7 @@ mod tests { assert_eq!(tier.tier_type.as_lowercase(), "s3"); let s3 = tier.s3.as_ref().expect("s3 payload survives"); assert_eq!(s3.bucket, "bucket-a"); - // secret_key is a serialized field (unlike Clone, marshal does not redact). + // Internal serialization preserves credentials; only explicit admin views redact them. assert_eq!(s3.secret_key, "sk"); } @@ -7374,7 +8424,8 @@ mod tests { } #[test] - fn test_external_blob_roundtrip_gcs_preserves_creds() { + fn test_external_blob_gcs_writer_stays_raw_for_v2_old_readers() { + const RAW_JSON: &str = r#"{"type":"service_account","project_id":"tier-௿"}"#; let mut mgr = empty_mgr(); mgr.tiers.insert( "COLD-G".to_string(), @@ -7385,7 +8436,7 @@ mod tests { gcs: Some(crate::services::tier::tier_config::TierGCS { name: "COLD-G".to_string(), endpoint: "https://storage.googleapis.com".to_string(), - creds: "service-account-json".to_string(), + creds: RAW_JSON.to_string(), bucket: "gbucket".to_string(), prefix: "gp".to_string(), region: "us".to_string(), @@ -7396,10 +8447,118 @@ mod tests { ); let bytes = encode_external_tiering_config_blob(&mgr).expect("encode gcs tier"); + assert_eq!(&bytes[0..2], &TIER_CONFIG_FORMAT.to_le_bytes()); + assert_eq!(&bytes[2..4], &TIER_CONFIG_VERSION.to_le_bytes()); + let external: ExternalTierConfigMgr = rmp_serde::from_slice(&bytes[4..]).expect("external GCS payload should decode"); + assert_eq!( + external.tiers["COLD-G"] + .gcs + .as_ref() + .expect("external GCS payload should exist") + .creds, + RAW_JSON + ); + serde_json::from_str::( + &external.tiers["COLD-G"] + .gcs + .as_ref() + .expect("external GCS payload should exist") + .creds, + ) + .expect("the baseline RustFS v2 reader must receive raw GCS credential JSON"); let decoded = decode_external_tiering_config_blob(&bytes).expect("decode gcs tier"); let tier = decoded.tiers.get("COLD-G").expect("gcs tier survives roundtrip"); assert_eq!(tier.tier_type.as_lowercase(), "gcs"); - assert_eq!(tier.gcs.as_ref().expect("gcs payload survives").creds, "service-account-json"); + assert_eq!(tier.gcs.as_ref().expect("gcs payload survives").creds, RAW_JSON); + + let rewritten = encode_external_tiering_config_blob(&decoded).expect("decoded GCS tier should remain v2-compatible"); + let rewritten: ExternalTierConfigMgr = + rmp_serde::from_slice(&rewritten[4..]).expect("rewritten external GCS payload should decode"); + assert_eq!( + rewritten.tiers["COLD-G"] + .gcs + .as_ref() + .expect("rewritten external GCS payload should exist") + .creds, + RAW_JSON + ); + } + + fn decode_hex_fixture(hex: &str) -> Vec { + assert_eq!(hex.len() % 2, 0, "hex fixture must contain complete bytes"); + hex.as_bytes() + .chunks_exact(2) + .map(|pair| { + let pair = std::str::from_utf8(pair).expect("hex fixture should be ASCII"); + u8::from_str_radix(pair, 16).expect("hex fixture should contain only hexadecimal digits") + }) + .collect() + } + + #[test] + fn test_external_blob_decodes_fixed_minio_gcs_fixture_and_rewrites_raw_v2_credentials() { + const RAW_JSON: &str = r#"{"type":"service_account","project_id":"tier-௿"}"#; + // Produced by MinIO RELEASE.2025-10-15T17-29-55Z with madmin-go/v3 + // v3.0.109. Keeping the bytes fixed prevents this compatibility test + // from accidentally validating RustFS against its own serializer. + const MINIO_GCS_BLOB_HEX: &str = concat!( + "0100020081a5546965727381a6434f4c442d4787a756657273696f6ea27631a45479706503a44e616d65a6434f4c442d47", + "a25333c0a5417a757265c0a347435386a8456e64706f696e74bf68747470733a2f2f73746f726167652e676f6f676c656170", + "69732e636f6d2fa54372656473d94465794a306558426c496a6f6963325679646d6c6a5a56396859324e7664573530496977", + "6963484a76616d566a644639705a434936496e52705a584974344b2d5f496e303da64275636b6574a7676275636b6574a650", + "7265666978a26770a6526567696f6ea27573ac53746f72616765436c617373a84e4541524c494e45a54d696e494fc0" + ); + const MINIO_GCS_BLOB_SHA256_HEX: &str = "bee1d4822d4936bc6f764c284381596d16f2096f40671b25500aca3b710ceac8"; + let fixture = decode_hex_fixture(MINIO_GCS_BLOB_HEX); + assert_eq!(fixture.len(), 246); + assert_eq!( + Sha256::digest(&fixture).as_slice(), + decode_hex_fixture(MINIO_GCS_BLOB_SHA256_HEX).as_slice(), + "fixed foreign fixture digest changed" + ); + + let decoded = decode_external_tiering_config_blob(&fixture) + .expect("the new reader should accept MinIO URL-safe-base64 GCS credentials"); + let gcs = decoded.tiers["COLD-G"] + .gcs + .as_ref() + .expect("decoded MinIO GCS payload should exist"); + assert_eq!(gcs.endpoint, "https://storage.googleapis.com/"); + assert_eq!(gcs.bucket, "gbucket"); + assert_eq!(gcs.prefix, "gp"); + assert_eq!(gcs.region, "us"); + assert_eq!(gcs.storage_class, "NEARLINE"); + assert_eq!(gcs.creds, RAW_JSON); + + let rewritten = encode_external_tiering_config_blob(&decoded) + .expect("MinIO credentials should rewrite in the existing RustFS v2 raw format"); + let rewritten: ExternalTierConfigMgr = + rmp_serde::from_slice(&rewritten[4..]).expect("rewritten MinIO fixture should decode"); + assert_eq!( + rewritten.tiers["COLD-G"] + .gcs + .as_ref() + .expect("rewritten MinIO GCS payload should exist") + .creds, + RAW_JSON + ); + } + + #[test] + fn external_gcs_credentials_accept_all_base64_alphabets_and_padding_modes() { + let raw = r#"{"type":"service_account","project_id":"tier-🚀"}"#; + for encoder in [ + base64_simd::STANDARD, + base64_simd::STANDARD_NO_PAD, + base64_simd::URL_SAFE, + base64_simd::URL_SAFE_NO_PAD, + ] { + let encoded = encoder.encode_to_string(raw.as_bytes()); + assert_eq!( + decode_external_gcs_credentials(&encoded).expect("supported GCS encoding should decode"), + raw + ); + } } // `TierConfigMgr` intentionally does not derive `Debug` (it holds live @@ -7546,6 +8705,7 @@ mod tests { struct LeaseTestBackend { id: &'static str, calls: Arc>>, + probe_present: Arc, remove_started: Option>, remove_release: Option>, backend_in_use: bool, @@ -7558,6 +8718,7 @@ mod tests { Self { id, calls: Arc::new(std::sync::Mutex::new(Vec::new())), + probe_present: Arc::new(AtomicBool::new(false)), remove_started: None, remove_release: None, backend_in_use: false, @@ -7603,6 +8764,7 @@ mod tests { #[async_trait::async_trait] impl WarmBackend for LeaseTestBackend { async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> io::Result { + self.probe_present.store(true, Ordering::SeqCst); Ok(self.id.to_string()) } @@ -7617,7 +8779,7 @@ mod tests { } async fn get(&self, _object: &str, _rv: &str, _opts: WarmBackendGetOpts) -> io::Result { - Ok(BufReader::new(Cursor::new(Vec::new()))) + Ok(BufReader::new(Cursor::new(b"RustFS".to_vec()))) } async fn remove(&self, _object: &str, _rv: &str) -> io::Result<()> { @@ -7635,9 +8797,18 @@ mod tests { .expect("lease test release semaphore should stay open") .forget(); } + self.probe_present.store(false, Ordering::SeqCst); Ok(()) } + async fn probe_transition_candidate(&self, _object: &str) -> io::Result { + if self.probe_present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent(self.id.to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + async fn in_use(&self) -> io::Result { if self.panic_in_use { panic!("simulated tier backend in-use panic"); @@ -7685,6 +8856,7 @@ mod tests { struct FakeTierMutationPeer { label: &'static str, calls: Arc>>, + captured_prepares: Option>>>, prepare: std::result::Result, prepare_definitely_rejected: bool, commit: std::result::Result, @@ -7709,6 +8881,7 @@ mod tests { Arc::new(Self { label, calls, + captured_prepares: None, prepare, prepare_definitely_rejected: false, commit, @@ -7716,6 +8889,22 @@ mod tests { }) } + fn boxed_capturing_prepare( + label: &'static str, + calls: Arc>>, + captured_prepares: Arc>>, + ) -> Arc { + Arc::new(Self { + label, + calls, + captured_prepares: Some(captured_prepares), + prepare: Ok(PeerTierMutationState::Prepared), + prepare_definitely_rejected: false, + commit: Ok(PeerTierMutationState::Committed), + abort: Ok(PeerTierMutationState::Aborted), + }) + } + fn record(&self, call: String) { lock_unpoisoned(&self.calls).push(call); } @@ -7732,6 +8921,10 @@ mod tests { mutation_id: uuid::Uuid, canonical_payload: Bytes, ) -> Result { + if let Some(captured_prepares) = self.captured_prepares.as_ref() { + let intent = TierMutationIntent::decode(mutation_id, &canonical_payload).map_err(Error::other)?; + lock_unpoisoned(captured_prepares).push(intent); + } self.record(format!("{}:prepare:{}:{}", self.label, mutation_id, canonical_payload.len())); match self.prepare { Ok(state) => Ok(state), @@ -8379,6 +9572,7 @@ mod tests { let old_peer = Arc::new(FakeTierMutationPeer { label: "peer-v3", calls: calls.clone(), + captured_prepares: None, prepare: Err("unsupported tier mutation peer protocol version: 4"), prepare_definitely_rejected: true, commit: Ok(PeerTierMutationState::Committed), @@ -8449,6 +9643,7 @@ mod tests { Arc::new(FakeTierMutationPeer { label: "peer-a", calls: calls.clone(), + captured_prepares: None, prepare: Ok(PeerTierMutationState::Prepared), prepare_definitely_rejected: false, commit: Ok(PeerTierMutationState::Committed), @@ -8961,41 +10156,44 @@ mod tests { let update = TierConfigMgr::admin_update_lock(&manager).await; let calls = Arc::new(Mutex::new(Vec::new())); - TIER_MUTATION_TEST_PEERS + TIER_DRIVER_TEST_FACTORY .scope( - vec![ - FakeTierMutationPeer::boxed_with_prepare_commit( - "peer-a", - calls.clone(), - Ok(PeerTierMutationState::Committed), - Ok(PeerTierMutationState::Committed), - ), - FakeTierMutationPeer::boxed_with_prepare_commit( - "peer-b", - calls.clone(), - Ok(PeerTierMutationState::Prepared), - Ok(PeerTierMutationState::Committed), - ), - FakeTierMutationPeer::boxed_with_prepare_commit( - "peer-c", - calls.clone(), - Ok(PeerTierMutationState::Prepared), - Ok(PeerTierMutationState::Committed), - ), - ], - async { - TierConfigMgr::update_candidate_owned( - &manager, - store.clone(), - candidate, - version, - TierCandidateMutation::Add(build_rustfs_tier("COLD-A"), true), - update, - None, - ) - .await - .expect("four-hot AddTier reporter replay should publish after prepared peers commit"); - }, + healthy_driver_factory(), + TIER_MUTATION_TEST_PEERS.scope( + vec![ + FakeTierMutationPeer::boxed_with_prepare_commit( + "peer-a", + calls.clone(), + Ok(PeerTierMutationState::Committed), + Ok(PeerTierMutationState::Committed), + ), + FakeTierMutationPeer::boxed_with_prepare_commit( + "peer-b", + calls.clone(), + Ok(PeerTierMutationState::Prepared), + Ok(PeerTierMutationState::Committed), + ), + FakeTierMutationPeer::boxed_with_prepare_commit( + "peer-c", + calls.clone(), + Ok(PeerTierMutationState::Prepared), + Ok(PeerTierMutationState::Committed), + ), + ], + async { + TierConfigMgr::update_candidate_owned( + &manager, + store.clone(), + candidate, + version, + TierCandidateMutation::Add(build_rustfs_tier("COLD-A"), true), + update, + None, + ) + .await + .expect("four-hot AddTier reporter replay should publish after prepared peers commit"); + }, + ), ) .await; @@ -10655,6 +11853,7 @@ mod tests { vec![Arc::new(FakeTierMutationPeer { label: "peer-a", calls: calls.clone(), + captured_prepares: None, prepare: Ok(PeerTierMutationState::Prepared), prepare_definitely_rejected: false, commit: Ok(PeerTierMutationState::Committed), @@ -11487,6 +12686,57 @@ mod tests { verify.await.expect("verify task should join").expect("verify should finish"); } + #[tokio::test] + async fn cancelled_admin_verify_finishes_probe_cleanup_and_releases_its_lease() { + let manager = TierConfigMgr::new(); + let backend = RecordingWarmBackend::new(); + let get_barrier = backend.arm_get_barrier().await; + { + let mut guard = manager.write().await; + guard.tiers.insert("COLD-A".to_string(), build_rustfs_tier("COLD-A")); + guard + .replace_driver("COLD-A", Box::new(backend.clone())) + .expect("verification backend generation should install"); + } + + let verify_manager = manager.clone(); + let verify = tokio::spawn(async move { TierConfigMgr::verify_without_manager_lock(&verify_manager, "COLD-A").await }); + get_barrier.wait_until_paused().await; + verify.abort(); + assert!( + verify + .await + .expect_err("the outer verify caller should be cancelled") + .is_cancelled() + ); + assert_eq!(TierConfigMgr::active_operation_lease_count(&manager, "COLD-A").await, 1); + + get_barrier.release(); + tokio::time::timeout(Duration::from_secs(1), async { + loop { + if backend.object_count().await == 0 && TierConfigMgr::active_operation_lease_count(&manager, "COLD-A").await == 0 + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("the detached verification must finish cleanup and release its lease"); + + let operations = backend.op_log().await; + assert!(matches!( + operations.as_slice(), + [ + MockWarmOp::Put { .. }, + MockWarmOp::Probe { .. }, + MockWarmOp::Get { .. }, + MockWarmOp::Remove { .. }, + MockWarmOp::Probe { .. } + ] + )); + } + #[tokio::test] async fn same_name_replacement_drains_old_generation_and_routes_new_work_to_new_driver() { let old = LeaseTestBackend::ready("old"); @@ -13021,6 +14271,480 @@ mod tests { } } + #[tokio::test] + async fn edit_and_save_with_omitted_credentials_preserves_persisted_and_runtime_secrets() { + const ACCESS_KEY: &str = "persisted-access-value"; + const SECRET_KEY: &str = "persisted-secret-value"; + + let store = Arc::new(CasConfigStore::default()); + let mut tier = build_rustfs_tier("COLD-R"); + let rustfs = tier.rustfs.as_mut().expect("RustFS payload should exist"); + rustfs.access_key = ACCESS_KEY.to_string(); + rustfs.secret_key = SECRET_KEY.to_string(); + let mut persisted = empty_mgr(); + persisted.tiers.insert("COLD-R".to_string(), tier); + persisted + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("credential-preservation fixture should persist"); + + let manager = TierConfigMgr::new(); + let backend = RecordingWarmBackend::new(); + let observed = Arc::new(Mutex::new(Vec::new())); + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend.clone(), observed.clone()), + TIER_MUTATION_TEST_PEERS.scope( + Vec::new(), + TierConfigMgr::edit_and_save_with(&manager, store.clone(), "COLD-R", TierCreds::default()), + ), + ) + .await + .expect("omitted credentials should preserve and publish the current credentials"); + + let reloaded = load_tier_config_for_update(store.clone()) + .await + .expect("edited tier config should reload") + .0; + let persisted_rustfs = reloaded.tiers["COLD-R"] + .rustfs + .as_ref() + .expect("persisted RustFS payload should exist"); + assert_eq!(persisted_rustfs.access_key, ACCESS_KEY); + assert_eq!(persisted_rustfs.secret_key, SECRET_KEY); + + let config_object = store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("binary tier config should remain persisted") + .0 + .clone(); + assert!( + !config_object + .windows(TIER_CREDENTIAL_REDACTED.len()) + .any(|window| window == TIER_CREDENTIAL_REDACTED.as_bytes()), + "the API placeholder must never be persisted as a credential" + ); + + let (runtime_access_key, runtime_secret_key) = { + let manager = manager.read().await; + let runtime = registered_tier_driver_runtime(&manager).expect("driver runtime should be registered"); + let runtime = lock_unpoisoned(&runtime); + let generation = runtime + .generations + .get("COLD-R") + .expect("edited generation should be installed"); + let rustfs = generation + .tier_config + .rustfs + .as_ref() + .expect("runtime RustFS payload should exist"); + (rustfs.access_key.clone(), rustfs.secret_key.clone()) + }; + assert_eq!(runtime_access_key, ACCESS_KEY); + assert_eq!(runtime_secret_key, SECRET_KEY); + + let api_view = manager + .read() + .await + .get("COLD-R") + .expect("edited tier should remain visible through the admin view"); + assert_eq!( + api_view.rustfs.expect("admin RustFS payload should exist").secret_key, + TIER_CREDENTIAL_REDACTED + ); + let observed = lock_unpoisoned(&observed); + assert_eq!(observed.len(), 1); + assert_eq!( + observed[0] + .rustfs + .as_ref() + .expect("backend factory should observe the RustFS payload") + .secret_key, + SECRET_KEY + ); + drop(observed); + + let operations = backend.op_log().await; + assert_eq!(operations.len(), 5); + assert!(matches!(&operations[0], MockWarmOp::Put { .. })); + assert!(matches!(&operations[1], MockWarmOp::Probe { .. })); + assert!(matches!(&operations[2], MockWarmOp::Get { .. })); + assert!(matches!(&operations[3], MockWarmOp::Remove { .. })); + assert!(matches!(&operations[4], MockWarmOp::Probe { .. })); + } + + #[tokio::test] + async fn edit_and_save_rotates_credentials_while_an_active_lifecycle_rule_references_the_tier() { + let store = Arc::new(CasConfigStore::default()); + let mut persisted = empty_mgr(); + persisted.tiers.insert("COLD-A".to_string(), build_rustfs_tier("COLD-A")); + persisted + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("credential rotation fixture should persist"); + store.add_listed_version(ObjectInfo { + bucket: "photos".to_string(), + name: "safe.txt".to_string(), + ..Default::default() + }); + store.add_lifecycle_config( + "photos", + BucketLifecycleConfiguration { + expiry_updated_at: None, + rules: vec![LifecycleRule { + status: ExpirationStatus::from_static(ExpirationStatus::ENABLED), + expiration: None, + abort_incomplete_multipart_upload: None, + del_marker_expiration: None, + filter: None, + id: Some("move-current".to_string()), + noncurrent_version_expiration: None, + noncurrent_version_transitions: None, + prefix: None, + transitions: Some(vec![Transition { + days: Some(1), + date: None, + storage_class: Some(TransitionStorageClass::from_static("COLD-A")), + }]), + }], + }, + ); + + let manager = TierConfigMgr::new(); + let backend = RecordingWarmBackend::new(); + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend, Arc::new(Mutex::new(Vec::new()))), + TIER_MUTATION_TEST_PEERS.scope( + Vec::new(), + TierConfigMgr::edit_and_save_with( + &manager, + store.clone(), + "COLD-A", + TierCreds { + access_key: "rotated-access".to_string(), + secret_key: "rotated-secret".to_string(), + ..Default::default() + }, + ), + ), + ) + .await + .expect("same-destination credential rotation must not be treated as a lifecycle rebind"); + + let reloaded = load_tier_config_for_update(store) + .await + .expect("rotated tier config should reload") + .0; + let rustfs = reloaded.tiers["COLD-A"] + .rustfs + .as_ref() + .expect("RustFS payload should remain"); + assert_eq!(rustfs.access_key, "rotated-access"); + assert_eq!(rustfs.secret_key, "rotated-secret"); + } + + async fn assert_edit_rejected_before_distributed_commit( + backend: RecordingWarmBackend, + credentials: TierCreds, + expected_code: &str, + expected_backend_builds: usize, + ) { + let store = Arc::new(CasConfigStore::default()); + let mut persisted = empty_mgr(); + persisted.tiers.insert("COLD-A".to_string(), build_rustfs_tier("COLD-A")); + persisted + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("rejected-edit fixture should persist"); + let before = store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should be present") + .clone(); + + let manager = TierConfigMgr::new(); + { + let mut manager = manager.write().await; + install_lease_backend(&mut manager, "COLD-A", LeaseTestBackend::ready("old")); + } + let old_generation = TierConfigMgr::acquire_operation_lease(&manager, "COLD-A") + .await + .expect("old generation should be available") + .generation(); + let peer_calls = Arc::new(Mutex::new(Vec::new())); + let observed = Arc::new(Mutex::new(Vec::new())); + let err = TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend, observed.clone()), + TIER_MUTATION_TEST_PEERS.scope( + vec![FakeTierMutationPeer::boxed( + "peer-a", + peer_calls.clone(), + Ok(PeerTierMutationState::Committed), + )], + TierConfigMgr::edit_and_save_with(&manager, store.clone(), "COLD-A", credentials), + ), + ) + .await + .expect_err("credential validation must fail before distributed prepare"); + + let TierConfigUpdateError::Mutation(err) = err else { + panic!("credential validation should be reported as a mutation error: {err:?}"); + }; + assert_eq!(err.code, expected_code); + assert!(lock_unpoisoned(&peer_calls).is_empty()); + assert_eq!(lock_unpoisoned(&observed).len(), expected_backend_builds); + assert_eq!( + store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should remain present"), + &before + ); + let current = TierConfigMgr::acquire_operation_lease(&manager, "COLD-A") + .await + .expect("the old generation should be restored after validation failure"); + assert_eq!(current.generation(), old_generation); + assert_eq!( + current + .inner + .tier_config + .rustfs + .as_ref() + .expect("runtime RustFS payload should remain") + .secret_key, + "sk" + ); + } + + #[tokio::test] + async fn edit_and_save_rejects_half_credentials_before_peer_prepare_or_config_cas() { + assert_edit_rejected_before_distributed_commit( + RecordingWarmBackend::new(), + TierCreds { + access_key: "rotated-access".to_string(), + ..Default::default() + }, + &ERR_TIER_MISSING_CREDENTIALS.code, + 0, + ) + .await; + } + + #[tokio::test] + async fn edit_and_save_rejects_canonical_azure_service_principal_before_peer_prepare() { + let credentials: TierCreds = serde_json::from_value(serde_json::json!({ + "azSP": { + "TenantID": "tenant", + "ClientID": "client", + "ClientSecret": "service-principal-secret" + } + })) + .expect("canonical madmin credentials should decode"); + assert_edit_rejected_before_distributed_commit( + RecordingWarmBackend::new(), + credentials, + &ERR_TIER_INVALID_CONFIG.code, + 0, + ) + .await; + } + + #[tokio::test] + async fn edit_and_save_rejects_legacy_azure_options_before_probe_or_peer_prepare() { + let store = Arc::new(CasConfigStore::default()); + let mut legacy = build_azure_tier("account-a"); + legacy.azure.as_mut().expect("Azure payload should exist").storage_class = "HOT".to_string(); + let mut persisted = empty_mgr(); + persisted + .tiers + .insert("COLD-AZURE".to_string(), legacy.clone_with_credentials()); + persisted + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("legacy Azure fixture should persist"); + let before = store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should be present") + .clone(); + + let manager = TierConfigMgr::new(); + { + let mut manager = manager.write().await; + manager.tiers.insert("COLD-AZURE".to_string(), legacy); + manager + .replace_driver("COLD-AZURE", Box::new(LeaseTestBackend::ready("old"))) + .expect("legacy generation should install"); + } + let old_generation = TierConfigMgr::acquire_operation_lease(&manager, "COLD-AZURE") + .await + .expect("legacy generation should be available") + .generation(); + let peer_calls = Arc::new(Mutex::new(Vec::new())); + let observed = Arc::new(Mutex::new(Vec::new())); + + let err = TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(RecordingWarmBackend::new(), observed.clone()), + TIER_MUTATION_TEST_PEERS.scope( + vec![FakeTierMutationPeer::boxed( + "peer-a", + peer_calls.clone(), + Ok(PeerTierMutationState::Committed), + )], + TierConfigMgr::edit_and_save_with(&manager, store.clone(), "COLD-AZURE", TierCreds::default()), + ), + ) + .await + .expect_err("legacy unsupported Azure options must fail closed on edit"); + + let TierConfigUpdateError::Mutation(err) = err else { + panic!("legacy Azure validation should be a mutation error: {err:?}"); + }; + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert!(err.message.contains("storageClass"), "{}", err.message); + assert!(lock_unpoisoned(&observed).is_empty(), "validation must precede backend preparation"); + assert!(lock_unpoisoned(&peer_calls).is_empty(), "validation must precede peer prepare"); + assert_eq!( + store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should remain present"), + &before + ); + let current = TierConfigMgr::acquire_operation_lease(&manager, "COLD-AZURE") + .await + .expect("legacy generation should remain available"); + assert_eq!(current.generation(), old_generation); + assert_eq!( + current + .inner + .tier_config + .azure + .as_ref() + .expect("runtime Azure payload should remain") + .storage_class, + "HOT" + ); + } + + #[tokio::test] + async fn edit_and_save_probe_failure_keeps_old_generation_and_skips_peer_prepare() { + let backend = RecordingWarmBackend::new(); + backend.set_reject_credentials(true).await; + assert_edit_rejected_before_distributed_commit( + backend, + TierCreds { + access_key: "rotated-access".to_string(), + secret_key: "rotated-secret".to_string(), + ..Default::default() + }, + &ERR_TIER_PERM_ERR.code, + 1, + ) + .await; + } + + #[tokio::test(start_paused = true)] + async fn edit_and_save_get_timeout_cleans_probe_and_keeps_config_and_generation() { + let store = Arc::new(CasConfigStore::default()); + let mut persisted = empty_mgr(); + persisted.tiers.insert("COLD-A".to_string(), build_rustfs_tier("COLD-A")); + persisted + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("timeout fixture should persist"); + let before = store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should be present") + .clone(); + + let manager = TierConfigMgr::new(); + { + let mut manager = manager.write().await; + install_lease_backend(&mut manager, "COLD-A", LeaseTestBackend::ready("old")); + } + let old_generation = TierConfigMgr::acquire_operation_lease(&manager, "COLD-A") + .await + .expect("old generation should be available") + .generation(); + let backend = RecordingWarmBackend::new(); + let get_barrier = backend.arm_get_barrier().await; + let peer_calls = Arc::new(Mutex::new(Vec::new())); + let edit = TIER_DRIVER_TEST_FACTORY.scope( + recording_driver_factory(backend.clone(), Arc::new(Mutex::new(Vec::new()))), + TIER_MUTATION_TEST_PEERS.scope( + vec![FakeTierMutationPeer::boxed( + "peer-a", + peer_calls.clone(), + Ok(PeerTierMutationState::Committed), + )], + TierConfigMgr::edit_and_save_with( + &manager, + store.clone(), + "COLD-A", + TierCreds { + access_key: "rotated-access".to_string(), + secret_key: "rotated-secret".to_string(), + ..Default::default() + }, + ), + ), + ); + tokio::pin!(edit); + tokio::select! { + _ = get_barrier.wait_until_paused() => {} + result = &mut edit => panic!("edit completed before the probe GET timeout: {result:?}"), + } + tokio::time::advance(WARM_BACKEND_PROBE_TIMEOUT + Duration::from_millis(1)).await; + let err = edit.await.expect_err("a hanging probe GET must time out"); + let TierConfigUpdateError::Mutation(err) = err else { + panic!("probe timeout should be reported as a mutation error: {err:?}"); + }; + assert_eq!(err.code, ERR_TIER_BACKEND_IN_USE.code); + assert!(lock_unpoisoned(&peer_calls).is_empty()); + assert_eq!( + store + .objects + .lock() + .await + .get(&tier_config_path(TIER_CONFIG_FILE)) + .expect("base config should remain present"), + &before + ); + assert_eq!( + TierConfigMgr::acquire_operation_lease(&manager, "COLD-A") + .await + .expect("old generation should remain available") + .generation(), + old_generation + ); + let operations = backend.op_log().await; + assert!(matches!(operations.first(), Some(MockWarmOp::Put { .. }))); + assert!( + operations + .iter() + .any(|operation| matches!(operation, MockWarmOp::Remove { .. })) + ); + assert!(matches!(operations.last(), Some(MockWarmOp::Probe { .. }))); + } + #[tokio::test] async fn remove_and_clear_full_update_paths_preserve_force() { let remove_store = Arc::new(CasConfigStore::default()); @@ -13690,13 +15414,17 @@ mod tests { .await .expect("add fixture should persist"); - TierConfigMgr::update_candidate_with_config_lock( - &manager, - store.clone(), - TierCandidateMutation::Add(build_rustfs_tier("COLD-B"), true), - ) - .await - .expect("add proof must ignore unchanged durable tiers missing from the stale manager"); + TIER_DRIVER_TEST_FACTORY + .scope( + healthy_driver_factory(), + TierConfigMgr::update_candidate_with_config_lock( + &manager, + store.clone(), + TierCandidateMutation::Add(build_rustfs_tier("COLD-B"), true), + ), + ) + .await + .expect("add proof must ignore unchanged durable tiers missing from the stale manager"); let reloaded = load_tier_config_for_update(store) .await @@ -13706,6 +15434,84 @@ mod tests { assert!(reloaded.tiers.contains_key("COLD-B")); } + #[tokio::test] + async fn nested_only_legacy_add_builds_nonempty_coordinator_target_and_fans_out() { + let manager = TierConfigMgr::new(); + let store = Arc::new(CasConfigStore::default()); + empty_mgr() + .save_tiering_config_if_current(store.clone(), None) + .await + .expect("empty tier config fixture should persist"); + let legacy_wire: TierConfig = serde_json::from_value(serde_json::json!({ + "type": "s3", + "s3": { + "name": "COLD-LEGACY", + "endpoint": "https://s3.example.invalid", + "accessKey": "access", + "secretKey": "secret", + "bucket": "archive", + "prefix": "objects", + "region": "us-east-1", + "storageClass": "STANDARD" + } + })) + .expect("legacy RustFS nested-name AddTier payload should decode"); + assert!(legacy_wire.name.is_empty()); + let mutation = TierCandidateMutation::add(legacy_wire, true) + .expect("legacy nested name must normalize before constructing the Add mutation"); + assert_eq!(mutation.explicit_tier_name(), Some("COLD-LEGACY")); + + let peer_calls = Arc::new(Mutex::new(Vec::new())); + let prepared_intents = Arc::new(Mutex::new(Vec::new())); + let backend = RecordingWarmBackend::new(); + TIER_DRIVER_TEST_FACTORY + .scope( + recording_driver_factory(backend, Arc::new(Mutex::new(Vec::new()))), + TIER_MUTATION_TEST_PEERS.scope( + vec![FakeTierMutationPeer::boxed_capturing_prepare( + "peer-a", + peer_calls.clone(), + prepared_intents.clone(), + )], + TierConfigMgr::update_candidate_with_config_lock(&manager, store.clone(), mutation), + ), + ) + .await + .expect("legacy nested-name Add must run the full coordinator fanout"); + + let prepared_intents = lock_unpoisoned(&prepared_intents); + assert_eq!(prepared_intents.len(), 1); + assert_eq!(prepared_intents[0].kind, TierMutationIntentKind::Add); + assert_eq!(prepared_intents[0].affected_targets.len(), 1); + assert_eq!(prepared_intents[0].affected_targets[0].tier_name, "COLD-LEGACY"); + assert!(prepared_intents[0].affected_targets[0].old_backend_identity.is_none()); + assert!(prepared_intents[0].affected_targets[0].new_backend_identity.is_some()); + drop(prepared_intents); + + let peer_calls = lock_unpoisoned(&peer_calls).clone(); + let prepare_index = peer_calls + .iter() + .position(|call| call.starts_with("peer-a:prepare:")) + .expect("legacy Add should prepare the peer"); + let commit_index = peer_calls + .iter() + .position(|call| call.starts_with("peer-a:commit:")) + .expect("legacy Add should commit the peer"); + assert!(prepare_index < commit_index, "{peer_calls:?}"); + + let reloaded = load_tier_config_for_update(store) + .await + .expect("legacy Add result should reload") + .0; + let persisted = reloaded + .tiers + .get("COLD-LEGACY") + .expect("normalized tier name should be persisted"); + assert_eq!(persisted.name, "COLD-LEGACY"); + assert_eq!(persisted.s3.as_ref().expect("persisted S3 payload should exist").name, "COLD-LEGACY"); + assert!(manager.read().await.tiers.contains_key("COLD-LEGACY")); + } + #[tokio::test] async fn config_update_refuses_pending_prepared_recovery_before_advancing_etag() { let manager = TierConfigMgr::new(); diff --git a/crates/ecstore/src/services/tier/tier_admin.rs b/crates/ecstore/src/services/tier/tier_admin.rs index 1b3519c02..32cd844a0 100644 --- a/crates/ecstore/src/services/tier/tier_admin.rs +++ b/crates/ecstore/src/services/tier/tier_admin.rs @@ -18,25 +18,203 @@ #![allow(unused_must_use)] #![allow(clippy::all)] -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; -#[derive(Serialize, Deserialize, Default, Debug, Clone)] +#[derive(Serialize, Deserialize, Default, Clone)] +#[serde(default)] +pub struct TierServicePrincipalAuth { + #[serde(rename = "TenantID", alias = "tenantID", alias = "tenant_id")] + pub tenant_id: String, + #[serde(rename = "ClientID", alias = "clientID", alias = "client_id")] + pub client_id: String, + #[serde(rename = "ClientSecret", alias = "clientSecret", alias = "client_secret")] + pub client_secret: String, +} + +impl TierServicePrincipalAuth { + pub(crate) fn is_empty(&self) -> bool { + self.tenant_id.is_empty() && self.client_id.is_empty() && self.client_secret.is_empty() + } +} + +impl std::fmt::Debug for TierServicePrincipalAuth { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TierServicePrincipalAuth") + .field("tenant_id", &self.tenant_id) + .field("client_id", &self.client_id) + .field("client_secret", &"REDACTED") + .finish() + } +} + +#[derive(Serialize, Deserialize, Default, Clone)] #[serde(default)] pub struct TierCreds { - #[serde(rename = "accessKey")] + #[serde(rename = "access", alias = "accessKey")] pub access_key: String, - #[serde(rename = "secretKey")] + #[serde(rename = "secret", alias = "secretKey")] pub secret_key: String, - #[serde(rename = "awsRole")] + #[serde(rename = "awsrole", alias = "awsRole")] pub aws_role: bool, - #[serde(rename = "awsRoleWebIdentityTokenFile")] + #[serde(rename = "awsroleWebIdentity", alias = "awsRoleWebIdentityTokenFile")] pub aws_role_web_identity_token_file: String, - #[serde(rename = "awsRoleArn")] + #[serde(rename = "awsroleARN", alias = "awsRoleArn", alias = "awsRoleARN")] pub aws_role_arn: String, - //azsp: ServicePrincipalAuth, + #[serde(rename = "azSP", alias = "azsp", skip_serializing_if = "TierServicePrincipalAuth::is_empty")] + pub azure_service_principal: TierServicePrincipalAuth, - //#[serde(rename = "credsJson")] + #[serde( + rename = "creds", + alias = "credsJson", + alias = "credsJSON", + alias = "creds_json", + default, + skip_serializing_if = "Vec::is_empty", + with = "base64_bytes" + )] pub creds_json: Vec, } + +impl std::fmt::Debug for TierCreds { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TierCreds") + .field("access_key", &self.access_key) + .field("secret_key", &"REDACTED") + .field("aws_role", &self.aws_role) + .field( + "aws_role_web_identity_token_file", + &(!self.aws_role_web_identity_token_file.is_empty()).then_some("REDACTED"), + ) + .field("aws_role_arn", &self.aws_role_arn) + .field("azure_service_principal", &self.azure_service_principal) + .field("creds_json", &(!self.creds_json.is_empty()).then_some("REDACTED")) + .finish() + } +} + +mod base64_bytes { + use super::*; + + pub(super) fn serialize(value: &[u8], serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&base64_simd::STANDARD.encode_to_string(value)) + } + + pub(super) fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(untagged)] + enum EncodedBytes { + Base64(String), + Legacy(Vec), + } + + match EncodedBytes::deserialize(deserializer)? { + EncodedBytes::Base64(value) => base64_simd::STANDARD + .decode_to_vec(value.as_bytes()) + .or_else(|_| base64_simd::STANDARD_NO_PAD.decode_to_vec(value.as_bytes())) + .or_else(|_| base64_simd::URL_SAFE.decode_to_vec(value.as_bytes())) + .or_else(|_| base64_simd::URL_SAFE_NO_PAD.decode_to_vec(value.as_bytes())) + .map_err(de::Error::custom), + EncodedBytes::Legacy(value) => Ok(value), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn tier_creds_accepts_madmin_wire_names_and_base64_gcs_json() { + let service_account = r#"{"type":"service_account","project_id":"tier-🚀x"}"#.as_bytes(); + let encoded = "eyJ0eXBlIjoic2VydmljZV9hY2NvdW50IiwicHJvamVjdF9pZCI6InRpZXIt8J+agHgifQ=="; + let creds: TierCreds = serde_json::from_value(serde_json::json!({ + "access": "access", + "secret": "secret", + "awsrole": false, + "creds": encoded, + })) + .expect("madmin tier credentials should decode"); + + assert_eq!(creds.access_key, "access"); + assert_eq!(creds.secret_key, "secret"); + assert_eq!(creds.creds_json.as_slice(), &service_account[..]); + + let wire = serde_json::to_value(&creds).expect("madmin tier credentials should encode"); + assert_eq!(wire["access"], "access"); + assert_eq!(wire["secret"], "secret"); + assert_eq!(wire["creds"], encoded); + assert!(wire.get("accessKey").is_none()); + assert!(wire.get("secretKey").is_none()); + + let legacy: TierCreds = serde_json::from_value(serde_json::json!({ + "accessKey": "legacy-access", + "secretKey": "legacy-secret", + "credsJson": service_account, + })) + .expect("the former RustFS field names and byte-array encoding should remain readable"); + assert_eq!(legacy.access_key, "legacy-access"); + assert_eq!(legacy.secret_key, "legacy-secret"); + assert_eq!(legacy.creds_json.as_slice(), &service_account[..]); + } + + #[test] + fn tier_creds_accepts_all_supported_base64_alphabets_and_padding_modes() { + let service_account = r#"{"type":"service_account","project_id":"tier-🚀"}"#.as_bytes(); + for encoder in [ + base64_simd::STANDARD, + base64_simd::STANDARD_NO_PAD, + base64_simd::URL_SAFE, + base64_simd::URL_SAFE_NO_PAD, + ] { + let encoded = encoder.encode_to_string(service_account); + let creds: TierCreds = serde_json::from_value(serde_json::json!({ "creds": encoded })) + .expect("all supported madmin base64 forms should decode"); + assert_eq!(creds.creds_json, service_account); + } + } + + #[test] + fn tier_creds_debug_redacts_secret_payloads() { + let creds = TierCreds { + access_key: "access".to_string(), + secret_key: "tier-secret-value".to_string(), + aws_role_web_identity_token_file: "/var/run/private-token".to_string(), + creds_json: br#"{"private_key":"gcs-private-key-value"}"#.to_vec(), + ..Default::default() + }; + + let rendered = format!("{creds:?}"); + assert!(!rendered.contains("tier-secret-value")); + assert!(!rendered.contains("/var/run/private-token")); + assert!(!rendered.contains("gcs-private-key-value")); + } + + #[test] + fn tier_creds_accepts_canonical_madmin_azure_service_principal_wire_shape() { + let creds: TierCreds = serde_json::from_value(serde_json::json!({ + "azSP": { + "TenantID": "tenant", + "ClientID": "client", + "ClientSecret": "service-principal-secret" + } + })) + .expect("canonical madmin azure service principal credentials should decode"); + + assert_eq!(creds.azure_service_principal.tenant_id, "tenant"); + assert_eq!(creds.azure_service_principal.client_id, "client"); + assert_eq!(creds.azure_service_principal.client_secret, "service-principal-secret"); + let wire = serde_json::to_value(&creds).expect("canonical madmin credentials should encode"); + assert_eq!(wire["azSP"]["TenantID"], "tenant"); + assert_eq!(wire["azSP"]["ClientID"], "client"); + assert_eq!(wire["azSP"]["ClientSecret"], "service-principal-secret"); + assert!(!format!("{creds:?}").contains("service-principal-secret")); + } +} diff --git a/crates/ecstore/src/services/tier/tier_config.rs b/crates/ecstore/src/services/tier/tier_config.rs index 94fe4e408..3d4833727 100644 --- a/crates/ecstore/src/services/tier/tier_config.rs +++ b/crates/ecstore/src/services/tier/tier_config.rs @@ -42,7 +42,7 @@ const WASABI_ALTERNATIVE_ENDPOINTS: &[(&str, &str)] = &[ pub enum TierType { #[default] Unsupported, - #[serde(rename = "s3")] + #[serde(rename = "s3", alias = "S3")] S3, #[serde(rename = "wasabi")] Wasabi, @@ -58,7 +58,7 @@ pub enum TierType { Huaweicloud, #[serde(rename = "azure")] Azure, - #[serde(rename = "gcs")] + #[serde(rename = "gcs", alias = "GCS")] GCS, #[serde(rename = "r2")] R2, @@ -138,16 +138,18 @@ impl TierType { } } -#[derive(Default, Debug, Serialize, Deserialize)] +pub(crate) const TIER_CREDENTIAL_REDACTED: &str = "REDACTED"; + +#[derive(Default, Serialize, Deserialize)] #[serde(default)] pub struct TierConfig { #[serde(skip)] pub version: String, - #[serde(rename = "type")] + #[serde(rename = "type", alias = "Type")] pub tier_type: TierType, - #[serde(skip)] + #[serde(rename = "Name", alias = "name", skip_serializing)] pub name: String, - #[serde(rename = "s3", skip_serializing_if = "Option::is_none")] + #[serde(rename = "s3", alias = "S3", skip_serializing_if = "Option::is_none")] pub s3: Option, #[serde(rename = "wasabi", skip_serializing_if = "Option::is_none")] pub wasabi: Option, @@ -159,7 +161,7 @@ pub struct TierConfig { pub huaweicloud: Option, #[serde(rename = "azure", skip_serializing_if = "Option::is_none")] pub azure: Option, - #[serde(rename = "gcs", skip_serializing_if = "Option::is_none")] + #[serde(rename = "gcs", alias = "GCS", skip_serializing_if = "Option::is_none")] pub gcs: Option, #[serde(rename = "r2", skip_serializing_if = "Option::is_none")] pub r2: Option, @@ -170,109 +172,91 @@ pub struct TierConfig { } impl Clone for TierConfig { - fn clone(&self) -> TierConfig { - let mut s3 = None; - let mut wasabi = None; - let mut r = None; - let mut compatible_backend = None; - let mut aliyun = None; - let mut tencent = None; - let mut huaweicloud = None; - let mut azure = None; - let mut gcs = None; - let mut r2 = None; - match self.tier_type { - TierType::S3 => { - if let Some(s3_) = self.s3.as_ref() { - let mut s3_clone = s3_.clone(); - s3_clone.secret_key = "REDACTED".to_string(); - s3 = Some(s3_clone); - } - } - TierType::Wasabi => { - if let Some(wasabi_) = self.wasabi.as_ref() { - let mut wasabi_clone = wasabi_.clone(); - wasabi_clone.secret_key = "REDACTED".to_string(); - wasabi = Some(wasabi_clone); - } - } - TierType::RustFS => { - if let Some(r_) = self.rustfs.as_ref() { - let mut r_clone = r_.clone(); - r_clone.secret_key = "REDACTED".to_string(); - r = Some(r_clone); - } - } - TierType::MinIO => { - if let Some(compatible_backend_) = self.minio.as_ref() { - let mut compatible_backend_clone = compatible_backend_.clone(); - compatible_backend_clone.secret_key = "REDACTED".to_string(); - compatible_backend = Some(compatible_backend_clone); - } - } - TierType::Aliyun => { - if let Some(aliyun_) = self.aliyun.as_ref() { - let mut aliyun_clone = aliyun_.clone(); - aliyun_clone.secret_key = "REDACTED".to_string(); - aliyun = Some(aliyun_clone); - } - } - TierType::Tencent => { - if let Some(tencent_) = self.tencent.as_ref() { - let mut tencent_clone = tencent_.clone(); - tencent_clone.secret_key = "REDACTED".to_string(); - tencent = Some(tencent_clone); - } - } - TierType::Huaweicloud => { - if let Some(huaweicloud_) = self.huaweicloud.as_ref() { - let mut huaweicloud_clone = huaweicloud_.clone(); - huaweicloud_clone.secret_key = "REDACTED".to_string(); - huaweicloud = Some(huaweicloud_clone); - } - } - TierType::Azure => { - if let Some(azure_) = self.azure.as_ref() { - let mut azure_clone = azure_.clone(); - azure_clone.secret_key = "REDACTED".to_string(); - azure = Some(azure_clone); - } - } - TierType::GCS => { - if let Some(gcs_) = self.gcs.as_ref() { - let mut gcs_clone = gcs_.clone(); - gcs_clone.creds = "REDACTED".to_string(); - gcs = Some(gcs_clone); - } - } - TierType::R2 => { - if let Some(r2_) = self.r2.as_ref() { - let mut r2_clone = r2_.clone(); - r2_clone.secret_key = "REDACTED".to_string(); - r2 = Some(r2_clone); - } - } - _ => (), - } - TierConfig { - version: self.version.clone(), - tier_type: self.tier_type.clone(), - name: self.name.clone(), - s3, - wasabi, - rustfs: r, - minio: compatible_backend, - aliyun, - tencent, - huaweicloud, - azure, - gcs, - r2, - } + fn clone(&self) -> Self { + self.redacted() } } impl TierConfig { + pub(crate) fn redacted(&self) -> Self { + let mut redacted = Self { + version: self.version.clone(), + tier_type: self.tier_type.clone(), + name: self.name.clone(), + ..Default::default() + }; + match self.tier_type { + TierType::S3 => { + redacted.s3 = self.s3.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + if !backend.aws_role_web_identity_token_file.is_empty() { + backend.aws_role_web_identity_token_file = TIER_CREDENTIAL_REDACTED.to_string(); + } + backend + }); + } + TierType::Wasabi => { + redacted.wasabi = self.wasabi.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::RustFS => { + redacted.rustfs = self.rustfs.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::MinIO => { + redacted.minio = self.minio.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::Aliyun => { + redacted.aliyun = self.aliyun.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::Tencent => { + redacted.tencent = self.tencent.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::Huaweicloud => { + redacted.huaweicloud = self.huaweicloud.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::Azure => { + redacted.azure = self.azure.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + if !backend.sp_auth.client_secret.is_empty() { + backend.sp_auth.client_secret = TIER_CREDENTIAL_REDACTED.to_string(); + } + backend + }); + } + TierType::GCS => { + redacted.gcs = self.gcs.clone().map(|mut backend| { + backend.creds = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::R2 => { + redacted.r2 = self.r2.clone().map(|mut backend| { + backend.secret_key = TIER_CREDENTIAL_REDACTED.to_string(); + backend + }); + } + TierType::Unsupported => {} + } + redacted + } + pub(crate) fn clone_with_credentials(&self) -> Self { Self { version: self.version.clone(), @@ -372,31 +356,61 @@ impl TierConfig { } } +impl std::fmt::Debug for TierConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let redacted = self.redacted(); + f.debug_struct("TierConfig") + .field("version", &redacted.version) + .field("tier_type", &redacted.tier_type) + .field("name", &redacted.name) + .field("s3", &redacted.s3) + .field("wasabi", &redacted.wasabi) + .field("aliyun", &redacted.aliyun) + .field("tencent", &redacted.tencent) + .field("huaweicloud", &redacted.huaweicloud) + .field("azure", &redacted.azure) + .field("gcs", &redacted.gcs) + .field("r2", &redacted.r2) + .field("rustfs", &redacted.rustfs) + .field("minio", &redacted.minio) + .finish() + } +} + //type S3Options = impl Fn(TierS3) -> Pin>> + Send + Sync + 'static; #[derive(Serialize, Deserialize, Default, Debug, Clone)] #[serde(default)] pub struct TierS3 { + #[serde(alias = "Name")] pub name: String, + #[serde(alias = "Endpoint")] pub endpoint: String, - #[serde(rename = "accessKey")] + #[serde(rename = "accessKey", alias = "AccessKey")] pub access_key: String, - #[serde(rename = "secretKey")] + #[serde(rename = "secretKey", alias = "SecretKey")] pub secret_key: String, + #[serde(alias = "Bucket")] pub bucket: String, + #[serde(alias = "Prefix")] pub prefix: String, + #[serde(alias = "Region")] pub region: String, - #[serde(rename = "storageClass")] + #[serde(rename = "storageClass", alias = "StorageClass")] pub storage_class: String, - #[serde(skip)] + #[serde(rename = "AWSRole", alias = "awsRole", skip_serializing)] pub aws_role: bool, - #[serde(skip)] + #[serde( + rename = "AWSRoleWebIdentityTokenFile", + alias = "awsRoleWebIdentityTokenFile", + skip_serializing + )] pub aws_role_web_identity_token_file: String, - #[serde(skip)] + #[serde(rename = "AWSRoleARN", alias = "awsRoleARN", alias = "awsRoleArn", skip_serializing)] pub aws_role_arn: String, - #[serde(skip)] + #[serde(rename = "AWSRoleSessionName", alias = "awsRoleSessionName", skip_serializing)] pub aws_role_session_name: String, - #[serde(skip)] + #[serde(rename = "AWSRoleDurationSeconds", alias = "awsRoleDurationSeconds", skip_serializing)] pub aws_role_duration_seconds: i32, } @@ -623,8 +637,11 @@ pub struct TierHuaweicloud { #[derive(Serialize, Deserialize, Default, Debug, Clone)] #[serde(default)] pub struct ServicePrincipalAuth { + #[serde(alias = "TenantID")] pub tenant_id: String, + #[serde(alias = "ClientID")] pub client_id: String, + #[serde(alias = "ClientSecret")] pub client_secret: String, } @@ -640,9 +657,9 @@ pub struct TierAzure { pub bucket: String, pub prefix: String, pub region: String, - #[serde(rename = "storageClass")] + #[serde(rename = "storageClass", alias = "StorageClass")] pub storage_class: String, - #[serde(rename = "spAuth")] + #[serde(rename = "spAuth", alias = "SPAuth")] pub sp_auth: ServicePrincipalAuth, } @@ -696,14 +713,19 @@ fn AzureStorageClass(sc string) func(az *TierAzure) error { #[derive(Serialize, Deserialize, Default, Debug, Clone)] #[serde(default)] pub struct TierGCS { + #[serde(alias = "Name")] pub name: String, + #[serde(alias = "Endpoint")] pub endpoint: String, - #[serde(rename = "creds")] + #[serde(rename = "creds", alias = "Creds")] pub creds: String, + #[serde(alias = "Bucket")] pub bucket: String, + #[serde(alias = "Prefix")] pub prefix: String, + #[serde(alias = "Region")] pub region: String, - #[serde(rename = "storageClass")] + #[serde(rename = "storageClass", alias = "StorageClass")] pub storage_class: String, } @@ -725,6 +747,43 @@ pub struct TierR2 { mod tests { use super::*; + #[test] + fn s3_gcs_type_uppercase_aliases_preserve_lowercase_output() { + let s3: TierType = serde_json::from_str(r#""S3""#).expect("uppercase S3 wire value should decode"); + let gcs: TierType = serde_json::from_str(r#""GCS""#).expect("uppercase GCS wire value should decode"); + assert!(matches!(s3, TierType::S3)); + assert!(matches!(gcs, TierType::GCS)); + assert_eq!(serde_json::to_string(&s3).expect("S3 type should encode"), r#""s3""#); + assert_eq!(serde_json::to_string(&gcs).expect("GCS type should encode"), r#""gcs""#); + } + + #[test] + fn azure_service_principal_accepts_canonical_madmin_field_names() { + for field in ["TenantID", "ClientID", "ClientSecret"] { + let mut sp_auth = serde_json::Map::new(); + sp_auth.insert(field.to_string(), serde_json::Value::String("present".to_string())); + let config: TierConfig = serde_json::from_value(serde_json::json!({ + "type": "azure", + "Name": "COLD-AZURE", + "azure": { + "name": "COLD-AZURE", + "endpoint": "https://azure.example.invalid", + "accessKey": "account", + "secretKey": "key", + "bucket": "archive", + "SPAuth": sp_auth + } + })) + .expect("mixed RustFS/madmin Azure payload should decode"); + let sp_auth = &config.azure.expect("Azure payload should exist").sp_auth; + + assert!( + !sp_auth.tenant_id.is_empty() || !sp_auth.client_id.is_empty() || !sp_auth.client_secret.is_empty(), + "canonical {field} must not be silently discarded" + ); + } + } + fn wasabi_config() -> TierWasabi { TierWasabi { name: "COLD-WASABI".to_string(), @@ -838,9 +897,14 @@ mod tests { let config = TierConfig { tier_type: TierType::Wasabi, wasabi: Some(wasabi_config()), + rustfs: Some(TierRustFS { + access_key: "inactive-access".to_string(), + secret_key: "inactive-secret".to_string(), + ..Default::default() + }), ..Default::default() }; - let redacted = config.clone(); + let redacted = config.redacted(); assert_eq!( redacted .wasabi @@ -849,21 +913,46 @@ mod tests { .secret_key, "REDACTED" ); + assert!(redacted.rustfs.is_none(), "the external view should retain only the active provider"); + let cloned = config.clone(); + assert_eq!(cloned.wasabi.expect("redacted Wasabi clone should remain").secret_key, "REDACTED"); + assert!(cloned.rustfs.is_none(), "ordinary Clone must retain its redacted API semantics"); + let preserved = config.clone_with_credentials(); assert_eq!( - config - .clone_with_credentials() + preserved .wasabi .as_ref() - .expect("credential-bearing Wasabi payload should remain") + .expect("credential-bearing Wasabi snapshot should remain") .secret_key, "secret" ); + assert_eq!( + preserved + .rustfs + .expect("credential-bearing snapshots should preserve inactive provider data") + .secret_key, + "inactive-secret" + ); let mut debug_config = wasabi_config(); debug_config.secret_key = "wasabi-debug-secret-value".to_string(); let debug = format!("{debug_config:?}"); assert!(debug.contains("REDACTED")); assert!(!debug.contains("wasabi-debug-secret-value")); + + let debug = format!( + "{:?}", + TierConfig { + tier_type: TierType::RustFS, + rustfs: Some(TierRustFS { + secret_key: "rustfs-debug-secret-value".to_string(), + ..Default::default() + }), + ..Default::default() + } + ); + assert!(debug.contains("REDACTED")); + assert!(!debug.contains("rustfs-debug-secret-value")); } #[test] @@ -894,7 +983,7 @@ mod tests { assert_eq!(encoded, expected); let decoded: TierConfig = serde_json::from_value(encoded).expect("Wasabi Admin JSON should decode"); assert!(matches!(decoded.tier_type, TierType::Wasabi)); - let redacted = config.clone(); + let redacted = config.redacted(); assert_eq!( config .wasabi @@ -916,4 +1005,87 @@ mod tests { "REDACTED" ); } + + #[test] + fn api_serialization_and_debug_redact_s3_gcs_and_azure_credentials() { + let cases = [ + ( + "s3", + TierConfig { + tier_type: TierType::S3, + s3: Some(TierS3 { + secret_key: "s3-secret-bytes".to_string(), + aws_role_web_identity_token_file: "/var/run/s3-private-token".to_string(), + ..Default::default() + }), + ..Default::default() + }, + vec!["s3-secret-bytes", "/var/run/s3-private-token"], + ), + ( + "gcs", + TierConfig { + tier_type: TierType::GCS, + gcs: Some(TierGCS { + creds: r#"{"type":"service_account","private_key":"gcs-private-key-bytes"}"#.to_string(), + ..Default::default() + }), + ..Default::default() + }, + vec!["gcs-private-key-bytes"], + ), + ( + "azure", + TierConfig { + tier_type: TierType::Azure, + azure: Some(TierAzure { + secret_key: "azure-account-secret-bytes".to_string(), + sp_auth: ServicePrincipalAuth { + client_secret: "azure-client-secret-bytes".to_string(), + ..Default::default() + }, + ..Default::default() + }), + ..Default::default() + }, + vec!["azure-account-secret-bytes", "azure-client-secret-bytes"], + ), + ]; + + for (provider, config, secrets) in cases { + let api = serde_json::to_string(&config.redacted()).expect("redacted API config should serialize"); + let debug = format!("{config:?}"); + assert!(api.contains(TIER_CREDENTIAL_REDACTED), "{provider} API output should be visibly redacted"); + assert!( + debug.contains(TIER_CREDENTIAL_REDACTED), + "{provider} Debug output should be visibly redacted" + ); + for secret in secrets { + assert!(!api.contains(secret), "{provider} API output exposed credential bytes"); + assert!(!debug.contains(secret), "{provider} Debug output exposed credential bytes"); + } + } + } + + #[test] + fn azure_static_account_redaction_preserves_an_empty_service_principal_secret() { + let config = TierConfig { + tier_type: TierType::Azure, + azure: Some(TierAzure { + secret_key: "azure-account-secret-bytes".to_string(), + sp_auth: ServicePrincipalAuth::default(), + ..Default::default() + }), + ..Default::default() + }; + + let api = serde_json::to_value(config.redacted()).expect("redacted Azure API config should serialize"); + let debug = format!("{config:?}"); + + assert_eq!(api["azure"]["secretKey"], TIER_CREDENTIAL_REDACTED); + assert_eq!(api["azure"]["spAuth"]["client_secret"], ""); + assert!(debug.contains("client_secret: \"\"")); + assert!(!debug.contains("client_secret: \"REDACTED\"")); + assert!(!debug.contains("azure-account-secret-bytes")); + } } diff --git a/crates/ecstore/src/services/tier/warm_backend.rs b/crates/ecstore/src/services/tier/warm_backend.rs index 8eb6e4058..ee48c116c 100644 --- a/crates/ecstore/src/services/tier/warm_backend.rs +++ b/crates/ecstore/src/services/tier/warm_backend.rs @@ -20,7 +20,7 @@ use crate::error::is_err_bucket_not_found; use crate::services::tier::{ - tier::{ERR_TIER_INVALID_CONFIG, ERR_TIER_TYPE_UNSUPPORTED}, + tier::{ERR_TIER_BACKEND_IN_USE, ERR_TIER_INVALID_CONFIG, ERR_TIER_TYPE_UNSUPPORTED}, tier_config::{TierConfig, TierType}, tier_handlers::{ERR_TIER_BUCKET_NOT_FOUND, ERR_TIER_NOT_FOUND, ERR_TIER_PERM_ERR}, warm_backend_aliyun::WarmBackendAliyun, @@ -55,18 +55,21 @@ use s3s::header::{ }; use std::collections::HashMap; use std::sync::Arc; +use std::time::Duration; use time::OffsetDateTime; use time::format_description::well_known::{Rfc2822, Rfc3339}; +use tokio::io::AsyncReadExt; use tracing::{info, warn}; pub type WarmBackendImpl = Box; -const PROBE_OBJECT: &str = "probeobject"; - /// Largest object the S3-compatible warm backends accept for a multipart put. pub(crate) const MAX_MULTIPART_PUT_OBJECT_SIZE: i64 = 1024 * 1024 * 1024 * 1024 * 5; /// Part-count ceiling S3-compatible services impose on a multipart upload. pub(crate) const MAX_PARTS_COUNT: i64 = 10000; +pub(crate) const WARM_BACKEND_PROBE_TIMEOUT: Duration = Duration::from_secs(30); +const WARM_BACKEND_PROBE_RECONCILE_INTERVAL: Duration = Duration::from_secs(1); +const WARM_BACKEND_PROBE_FINAL_RECONCILE_TIMEOUT: Duration = Duration::from_secs(1); #[derive(Default)] pub struct WarmBackendGetOpts { @@ -260,6 +263,23 @@ pub(crate) struct S3CompatibleWarmBackendParams<'a> { pub validate_endpoint: fn(&url::Url) -> Result<(), rustfs_utils::egress::OutboundUrlError>, } +/// Return the authority format accepted by `TransitionClient::new` while +/// retaining an explicitly configured port. `url::Url::host_str()` omits the +/// brackets needed when an IPv6 literal is combined with a port. +pub(crate) fn endpoint_authority(url: &url::Url) -> Result { + let host = url + .host_str() + .ok_or_else(|| std::io::Error::other("Invalid endpoint URL: missing host"))?; + let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); + if host.starts_with('[') && host.ends_with(']') { + Ok(format!("{host}:{port}")) + } else if host.contains(':') { + Ok(format!("[{host}]:{port}")) + } else { + Ok(format!("{host}:{port}")) + } +} + /// Build the [`WarmBackendS3`] shared by the S3-compatible warm backend providers. /// /// Credential, bucket, and endpoint validation run in this order because the @@ -298,17 +318,11 @@ pub(crate) async fn new_s3_compatible_warm_backend( bucket_lookup: params.bucket_lookup, ..Default::default() }; - let scheme = u.scheme(); - let default_port = if scheme == "https" { 443 } else { 80 }; - let host = u - .host_str() - .ok_or_else(|| std::io::Error::other("Invalid endpoint URL: missing host"))?; - // Runs after the host-presence check above (not immediately after Url::parse) so a - // host-less endpoint still reports this constructor's own "missing host" text instead of - // validate_endpoint's differently-worded rejection for the same input. + let endpoint = endpoint_authority(&u)?; + // Run the SSRF guard after the host-presence check so a host-less endpoint + // keeps this constructor's stable error text. (params.validate_endpoint)(&u).map_err(|err| std::io::Error::other(format!("tier endpoint is not allowed: {err}")))?; - let client = - TransitionClient::new(&format!("{}:{}", host, u.port().unwrap_or(default_port)), opts, params.provider_tag).await?; + let client = TransitionClient::new(&endpoint, opts, params.provider_tag).await?; let client = Arc::new(client); let core = TransitionCore(Arc::clone(&client)); @@ -451,25 +465,187 @@ impl TransitionCandidateReconciler for MeteredTransitionCandidateReconciler { } } -pub async fn check_warm_backend(w: Option<&WarmBackendImpl>) -> Result<(), AdminError> { - let w = w.ok_or_else(|| ERR_TIER_NOT_FOUND.clone())?; - w.validate().await.map_err(|_| ERR_TIER_INVALID_CONFIG.clone())?; - let remote_version_id = w - .put(PROBE_OBJECT, ReaderImpl::Body(Bytes::from("RustFS".as_bytes().to_vec())), 5) - .await - .map_err(|_| ERR_TIER_PERM_ERR.clone())?; +async fn remove_discovered_probe_candidate( + w: &WarmBackendImpl, + probe_object: &str, + candidate: TransitionCandidateProbe, +) -> Result { + match candidate { + TransitionCandidateProbe::Missing => Ok(false), + TransitionCandidateProbe::VersionedPresent(remote_version_id) => { + w.remove_exact(probe_object, &remote_version_id).await?; + Ok(true) + } + TransitionCandidateProbe::UnversionedPresent => { + w.remove(probe_object, "").await?; + Ok(true) + } + TransitionCandidateProbe::Ambiguous => { + Err(std::io::Error::other("remote tier probe PUT produced multiple possible versions")) + } + TransitionCandidateProbe::Unsupported => Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "remote tier cannot discover the outcome of a probe PUT", + )), + } +} - if w.validate_remote_version_id(&remote_version_id).is_err() { - w.remove_exact(PROBE_OBJECT, &remote_version_id) +async fn compensate_uncertain_probe_put( + w: &WarmBackendImpl, + probe_object: &str, + settle_deadline: tokio::time::Instant, +) -> Result<(), std::io::Error> { + let final_deadline = settle_deadline + WARM_BACKEND_PROBE_FINAL_RECONCILE_TIMEOUT; + let mut removed_any = false; + while tokio::time::Instant::now() < settle_deadline { + let candidate = match tokio::time::timeout_at(settle_deadline, w.probe_transition_candidate(probe_object)).await { + Ok(candidate) => candidate?, + Err(_) => break, + }; + if matches!(candidate, TransitionCandidateProbe::Missing) && removed_any { + break; + } + removed_any |= tokio::time::timeout_at(settle_deadline, remove_discovered_probe_candidate(w, probe_object, candidate)) .await - .map_err(|_| ERR_TIER_PERM_ERR.clone())?; - return Err(ERR_TIER_INVALID_CONFIG.clone()); + .map_err(|_| std::io::Error::new(std::io::ErrorKind::TimedOut, "timed out reconciling a remote tier probe PUT"))??; + + let now = tokio::time::Instant::now(); + if now >= settle_deadline { + break; + } + tokio::time::sleep_until(std::cmp::min(settle_deadline, now + WARM_BACKEND_PROBE_RECONCILE_INTERVAL)).await; } - let read_result = w.get(PROBE_OBJECT, &remote_version_id, WarmBackendGetOpts::default()).await; - let remove_result = w.remove(PROBE_OBJECT, &remote_version_id).await; - //xhttp.DrainBody(r); - if read_result.is_err() || remove_result.is_err() { + let candidate = tokio::time::timeout_at(final_deadline, w.probe_transition_candidate(probe_object)) + .await + .map_err(|_| { + std::io::Error::new(std::io::ErrorKind::TimedOut, "timed out confirming the final remote tier probe state") + })??; + if !tokio::time::timeout_at(final_deadline, remove_discovered_probe_candidate(w, probe_object, candidate)) + .await + .map_err(|_| { + std::io::Error::new(std::io::ErrorKind::TimedOut, "timed out removing the final remote tier probe candidate") + })?? + { + return Ok(()); + } + + let final_candidate = tokio::time::timeout_at(final_deadline, w.probe_transition_candidate(probe_object)) + .await + .map_err(|_| std::io::Error::new(std::io::ErrorKind::TimedOut, "timed out confirming remote tier probe cleanup"))??; + match final_candidate { + TransitionCandidateProbe::Missing => Ok(()), + _ => Err(std::io::Error::other("remote tier probe cleanup could not be confirmed")), + } +} + +fn probe_cleanup_incomplete_error() -> AdminError { + let mut err = ERR_TIER_PERM_ERR.clone(); + err.message = "Remote tier probe outcome is uncertain; cleanup is incomplete".to_string(); + err +} + +async fn check_warm_backend_with_deadlines( + w: Option<&WarmBackendImpl>, + deadline: tokio::time::Instant, + cleanup_deadline: tokio::time::Instant, +) -> Result<(), AdminError> { + let w = w.ok_or_else(|| ERR_TIER_NOT_FOUND.clone())?; + let probe_object = format!("rustfs-tier-probe-{}", uuid::Uuid::new_v4()); + let timeout_error = || { + let mut err = ERR_TIER_BACKEND_IN_USE.clone(); + err.message = "Timed out validating the remote tier mutation".to_string(); + err + }; + tokio::time::timeout_at(deadline, w.validate()) + .await + .map_err(|_| timeout_error())? + .map_err(|_| ERR_TIER_INVALID_CONFIG.clone())?; + let put_result = + tokio::time::timeout_at(deadline, w.put(&probe_object, ReaderImpl::Body(Bytes::from_static(b"RustFS")), 6)).await; + let remote_version_id = match put_result { + Ok(Ok(remote_version_id)) => remote_version_id, + Ok(Err(_)) => { + return Err(match compensate_uncertain_probe_put(w, &probe_object, cleanup_deadline).await { + Ok(()) => ERR_TIER_PERM_ERR.clone(), + Err(_) => probe_cleanup_incomplete_error(), + }); + } + Err(_) => { + let err = timeout_error(); + return Err(match compensate_uncertain_probe_put(w, &probe_object, cleanup_deadline).await { + Ok(()) => err, + Err(_) => probe_cleanup_incomplete_error(), + }); + } + }; + + // S3-family backends do not replay a failed request before returning `Ok`, + // while GCS discovers every matching generation. The authoritative probe + // below therefore closes the acknowledged-PUT path; only an error or + // timeout needs the longer visibility reconciliation above. + let authoritative_candidate = match tokio::time::timeout_at(deadline, w.probe_transition_candidate(&probe_object)).await { + Ok(Ok(candidate)) => candidate, + Ok(Err(_)) | Err(_) => { + return Err(match compensate_uncertain_probe_put(w, &probe_object, cleanup_deadline).await { + Ok(()) => ERR_TIER_INVALID_CONFIG.clone(), + Err(_) => probe_cleanup_incomplete_error(), + }); + } + }; + let response_version_is_valid = w.validate_remote_version_id(&remote_version_id).is_ok(); + let response_matches_candidate = match &authoritative_candidate { + TransitionCandidateProbe::UnversionedPresent => remote_version_id.is_empty(), + TransitionCandidateProbe::VersionedPresent(candidate_version) => candidate_version == &remote_version_id, + TransitionCandidateProbe::Missing | TransitionCandidateProbe::Ambiguous | TransitionCandidateProbe::Unsupported => false, + }; + if !response_version_is_valid || !response_matches_candidate { + return Err(match compensate_uncertain_probe_put(w, &probe_object, cleanup_deadline).await { + Ok(()) => ERR_TIER_INVALID_CONFIG.clone(), + Err(_) => probe_cleanup_incomplete_error(), + }); + } + + let read_result = tokio::time::timeout_at(deadline, async { + let mut reader = w + .get( + &probe_object, + &remote_version_id, + WarmBackendGetOpts { + start_offset: 0, + length: 7, + }, + ) + .await + .map_err(|_| ERR_TIER_PERM_ERR.clone())?; + let mut body = Vec::new(); + reader + .take(7) + .read_to_end(&mut body) + .await + .map_err(|_| ERR_TIER_PERM_ERR.clone())?; + if body != b"RustFS" { + return Err(ERR_TIER_PERM_ERR.clone()); + } + Ok(()) + }) + .await + .map_err(|_| timeout_error()) + .and_then(|result| result); + let cleanup_result = tokio::time::timeout_at(cleanup_deadline, async { + if !remove_discovered_probe_candidate(w, &probe_object, authoritative_candidate).await? { + return Err(std::io::Error::other("remote tier probe disappeared before cleanup")); + } + match w.probe_transition_candidate(&probe_object).await? { + TransitionCandidateProbe::Missing => Ok(()), + _ => Err(std::io::Error::other("remote tier probe remained after cleanup")), + } + }) + .await; + if !matches!(cleanup_result, Ok(Ok(()))) { + return Err(probe_cleanup_incomplete_error()); + } + if let Err(err) = read_result { //if is_err_bucket_not_found(&err) { // return Err(ERR_TIER_BUCKET_NOT_FOUND); //} @@ -477,12 +653,28 @@ pub async fn check_warm_backend(w: Option<&WarmBackendImpl>) -> Result<(), Admin return Err(ERR_TIER_MISSING_CREDENTIALS); }*/ //else { - return Err(ERR_TIER_PERM_ERR.clone()); + return Err(err); //} } Ok(()) } +/// Validate a backend using a caller-owned deadline while retaining a bounded +/// reconciliation window for an uncertain probe PUT. The validation future is +/// kept alive through cleanup so an outer timeout cannot abandon the remote +/// probe object. +pub(crate) async fn check_warm_backend_until( + w: Option<&WarmBackendImpl>, + deadline: tokio::time::Instant, +) -> Result<(), AdminError> { + check_warm_backend_with_deadlines(w, deadline, deadline + WARM_BACKEND_PROBE_FINAL_RECONCILE_TIMEOUT).await +} + +pub async fn check_warm_backend(w: Option<&WarmBackendImpl>) -> Result<(), AdminError> { + let deadline = tokio::time::Instant::now() + WARM_BACKEND_PROBE_TIMEOUT; + check_warm_backend_with_deadlines(w, deadline, deadline + WARM_BACKEND_PROBE_TIMEOUT).await +} + pub async fn new_warm_backend(tier: &TierConfig, probe: bool) -> Result { let mut d: Option = None; match tier.tier_type { @@ -701,7 +893,7 @@ pub async fn new_warm_backend(tier: &TierConfig, probe: bool) -> Result, + present: Arc, removed_versions: Arc>>, + returned_version: String, } struct RecordingProbeBackend { get_versions: Arc>>, + present: Arc, removed_versions: Arc>>, + remove_clears_candidate: bool, fail_get: bool, + body: ProbeBody, + } + + struct HangingProbePutBackend { + put_started: Arc, + present: Arc, + probes: Arc, + removed_versions: Arc>>, + } + + struct LateVisibleProbeBackend { + visible_at: tokio::time::Instant, + removed: Arc, + probes: Arc, + removed_versions: Arc>>, + } + + #[derive(Clone, Copy)] + enum ProbeBody { + Exact, + Mismatch, } #[async_trait::async_trait] @@ -976,7 +1194,7 @@ mod tests { } async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> Result { - Ok(uuid::Uuid::nil().to_string()) + Ok(self.returned_version.clone()) } async fn put_with_meta( @@ -999,10 +1217,19 @@ mod tests { } async fn remove_exact(&self, _object: &str, rv: &str) -> Result<(), std::io::Error> { + self.present.store(false, Ordering::SeqCst); self.removed_versions.lock().await.push(rv.to_string()); Ok(()) } + async fn probe_transition_candidate(&self, _object: &str) -> Result { + if self.present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent(PROBE_VERSION.to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + async fn in_use(&self) -> Result { Ok(false) } @@ -1029,15 +1256,121 @@ mod tests { if self.fail_get { Err(std::io::Error::other("probe GET failed")) } else { - Ok(ReadCloser::new(std::io::Cursor::new(Vec::new()))) + match self.body { + ProbeBody::Exact => Ok(ReadCloser::new(std::io::Cursor::new(b"RustFS".to_vec()))), + ProbeBody::Mismatch => Ok(ReadCloser::new(std::io::Cursor::new(b"RustFT".to_vec()))), + } } } async fn remove(&self, _object: &str, rv: &str) -> Result<(), std::io::Error> { + if self.remove_clears_candidate { + self.present.store(false, Ordering::SeqCst); + } self.removed_versions.lock().await.push(rv.to_string()); Ok(()) } + async fn probe_transition_candidate(&self, _object: &str) -> Result { + if self.present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent(PROBE_VERSION.to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + + async fn in_use(&self) -> Result { + Ok(false) + } + } + + #[async_trait::async_trait] + impl WarmBackend for HangingProbePutBackend { + async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> Result { + self.put_started.notify_one(); + std::future::pending().await + } + + async fn put_with_meta( + &self, + object: &str, + r: ReaderImpl, + length: i64, + _meta: HashMap, + ) -> Result { + self.put(object, r, length).await + } + + async fn get(&self, _object: &str, _rv: &str, _opts: WarmBackendGetOpts) -> Result { + Err(std::io::Error::other("GET must not run after a timed out probe PUT")) + } + + async fn remove(&self, _object: &str, _rv: &str) -> Result<(), std::io::Error> { + Err(std::io::Error::other("generic remove must not replace exact probe cleanup")) + } + + async fn remove_exact(&self, _object: &str, rv: &str) -> Result<(), std::io::Error> { + self.present.store(false, Ordering::SeqCst); + self.removed_versions.lock().await.push(rv.to_string()); + Ok(()) + } + + async fn probe_transition_candidate(&self, _object: &str) -> Result { + self.probes.fetch_add(1, Ordering::SeqCst); + if self.present.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent(PROBE_VERSION.to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + + async fn in_use(&self) -> Result { + Ok(false) + } + } + + #[async_trait::async_trait] + impl WarmBackend for LateVisibleProbeBackend { + async fn put(&self, _object: &str, _r: ReaderImpl, _length: i64) -> Result { + Err(std::io::Error::new( + std::io::ErrorKind::ConnectionReset, + "probe PUT response was lost before the object became visible", + )) + } + + async fn put_with_meta( + &self, + object: &str, + r: ReaderImpl, + length: i64, + _meta: HashMap, + ) -> Result { + self.put(object, r, length).await + } + + async fn get(&self, _object: &str, _rv: &str, _opts: WarmBackendGetOpts) -> Result { + Err(std::io::Error::other("GET must not run after a lost probe PUT response")) + } + + async fn remove(&self, _object: &str, _rv: &str) -> Result<(), std::io::Error> { + Err(std::io::Error::other("generic remove must not replace exact probe cleanup")) + } + + async fn remove_exact(&self, _object: &str, rv: &str) -> Result<(), std::io::Error> { + self.removed.store(true, Ordering::SeqCst); + self.removed_versions.lock().await.push(rv.to_string()); + Ok(()) + } + + async fn probe_transition_candidate(&self, _object: &str) -> Result { + self.probes.fetch_add(1, Ordering::SeqCst); + if tokio::time::Instant::now() >= self.visible_at && !self.removed.load(Ordering::SeqCst) { + Ok(TransitionCandidateProbe::VersionedPresent(PROBE_VERSION.to_string())) + } else { + Ok(TransitionCandidateProbe::Missing) + } + } + async fn in_use(&self) -> Result { Ok(false) } @@ -1098,13 +1431,15 @@ mod tests { assert_eq!(probe, TransitionCandidateProbe::Unsupported); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn check_warm_backend_removes_exact_probe_when_versioning_drifts() { let gets = Arc::new(AtomicUsize::new(0)); let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); let backend: WarmBackendImpl = Box::new(RejectingProbeVersionBackend { gets: gets.clone(), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), removed_versions: removed_versions.clone(), + returned_version: uuid::Uuid::nil().to_string(), }); let err = check_warm_backend(Some(&backend)) @@ -1113,7 +1448,27 @@ mod tests { assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); assert_eq!(gets.load(Ordering::SeqCst), 0); - assert_eq!(removed_versions.lock().await.as_slice(), [uuid::Uuid::nil().to_string()]); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); + } + + #[tokio::test(start_paused = true)] + async fn check_warm_backend_rejects_empty_put_version_for_a_versioned_candidate() { + let gets = Arc::new(AtomicUsize::new(0)); + let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let backend: WarmBackendImpl = Box::new(RejectingProbeVersionBackend { + gets: gets.clone(), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), + removed_versions: removed_versions.clone(), + returned_version: String::new(), + }); + + let err = check_warm_backend(Some(&backend)) + .await + .expect_err("an empty PUT version must not read or generically delete a versioned object"); + + assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code); + assert_eq!(gets.load(Ordering::SeqCst), 0); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); } #[tokio::test] @@ -1122,8 +1477,11 @@ mod tests { let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); let backend: WarmBackendImpl = Box::new(RecordingProbeBackend { get_versions: get_versions.clone(), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), removed_versions: removed_versions.clone(), + remove_clears_candidate: true, fail_get: false, + body: ProbeBody::Exact, }); check_warm_backend(Some(&backend)) @@ -1140,8 +1498,11 @@ mod tests { let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); let backend: WarmBackendImpl = Box::new(RecordingProbeBackend { get_versions: get_versions.clone(), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), removed_versions: removed_versions.clone(), + remove_clears_candidate: true, fail_get: true, + body: ProbeBody::Exact, }); let err = check_warm_backend(Some(&backend)) @@ -1153,6 +1514,169 @@ mod tests { assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); } + #[tokio::test] + async fn check_warm_backend_removes_probe_after_body_mismatch() { + let get_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let backend: WarmBackendImpl = Box::new(RecordingProbeBackend { + get_versions, + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), + removed_versions: removed_versions.clone(), + remove_clears_candidate: true, + fail_get: false, + body: ProbeBody::Mismatch, + }); + + let err = check_warm_backend(Some(&backend)) + .await + .expect_err("a mismatched body should fail after cleanup"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); + } + + #[tokio::test] + async fn check_warm_backend_rejects_a_stale_candidate_after_successful_delete() { + let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let backend: WarmBackendImpl = Box::new(RecordingProbeBackend { + get_versions: Arc::new(tokio::sync::Mutex::new(Vec::new())), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), + removed_versions: removed_versions.clone(), + remove_clears_candidate: false, + fail_get: false, + body: ProbeBody::Exact, + }); + + let err = check_warm_backend(Some(&backend)) + .await + .expect_err("cleanup must not succeed while the deleted candidate remains visible"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert!(err.message.contains("cleanup is incomplete")); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); + } + + #[tokio::test(start_paused = true)] + async fn check_warm_backend_reconciles_a_lost_put_response() { + let backend = MockWarmBackend::new(); + backend.lose_next_put_response(); + let driver: WarmBackendImpl = Box::new(backend.clone()); + + let err = check_warm_backend(Some(&driver)) + .await + .expect_err("a lost probe PUT response must fail after compensation"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert_eq!(backend.object_count().await, 0); + assert_eq!(backend.exact_remove_count(), 1); + let operations = backend.op_log().await; + let put = operations.iter().find_map(|operation| match operation { + MockWarmOp::Put { object } => Some(object), + _ => None, + }); + let probe = operations.iter().find_map(|operation| match operation { + MockWarmOp::Probe { object } => Some(object), + _ => None, + }); + let remove = operations.iter().find_map(|operation| match operation { + MockWarmOp::Remove { object } => Some(object), + _ => None, + }); + let (Some(put), Some(probe), Some(remove)) = (put, probe, remove) else { + panic!("lost-response compensation should PUT, probe, and remove"); + }; + assert_eq!(put, probe); + assert_eq!(probe, remove); + } + + #[tokio::test(start_paused = true)] + async fn check_warm_backend_retries_until_a_late_put_becomes_visible() { + let probes = Arc::new(AtomicUsize::new(0)); + let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let driver: WarmBackendImpl = Box::new(LateVisibleProbeBackend { + visible_at: tokio::time::Instant::now() + Duration::from_secs(5), + removed: Arc::new(std::sync::atomic::AtomicBool::new(false)), + probes: probes.clone(), + removed_versions: removed_versions.clone(), + }); + + let err = check_warm_backend(Some(&driver)) + .await + .expect_err("a late-visible probe PUT must still report the lost response"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert!( + probes.load(Ordering::SeqCst) > 5, + "reconciliation must not stop at the first Missing result" + ); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); + } + + #[tokio::test] + async fn check_warm_backend_reports_incomplete_cleanup_without_guessing() { + for candidate in [TransitionCandidateProbe::Unsupported, TransitionCandidateProbe::Ambiguous] { + let backend = MockWarmBackend::new(); + backend.set_transition_candidate_probe_override(Some(candidate)).await; + backend.lose_next_put_response(); + let driver: WarmBackendImpl = Box::new(backend.clone()); + + let err = check_warm_backend(Some(&driver)) + .await + .expect_err("an uncertain candidate must fail without a guessed delete"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert!(err.message.contains("cleanup is incomplete")); + assert_eq!(backend.remove_count().await, 0); + assert_eq!(backend.object_count().await, 1); + } + } + + #[tokio::test] + async fn check_warm_backend_reports_an_exact_cleanup_failure() { + let backend = MockWarmBackend::new(); + backend.set_remove_failure(true); + backend.lose_next_put_response(); + let driver: WarmBackendImpl = Box::new(backend.clone()); + + let err = check_warm_backend(Some(&driver)) + .await + .expect_err("an exact cleanup failure must replace the ambiguous PUT error"); + + assert_eq!(err.code, ERR_TIER_PERM_ERR.code); + assert!(err.message.contains("cleanup is incomplete")); + assert_eq!(backend.exact_remove_count(), 1); + assert_eq!(backend.object_count().await, 1); + } + + #[tokio::test(start_paused = true)] + async fn check_warm_backend_reconciles_a_timed_out_put() { + let put_started = Arc::new(tokio::sync::Notify::new()); + let probes = Arc::new(AtomicUsize::new(0)); + let removed_versions = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let driver: WarmBackendImpl = Box::new(HangingProbePutBackend { + put_started: put_started.clone(), + present: Arc::new(std::sync::atomic::AtomicBool::new(true)), + probes: probes.clone(), + removed_versions: removed_versions.clone(), + }); + let check = check_warm_backend(Some(&driver)); + tokio::pin!(check); + tokio::select! { + _ = put_started.notified() => {} + result = &mut check => panic!("probe completed before the PUT timeout: {result:?}"), + } + + tokio::time::advance(WARM_BACKEND_PROBE_TIMEOUT + Duration::from_millis(1)).await; + let err = check.await.expect_err("a timed out probe PUT must fail after compensation"); + + assert_eq!(err.code, ERR_TIER_BACKEND_IN_USE.code); + assert!( + probes.load(Ordering::SeqCst) > 1, + "timed-out PUT reconciliation must keep checking through the visibility window" + ); + assert_eq!(removed_versions.lock().await.as_slice(), [PROBE_VERSION]); + } + #[tokio::test] async fn new_wasabi_backend_honors_probe_flag() { let tier = TierConfig { @@ -1298,6 +1822,15 @@ mod tests { assert_eq!(insecure.client.endpoint_url.port_or_known_default(), Some(80)); } + #[test] + fn endpoint_authority_preserves_ipv6_brackets_and_explicit_port() { + let url = url::Url::parse("https://[2001:db8::1]:9443").expect("the IPv6 endpoint should parse"); + assert_eq!( + endpoint_authority(&url).expect("the endpoint should have an authority"), + "[2001:db8::1]:9443" + ); + } + #[tokio::test] async fn s3_compatible_backend_strips_only_a_trailing_prefix_separator() { let mut params = s3_compatible_params("http://tier.example.com:9000"); diff --git a/crates/ecstore/src/services/tier/warm_backend_aliyun.rs b/crates/ecstore/src/services/tier/warm_backend_aliyun.rs index e848f4f29..27fb7decd 100644 --- a/crates/ecstore/src/services/tier/warm_backend_aliyun.rs +++ b/crates/ecstore/src/services/tier/warm_backend_aliyun.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use crate::services::tier::{ tier_config::TierAliyun, warm_backend::{ - S3CompatibleWarmBackendParams, WarmBackend, WarmBackendGetOpts, build_transition_put_options, + S3CompatibleWarmBackendParams, TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts, build_transition_put_options, new_s3_compatible_warm_backend, optimal_part_size, }, warm_backend_s3::WarmBackendS3, @@ -89,6 +89,10 @@ impl WarmBackend for WarmBackendAliyun { self.0.remove(object, rv).await } + async fn probe_transition_candidate(&self, object: &str) -> Result { + self.0.probe_transition_candidate(object).await + } + async fn in_use(&self) -> Result { self.0.in_use().await } diff --git a/crates/ecstore/src/services/tier/warm_backend_azure.rs b/crates/ecstore/src/services/tier/warm_backend_azure.rs index 7b3723eac..ed9ef0cf8 100644 --- a/crates/ecstore/src/services/tier/warm_backend_azure.rs +++ b/crates/ecstore/src/services/tier/warm_backend_azure.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use crate::services::tier::{ tier_config::TierAzure, warm_backend::{ - S3CompatibleWarmBackendParams, WarmBackend, WarmBackendGetOpts, build_transition_put_options, + S3CompatibleWarmBackendParams, TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts, build_transition_put_options, new_s3_compatible_warm_backend, optimal_part_size, }, warm_backend_s3::WarmBackendS3, @@ -89,6 +89,16 @@ impl WarmBackend for WarmBackendAzure { self.0.remove(object, rv).await } + async fn probe_transition_candidate(&self, object: &str) -> Result { + // Azure currently uses the shared S3/SigV4 transport, but its normal + // object path cannot persist exact remote versions across mixed + // RustFS releases. The mutation probe may still detect and precisely + // remove a versioned test object before rejecting that configuration. + self.0 + .probe_transition_candidate_with_raw_version_header(object, "x-amz-version-id") + .await + } + async fn in_use(&self) -> Result { self.0.in_use().await } @@ -98,6 +108,23 @@ impl WarmBackend for WarmBackendAzure { mod tests { use super::*; use crate::services::tier::tier_config::TierAzure; + use rustfs_s3_client::{ + credentials::{Credentials, SignatureType, Static, Value}, + transition_api::{Options, TransitionClient, TransitionCore}, + }; + use std::sync::Arc; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + async fn read_request_head(stream: &mut tokio::net::TcpStream) -> String { + let mut request = Vec::new(); + let mut buffer = [0_u8; 1024]; + while !request.windows(4).any(|window| window == b"\r\n\r\n") { + let read = stream.read(&mut buffer).await.expect("fixture request should be readable"); + assert_ne!(read, 0, "connection closed before request headers were received"); + request.extend_from_slice(&buffer[..read]); + } + String::from_utf8_lossy(&request).into_owned() + } /// The SSRF guard itself is exercised once, generically, in /// `warm_backend::tests` (see backlog#2040/backlog#2041 and @@ -119,4 +146,85 @@ mod tests { Err(err) => assert!(err.to_string().contains("not allowed")), } } + + #[tokio::test] + async fn versioned_candidate_cleanup_uses_the_exact_s3_version_without_enabling_data_versions() { + let listener = match tokio::net::TcpListener::bind("127.0.0.1:0").await { + Ok(listener) => listener, + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return, + Err(err) => panic!("test listener should bind: {err}"), + }; + let endpoint = listener + .local_addr() + .expect("listener local address should be available") + .to_string(); + let fixture = tokio::spawn(async move { + let (mut get_stream, _) = listener.accept().await.expect("fixture should accept candidate GET"); + let get_request = read_request_head(&mut get_stream).await; + get_stream + .write_all( + b"HTTP/1.1 206 Partial Content\r\nContent-Length: 1\r\nx-amz-version-id: azure-version\r\nConnection: close\r\n\r\nx", + ) + .await + .expect("fixture should write candidate GET response"); + + let (mut delete_stream, _) = listener.accept().await.expect("fixture should accept exact DELETE"); + let delete_request = read_request_head(&mut delete_stream).await; + delete_stream + .write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .await + .expect("fixture should write exact DELETE response"); + (get_request, delete_request) + }); + let client = Arc::new( + TransitionClient::new( + &endpoint, + Options { + creds: Credentials::new(Static(Value { + access_key_id: "access-key".to_string(), + secret_access_key: "secret-key".to_string(), + signer_type: SignatureType::SignatureV4, + ..Default::default() + })), + region: "us-east-1".to_string(), + bucket_lookup: BucketLookupType::BucketLookupPath, + max_retries: 1, + ..Default::default() + }, + "azure", + ) + .await + .expect("fixture client should build"), + ); + let backend = WarmBackendAzure(WarmBackendS3 { + core: TransitionCore(Arc::clone(&client)), + client, + bucket: "bucket".to_string(), + prefix: String::new(), + storage_class: String::new(), + }); + assert!( + !backend.0.client.provider_version_capabilities().exact_get_delete, + "probe-only version discovery must not change Azure's persisted data-path contract" + ); + + let candidate = backend + .probe_transition_candidate("probe") + .await + .expect("Azure candidate should be discovered"); + assert_eq!(candidate, TransitionCandidateProbe::VersionedPresent("azure-version".to_string())); + backend + .remove_exact("probe", "azure-version") + .await + .expect("Azure candidate should be deleted by exact version"); + + let (get_request, delete_request) = fixture.await.expect("fixture should join"); + assert!(get_request.to_ascii_lowercase().contains("\r\nrange: bytes=0-0\r\n")); + assert!( + delete_request + .lines() + .next() + .is_some_and(|line| line.contains("DELETE /bucket/probe?versionId=azure-version ")) + ); + } } diff --git a/crates/ecstore/src/services/tier/warm_backend_gcs.rs b/crates/ecstore/src/services/tier/warm_backend_gcs.rs index 63c9a62c1..03c2df988 100644 --- a/crates/ecstore/src/services/tier/warm_backend_gcs.rs +++ b/crates/ecstore/src/services/tier/warm_backend_gcs.rs @@ -18,13 +18,13 @@ #![allow(unused_must_use)] #![allow(clippy::all)] -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; +use std::future::Future; use std::io::{Error, ErrorKind}; use std::sync::Arc; use bytes::Bytes; -use google_cloud_auth::credentials::Credentials; -use google_cloud_auth::credentials::user_account::Builder; +use google_cloud_auth::credentials::service_account::Builder; use google_cloud_storage as gcs; use google_cloud_storage::client::Storage; use google_cloud_storage::client::StorageControl; @@ -32,7 +32,7 @@ use std::convert::TryFrom; use crate::services::tier::{ tier_config::TierGCS, - warm_backend::{WarmBackend, WarmBackendGetOpts}, + warm_backend::{TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts}, }; use rustfs_s3_client::{ admin_handler_utils::AdminError, @@ -43,6 +43,7 @@ use rustfs_utils::egress::validate_outbound_url; use tracing::warn; const _MAX_PART_SIZE: i64 = 1024 * 1024 * 1024 * 5; +const MAX_GCS_CANDIDATE_PAGES: usize = 64; fn parse_generation(remote_version: &str) -> Result, Error> { if remote_version.is_empty() { @@ -57,6 +58,85 @@ fn parse_generation(remote_version: &str) -> Result, Error> { Ok(Some(generation)) } +fn append_gcs_chunk( + contents: &mut Vec, + chunk: Result, + max_response_bytes: Option, +) -> std::io::Result<()> { + let chunk = chunk.map_err(|err| std::io::Error::other(err.to_string()))?; + if max_response_bytes.is_some_and(|limit| contents.len().saturating_add(chunk.len()) > limit) { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "GCS object response exceeded the configured byte limit", + )); + } + contents.extend_from_slice(&chunk); + Ok(()) +} + +fn gcs_bucket_resource_name(bucket: &str) -> String { + format!("projects/_/buckets/{bucket}") +} + +struct GcsCandidateObject { + name: String, + generation: i64, +} + +struct GcsCandidatePage { + objects: Vec, + next_page_token: String, +} + +async fn probe_exact_gcs_candidate( + remote_object: &str, + mut fetch_page: F, +) -> Result +where + F: FnMut(String) -> Fut, + Fut: Future>, +{ + let mut page_token = String::new(); + let mut seen_page_tokens = HashSet::new(); + let mut generation = None; + let mut pages_seen = 0_usize; + + loop { + pages_seen += 1; + if pages_seen > MAX_GCS_CANDIDATE_PAGES { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "GCS candidate listing exceeded the page limit", + )); + } + let response = fetch_page(page_token.clone()).await?; + for candidate in response.objects.iter().filter(|candidate| candidate.name == remote_object) { + if candidate.generation <= 0 { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "GCS candidate listing returned a non-positive generation", + )); + } + if generation.replace(candidate.generation).is_some() { + return Ok(TransitionCandidateProbe::Ambiguous); + } + } + + if response.next_page_token.is_empty() { + break; + } + if !seen_page_tokens.insert(response.next_page_token.clone()) { + return Err(std::io::Error::new(ErrorKind::InvalidData, "GCS candidate listing repeated a page token")); + } + page_token = response.next_page_token; + } + + Ok(match generation { + Some(generation) => TransitionCandidateProbe::VersionedPresent(generation.to_string()), + None => TransitionCandidateProbe::Missing, + }) +} + pub struct WarmBackendGCS { pub client: Arc, pub control: Arc, @@ -80,8 +160,8 @@ impl WarmBackendGCS { .map_err(|err| std::io::Error::other(format!("tier endpoint is not allowed: {err}")))?; } - let authorized_user = serde_json::from_str(&conf.creds)?; - let credentials = Builder::new(authorized_user) + let service_account = serde_json::from_str(&conf.creds)?; + let credentials = Builder::new(service_account) //.with_retry_policy(AlwaysRetry.with_attempt_limit(3)) //.with_backoff_policy(backoff) .build() @@ -98,7 +178,11 @@ impl WarmBackendGCS { let client = Arc::new(client); // Control-plane client: the data-plane `Storage` client cannot delete or list objects; // delete_object/list_objects live on StorageControl. - let Ok(control) = StorageControl::builder().with_credentials(credentials).build().await else { + let mut control_builder = StorageControl::builder().with_credentials(credentials); + if !conf.endpoint.is_empty() { + control_builder = control_builder.with_endpoint(conf.endpoint.clone()); + } + let Ok(control) = control_builder.build().await else { return Err(std::io::Error::other("StorageControl::builder error")); }; let control = Arc::new(control); @@ -136,9 +220,10 @@ impl WarmBackend for WarmBackendGCS { ReaderImpl::Body(content_body) => content_body.to_vec(), ReaderImpl::ObjectBody(mut content_body) => content_body.read_all().await?, }; + let bucket = gcs_bucket_resource_name(&self.bucket); let Ok(res) = Box::pin( self.client - .write_object(&self.bucket, &self.get_dest(object), Bytes::from(d)) + .write_object(&bucket, &self.get_dest(object), Bytes::from(d)) .send_buffered(), ) .await @@ -154,7 +239,9 @@ impl WarmBackend for WarmBackendGCS { } async fn get(&self, object: &str, rv: &str, opts: WarmBackendGetOpts) -> Result { - let mut req = self.client.read_object(&self.bucket, &self.get_dest(object)); + let bucket = gcs_bucket_resource_name(&self.bucket); + let mut req = self.client.read_object(&bucket, &self.get_dest(object)); + let mut max_response_bytes = None; if let Some(generation) = parse_generation(rv)? { req = req.set_generation(generation); } @@ -170,6 +257,11 @@ impl WarmBackend for WarmBackendGCS { .length .try_into() .map_err(|_| std::io::Error::other("invalid range: negative length"))?; + max_response_bytes = Some( + opts.length + .try_into() + .map_err(|_| std::io::Error::other("invalid range: length does not fit in memory"))?, + ); req = req.set_read_range(google_cloud_storage::model_ext::ReadRange::segment(offset, count)); } @@ -177,8 +269,8 @@ impl WarmBackend for WarmBackendGCS { return Err(std::io::Error::other("read_object error")); }; let mut contents = Vec::new(); - while let Ok(Some(chunk)) = reader.next().await.transpose() { - contents.extend_from_slice(&chunk); + while let Some(chunk) = reader.next().await { + append_gcs_chunk(&mut contents, chunk, max_response_bytes)?; } Ok(ReadCloser::new(std::io::Cursor::new(contents))) } @@ -190,7 +282,7 @@ impl WarmBackend for WarmBackendGCS { let mut req = self .control .delete_object() - .set_bucket(format!("projects/_/buckets/{}", self.bucket)) + .set_bucket(gcs_bucket_resource_name(&self.bucket)) .set_object(self.get_dest(object)); if let Some(generation) = parse_generation(rv)? { req = req.set_generation(generation); @@ -199,13 +291,47 @@ impl WarmBackend for WarmBackendGCS { Ok(()) } + async fn probe_transition_candidate(&self, object: &str) -> Result { + let remote_object = self.get_dest(object); + let parent = gcs_bucket_resource_name(&self.bucket); + probe_exact_gcs_candidate(&remote_object, |page_token| { + let control = self.control.clone(); + let parent = parent.clone(); + let prefix = remote_object.clone(); + async move { + let response = control + .list_objects() + .set_parent(parent) + .set_prefix(prefix) + .set_versions(true) + .set_page_size(2) + .set_page_token(page_token) + .send() + .await + .map_err(|err| std::io::Error::other(err.to_string()))?; + Ok(GcsCandidatePage { + objects: response + .objects + .into_iter() + .map(|candidate| GcsCandidateObject { + name: candidate.name, + generation: candidate.generation, + }) + .collect(), + next_page_token: response.next_page_token, + }) + } + }) + .await + } + async fn in_use(&self) -> Result { // Scope the listing to this tier's prefix (matching the other warm backends) and only // need to know whether a single object exists. let resp = self .control .list_objects() - .set_parent(format!("projects/_/buckets/{}", self.bucket)) + .set_parent(gcs_bucket_resource_name(&self.bucket)) .set_prefix(self.prefix.clone()) .set_page_size(1) .send() @@ -218,10 +344,126 @@ impl WarmBackend for WarmBackendGCS { #[cfg(test)] mod tests { + use super::GcsCandidateObject; + use super::GcsCandidatePage; + use super::MAX_GCS_CANDIDATE_PAGES; use super::WarmBackendGCS; + use super::append_gcs_chunk; + use super::gcs_bucket_resource_name; use super::parse_generation; + use super::probe_exact_gcs_candidate; use crate::services::tier::tier_config::TierGCS; + use crate::services::tier::warm_backend::{TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts}; + use google_cloud_auth::credentials::anonymous::Builder as Anonymous; + use google_cloud_storage::client::{Storage, StorageControl}; use std::io::ErrorKind; + use std::sync::Arc; + use std::time::Duration; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + async fn serve_data_plane_fixture(listener: TcpListener) -> Vec { + let upload_body = r#"{"name":"probe","bucket":"tier-bucket","generation":"123"}"#; + let responses = [ + format!( + "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{upload_body}", + upload_body.len() + ), + "HTTP/1.1 206 Partial Content\r\ncontent-type: application/octet-stream\r\ncontent-range: bytes 0-6/7\r\nx-goog-generation: 123\r\ncontent-length: 7\r\nconnection: close\r\n\r\nRustFS!" + .to_string(), + "HTTP/1.1 206 Partial Content\r\ncontent-type: application/octet-stream\r\ncontent-range: bytes 0-7/8\r\nx-goog-generation: 123\r\ncontent-length: 8\r\nconnection: close\r\n\r\nRustFS!!" + .to_string(), + ]; + let mut requests = Vec::new(); + + for response in responses { + let (mut stream, _) = listener.accept().await.expect("the GCS fixture should accept a request"); + let mut request = Vec::new(); + loop { + let mut chunk = [0_u8; 1024]; + let count = stream + .read(&mut chunk) + .await + .expect("the GCS fixture should read request headers"); + if count == 0 { + break; + } + request.extend_from_slice(&chunk[..count]); + if request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + let header_end = request + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|position| position + 4) + .expect("the GCS fixture should receive complete request headers"); + let headers = String::from_utf8_lossy(&request[..header_end]); + if headers.lines().any(|line| line.eq_ignore_ascii_case("expect: 100-continue")) { + stream + .write_all(b"HTTP/1.1 100 Continue\r\n\r\n") + .await + .expect("the GCS fixture should acknowledge 100-continue"); + } + let content_length = headers + .lines() + .find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().expect("content-length should be numeric")) + }) + .unwrap_or_default(); + while request.len() < header_end.saturating_add(content_length) { + let mut chunk = [0_u8; 1024]; + let count = stream + .read(&mut chunk) + .await + .expect("the GCS fixture should read the request body"); + if count == 0 { + break; + } + request.extend_from_slice(&chunk[..count]); + } + requests.push(String::from_utf8_lossy(&request).into_owned()); + stream + .write_all(response.as_bytes()) + .await + .expect("the GCS fixture should write its response"); + } + + requests + } + + fn candidate_page(objects: &[(&str, i64)], next_page_token: &str) -> GcsCandidatePage { + GcsCandidatePage { + objects: objects + .iter() + .map(|(name, generation)| GcsCandidateObject { + name: (*name).to_string(), + generation: *generation, + }) + .collect(), + next_page_token: next_page_token.to_string(), + } + } + + async fn probe_candidate_pages( + remote_object: &str, + pages: Vec, + ) -> (Result, Vec) { + let mut pages = pages.into_iter(); + let mut requested_tokens = Vec::new(); + let result = probe_exact_gcs_candidate(remote_object, |page_token| { + requested_tokens.push(page_token); + std::future::ready( + pages + .next() + .ok_or_else(|| std::io::Error::new(ErrorKind::UnexpectedEof, "test fixture ran out of GCS pages")), + ) + }) + .await; + (result, requested_tokens) + } #[test] fn generation_parser_preserves_exact_numeric_versions() { @@ -241,6 +483,243 @@ mod tests { } } + #[test] + fn body_collection_propagates_an_error_after_a_complete_prefix() { + let mut contents = Vec::new(); + append_gcs_chunk::(&mut contents, Ok(bytes::Bytes::from_static(b"RustFS")), Some(7)) + .expect("the prefix chunk should be collected"); + let err = append_gcs_chunk(&mut contents, Err(std::io::Error::other("trailing stream failure")), Some(7)) + .expect_err("a trailing stream error must not be mistaken for EOF"); + + assert_eq!(contents, b"RustFS"); + assert!(err.to_string().contains("trailing stream failure")); + } + + #[test] + fn body_collection_rejects_a_chunk_that_exceeds_the_probe_limit() { + let mut contents = Vec::new(); + let err = append_gcs_chunk::(&mut contents, Ok(bytes::Bytes::from_static(b"RustFSxx")), Some(7)) + .expect_err("the GCS collection layer must reject an oversized probe response"); + + assert!(contents.is_empty()); + assert_eq!(err.kind(), ErrorKind::InvalidData); + } + + #[tokio::test] + async fn candidate_probe_finds_exact_name_on_first_or_later_page() { + let (first, first_tokens) = + probe_candidate_pages("prefix/object", vec![candidate_page(&[("prefix/object", 7)], "")]).await; + assert_eq!( + first.expect("an exact first-page object should be discovered"), + TransitionCandidateProbe::VersionedPresent("7".to_string()) + ); + assert_eq!(first_tokens, [""]); + + let (later, later_tokens) = probe_candidate_pages( + "prefix/object", + vec![ + candidate_page(&[("prefix/object-shadow", 8)], "next"), + candidate_page(&[("prefix/object", 9)], ""), + ], + ) + .await; + assert_eq!( + later.expect("an exact later-page object should be discovered"), + TransitionCandidateProbe::VersionedPresent("9".to_string()) + ); + assert_eq!(later_tokens, ["", "next"]); + } + + #[tokio::test] + async fn candidate_probe_ignores_non_exact_prefix_matches() { + let (probe, _) = probe_candidate_pages( + "prefix/object", + vec![candidate_page( + &[("prefix/object-shadow", 8), ("prefix/object/child", 9), ("prefix/object", 7)], + "", + )], + ) + .await; + + assert_eq!( + probe.expect("prefix-only matches should not hide the exact object"), + TransitionCandidateProbe::VersionedPresent("7".to_string()) + ); + } + + #[tokio::test] + async fn candidate_probe_reports_duplicate_exact_names_as_ambiguous() { + let (probe, _) = probe_candidate_pages( + "prefix/object", + vec![ + candidate_page(&[("prefix/object", 7)], "next"), + candidate_page(&[("prefix/object", 8)], ""), + ], + ) + .await; + + assert_eq!( + probe.expect("multiple exact generations should produce a conservative result"), + TransitionCandidateProbe::Ambiguous + ); + } + + #[tokio::test] + async fn candidate_probe_reports_missing_without_an_exact_name() { + let (probe, _) = probe_candidate_pages("prefix/object", vec![candidate_page(&[("prefix/object-shadow", 8)], "")]).await; + + assert_eq!( + probe.expect("a complete listing without an exact name should be definitive"), + TransitionCandidateProbe::Missing + ); + } + + #[tokio::test] + async fn candidate_probe_rejects_non_positive_generations() { + for generation in [0, -1] { + let (probe, _) = + probe_candidate_pages("prefix/object", vec![candidate_page(&[("prefix/object", generation)], "")]).await; + let err = probe.expect_err("a non-positive GCS generation must fail closed"); + assert_eq!(err.kind(), ErrorKind::InvalidData, "generation {generation}"); + } + } + + #[tokio::test] + async fn candidate_probe_rejects_a_page_token_that_does_not_advance() { + let (probe, requested_tokens) = + probe_candidate_pages("prefix/object", vec![candidate_page(&[], "next"), candidate_page(&[], "next")]).await; + + let err = probe.expect_err("a repeated GCS page token must fail closed"); + assert_eq!(err.kind(), ErrorKind::InvalidData); + assert_eq!(requested_tokens, ["", "next"]); + } + + #[tokio::test] + async fn candidate_probe_rejects_a_non_adjacent_page_token_cycle() { + let (probe, requested_tokens) = probe_candidate_pages( + "prefix/object", + vec![candidate_page(&[], "a"), candidate_page(&[], "b"), candidate_page(&[], "a")], + ) + .await; + + let err = probe.expect_err("a non-adjacent GCS page token cycle must fail closed"); + assert_eq!(err.kind(), ErrorKind::InvalidData); + assert_eq!(requested_tokens, ["", "a", "b"]); + } + + #[tokio::test] + async fn candidate_probe_rejects_an_unbounded_unique_token_chain() { + let pages = (0..MAX_GCS_CANDIDATE_PAGES) + .map(|index| candidate_page(&[], &format!("token-{index}"))) + .collect(); + let (probe, requested_tokens) = probe_candidate_pages("prefix/object", pages).await; + + let err = probe.expect_err("an unbounded unique page-token chain must fail closed"); + assert_eq!(err.kind(), ErrorKind::InvalidData); + assert_eq!(requested_tokens.len(), MAX_GCS_CANDIDATE_PAGES); + } + + #[tokio::test] + async fn plain_bucket_reaches_gcs_put_and_get_transport_with_resource_name() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("the GCS fixture should bind a loopback port"); + let endpoint = format!("http://{}", listener.local_addr().expect("the GCS fixture should have a local address")); + let fixture = tokio::spawn(serve_data_plane_fixture(listener)); + let credentials = Anonymous::new().build(); + let client = Storage::builder() + .with_endpoint(endpoint.clone()) + .with_credentials(credentials.clone()) + .build() + .await + .expect("the GCS data client should build"); + let control = StorageControl::builder() + .with_endpoint(endpoint) + .with_credentials(credentials) + .build() + .await + .expect("the GCS control client should build"); + let backend = WarmBackendGCS { + client: Arc::new(client), + control: Arc::new(control), + bucket: "tier-bucket".to_string(), + prefix: String::new(), + }; + + let (version, body, oversized_error_kind, requests) = tokio::time::timeout(Duration::from_secs(5), async { + let version = backend + .put( + "probe", + rustfs_s3_client::transition_api::ReaderImpl::Body(bytes::Bytes::from_static(b"RustFS")), + 6, + ) + .await + .expect("a plain configured bucket should reach the GCS upload transport"); + let mut reader = backend + .get( + "probe", + &version, + WarmBackendGetOpts { + start_offset: 0, + length: 7, + }, + ) + .await + .expect("a plain configured bucket should reach the GCS read transport"); + let mut body = Vec::new(); + reader + .read_to_end(&mut body) + .await + .expect("the fixture body should be readable"); + let oversized_error = match backend + .get( + "probe", + &version, + WarmBackendGetOpts { + start_offset: 0, + length: 7, + }, + ) + .await + { + Ok(_) => panic!("an eight-byte response must not pass a seven-byte collection limit"), + Err(err) => err, + }; + let requests = fixture.await.expect("the GCS fixture task should finish"); + (version, body, oversized_error.kind(), requests) + }) + .await + .expect("the GCS data-plane requests should not be rejected before transport"); + + assert_eq!(gcs_bucket_resource_name("tier-bucket"), "projects/_/buckets/tier-bucket"); + assert_eq!(version, "123"); + assert_eq!(body, b"RustFS!"); + assert_eq!(oversized_error_kind, ErrorKind::InvalidData); + assert!( + requests[0].starts_with("POST /upload/storage/v1/b/tier-bucket/o?"), + "unexpected upload request line: {}", + requests[0].lines().next().unwrap_or_default() + ); + assert!( + requests[1].starts_with("GET /storage/v1/b/tier-bucket/o/probe?"), + "unexpected read request line: {}", + requests[1].lines().next().unwrap_or_default() + ); + assert!( + requests[1].to_ascii_lowercase().contains("\r\nrange: bytes=0-6\r\n"), + "the GCS probe read must preserve its seven-byte range" + ); + assert!( + requests[2].starts_with("GET /storage/v1/b/tier-bucket/o/probe?"), + "unexpected oversized read request line: {}", + requests[2].lines().next().unwrap_or_default() + ); + assert!( + requests[2].to_ascii_lowercase().contains("\r\nrange: bytes=0-6\r\n"), + "the oversized response must be fetched under the same seven-byte request boundary" + ); + } + #[tokio::test] async fn new_rejects_loopback_endpoint_before_credential_setup() { let conf = TierGCS { diff --git a/crates/ecstore/src/services/tier/warm_backend_huaweicloud.rs b/crates/ecstore/src/services/tier/warm_backend_huaweicloud.rs index f4f489e9c..73ced2cef 100644 --- a/crates/ecstore/src/services/tier/warm_backend_huaweicloud.rs +++ b/crates/ecstore/src/services/tier/warm_backend_huaweicloud.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use crate::services::tier::{ tier_config::TierHuaweicloud, warm_backend::{ - S3CompatibleWarmBackendParams, WarmBackend, WarmBackendGetOpts, build_transition_put_options, + S3CompatibleWarmBackendParams, TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts, build_transition_put_options, new_s3_compatible_warm_backend, optimal_part_size, }, warm_backend_s3::WarmBackendS3, @@ -89,6 +89,10 @@ impl WarmBackend for WarmBackendHuaweicloud { self.0.remove(object, rv).await } + async fn probe_transition_candidate(&self, object: &str) -> Result { + self.0.probe_transition_candidate(object).await + } + async fn in_use(&self) -> Result { self.0.in_use().await } diff --git a/crates/ecstore/src/services/tier/warm_backend_s3.rs b/crates/ecstore/src/services/tier/warm_backend_s3.rs index 55a29418c..5462fc52c 100644 --- a/crates/ecstore/src/services/tier/warm_backend_s3.rs +++ b/crates/ecstore/src/services/tier/warm_backend_s3.rs @@ -26,11 +26,12 @@ use crate::services::tier::{ tier_config::TierS3, warm_backend::{ TransitionCandidateIdentity, TransitionCandidateProbe, TransitionCandidateReconciler, WarmBackend, WarmBackendGetOpts, - build_transition_put_options, + build_transition_put_options, endpoint_authority, }, }; use http::HeaderMap; use rustfs_s3_client::{ + api_error_response::to_error_response, api_get_options::GetObjectOptions, api_list::ListObjectsOptions, api_put_object::PutObjectOptions, @@ -43,7 +44,7 @@ use rustfs_s3_client::{ }; use rustfs_utils::egress::validate_outbound_url; use rustfs_utils::path::SLASH_SEPARATOR; -use s3s::dto::BucketVersioningStatus; +use s3s::{S3ErrorCode, dto::BucketVersioningStatus}; pub struct WarmBackendS3 { pub client: Arc, @@ -74,6 +75,19 @@ fn remote_bucket_versioning_from_status(status: Option<&str>) -> Result Result, std::io::Error> { + if opts.start_offset < 0 || opts.length <= 0 { + return Ok(None); + } + usize::try_from(opts.length) + .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid range: length does not fit in memory"))?; + let end_offset = opts + .start_offset + .checked_add(opts.length - 1) + .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid range: end offset overflow"))?; + Ok(Some((opts.start_offset, end_offset))) +} + impl WarmBackendS3 { pub async fn new(conf: &TierS3, _tier: &str) -> Result { Self::new_with_bucket_lookup(conf, BucketLookupType::BucketLookupAuto, "s3").await @@ -132,10 +146,8 @@ impl WarmBackendS3 { bucket_lookup, ..Default::default() }; - let host = u - .host() - .ok_or_else(|| std::io::Error::other("Invalid endpoint URL: missing host"))?; - let client = TransitionClient::new(&host.to_string(), opts, tier_type).await?; + let endpoint = endpoint_authority(&u)?; + let client = TransitionClient::new(&endpoint, opts, tier_type).await?; let client = Arc::new(client); let core = TransitionCore(Arc::clone(&client)); @@ -177,10 +189,8 @@ impl WarmBackendS3 { if !rv.is_empty() { gopts.version_id = rv.to_string(); } - if opts.start_offset >= 0 && opts.length > 0 { - gopts - .set_range(opts.start_offset, opts.start_offset + opts.length - 1) - .map_err(std::io::Error::other)?; + if let Some((start_offset, end_offset)) = bounded_get_range(&opts)? { + gopts.set_range(start_offset, end_offset)?; } let (_, headers, reader) = self.core.get_object(&self.bucket, &self.get_dest(object), &gopts).await?; Ok((headers, reader)) @@ -191,36 +201,64 @@ impl WarmBackendS3 { remote_bucket_versioning_from_status(config.status.as_ref().map(|status| status.as_str())) } - async fn probe_transition_candidate_versions( + async fn probe_current_transition_candidate_with_header( &self, object: &str, - bucket_versioning: RemoteBucketVersioning, + raw_version_header: Option<&'static str>, ) -> Result { - let remote_object = self.get_dest(object); - let mut opts = ListObjectsOptions::default(); - opts.set("prefix", &remote_object); - opts.set("max-keys", "1000"); - - let mut key_marker = String::new(); - let mut version_id_marker = String::new(); - let mut candidates = TransitionCandidateVersions::default(); - loop { - let versions = self - .client - .list_object_versions_query(&self.bucket, &opts, &key_marker, &version_id_marker, "") - .await?; - candidates.extend(&remote_object, &versions); - if candidates.is_ambiguous() { - return Ok(TransitionCandidateProbe::Ambiguous); + match self + .get_with_headers( + object, + "", + WarmBackendGetOpts { + start_offset: 0, + length: 1, + }, + ) + .await + { + Ok((headers, _)) => { + let version_id = match raw_version_header { + Some(header_name) => match headers.get(header_name) { + Some(value) => { + let version_id = value.to_str().map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "remote object version id is not valid ASCII", + ) + })?; + validate_remote_version_id(version_id)?; + Some(version_id) + } + None => None, + }, + None => self.client.raw_version_id(&headers)?, + }; + Ok(match version_id { + Some(version_id) => TransitionCandidateProbe::VersionedPresent(version_id.to_string()), + None => TransitionCandidateProbe::UnversionedPresent, + }) } - if !versions.is_truncated { - return classify_transition_candidates(candidates, bucket_versioning); + Err(err) => { + let response = to_error_response(&err); + if response.code == S3ErrorCode::NoSuchKey { + Ok(TransitionCandidateProbe::Missing) + } else { + Err(err) + } } - - advance_version_markers(&mut key_marker, &mut version_id_marker, &versions)?; } } + pub(crate) async fn probe_transition_candidate_with_raw_version_header( + &self, + object: &str, + raw_version_header: &'static str, + ) -> Result { + self.probe_current_transition_candidate_with_header(object, Some(raw_version_header)) + .await + } + async fn probe_transition_candidate_identity( &self, object: &str, @@ -343,6 +381,7 @@ struct TransitionCandidateVersions { } impl TransitionCandidateVersions { + #[cfg(test)] fn extend(&mut self, remote_object: &str, versions: &ListVersionsResult) { for version in versions.versions.iter().filter(|version| version.key == remote_object) { if self.version_id.is_some() { @@ -353,10 +392,6 @@ impl TransitionCandidateVersions { } } - fn is_ambiguous(&self) -> bool { - self.ambiguous - } - fn classify(self, bucket_versioning: RemoteBucketVersioning) -> TransitionCandidateProbe { if self.ambiguous { return TransitionCandidateProbe::Ambiguous; @@ -380,6 +415,8 @@ impl TransitionCandidateVersions { mod tests { use super::*; use rustfs_s3_client::api_s3_datatypes::{ListVersionsResult, Version}; + use std::time::Duration; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; #[tokio::test] async fn new_rejects_loopback_endpoint_before_network_setup() { @@ -398,6 +435,204 @@ mod tests { } } + #[tokio::test] + async fn new_preserves_an_explicit_endpoint_port() { + let conf = TierS3 { + endpoint: "https://tier.example.com:9443".to_string(), + bucket: "tier-bucket".to_string(), + access_key: "access".to_string(), + secret_key: "secret".to_string(), + region: "us-east-1".to_string(), + ..Default::default() + }; + + let backend = WarmBackendS3::new(&conf, "tier") + .await + .expect("a well-formed S3 endpoint should initialize without network I/O"); + assert_eq!(backend.client.endpoint_url.host_str(), Some("tier.example.com")); + assert_eq!(backend.client.endpoint_url.port(), Some(9443)); + } + + #[tokio::test] + async fn overflowing_get_range_is_rejected_before_network_io() { + let listener = match tokio::net::TcpListener::bind("127.0.0.1:0").await { + Ok(listener) => listener, + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return, + Err(err) => panic!("test listener should bind: {err}"), + }; + let endpoint = listener + .local_addr() + .expect("listener local address should be available") + .to_string(); + let client = Arc::new( + TransitionClient::new( + &endpoint, + Options { + creds: Credentials::new(Static(Value { + access_key_id: "access-key".to_string(), + secret_access_key: "secret-key".to_string(), + signer_type: SignatureType::SignatureV4, + ..Default::default() + })), + region: "us-east-1".to_string(), + bucket_lookup: BucketLookupType::BucketLookupPath, + max_retries: 1, + ..Default::default() + }, + "s3", + ) + .await + .expect("fixture client should build"), + ); + let backend = WarmBackendS3 { + core: TransitionCore(Arc::clone(&client)), + client, + bucket: "bucket".to_string(), + prefix: String::new(), + storage_class: String::new(), + }; + + let err = backend + .get_with_headers( + "probe", + "", + WarmBackendGetOpts { + start_offset: i64::MAX, + length: 2, + }, + ) + .await + .expect_err("an overflowing range must fail before issuing a GET"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + tokio::time::timeout(Duration::from_millis(100), listener.accept()) + .await + .is_err() + ); + } + + async fn candidate_probe_fixture() -> Option<(WarmBackendS3, tokio::task::JoinHandle>)> { + let listener = match tokio::net::TcpListener::bind("127.0.0.1:0").await { + Ok(listener) => listener, + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return None, + Err(err) => panic!("test listener should bind: {err}"), + }; + let endpoint = listener + .local_addr() + .expect("listener local address should be available") + .to_string(); + let fixture = tokio::spawn(async move { + let responses = [ + "HTTP/1.1 206 Partial Content\r\nContent-Length: 1\r\nx-amz-version-id: opaque-version\r\nConnection: close\r\n\r\nx", + "HTTP/1.1 206 Partial Content\r\nContent-Length: 1\r\nConnection: close\r\n\r\nx", + "HTTP/1.1 404 Not Found\r\nContent-Type: application/xml\r\nContent-Length: 63\r\nConnection: close\r\n\r\nNoSuchKeymissing", + "HTTP/1.1 404 Not Found\r\nContent-Type: application/xml\r\nContent-Length: 66\r\nConnection: close\r\n\r\nNoSuchObjectmissing", + "HTTP/1.1 403 Forbidden\r\nContent-Type: application/xml\r\nContent-Length: 65\r\nConnection: close\r\n\r\nAccessDenieddenied", + ]; + let mut requests = Vec::new(); + for response in responses { + let (mut stream, _) = listener.accept().await.expect("fixture should accept candidate GET"); + let mut request = Vec::new(); + let mut buffer = [0; 1024]; + loop { + let read = stream.read(&mut buffer).await.expect("fixture should read request headers"); + assert_ne!(read, 0, "connection closed before request headers were received"); + request.extend_from_slice(&buffer[..read]); + if request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + requests.push(String::from_utf8_lossy(&request).into_owned()); + stream + .write_all(response.as_bytes()) + .await + .expect("fixture should write candidate response"); + } + requests + }); + let client = Arc::new( + TransitionClient::new( + &endpoint, + Options { + creds: Credentials::new(Static(Value { + access_key_id: "access-key".to_string(), + secret_access_key: "secret-key".to_string(), + signer_type: SignatureType::SignatureV4, + ..Default::default() + })), + region: "us-east-1".to_string(), + bucket_lookup: BucketLookupType::BucketLookupPath, + max_retries: 1, + ..Default::default() + }, + "s3", + ) + .await + .expect("fixture client should build"), + ); + Some(( + WarmBackendS3 { + core: TransitionCore(Arc::clone(&client)), + client, + bucket: "bucket".to_string(), + prefix: String::new(), + storage_class: String::new(), + }, + fixture, + )) + } + + #[tokio::test] + async fn candidate_probe_uses_only_exact_bounded_get_permissions() { + let Some((backend, fixture)) = candidate_probe_fixture().await else { + return; + }; + + assert_eq!( + backend + .probe_transition_candidate("versioned-probe") + .await + .expect("versioned candidate should be discovered"), + TransitionCandidateProbe::VersionedPresent("opaque-version".to_string()) + ); + assert_eq!( + backend + .probe_transition_candidate("unversioned-probe") + .await + .expect("unversioned candidate should be discovered"), + TransitionCandidateProbe::UnversionedPresent + ); + assert_eq!( + backend + .probe_transition_candidate("missing-probe") + .await + .expect("a missing key should be classified"), + TransitionCandidateProbe::Missing + ); + assert_eq!( + backend + .probe_transition_candidate("provider-missing-probe") + .await + .expect("a provider-specific missing code should be classified"), + TransitionCandidateProbe::Missing + ); + let err = backend + .probe_transition_candidate("forbidden-probe") + .await + .expect_err("an authorization failure must not be mistaken for a missing key"); + assert_eq!(to_error_response(&err).code, S3ErrorCode::AccessDenied); + + let requests = fixture.await.expect("candidate fixture should join"); + for request in requests { + let request = request.to_ascii_lowercase(); + assert!(request.starts_with("get /bucket/"), "candidate discovery must use object GET"); + assert!(request.contains("\r\nrange: bytes=0-0\r\n")); + assert!(!request.contains("?versioning")); + assert!(!request.contains("?versions")); + } + } + fn list_versions(versions: &[(&str, &str)], delete_markers: &[(&str, &str)], is_truncated: bool) -> ListVersionsResult { ListVersionsResult { versions: versions @@ -631,8 +866,7 @@ impl WarmBackend for WarmBackendS3 { } async fn probe_transition_candidate(&self, object: &str) -> Result { - let bucket_versioning = self.remote_bucket_versioning().await?; - self.probe_transition_candidate_versions(object, bucket_versioning).await + self.probe_current_transition_candidate_with_header(object, None).await } async fn in_use(&self) -> Result { diff --git a/crates/ecstore/src/services/tier/warm_backend_tencent.rs b/crates/ecstore/src/services/tier/warm_backend_tencent.rs index e2810e316..20eb7ee81 100644 --- a/crates/ecstore/src/services/tier/warm_backend_tencent.rs +++ b/crates/ecstore/src/services/tier/warm_backend_tencent.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use crate::services::tier::{ tier_config::TierTencent, warm_backend::{ - S3CompatibleWarmBackendParams, WarmBackend, WarmBackendGetOpts, build_transition_put_options, + S3CompatibleWarmBackendParams, TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts, build_transition_put_options, new_s3_compatible_warm_backend, optimal_part_size, }, warm_backend_s3::WarmBackendS3, @@ -89,6 +89,10 @@ impl WarmBackend for WarmBackendTencent { self.0.remove(object, rv).await } + async fn probe_transition_candidate(&self, object: &str) -> Result { + self.0.probe_transition_candidate(object).await + } + async fn in_use(&self) -> Result { self.0.in_use().await } diff --git a/crates/ecstore/src/services/tier/warm_backend_wasabi.rs b/crates/ecstore/src/services/tier/warm_backend_wasabi.rs index 2311e6576..8398aec52 100644 --- a/crates/ecstore/src/services/tier/warm_backend_wasabi.rs +++ b/crates/ecstore/src/services/tier/warm_backend_wasabi.rs @@ -23,7 +23,7 @@ use uuid::Uuid; use crate::services::tier::{ tier_config::{TierS3, TierWasabi}, - warm_backend::{WarmBackend, WarmBackendGetOpts}, + warm_backend::{TransitionCandidateProbe, WarmBackend, WarmBackendGetOpts}, warm_backend_s3::WarmBackendS3, }; use rustfs_s3_client::transition_api::{BucketLookupType, ReadCloser, ReaderImpl}; @@ -169,6 +169,10 @@ impl WarmBackend for WarmBackendWasabi { self.s3.remove(object, rv).await } + async fn probe_transition_candidate(&self, object: &str) -> io::Result { + self.s3.probe_transition_candidate(object).await + } + async fn in_use(&self) -> io::Result { self.check_remote_bucket_unversioned().await?; let in_use = self.s3.in_use().await?; diff --git a/crates/ecstore/src/store/init.rs b/crates/ecstore/src/store/init.rs index 2f17c7d1a..7037ad605 100644 --- a/crates/ecstore/src/store/init.rs +++ b/crates/ecstore/src/store/init.rs @@ -10339,9 +10339,17 @@ mod tests { } #[cfg(feature = "test-util")] - #[tokio::test] + #[test] #[serial_test::serial(storage_class_env)] - async fn v6_decommission_checkpoint_no_lock_put_rejects_lost_publication_fence() { + fn v6_decommission_checkpoint_no_lock_put_rejects_lost_publication_fence() { + run_large_stack_async_test( + "v6-checkpoint-fence-loss", + v6_decommission_checkpoint_no_lock_put_rejects_lost_publication_fence_case, + ); + } + + #[cfg(feature = "test-util")] + async fn v6_decommission_checkpoint_no_lock_put_rejects_lost_publication_fence_case() { let temp_dir = tempfile::tempdir().expect("create v6 checkpoint fence-loss store dir"); let (_ctx, store, _shutdown) = without_storage_class_env(build_isolated_test_store(temp_dir.path(), "v6-checkpoint-fence-loss", &[4, 4])).await; diff --git a/crates/s3-client/src/api_error_response.rs b/crates/s3-client/src/api_error_response.rs index 8373517e7..7c653d6a8 100644 --- a/crates/s3-client/src/api_error_response.rs +++ b/crates/s3-client/src/api_error_response.rs @@ -57,7 +57,11 @@ fn deserialize_code<'de, D>(d: D) -> Result where D: Deserializer<'de>, { - Ok(S3ErrorCode::from_bytes(String::deserialize(d)?.as_bytes()).unwrap_or(S3ErrorCode::Custom("".into()))) + let code = String::deserialize(d)?; + if code == "NoSuchObject" { + return Ok(S3ErrorCode::NoSuchKey); + } + Ok(S3ErrorCode::from_bytes(code.as_bytes()).unwrap_or(S3ErrorCode::Custom("".into()))) } impl Default for ErrorResponse { @@ -325,4 +329,21 @@ mod tests { assert_eq!(response.code, S3ErrorCode::NoSuchVersion); assert_eq!(response.status_code, StatusCode::NOT_FOUND); } + + #[test] + fn normalizes_provider_specific_missing_object_code() { + let mut headers = HeaderMap::new(); + headers.insert("x-amz-request-id", "request-id".parse().expect("request ID header should parse")); + + let response = http_resp_to_error_response( + StatusCode::NOT_FOUND, + &headers, + b"NoSuchObjectremote detail".to_vec(), + "bucket", + "object", + ); + + assert_eq!(response.code, S3ErrorCode::NoSuchKey); + assert_eq!(response.status_code, StatusCode::NOT_FOUND); + } } diff --git a/crates/s3-client/src/api_get_object.rs b/crates/s3-client/src/api_get_object.rs index cc06fe388..872c1a90d 100644 --- a/crates/s3-client/src/api_get_object.rs +++ b/crates/s3-client/src/api_get_object.rs @@ -30,7 +30,9 @@ use tokio_util::io::StreamReader; use crate::{ api_error_response::err_invalid_argument, api_get_options::GetObjectOptions, - transition_api::{ObjectInfo, ReadCloser, ReaderImpl, RequestMetadata, TransitionClient, to_object_info_for_provider}, + transition_api::{ + ObjectInfo, ReadCloser, ReaderImpl, RequestMetadata, TransitionClient, collect_response_body, to_object_info_for_provider, + }, }; use futures_util::StreamExt; use http_body_util::BodyExt; @@ -39,6 +41,42 @@ use hyper::body::Bytes; use rustfs_utils::hash::EMPTY_STRING_SHA256_HASH; use tokio_util::io::ReaderStream; +fn response_limit_from_range(opts: &GetObjectOptions) -> Result, std::io::Error> { + let Some(range) = opts + .headers + .iter() + .find_map(|(name, value)| name.eq_ignore_ascii_case("range").then_some(value.as_str())) + else { + return Ok(None); + }; + let Some((unit, bounds)) = range.split_once('=') else { + return Ok(None); + }; + if !unit.eq_ignore_ascii_case("bytes") { + return Ok(None); + } + let Some((start, end)) = bounds.split_once('-') else { + return Ok(None); + }; + if start.is_empty() || end.is_empty() || end.contains(',') { + return Ok(None); + } + let start = start + .parse::() + .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "closed response range start is invalid"))?; + let end = end + .parse::() + .map_err(|_| std::io::Error::new(std::io::ErrorKind::InvalidInput, "closed response range end is invalid"))?; + let length = end + .checked_sub(start) + .and_then(|length| length.checked_add(1)) + .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidInput, "closed response range length overflows"))?; + let limit = usize::try_from(length).map_err(|_| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, "closed response range length does not fit in memory") + })?; + Ok(Some(limit)) +} + impl TransitionClient { pub fn get_object(&self, bucket_name: &str, object_name: &str, opts: &GetObjectOptions) -> Result { let _ = opts; @@ -54,6 +92,7 @@ impl TransitionClient { object_name: &str, opts: &GetObjectOptions, ) -> Result<(ObjectInfo, HeaderMap, ReadCloser), std::io::Error> { + let max_response_bytes = response_limit_from_range(opts)?; let resp = self .execute_method( http::Method::GET, @@ -81,18 +120,214 @@ impl TransitionClient { let h = resp.headers().clone(); - let mut body_vec = Vec::new(); let mut body = resp.into_body(); - while let Some(frame) = body.frame().await { - let frame = frame.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?; - if let Some(data) = frame.data_ref() { - body_vec.extend_from_slice(data); + let body_vec = if let Some(limit) = max_response_bytes { + collect_response_body(body, limit).await? + } else { + let mut body_vec = Vec::new(); + while let Some(frame) = body.frame().await { + let frame = frame.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?; + if let Some(data) = frame.data_ref() { + body_vec.extend_from_slice(data); + } } - } + body_vec + }; Ok((object_stat, h, BufReader::new(Cursor::new(body_vec)))) } } +#[cfg(test)] +mod bounded_response_tests { + use super::response_limit_from_range; + use crate::{ + api_get_options::GetObjectOptions, + credentials::{Credentials, SignatureType, Static, Value}, + transition_api::{BucketLookupType, Options, TransitionClient, collect_response_body}, + }; + use http_body_util::Full; + use hyper::body::Bytes; + use std::time::Duration; + use tokio::{ + io::{AsyncReadExt, AsyncWriteExt}, + net::TcpListener, + }; + + #[test] + fn closed_range_derives_a_collection_limit_without_new_public_options() { + let mut opts = GetObjectOptions::default(); + opts.set_range(5, 11).expect("the closed range should be valid"); + + assert_eq!(response_limit_from_range(&opts).expect("the range should parse"), Some(7)); + } + + #[tokio::test] + async fn response_collection_rejects_the_body_that_exceeds_its_range_limit() { + let mut opts = GetObjectOptions::default(); + opts.set_range(0, 6).expect("the probe range should be valid"); + let max_response_bytes = response_limit_from_range(&opts) + .expect("the range should parse") + .expect("the closed range should have a limit"); + let err = collect_response_body(Full::new(Bytes::from_static(b"RustFSxx")), max_response_bytes) + .await + .expect_err("the collection layer must reject a response larger than its limit"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + } + + async fn bounded_get_fixture(body: &'static [u8]) -> Option<(TransitionClient, tokio::task::JoinHandle)> { + let listener = match TcpListener::bind("127.0.0.1:0").await { + Ok(listener) => listener, + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return None, + Err(err) => panic!("test listener should bind: {err}"), + }; + let endpoint = listener + .local_addr() + .expect("listener local address should be available") + .to_string(); + let request = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.expect("fixture should accept one GET"); + let mut request = Vec::new(); + let mut buffer = [0; 1024]; + loop { + let read = stream.read(&mut buffer).await.expect("fixture should read request headers"); + assert_ne!(read, 0, "connection closed before request headers were received"); + request.extend_from_slice(&buffer[..read]); + if request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + let request = String::from_utf8_lossy(&request).into_owned(); + let response = format!( + "HTTP/1.1 206 Partial Content\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ); + stream + .write_all(response.as_bytes()) + .await + .expect("fixture should write response headers"); + stream.write_all(body).await.expect("fixture should write response body"); + request + }); + let client = TransitionClient::new( + &endpoint, + Options { + creds: Credentials::new(Static(Value { + access_key_id: "access-key".to_string(), + secret_access_key: "secret-key".to_string(), + signer_type: SignatureType::SignatureV4, + ..Default::default() + })), + region: "us-east-1".to_string(), + bucket_lookup: BucketLookupType::BucketLookupPath, + max_retries: 1, + ..Default::default() + }, + "", + ) + .await + .expect("fixture client should build"); + Some((client, request)) + } + + #[tokio::test] + async fn real_transport_accepts_the_exact_closed_range_length() { + let Some((client, request)) = bounded_get_fixture(b"RustFS!").await else { + return; + }; + let mut opts = GetObjectOptions::default(); + opts.set_range(0, 6).expect("the probe range should be valid"); + + let (_, _, mut reader) = client + .get_object_inner("bucket", "probe", &opts) + .await + .expect("a seven-byte response should fit the requested range"); + let mut body = Vec::new(); + reader + .read_to_end(&mut body) + .await + .expect("bounded response should be readable"); + + assert_eq!(body, b"RustFS!"); + assert!( + request + .await + .expect("fixture should join") + .to_ascii_lowercase() + .contains("\r\nrange: bytes=0-6\r\n") + ); + } + + #[tokio::test] + async fn real_transport_rejects_a_body_larger_than_the_closed_range() { + let Some((client, request)) = bounded_get_fixture(b"RustFS!!").await else { + return; + }; + let mut opts = GetObjectOptions::default(); + opts.set_range(0, 6).expect("the probe range should be valid"); + + let err = client + .get_object_inner("bucket", "probe", &opts) + .await + .expect_err("an eight-byte response must exceed the seven-byte range limit"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + assert!( + request + .await + .expect("fixture should join") + .to_ascii_lowercase() + .contains("\r\nrange: bytes=0-6\r\n") + ); + } + + #[tokio::test] + async fn overflowing_closed_range_is_rejected_before_network_io() { + let listener = match TcpListener::bind("127.0.0.1:0").await { + Ok(listener) => listener, + Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return, + Err(err) => panic!("test listener should bind: {err}"), + }; + let endpoint = listener + .local_addr() + .expect("listener local address should be available") + .to_string(); + let client = TransitionClient::new( + &endpoint, + Options { + creds: Credentials::new(Static(Value { + access_key_id: "access-key".to_string(), + secret_access_key: "secret-key".to_string(), + signer_type: SignatureType::SignatureV4, + ..Default::default() + })), + region: "us-east-1".to_string(), + bucket_lookup: BucketLookupType::BucketLookupPath, + max_retries: 1, + ..Default::default() + }, + "", + ) + .await + .expect("fixture client should build"); + let mut opts = GetObjectOptions::default(); + opts.headers + .insert("range".to_string(), "bytes=0-18446744073709551615".to_string()); + + let err = client + .get_object_inner("bucket", "probe", &opts) + .await + .expect_err("an overflowing closed range must be rejected locally"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + tokio::time::timeout(Duration::from_millis(100), listener.accept()) + .await + .is_err() + ); + } +} + #[derive(Default)] pub struct GetRequest { pub buffer: Vec, diff --git a/rustfs/src/admin/handlers/tier.rs b/rustfs/src/admin/handlers/tier.rs index 535554a46..310212cc4 100644 --- a/rustfs/src/admin/handlers/tier.rs +++ b/rustfs/src/admin/handlers/tier.rs @@ -80,6 +80,82 @@ fn wasabi_payload_name(config: &TierConfig) -> S3Result { .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Wasabi configuration")) } +fn normalize_add_tier_payload_name(config: &mut TierConfig) -> S3Result<()> { + match config.tier_type { + TierType::S3 => { + let _ = config + .s3 + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing S3 configuration"))?; + } + TierType::Wasabi => config.name = wasabi_payload_name(config)?, + TierType::RustFS => { + config.name = config + .rustfs + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing RustFS configuration"))? + .name + .clone(); + } + TierType::MinIO => { + config.name = config + .minio + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing MinIO configuration"))? + .name + .clone(); + } + TierType::Aliyun => { + config.name = config + .aliyun + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Aliyun configuration"))? + .name + .clone(); + } + TierType::Tencent => { + config.name = config + .tencent + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Tencent configuration"))? + .name + .clone(); + } + TierType::Huaweicloud => { + config.name = config + .huaweicloud + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Huawei Cloud configuration"))? + .name + .clone(); + } + TierType::Azure => { + config.name = config + .azure + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Azure configuration"))? + .name + .clone(); + } + TierType::GCS => { + let _ = config + .gcs + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing GCS configuration"))?; + } + TierType::R2 => { + config.name = config + .r2 + .as_ref() + .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing R2 configuration"))? + .name + .clone(); + } + TierType::Unsupported => {} + } + Ok(()) +} + fn spawn_transition_tier_config_propagation(action: &'static str) { if let Some(notification_sys) = current_notification_system() { debug!( @@ -263,75 +339,7 @@ impl Operation for AddTier { let mut args: TierConfig = serde_json::from_slice(&body) .map_err(|e| S3Error::with_message(S3ErrorCode::InvalidRequest, format!("invalid JSON: {e}")))?; - match args.tier_type { - TierType::S3 => { - args.name = args - .s3 - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing S3 configuration"))? - .name; - } - TierType::Wasabi => { - args.name = wasabi_payload_name(&args)?; - } - TierType::RustFS => { - args.name = args - .rustfs - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing RustFS configuration"))? - .name; - } - TierType::MinIO => { - args.name = args - .minio - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing MinIO configuration"))? - .name; - } - TierType::Aliyun => { - args.name = args - .aliyun - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Aliyun configuration"))? - .name; - } - TierType::Tencent => { - args.name = args - .tencent - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Tencent configuration"))? - .name; - } - TierType::Huaweicloud => { - args.name = args - .huaweicloud - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Huawei Cloud configuration"))? - .name; - } - TierType::Azure => { - args.name = args - .azure - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing Azure configuration"))? - .name; - } - TierType::GCS => { - args.name = args - .gcs - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing GCS configuration"))? - .name; - } - TierType::R2 => { - args.name = args - .r2 - .clone() - .ok_or_else(|| S3Error::with_message(S3ErrorCode::InvalidRequest, "missing R2 configuration"))? - .name; - } - _ => (), - } + normalize_add_tier_payload_name(&mut args)?; debug!( event = EVENT_ADMIN_TIER_STATE, component = LOG_COMPONENT_ADMIN, @@ -1150,6 +1158,44 @@ mod tests { assert_eq!(err.message(), Some("missing Wasabi configuration")); } + #[test] + fn add_tier_payload_preserves_canonical_madmin_s3_and_gcs_names() { + for (provider, wire) in [ + ( + "S3", + serde_json::json!({ + "Type": "s3", + "Name": "COLD-S3", + "S3": { + "Endpoint": "https://s3.example.invalid", + "AccessKey": "access", + "SecretKey": "secret", + "Bucket": "archive" + } + }), + ), + ( + "GCS", + serde_json::json!({ + "Type": "gcs", + "Name": "COLD-GCS", + "GCS": { + "Endpoint": "https://storage.googleapis.com", + "Creds": "e30=", + "Bucket": "archive" + } + }), + ), + ] { + let mut config: TierConfig = serde_json::from_value(wire).expect("canonical madmin payload should decode"); + let expected = config.name.clone(); + + normalize_add_tier_payload_name(&mut config).expect("canonical madmin payload should pass the handler boundary"); + + assert_eq!(config.name, expected, "{provider} top-level Name must not be cleared"); + } + } + #[test] fn resolve_tier_name_prefers_path_parameter() { let uri: Uri = "/rustfs/admin/v3/tier/HOT?tier=COLD".parse().expect("uri should parse"); @@ -1756,5 +1802,11 @@ mod tests { } assert!(!production.contains("check_key_valid(get_session_token")); + + let add_tier = source_block(production, "impl Operation for AddTier"); + assert!( + add_tier.contains("normalize_add_tier_payload_name(&mut args)?;"), + "AddTier must preserve canonical top-level provider names through the tested boundary helper" + ); } } diff --git a/rustfs/src/app/object/mod.rs b/rustfs/src/app/object/mod.rs index 5360948e3..d5751d5c4 100644 --- a/rustfs/src/app/object/mod.rs +++ b/rustfs/src/app/object/mod.rs @@ -170,12 +170,12 @@ use s3s::dto::{ DeleteObjectsOutput, DeletedObject, ETag, GetObjectAttributesInput, GetObjectAttributesOutput, GetObjectAttributesParts, GetObjectInput, GetObjectOutput, HeadObjectInput, HeadObjectOutput, MetadataDirective, ObjectAttributes, ObjectLockLegalHold, ObjectLockLegalHoldStatus, ObjectLockMode, ObjectLockRetention, ObjectLockRetentionMode, ObjectPart, PutObjectInput, - PutObjectOutput, Range, RequestCharged, RestoreObjectInput, RestoreObjectOutput, RestoreStatus, SSECustomerAlgorithm, - SSECustomerKeyMD5, SSEKMSKeyId, SelectObjectContentInput, SelectObjectContentOutput, ServerSideEncryption, - ServerSideEncryptionConfiguration, StorageClass, StreamingBlob, TaggingDirective, TaggingHeader, Timestamp, TimestampFormat, - WebsiteRedirectLocation, + PutObjectOutput, Range, RequestCharged, RestoreObjectInput, RestoreObjectOutput, RestoreRequestType, RestoreStatus, + SSECustomerAlgorithm, SSECustomerKeyMD5, SSEKMSKeyId, SelectObjectContentInput, SelectObjectContentOutput, + ServerSideEncryption, ServerSideEncryptionConfiguration, StorageClass, StreamingBlob, TaggingDirective, TaggingHeader, + Timestamp, TimestampFormat, WebsiteRedirectLocation, }; -use s3s::header::{X_AMZ_RESTORE, X_AMZ_RESTORE_OUTPUT_PATH}; +use s3s::header::X_AMZ_RESTORE; use s3s::stream::{ByteStream, DynByteStream, RemainingLength}; use s3s::{S3Error, S3ErrorCode, S3Request, S3Response, S3Result, s3_error}; diff --git a/rustfs/src/app/object/restore.rs b/rustfs/src/app/object/restore.rs index 56319265d..96201a670 100644 --- a/rustfs/src/app/object/restore.rs +++ b/rustfs/src/app/object/restore.rs @@ -165,18 +165,49 @@ impl DefaultObjectUsecase { validate_table_catalog_object_mutation(&bucket, &object).await?; - let rreq = rreq.ok_or_else(|| { - S3Error::with_message(S3ErrorCode::Custom("ErrValidRestoreObject".into()), "restore request is required") - })?; + // Typed S3 errors on every RestoreObject failure (backlog#2205): a + // `Custom` code serializes as a generic 500, which makes SDK clients + // retry client errors and conflicts alike. + let rreq = rreq.ok_or_else(|| S3Error::with_message(S3ErrorCode::MalformedXML, "restore request is required"))?; + + // SELECT-type restore is not supported (backlog#1341). The restore + // path can only write the retrieved bytes back to the source key, so + // honouring a SELECT request overwrote the source object with + // SELECT-only metadata (dropping `x-amz-restore`, user metadata and + // tags on an unversioned bucket, or publishing a bogus latest version + // on a versioned one) while never writing anything to + // `OutputLocation.S3`. Reject before any guard, metadata write or + // fabricated `x-amz-restore-output-path` response header. + if rreq + .type_ + .as_ref() + .is_some_and(|type_| type_.as_str() == RestoreRequestType::SELECT) + { + return Err(S3Error::with_message( + S3ErrorCode::NotImplemented, + "SELECT restore requests are not supported.", + )); + } let Some(store) = self.object_store() else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; + // Validate the request shape before taking any lock or reading the + // object: a malformed request or an illegal `Days` value is a client + // error, and the validator messages are static — they carry no + // backend or credential detail. + if let Err(e) = validate_restore_request(&rreq, store.clone()) { + return Err(S3Error::with_message( + S3ErrorCode::InvalidRequest, + format!("Restore object validation failed: {e}"), + )); + } + let version_id_str = version_id.clone().unwrap_or_default(); let mut opts = post_restore_opts(&version_id_str, &bucket, &object) .await - .map_err(|_| S3Error::with_message(S3ErrorCode::Custom("ErrPostRestoreOpts".into()), "restore object failed."))?; + .map_err(ApiError::from)?; apply_bucket_generation_guard(&req, &bucket, &mut opts)?; // `apply_bucket_generation_guard` deliberately tolerates a missing guard // (only the S3 access layer installs one), so this must not hard-require @@ -192,11 +223,7 @@ impl DefaultObjectUsecase { } }; - // SELECT-type restores skip both the ongoing check and the metadata - // write below, so the accept guard would protect nothing for them — - // they keep the plain (read-locked) accept path. - let is_select = rreq.type_.as_ref().is_some_and(|t| t.as_str() == "SELECT"); - let restore_operation_id = (!is_select).then(Uuid::new_v4); + let restore_operation_id = Some(Uuid::new_v4()); let mut restore_worker_guard = if let Some(operation_id) = restore_operation_id { Some( store @@ -210,10 +237,10 @@ impl DefaultObjectUsecase { // Hold the restore-accept guard across the restore-status read, the // ongoing/already-restored decision, and the metadata write below, so - // two concurrent (non-SELECT) POST ?restore cannot both observe - // ongoing=false and both start a copy-back (backlog#1304). Reads and - // writes inside this scope run with no_lock; the guard is dropped - // before the copy-back is spawned so it never blocks readers. + // two concurrent POST ?restore cannot both observe ongoing=false and + // both start a copy-back (backlog#1304). Reads and writes inside this + // scope run with no_lock; the guard is dropped before the copy-back is + // spawned so it never blocks readers. // Contention on the accept guard (e.g. a concurrent accept or an // in-flight commit on the same object) is transient — answer 503 // SlowDown so SDK clients back off and retry instead of treating it @@ -222,9 +249,7 @@ impl DefaultObjectUsecase { if store.bucket_incarnation_id_from_disk(&bucket).await.map_err(ApiError::from)? != restore_bucket_incarnation_id { return Err(ApiError::from(StorageError::BucketNotFound(bucket.clone())).into()); } - let mut accept_guard = if is_select { - None - } else { + let mut accept_guard = { let guard = store .acquire_restore_accept_guard(&bucket, &object) .await @@ -233,24 +258,17 @@ impl DefaultObjectUsecase { Some(guard) }; - let mut obj_info = store - .get_object_info(&bucket, &object, &opts) - .await - .map_err(|_| S3Error::with_message(S3ErrorCode::Custom("ErrInvalidObjectState".into()), "restore object failed."))?; + // A missing key or version must stay NoSuchKey / NoSuchVersion, and an + // authorization or storage failure must keep its own identity, so map + // the storage error instead of flattening it (backlog#2205). + let mut obj_info = store.get_object_info(&bucket, &object, &opts).await.map_err(ApiError::from)?; - // Check if object is in a transitioned state + // Restoring an object that was never transitioned is the S3 + // InvalidObjectState case, not an internal error. if obj_info.transitioned_object.status != lifecycle::TRANSITION_COMPLETE { return Err(S3Error::with_message( - S3ErrorCode::Custom("ErrInvalidTransitionedState".into()), - "restore object failed.", - )); - } - - // Validate restore request - if let Err(e) = validate_restore_request(&rreq, store.clone()) { - return Err(S3Error::with_message( - S3ErrorCode::Custom("ErrValidRestoreObject".into()), - format!("Restore object validation failed: {}", e), + S3ErrorCode::InvalidObjectState, + "The operation is not valid for the object's storage class.", )); } @@ -260,7 +278,7 @@ impl DefaultObjectUsecase { // would create an ABBA cycle. If the probe succeeds, reacquire and // re-read the object before replacing the exact orphan generation. let mut superseded_worker_guard = None; - if obj_info.restore_ongoing && !is_select { + if obj_info.restore_ongoing { match classify_ongoing_restore(obj_info.user_defined.as_ref(), OffsetDateTime::now_utc()) { OngoingRestoreRecovery::ActiveOrUnsafe => { return Err(S3Error::with_message( @@ -293,13 +311,11 @@ impl DefaultObjectUsecase { .map_err(|_| S3Error::with_message(S3ErrorCode::SlowDown, "restore object failed."))?, ); opts.no_lock = true; - obj_info = store.get_object_info(&bucket, &object, &opts).await.map_err(|_| { - S3Error::with_message(S3ErrorCode::Custom("ErrInvalidObjectState".into()), "restore object failed.") - })?; + obj_info = store.get_object_info(&bucket, &object, &opts).await.map_err(ApiError::from)?; if obj_info.transitioned_object.status != lifecycle::TRANSITION_COMPLETE { return Err(S3Error::with_message( - S3ErrorCode::Custom("ErrInvalidTransitionedState".into()), - "restore object failed.", + S3ErrorCode::InvalidObjectState, + "The operation is not valid for the object's storage class.", )); } if obj_info.restore_ongoing { @@ -327,11 +343,11 @@ impl DefaultObjectUsecase { remove_str(&mut metadata, SUFFIX_RESTORE_OPERATION_ID); remove_str(&mut metadata, SUFFIX_RESTORE_WORKER_LOCK); - let mut header = HeaderMap::new(); - let event_object_info = obj_info.clone(); let obj_info_ = obj_info.clone(); - if !is_select { + // Scopes the accept-guarded metadata write: everything below runs + // inside the accept critical section, which is released right after. + { obj_info.metadata_only = true; metadata.insert(AMZ_RESTORE_EXPIRY_DAYS.to_string(), rreq.days.unwrap_or(1).to_string()); let request_date = OffsetDateTime::now_utc().format(&Rfc3339).map_err(|e| { @@ -403,7 +419,7 @@ impl DefaultObjectUsecase { &restore_dst_opts, ) .await - .map_err(|_| S3Error::with_message(S3ErrorCode::Custom("ErrCopyObject".into()), "restore object failed."))?; + .map_err(ApiError::from)?; rustfs_scanner::record_dirty_usage_bucket(&bucket); #[cfg(test)] maybe_pause_after_restore_status_commit(&bucket, &object).await; @@ -429,17 +445,6 @@ impl DefaultObjectUsecase { drop(accept_guard); drop(restore_bucket_lifecycle_guard); - // Handle output location for SELECT requests - if let Some(output_location) = &rreq.output_location - && let Some(s3) = &output_location.s3 - && !s3.bucket_name.is_empty() - { - let restore_object = Uuid::new_v4().to_string(); - if let Ok(header_value) = format!("{}{}{}", s3.bucket_name, s3.prefix, restore_object).parse() { - header.insert(X_AMZ_RESTORE_OUTPUT_PATH, header_value); - } - } - // Spawn restoration task in the background. Pin the copy-back to the // version the accept resolved and flagged: with a versionless request // on a versioned bucket, a PUT landing between the accept and the @@ -499,7 +504,7 @@ impl DefaultObjectUsecase { restore_output_path: None, }; helper = helper.object(event_object_info).version_id(version_id_str); - let result = Ok(S3Response::with_headers(output, header)); + let result = Ok(S3Response::new(output)); let _ = helper.complete(&result); result } @@ -629,6 +634,28 @@ mod tests { assert_eq!(classify_ongoing_restore(&conflicting_date, now), OngoingRestoreRecovery::ActiveOrUnsafe); } + fn restore_request(days: Option) -> RestoreRequest { + RestoreRequest { + days, + description: None, + glacier_job_parameters: None, + output_location: None, + select_parameters: None, + tier: None, + type_: None, + } + } + + fn restore_input(bucket: &str, key: &str, rreq: RestoreRequest) -> RestoreObjectInput { + RestoreObjectInput::builder() + .bucket(bucket.to_string()) + .key(key.to_string()) + .restore_request(Some(rreq)) + .build() + .expect("restore input should build") + } + + /// backlog#2205: a missing restore body is a client error, not a 500. #[tokio::test] async fn execute_restore_object_rejects_missing_restore_request() { let input = RestoreObjectInput::builder() @@ -641,10 +668,92 @@ mod tests { let usecase = DefaultObjectUsecase::without_context(); let err = usecase.execute_restore_object(req).await.unwrap_err(); - match err.code() { - S3ErrorCode::Custom(code) => assert_eq!(code, "ErrValidRestoreObject"), - code => panic!("unexpected error code: {:?}", code), - } + assert_eq!(err.code(), &S3ErrorCode::MalformedXML); + } + + /// backlog#1341: a SELECT restore must be rejected outright — the restore + /// path can only write back to the source key, never to + /// `OutputLocation.S3`. Rejection happens before the store is resolved, so + /// an uninitialized usecase still answers NotImplemented rather than the + /// InternalError every request that gets past this point returns. + #[tokio::test] + async fn execute_restore_object_rejects_select_type() { + let mut rreq = restore_request(None); + rreq.type_ = Some(s3s::dto::RestoreRequestType::from_static(s3s::dto::RestoreRequestType::SELECT)); + + let req = build_request(restore_input("test-bucket", "test-key", rreq), Method::POST); + let usecase = DefaultObjectUsecase::without_context(); + + let err = usecase.execute_restore_object(req).await.unwrap_err(); + assert_eq!(err.code(), &S3ErrorCode::NotImplemented); + } + + /// backlog#2205: every RestoreObject failure that reaches storage must + /// keep its typed S3 identity. Before this, a missing key, a malformed + /// version-id, an illegal `Days` and an object that was never transitioned + /// all collapsed into `Custom(...)` codes, which serialize as a retryable + /// HTTP 500. + #[tokio::test] + #[serial_test::serial] + async fn execute_restore_object_maps_failures_to_typed_s3_errors() { + use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions}; + + let store = crate::app::gating_test_env::shared_gating_ecstore().await; + let context = crate::app::gating_test_env::shared_gating_ambient().await; + let bucket = format!("restore-typed-errors-{}", Uuid::new_v4().simple()); + store + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("create restore test bucket"); + let mut reader = PutObjReader::from_vec(b"never transitioned".to_vec()); + store + .put_object(&bucket, "local-object", &mut reader, &ObjectOptions::default()) + .await + .expect("put untransitioned test object"); + + let usecase = DefaultObjectUsecase::with_context(Some(context)); + + // An illegal `Days` is a client error, rejected before any lock or + // object read. + let err = usecase + .execute_restore_object(build_request( + restore_input(&bucket, "local-object", restore_request(Some(0))), + Method::POST, + )) + .await + .expect_err("days=0 must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + + // A malformed version-id keeps InvalidArgument instead of being + // flattened inside `post_restore_opts`. + let mut input = restore_input(&bucket, "local-object", restore_request(Some(1))); + input.version_id = Some("not-a-uuid".to_string()); + let err = usecase + .execute_restore_object(build_request(input, Method::POST)) + .await + .expect_err("malformed version-id must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidArgument); + + // A missing key stays NoSuchKey. + let err = usecase + .execute_restore_object(build_request( + restore_input(&bucket, "missing-object", restore_request(Some(1))), + Method::POST, + )) + .await + .expect_err("missing key must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::NoSuchKey); + + // Restoring an object that was never transitioned is the S3 + // InvalidObjectState case, not an internal error. + let err = usecase + .execute_restore_object(build_request( + restore_input(&bucket, "local-object", restore_request(Some(1))), + Method::POST, + )) + .await + .expect_err("untransitioned object must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidObjectState); } #[tokio::test] diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index e90f9d9ae..19548aa66 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -368,7 +368,8 @@ pub(crate) mod bucket { version_id: &str, bucket: &str, object: &str, - ) -> Result { + ) -> Result + { crate::storage::storage_api::ecstore_bucket::lifecycle::bucket_lifecycle_ops::post_restore_opts( version_id, bucket, object, ) diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 51aef9ef5..c2f3325c9 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -14,6 +14,8 @@ #[cfg(all(feature = "hotpath", feature = "hotpath-alloc", not(target_os = "windows")))] use std::alloc::{GlobalAlloc, Layout}; +#[cfg(all(feature = "hotpath", feature = "hotpath-alloc", not(target_os = "windows")))] +use std::ptr::NonNull; #[cfg(all(feature = "hotpath", feature = "hotpath-alloc", not(target_os = "windows")))] #[derive(Default)] @@ -35,8 +37,10 @@ unsafe impl GlobalAlloc for MiMallocAllocator { } unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - // SAFETY: ptr came from this allocator and layout.size() is the original allocation size. - unsafe { rustfs_mimalloc::MiMalloc::free_csize(ptr, layout.size()) } + // SAFETY: ptr came from this allocator, is non-null by GlobalAlloc's + // dealloc contract, and layout.size() is the original allocation size. + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + unsafe { rustfs_mimalloc::MiMalloc::free_csize_nonnull(ptr, layout.size()) } } unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {