From 2fc3494efbbae35b4c1a7f973ec808256cb4d485 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 14 Sep 2026 03:05:49 +0800 Subject: [PATCH] refactor(filemeta): own RestoreStatus and drop the s3s dependency (A3c) (#7782) Replace s3s::dto::{RestoreStatus, Timestamp} in filemeta with a filemeta-owned RestoreStatus (same field names, OffsetDateTime expiry). RestoreStatusOps and parse_restore_obj_status keep their signatures and persisted rendering; the ecstore restore finalize / lifecycle restore writers and rustfs RestoreObject now build the filemeta type. - crates/filemeta: drop s3s; tokio "time" becomes a dev-dependency (the metacache tests got it through s3s feature unification). - metadata_keys: drop the s3s half of the historical-source cross-check; PINNED, the pre-A3a fixture and per-character mutation still pin keys. - new test re-renders the pre-A3a fixture restore value byte-for-byte. - s3s footprint baselines 210 -> 209 files, ecstore 37 -> 36. Refs rustfs/backlog#1735 --- Cargo.lock | 1 - .../bucket/lifecycle/bucket_lifecycle_ops.rs | 10 ++- crates/ecstore/src/set_disk/replication.rs | 5 +- crates/filemeta/Cargo.toml | 4 +- crates/filemeta/src/fileinfo.rs | 68 ++++++++++++++++--- crates/filemeta/src/metadata_keys.rs | 17 +---- rustfs/src/app/object/mod.rs | 8 +-- rustfs/src/app/object/restore.rs | 5 +- scripts/check_s3s_footprint.sh | 4 +- 9 files changed, 79 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 676d0c149..6bff51998 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9906,7 +9906,6 @@ dependencies = [ "rmp-serde", "rustfs-config", "rustfs-utils", - "s3s", "serde", "serde_json", "tempfile", diff --git a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs index 3537e30dc..1caea50cb 100644 --- a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs +++ b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs @@ -84,16 +84,14 @@ use rustfs_config::{ use rustfs_data_usage::TierStats; use rustfs_filemeta::metadata_keys; use rustfs_filemeta::{ - FileInfo, FileInfoOpts, NULL_VERSION_ID, RestoreStatusOps, TRANSITION_COMPLETE, get_file_info, is_restored_object_on_disk, + FileInfo, FileInfoOpts, NULL_VERSION_ID, RestoreStatus, RestoreStatusOps, TRANSITION_COMPLETE, get_file_info, + is_restored_object_on_disk, }; 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}; -use s3s::dto::{ - BucketLifecycleConfiguration, ExpirationStatus, ObjectLockConfiguration, RestoreRequest, RestoreRequestType, RestoreStatus, - Timestamp, -}; +use s3s::dto::{BucketLifecycleConfiguration, ExpirationStatus, ObjectLockConfiguration, RestoreRequest, RestoreRequestType}; use sha2::{Digest, Sha256}; use std::any::Any; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -5177,7 +5175,7 @@ pub async fn put_restore_opts( metadata_keys::RESTORE.to_string(), RestoreStatus { is_restore_in_progress: Some(false), - restore_expiry_date: Some(Timestamp::from(restore_expiry)), + restore_expiry_date: Some(restore_expiry), } .to_string(), ); diff --git a/crates/ecstore/src/set_disk/replication.rs b/crates/ecstore/src/set_disk/replication.rs index 81b0effbd..8964a9420 100644 --- a/crates/ecstore/src/set_disk/replication.rs +++ b/crates/ecstore/src/set_disk/replication.rs @@ -18,9 +18,8 @@ use super::{ }; use crate::bucket::lifecycle::lifecycle; use crate::core::pools::DecommissionCapacityAdmission; -use rustfs_filemeta::RestoreStatusOps; use rustfs_filemeta::metadata_keys; -use s3s::dto::{RestoreStatus, Timestamp}; +use rustfs_filemeta::{RestoreStatus, RestoreStatusOps}; #[cfg(all(test, feature = "test-util"))] use std::sync::Arc; @@ -216,7 +215,7 @@ impl SetDisks { metadata_keys::RESTORE.to_string(), RestoreStatus { is_restore_in_progress: Some(false), - restore_expiry_date: Some(Timestamp::from(restore_expiry)), + restore_expiry_date: Some(restore_expiry), } .to_string(), ); diff --git a/crates/filemeta/Cargo.toml b/crates/filemeta/Cargo.toml index 369978881..aade6a445 100644 --- a/crates/filemeta/Cargo.toml +++ b/crates/filemeta/Cargo.toml @@ -48,12 +48,14 @@ rustfs-config = { workspace = true, features = ["constants"] } byteorder = { workspace = true } tracing.workspace = true thiserror.workspace = true -s3s = { workspace = true, features = ["minio"] } regex.workspace = true arc-swap.workspace = true [dev-dependencies] criterion = { workspace = true, features = ["html_reports"] } +# The metacache tests use tokio::time; s3s used to enable the feature via +# feature unification before filemeta dropped it (backlog#1735 A3c). +tokio = { workspace = true, features = ["time"] } tempfile = { workspace = true } proptest = "1" diff --git a/crates/filemeta/src/fileinfo.rs b/crates/filemeta/src/fileinfo.rs index 4fd556aee..7cc309a4c 100644 --- a/crates/filemeta/src/fileinfo.rs +++ b/crates/filemeta/src/fileinfo.rs @@ -23,7 +23,6 @@ use rustfs_utils::http::{ contains_key_str, get_consistent_str, get_str, has_internal_suffix, insert_str, is_encryption_metadata_key, starts_with_ignore_ascii_case, }; -use s3s::dto::{RestoreStatus, Timestamp}; use serde::de::{self, MapAccess, SeqAccess, Visitor, value::MapAccessDeserializer}; use serde::ser::SerializeMap; use serde::{Deserialize, Serialize}; @@ -1359,6 +1358,18 @@ pub struct FilesInfo { pub is_truncated: bool, } +/// Parsed restore status of a restored object: the value persisted under +/// [`metadata_keys::RESTORE`]. filemeta owns this type so the persisted format +/// does not depend on an HTTP library DTO (backlog#1735 A3c); the field names +/// match the former `s3s::dto::RestoreStatus` so the rendered bytes are unchanged. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct RestoreStatus { + /// `ongoing-request`; `None` reads as not in progress. + pub is_restore_in_progress: Option, + /// `expiry-date`; required to render a finished restore. + pub restore_expiry_date: Option, +} + pub trait RestoreStatusOps { fn expiry(&self) -> Option; fn on_going(&self) -> bool; @@ -1372,7 +1383,7 @@ impl RestoreStatusOps for RestoreStatus { if self.on_going() { return None; } - self.restore_expiry_date.clone().map(OffsetDateTime::from) + self.restore_expiry_date } fn on_going(&self) -> bool { @@ -1398,9 +1409,7 @@ impl RestoreStatusOps for RestoreStatus { } format!( "ongoing-request=\"false\", expiry-date=\"{}\"", - OffsetDateTime::from(self.restore_expiry_date.clone().unwrap()) - .format(&Rfc3339) - .unwrap() + self.restore_expiry_date.unwrap().format(&Rfc3339).unwrap() ) } @@ -1410,9 +1419,7 @@ impl RestoreStatusOps for RestoreStatus { } format!( "ongoing-request=\"false\", expiry-date=\"{}\"", - OffsetDateTime::from(self.restore_expiry_date.clone().unwrap()) - .format(&RFC1123) - .unwrap() + self.restore_expiry_date.unwrap().format(&RFC1123).unwrap() ) } } @@ -1473,7 +1480,7 @@ pub fn parse_restore_obj_status(restore_hdr: &str) -> Result { let expiry = parse_restore_expiry_date(expiry_tokens[1].trim_matches('"'))?; return Ok(RestoreStatus { is_restore_in_progress: Some(false), - restore_expiry_date: Some(Timestamp::from(expiry)), + restore_expiry_date: Some(expiry), }); } _ => (), @@ -2740,7 +2747,7 @@ mod tests { fn restore_status_round_trips_through_both_formats() { let status = RestoreStatus { is_restore_in_progress: Some(false), - restore_expiry_date: Some(Timestamp::from(datetime!(2030-06-15 07:08:09 UTC))), + restore_expiry_date: Some(datetime!(2030-06-15 07:08:09 UTC)), }; for rendered in [RestoreStatusOps::to_string(&status), status.to_string2()] { let parsed = parse_restore_obj_status(&rendered).unwrap_or_else(|e| panic!("{rendered} must parse: {e}")); @@ -2748,6 +2755,47 @@ mod tests { } } + /// backlog#1735 A3c: filemeta's own `RestoreStatus` must persist the same + /// bytes the s3s DTO did. The restore value in the pre-`metadata_keys` + /// fixture was rendered by the s3s-backed writer; parsing and rendering it + /// again must reproduce it exactly. + #[test] + fn restore_status_rerenders_pre_module_fixture_bytes() { + let fi = crate::FileMeta::load(&crate::test_data::create_pre_metadata_keys_xlmeta().expect("decode fixture hex")) + .expect("load fixture xl.meta") + .into_fileinfo("bucket", "object", "0b1e5a3a-1735-4a3a-8000-00000000a3a0", false, false, false) + .expect("fixture version to FileInfo"); + let stored = fi.metadata.get(metadata_keys::RESTORE).expect("fixture restore value"); + assert_eq!(stored, "ongoing-request=\"false\", expiry-date=\"9999-01-01T00:00:00Z\""); + + let parsed = parse_restore_obj_status(stored).expect("fixture restore value parses"); + assert_eq!( + parsed, + RestoreStatus { + is_restore_in_progress: Some(false), + restore_expiry_date: Some(datetime!(9999-01-01 00:00:00 UTC)), + } + ); + assert_eq!(RestoreStatusOps::to_string(&parsed), *stored); + assert_eq!( + parsed.to_string2(), + "ongoing-request=\"false\", expiry-date=\"Fri, 01 Jan 9999 00:00:00 GMT\"" + ); + + let ongoing = RestoreStatus { + is_restore_in_progress: Some(true), + restore_expiry_date: Some(datetime!(2030-06-15 07:08:09 UTC)), + }; + assert_eq!(RestoreStatusOps::to_string(&ongoing), "ongoing-request=\"true\""); + assert_eq!( + parse_restore_obj_status("ongoing-request=\"true\"").expect("in-progress form parses"), + RestoreStatus { + is_restore_in_progress: Some(true), + restore_expiry_date: None, + } + ); + } + /// A restored object migrated from MinIO must still be recognised as /// on-disk: `is_restored_object_on_disk` fails open, and /// `MetaObject::uses_data_dir` uses it to decide whether a data dir is diff --git a/crates/filemeta/src/metadata_keys.rs b/crates/filemeta/src/metadata_keys.rs index a627493d5..3b8c70ef4 100644 --- a/crates/filemeta/src/metadata_keys.rs +++ b/crates/filemeta/src/metadata_keys.rs @@ -128,8 +128,9 @@ mod tests { } /// Migration-period cross-check: the constants must equal the historical - /// sources callers used before this module existed. Drop the `s3s` half - /// together with filemeta's `s3s` dependency. + /// `rustfs_utils` sources callers used before this module existed. The + /// `s3s::header` half was dropped with filemeta's `s3s` dependency (A3c); + /// `PINNED` and the pre-module fixture keep pinning those bytes. #[test] fn persisted_metadata_keys_match_their_historical_sources() { use rustfs_utils::http::AMZ_BUCKET_REPLICATION_STATUS; @@ -137,18 +138,6 @@ mod tests { AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER, AMZ_OBJECT_LOCK_MODE_LOWER, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER, AMZ_RESTORE, AMZ_RESTORE_EXPIRY_DAYS, AMZ_RESTORE_REQUEST_DATE, AMZ_STORAGE_CLASS, }; - use s3s::header::{ - X_AMZ_OBJECT_LOCK_LEGAL_HOLD, X_AMZ_OBJECT_LOCK_MODE, X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE, X_AMZ_RESTORE, - X_AMZ_SERVER_SIDE_ENCRYPTION, X_AMZ_STORAGE_CLASS, - }; - - assert_eq!(OBJECT_LOCK_LEGAL_HOLD, X_AMZ_OBJECT_LOCK_LEGAL_HOLD.as_str()); - assert_eq!(OBJECT_LOCK_MODE, X_AMZ_OBJECT_LOCK_MODE.as_str()); - assert_eq!(OBJECT_LOCK_RETAIN_UNTIL_DATE, X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str()); - assert_eq!(RESTORE, X_AMZ_RESTORE.as_str()); - assert_eq!(SERVER_SIDE_ENCRYPTION, X_AMZ_SERVER_SIDE_ENCRYPTION.as_str()); - assert_eq!(STORAGE_CLASS, X_AMZ_STORAGE_CLASS.as_str()); - assert_eq!(OBJECT_LOCK_LEGAL_HOLD, AMZ_OBJECT_LOCK_LEGAL_HOLD_LOWER); assert_eq!(OBJECT_LOCK_MODE, AMZ_OBJECT_LOCK_MODE_LOWER); assert_eq!(OBJECT_LOCK_RETAIN_UNTIL_DATE, AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE_LOWER); diff --git a/rustfs/src/app/object/mod.rs b/rustfs/src/app/object/mod.rs index b10143743..a0b4f1aea 100644 --- a/rustfs/src/app/object/mod.rs +++ b/rustfs/src/app/object/mod.rs @@ -171,10 +171,10 @@ 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, RestoreRequestType, RestoreStatus, - SSECustomerAlgorithm, SSECustomerKeyMD5, SSEKMSKeyId, SelectObjectContentInput, SelectObjectContentOutput, - ServerSideEncryption, ServerSideEncryptionConfiguration, StorageClass, StreamingBlob, TaggingDirective, TaggingHeader, - Timestamp, TimestampFormat, WebsiteRedirectLocation, + PutObjectOutput, Range, RequestCharged, RestoreObjectInput, RestoreObjectOutput, RestoreRequestType, SSECustomerAlgorithm, + SSECustomerKeyMD5, SSEKMSKeyId, SelectObjectContentInput, SelectObjectContentOutput, ServerSideEncryption, + ServerSideEncryptionConfiguration, StorageClass, StreamingBlob, TaggingDirective, TaggingHeader, Timestamp, TimestampFormat, + WebsiteRedirectLocation, }; use s3s::header::X_AMZ_RESTORE; use s3s::stream::{ByteStream, DynByteStream, RemainingLength}; diff --git a/rustfs/src/app/object/restore.rs b/rustfs/src/app/object/restore.rs index f941e7e3d..59445ec98 100644 --- a/rustfs/src/app/object/restore.rs +++ b/rustfs/src/app/object/restore.rs @@ -15,6 +15,7 @@ //! RestoreObject path. use super::*; +use rustfs_filemeta::RestoreStatus; // RUSTFS_COMPAT_TODO(backlog-1337): legacy restores lack a liveness marker. Remove after the minimum supported release writes v1 on every restore. const LEGACY_RESTORE_ORPHAN_GRACE: time::Duration = time::Duration::hours(24); @@ -359,7 +360,7 @@ impl DefaultObjectUsecase { X_AMZ_RESTORE.as_str().to_string(), RestoreStatus { is_restore_in_progress: Some(false), - restore_expiry_date: Some(Timestamp::from(restore_expiry)), + restore_expiry_date: Some(restore_expiry), } .to_string(), ); @@ -368,7 +369,7 @@ impl DefaultObjectUsecase { X_AMZ_RESTORE.as_str().to_string(), RestoreStatus { is_restore_in_progress: Some(true), - restore_expiry_date: Some(Timestamp::from(OffsetDateTime::now_utc())), + restore_expiry_date: Some(OffsetDateTime::now_utc()), } .to_string(), ); diff --git a/scripts/check_s3s_footprint.sh b/scripts/check_s3s_footprint.sh index 0e6991b78..1764417db 100755 --- a/scripts/check_s3s_footprint.sh +++ b/scripts/check_s3s_footprint.sh @@ -61,14 +61,14 @@ cd "$(dirname "$0")/.." # 213 -> 212 on 2026-09-14: rustfs/backlog#1735 A4 moved rio's trailer # handle behind rustfs_rio::TrailerSource; the only adapter imports s3s through # the app storage_api shim, so crates/rio no longer references s3s. -S3S_IMPORT_FILES_BASELINE=210 +S3S_IMPORT_FILES_BASELINE=209 S3_ERROR_LINES_BASELINE=1588 # ecstore-scoped ratchet (rustfs/backlog#1842): the storage engine must not # know S3 wire/DTO types (ARCHITECTURE.md invariant 4). The S3-*consuming* # client was extracted to crates/s3-client, where s3s usage is legitimate; # this counter ratchets the remaining serving-side s3s references out of # crates/ecstore. Baseline verified on 2026-08-26. -S3S_ECSTORE_FILES_BASELINE=37 +S3S_ECSTORE_FILES_BASELINE=36 S3S_PATH_PATTERN='(^|[^"[:alnum:]_])s3s::' E2E_TEST_GLOB='--glob=!crates/e2e_test/**'