From bb5d9565a65c5c832a873ab9f4c2ed279456e920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Wed, 10 Jun 2026 15:16:14 +0800 Subject: [PATCH] feat(storage-api): add bucket DTO contract (#3314) * feat(storage-api): add bucket DTO contract * ci(build): increase workflow timeout to 90 minutes --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: houseme --- .github/workflows/build.yml | 2 +- Cargo.lock | 5 + crates/ecstore/Cargo.toml | 2 +- crates/ecstore/src/store_api.rs | 1 - crates/ecstore/src/store_api/readers.rs | 2 + crates/ecstore/src/store_api/types.rs | 42 +------- .../ecstore/tests/storage_api_compat_test.rs | 38 +++++++ crates/storage-api/Cargo.toml | 7 ++ crates/storage-api/src/bucket.rs | 100 ++++++++++++++++++ crates/storage-api/src/lib.rs | 2 + docs/architecture/compat-cleanup-register.md | 6 ++ docs/architecture/migration-progress.md | 91 ++++++++++------ 12 files changed, 224 insertions(+), 74 deletions(-) create mode 100644 crates/ecstore/tests/storage_api_compat_test.rs create mode 100644 crates/storage-api/src/bucket.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 53d1c93b7..57d90a0ba 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -202,7 +202,7 @@ jobs: needs: [ build-check, prepare-platform-matrix ] if: needs.build-check.outputs.should_build == 'true' && needs.prepare-platform-matrix.result == 'success' runs-on: ${{ matrix.os }} - timeout-minutes: 60 + timeout-minutes: 90 env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" # Always enable Tokio unstable features (required by dial9-tokio-telemetry). diff --git a/Cargo.lock b/Cargo.lock index deac367ff..6709dae4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9990,6 +9990,11 @@ dependencies = [ [[package]] name = "rustfs-storage-api" version = "1.0.0-beta.8" +dependencies = [ + "serde", + "serde_json", + "time", +] [[package]] name = "rustfs-targets" diff --git a/crates/ecstore/Cargo.toml b/crates/ecstore/Cargo.toml index 5d0425383..687b53e11 100644 --- a/crates/ecstore/Cargo.toml +++ b/crates/ecstore/Cargo.toml @@ -40,6 +40,7 @@ rustfs-utils = { workspace = true, features = ["full"] } rustfs-rio.workspace = true rustfs-rio-v2 = { workspace = true, optional = true } rustfs-signer.workspace = true +rustfs-storage-api.workspace = true rustfs-tls-runtime.workspace = true rustfs-checksums.workspace = true rustfs-config = { workspace = true, features = ["constants", "notify", "audit"] } @@ -51,7 +52,6 @@ rustfs-kms.workspace = true rustfs-s3-types = { workspace = true } rustfs-data-usage.workspace = true rustfs-object-capacity.workspace = true -rustfs-storage-api.workspace = true async-trait.workspace = true bytes.workspace = true byteorder = { workspace = true } diff --git a/crates/ecstore/src/store_api.rs b/crates/ecstore/src/store_api.rs index 8a99ea7ee..921f2001f 100644 --- a/crates/ecstore/src/store_api.rs +++ b/crates/ecstore/src/store_api.rs @@ -39,7 +39,6 @@ use rustfs_utils::CompressionAlgorithm; use rustfs_utils::http::headers::AMZ_OBJECT_TAGGING; use rustfs_utils::http::{AMZ_BUCKET_REPLICATION_STATUS, AMZ_RESTORE, AMZ_STORAGE_CLASS}; use rustfs_utils::path::decode_dir_object; -use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fmt::Debug; use std::io::Cursor; diff --git a/crates/ecstore/src/store_api/readers.rs b/crates/ecstore/src/store_api/readers.rs index 4db385e7e..ed83ac00e 100644 --- a/crates/ecstore/src/store_api/readers.rs +++ b/crates/ecstore/src/store_api/readers.rs @@ -11,6 +11,8 @@ use rustfs_kms::{service_manager::get_global_encryption_service, types::ObjectEn use rustfs_utils::http::{SSEC_ALGORITHM_HEADER, SSEC_KEY_HEADER, SSEC_KEY_MD5_HEADER}; use rustfs_utils::path::path_join_buf; #[cfg(feature = "rio-v2")] +use serde::Deserialize; +#[cfg(feature = "rio-v2")] use sha2::Sha256; use std::collections::HashMap; use std::env; diff --git a/crates/ecstore/src/store_api/types.rs b/crates/ecstore/src/store_api/types.rs index 697fa69f4..971bdaf04 100644 --- a/crates/ecstore/src/store_api/types.rs +++ b/crates/ecstore/src/store_api/types.rs @@ -1,29 +1,7 @@ use super::*; -#[derive(Debug, Default, Serialize, Deserialize)] -pub struct MakeBucketOptions { - pub lock_enabled: bool, - pub versioning_enabled: bool, - pub force_create: bool, // Create buckets even if they are already created. - pub created_at: Option, // only for site replication - pub no_lock: bool, -} - -#[derive(Debug, Default, Clone, PartialEq)] -pub enum SRBucketDeleteOp { - #[default] - NoOp, - MarkDelete, - Purge, -} - -#[derive(Debug, Default, Clone)] -pub struct DeleteBucketOptions { - pub no_lock: bool, - pub no_recreate: bool, - pub force: bool, // Force deletion - pub srdelete_op: SRBucketDeleteOp, -} +// RUSTFS_COMPAT_TODO(API-003): keep old ecstore::store_api bucket DTO import paths while storage API consumers migrate. Remove after all consumers import these DTOs from rustfs_storage_api. +pub use rustfs_storage_api::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp}; #[derive(Debug, Default, Clone)] pub struct HTTPPreconditions { @@ -225,22 +203,6 @@ fn is_modified_since(mod_time: &OffsetDateTime, given_time: &OffsetDateTime) -> mod_secs > given_secs } -#[derive(Debug, Default, Serialize, Deserialize)] -pub struct BucketOptions { - pub deleted: bool, // true only when site replication is enabled - pub cached: bool, // true only when we are requesting a cached response instead of hitting the disk for example ListBuckets() call. - pub no_metadata: bool, -} - -#[derive(Debug, Clone, Serialize, Deserialize, Default)] -pub struct BucketInfo { - pub name: String, - pub created: Option, - pub deleted: Option, - pub versioning: bool, - pub object_locking: bool, -} - #[derive(Debug, Default, Clone)] pub struct MultipartUploadResult { pub upload_id: String, diff --git a/crates/ecstore/tests/storage_api_compat_test.rs b/crates/ecstore/tests/storage_api_compat_test.rs new file mode 100644 index 000000000..97b43054a --- /dev/null +++ b/crates/ecstore/tests/storage_api_compat_test.rs @@ -0,0 +1,38 @@ +// 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. + +use rustfs_ecstore::store_api::{ + BucketInfo as EcstoreBucketInfo, BucketOptions as EcstoreBucketOptions, MakeBucketOptions as EcstoreMakeBucketOptions, +}; +use rustfs_storage_api::{ + BucketInfo as ApiBucketInfo, BucketOptions as ApiBucketOptions, MakeBucketOptions as ApiMakeBucketOptions, +}; + +#[test] +fn old_store_api_bucket_dto_path_reexports_storage_api_types() { + let ecstore_bucket: EcstoreBucketInfo = ApiBucketInfo { + name: "photos".to_owned(), + versioning: true, + ..Default::default() + }; + let api_bucket: ApiBucketInfo = ecstore_bucket; + + let ecstore_make: EcstoreMakeBucketOptions = ApiMakeBucketOptions::default(); + let api_options: ApiBucketOptions = EcstoreBucketOptions::default(); + + assert_eq!(api_bucket.name, "photos"); + assert!(api_bucket.versioning); + assert!(!ecstore_make.lock_enabled); + assert!(!api_options.no_metadata); +} diff --git a/crates/storage-api/Cargo.toml b/crates/storage-api/Cargo.toml index 35a6a3cb0..a2ef260d6 100644 --- a/crates/storage-api/Cargo.toml +++ b/crates/storage-api/Cargo.toml @@ -27,5 +27,12 @@ categories = ["web-programming", "development-tools", "filesystem"] [lib] doctest = false +[dependencies] +serde.workspace = true +time.workspace = true + +[dev-dependencies] +serde_json.workspace = true + [lints] workspace = true diff --git a/crates/storage-api/src/bucket.rs b/crates/storage-api/src/bucket.rs new file mode 100644 index 000000000..b25214f27 --- /dev/null +++ b/crates/storage-api/src/bucket.rs @@ -0,0 +1,100 @@ +// 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. + +use serde::{Deserialize, Serialize}; +use time::OffsetDateTime; + +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct MakeBucketOptions { + pub lock_enabled: bool, + pub versioning_enabled: bool, + pub force_create: bool, + pub created_at: Option, + pub no_lock: bool, +} + +#[derive(Debug, Default, Clone, PartialEq)] +pub enum SRBucketDeleteOp { + #[default] + NoOp, + MarkDelete, + Purge, +} + +#[derive(Debug, Default, Clone)] +pub struct DeleteBucketOptions { + pub no_lock: bool, + pub no_recreate: bool, + pub force: bool, + pub srdelete_op: SRBucketDeleteOp, +} + +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct BucketOptions { + pub deleted: bool, + pub cached: bool, + pub no_metadata: bool, +} + +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct BucketInfo { + pub name: String, + pub created: Option, + pub deleted: Option, + pub versioning: bool, + pub object_locking: bool, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn bucket_info_serializes_with_existing_fields() { + let bucket = BucketInfo { + name: "photos".to_owned(), + created: Some(OffsetDateTime::UNIX_EPOCH), + deleted: None, + versioning: true, + object_locking: false, + }; + + let encoded = serde_json::to_string(&bucket).expect("serialize BucketInfo to JSON"); + let decoded: BucketInfo = serde_json::from_str(&encoded).expect("deserialize BucketInfo from JSON"); + + assert_eq!(decoded.name, "photos"); + assert_eq!(decoded.created, Some(OffsetDateTime::UNIX_EPOCH)); + assert!(decoded.versioning); + assert!(!decoded.object_locking); + } + + #[test] + fn default_bucket_options_preserve_false_flags() { + let opts = BucketOptions::default(); + + assert!(!opts.deleted); + assert!(!opts.cached); + assert!(!opts.no_metadata); + } + + #[test] + fn default_delete_bucket_options_use_noop_site_replication() { + let opts = DeleteBucketOptions::default(); + + assert_eq!(opts.srdelete_op, SRBucketDeleteOp::NoOp); + assert!(!opts.no_lock); + assert!(!opts.no_recreate); + assert!(!opts.force); + } +} diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index 4e3dd4a02..8b36b1420 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -14,6 +14,8 @@ //! Storage API contracts for RustFS. +pub mod bucket; pub mod error; +pub use bucket::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp}; pub use error::{StorageErrorCode, StorageResult}; diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index 54b3e5d15..546136cbd 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -18,6 +18,12 @@ for later deletion. - Why: legacy KMS create-key and key-status admin grants must keep working during the dedicated KMS policy migration. - Removal condition: remove after KMS admin clients and built-in policies use `kms:Configure`, `kms:DescribeKey`, and `kms:ListKeys`. - Status: planned cleanup. +- `RUSTFS_COMPAT_TODO(API-003)` + - Task: `API-003` + - File: `crates/ecstore/src/store_api/types.rs` + - Why: old `ecstore::store_api` bucket DTO import paths must keep compiling while storage API consumers migrate. + - Removal condition: remove after all consumers import bucket DTOs from the storage API crate. + - Status: planned cleanup. ## Review Checklist diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 9eb2e3660..a63148485 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,15 +5,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-kms-defaults-inventory` -- Baseline: `upstream/main` at `f80162b5c99d6825b489ad94b81dfaeca6bfa874` (after CFG-002) -- PR type for this branch: `docs-only` -- Runtime behavior changes: none -- Rust code changes: none +- Branch: `overtrue/arch-storage-api-bucket-dtos` +- Baseline: `upstream/main` at `5fef10548477d9d25b0d391874f8280bf259d10e` +- PR type for this branch: `api-extraction` +- Runtime behavior changes: none. +- Rust code changes: move the pure bucket/options DTO subset from + `rustfs-ecstore` into `rustfs-storage-api`, while preserving old + `ecstore::store_api` import paths through a temporary compatibility + re-export. - CI/script changes: none -- Docs changes: add KMSD-001 inventory for current KMS development defaults, - classify each default as production-safe, dev-only, or invalid for - production, and record follow-up hardening boundaries. +- Docs changes: record API-003 bucket DTO compatibility cleanup. ## Phase 0 Tasks @@ -151,10 +152,11 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Acceptance: `rustfs-storage-api` is a workspace member and remains a dependency-free contract crate. - Verification: `cargo check -p rustfs-storage-api`. -- [~] `API-002` Move public storage error/result contracts. - - Current slice: add public `StorageErrorCode` and `StorageResult` contracts - in `rustfs-storage-api`, then make ECStore `StorageError::to_u32/from_u32` - consume the shared code table. +- [x] `API-002` Move public storage error/result contracts. + - Current PR: `rustfs/rustfs#3313` merged. + - Completed slice: add public `StorageErrorCode` and `StorageResult` + contracts in `rustfs-storage-api`, then make ECStore + `StorageError::to_u32/from_u32` consume the shared code table. - Deferred: keep the full ECStore `StorageError` enum and ECStore-specific conversions in `rustfs-ecstore` until the `DiskError`, filemeta, lock, and `std::io::Error` downcast boundary is proven safe. @@ -165,6 +167,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block quorum classification, and reserved code gaps `0x2B/0x2C`. - Risk defense: no storage hot-path enum move in this PR; only numeric code mapping uses the new contract. +- [~] `API-003` Move DTOs. + - Current branch: move the pure bucket/options DTO subset: + `MakeBucketOptions`, `SRBucketDeleteOp`, `DeleteBucketOptions`, + `BucketOptions`, and `BucketInfo`. + - Acceptance: `rustfs-storage-api` exports these DTOs, ECStore re-exports + them from the old `ecstore::store_api` path, and compatibility cleanup is + registered with `RUSTFS_COMPAT_TODO(API-003)`. + - Must preserve: no `ObjectOptions`, `ObjectInfo`, reader, compression, + encryption, filemeta conversion, multipart conversion, route, storage, or + runtime behavior changes in this PR. ## Phase 8 Background Controller Tasks @@ -189,14 +201,12 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Next PRs -1. Continue `API-002` only after reviewing whether `DiskError` and - `std::io::Error` conversion ownership can move without orphan-rule or - downcast behavior loss. -2. `contract`: move DTOs that are contract-only in `API-003`; keep ECStore - implementation, KMS/SSE readers, erasure logic, and remote disk internals out - of rustfs-storage-api. -3. `test-only`: add focused compatibility checks before moving store traits or - consumer imports. +1. `api-extraction`: continue API-003 with the next pure DTO subset only after + the bucket/options compatibility re-export is reviewed. +2. `contract`: wait for API-002/#3313 before adding error-aware storage API + traits. +3. `test-only`: add focused preservation tests before moving scanner, heal, + replication, lifecycle, or disk health workers. 4. `api-extraction`: move only the pure server-config model into rustfs-config as CFG-003. 5. `api-extraction`: keep the old rustfs_ecstore::config::* path with @@ -212,6 +222,12 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| +| Quality/architecture | pass | Only pure bucket/options DTOs moved into `rustfs-storage-api`; object, reader, compression, encryption, filemeta, multipart, storage, and runtime logic stayed in ECStore. | +| Migration preservation | pass | Old `ecstore::store_api` import paths remain through `RUSTFS_COMPAT_TODO(API-003)` compatibility re-export, with cleanup registered. | +| Testing/verification | pass | Focused DTO tests, ECStore compatibility test, migration guards, formatting, rio-v2 clippy, dependency review, diff checks, and pre-commit passed. | +| Quality/architecture | pass | Single `docs-only` PR; ADR chooses existing rustfs-config, records module path and dependency boundaries, and avoids a speculative new crate. | +| Migration preservation | pass | No code movement; ADR explicitly keeps persistence helpers, global server-config state, startup order, and old-path compatibility requirements out of CFG-002. | +| Testing/verification | pass | Docs-only verification uses migration guard scripts, metrics reference guard, layer dependency guard, and whitespace checks. | | Quality/architecture | pass | Single `docs-only` PR; the inventory is isolated under `docs/architecture`, uses existing KMS source files as evidence, and introduces no new abstraction or dependency edge. | | Migration preservation | pass | No runtime code, config persistence, admin authorization, startup order, storage path, global state, or crate boundary changes are made. | | Testing/verification | pass | Docs-only verification is bounded to diff review, architecture migration rules, metrics reference guard, layer dependency guard, and whitespace checks. | @@ -219,26 +235,39 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Verification Notes Passed: + +- `cargo test -p rustfs-storage-api` +- `cargo test -p rustfs-ecstore --test storage_api_compat_test` +- `cargo check -p rustfs-storage-api -p rustfs-ecstore` +- `cargo check -p rustfs-ecstore --features rio-v2` +- `cargo clippy -p rustfs-ecstore --features rio-v2 --all-targets -- -D warnings` +- `cargo fmt --all` +- `cargo fmt --all --check` +- `cargo test -p rustfs-ecstore error -- --nocapture` - `./scripts/check_architecture_migration_rules.sh` - `./scripts/check_layer_dependencies.sh` - `./scripts/check_metrics_migration_refs.sh` +- `./scripts/check_unsafe_code_allowances.sh` - `git diff --check` -- `git diff --name-only -- '*.rs' 'Cargo.toml' 'Cargo.lock' '.github/**' 'Makefile' 'Justfile'` +- `cargo tree -p rustfs-storage-api --edges normal` +- `make NUM_CORES=1 pre-commit` Notes: -- This branch changes architecture documentation only. -- No Rust source, Cargo manifest, workflow, script, or runtime configuration is - changed. -- `make pre-commit` is intentionally not required for this docs-only PR. +- Plain `make pre-commit` runs `fmt` and `unsafe-code-check` concurrently via + global Makefile parallelism; `unsafe-code-check` passes standalone, and the + full pre-commit target passed when run serially with `NUM_CORES=1`. +- Full nextest in pre-commit: 5704 passed, 111 skipped. +- Workspace doctests passed. ## Handoff Notes -- Keep this KMSD-001 branch as a focused `docs-only` PR. Do not change KMS - defaults, admin authorization, admin route registration shape, config moves, - Storage API moves, runtime moves, or ECStore moves. -- `rustfs` may depend on `rustfs-security-governance` for contract metadata; - the security-governance crate must stay independent from implementation - crates and runtime state. +- Keep this API-003 branch as a focused `api-extraction` PR for bucket/options + DTOs only. +- Do not move `ObjectOptions`, `ObjectInfo`, `CompletePart`, reader types, + compression/encryption helpers, filemeta conversions, S3 DTO conversions, or + storage traits in this PR. +- Keep the old `ecstore::store_api` compatibility re-export until all consumers + import bucket DTOs from `rustfs_storage_api`. - Do not add temporary compatibility code without a matching `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry. - KMS production default hardening remains a separate task group; do not bundle