From f80162b5c99d6825b489ad94b81dfaeca6bfa874 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 10:02:53 +0800 Subject: [PATCH] docs: decide config model boundary (#3307) --- .../architecture/config-model-boundary-adr.md | 162 ++++++++++++++++++ docs/architecture/crate-boundaries.md | 7 + docs/architecture/migration-progress.md | 78 ++++++--- docs/architecture/overview.md | 4 + 4 files changed, 223 insertions(+), 28 deletions(-) create mode 100644 docs/architecture/config-model-boundary-adr.md diff --git a/docs/architecture/config-model-boundary-adr.md b/docs/architecture/config-model-boundary-adr.md new file mode 100644 index 000000000..4e07c0116 --- /dev/null +++ b/docs/architecture/config-model-boundary-adr.md @@ -0,0 +1,162 @@ +# Config Model Boundary ADR + +Related issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) + +Task: `CFG-002` + +## Decision + +Use the existing `crates/config` package (`rustfs-config`) as the target owner +for the pure server-config model. Do not create a new config-model crate for +the first extraction. + +The next model extraction PR should introduce the model under: + +```text +crates/config/src/server_config.rs +``` + +The exported path should be: + +```rust +rustfs_config::server_config::{Config, KV, KVS} +``` + +The existing path must remain available through a temporary compatibility +re-export: + +```rust +rustfs_ecstore::config::{Config, KV, KVS} +``` + +That re-export must include `RUSTFS_COMPAT_TODO(CFG-004)` and a matching entry in +[`compat-cleanup-register.md`](compat-cleanup-register.md). + +## Why `rustfs-config` + +`rustfs-config` is already the lowest RustFS crate for configuration constants +and subsystem identifiers used by ECStore, notify, audit, targets, scanner, IAM, +and admin code. The current `ecstore::config::{Config, KV, KVS}` model already +uses `rustfs-config` constants, so moving the pure model upward to +`rustfs-config` cuts the wrong dependency direction without adding another crate. + +Creating a new crate now would add a second config namespace before consumers +are migrated. That would increase re-export and compatibility surface while not +removing any storage or runtime dependency by itself. + +## Allowed Dependencies + +The server-config model module may use only: + +- `std::collections::HashMap` +- `std::sync::{LazyLock, OnceLock}` for the default `KVS` registration surface +- `serde` for `KV` and `KVS` serialization compatibility +- `serde_json` for `Config::marshal` and `Config::unmarshal` +- existing `rustfs-config` constants and subsystem modules + +If `serde` and `serde_json` are added to `rustfs-config`, they should be attached +only to a model feature such as `server-config-model` unless the implementation +PR proves that making them non-optional is simpler and harmless for downstream +builds. + +## Forbidden Dependencies + +The model module must not depend on: + +- `rustfs-ecstore` +- `rustfs` +- `StorageAPI` or object persistence helpers +- notify, audit, targets, IAM, scanner, KMS, or admin handler crates +- async runtimes, HTTP/router crates, object-store crates, or runtime lifecycle + state +- global server-config snapshot state such as `GLOBAL_SERVER_CONFIG` +- `ConfigSys`, `read_config_without_migrate`, `save_server_config`, or any + `com.rs` persistence helper + +## Boundary Split + +Move in the first extraction: + +- `KV` +- `KVS` +- `Config` +- `DEFAULT_KVS` +- `register_default_kvs` +- `Config::new` +- `Config::get_value` +- `Config::set_defaults` +- `Config::marshal` +- `Config::unmarshal` +- `Config::merge` + +Keep in `ecstore`: + +- `ConfigSys` +- `GLOBAL_SERVER_CONFIG` +- `get_global_server_config` +- `set_global_server_config` +- `init_global_config_sys` +- `try_migrate_server_config` +- `read_config_without_migrate` +- `save_server_config` +- generic `com.rs` config-object helpers +- storage-class runtime global state + +Keep default registration wiring in `ecstore::config::init` until a later PR +extracts a dedicated default-registration contract. The values may be registered +through the moved `rustfs_config::server_config::register_default_kvs`, but the +startup order and caller remain unchanged. + +## Required Shape Preservation + +The extraction PR must preserve: + +- `KV { key, value, hidden_if_empty }` +- `#[serde(default, alias = "hiddenIfEmpty")]` on `KV::hidden_if_empty` +- `KVS(pub Vec)` +- `Config(pub HashMap>)` +- `KVS::new`, `get`, `lookup`, `is_empty`, `keys`, `insert`, and `extend` +- `Config::new`, `get_value`, `set_defaults`, `marshal`, `unmarshal`, and + `merge` +- `Config::new()` default application after `ecstore::config::init()` +- existing persisted server-config JSON shape +- existing target, notify, audit, scanner, OIDC, and admin interpretation of + `Config` and `KVS` + +## Next PR Requirements + +`CFG-003` should be a pure model extraction or narrow `api-extraction` PR. It +must not migrate consumers, change persistence helpers, or alter runtime +behavior. + +`CFG-004` should keep the old `rustfs_ecstore::config::*` path as a temporary +compatibility shim and register its removal condition. + +`CFG-005` should migrate external consumers one group at a time after the model +and compatibility path are stable. + +## Verification Gate + +Before pushing an extraction PR, run: + +- serde roundtrip tests for old and new paths +- tests for `hiddenIfEmpty` alias compatibility +- tests for `KVS` insertion, lookup, extension, and keys behavior +- tests for `Config::new`, `set_defaults`, `marshal`, `unmarshal`, and `merge` +- a compile smoke test that the old `rustfs_ecstore::config::{Config, KV, KVS}` + path still works +- `cargo tree -p rustfs-config --edges normal` +- `cargo tree -p rustfs-ecstore --edges normal` +- `./scripts/check_layer_dependencies.sh` +- `./scripts/check_architecture_migration_rules.sh` +- `./scripts/check_metrics_migration_refs.sh` +- `cargo fmt --all --check` +- `make pre-commit` + +## Non-Goals + +- No consumer migration in `CFG-002`. +- No code movement in `CFG-002`. +- No new crate in `CFG-002`. +- No `com.rs` or `StorageAPI` movement in the first model extraction. +- No global server-config state migration until the model path is stable. diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index bbd1745de..927fc6abb 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -75,3 +75,10 @@ Rules: `ecstore::config::{Config, KV, KVS}` should move before extension config adapters or config-schema work. First inventory consumers, then decide whether existing `crates/config` is enough or whether a smaller model crate is required. + +The current decision is recorded in +[`config-model-boundary-adr.md`](config-model-boundary-adr.md): use the existing +`rustfs-config` package for the pure server-config model, keep persistence and +global server-config state in `ecstore`, and preserve the old +`rustfs_ecstore::config::*` path with a temporary compatibility marker during +the first extraction. diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index ca06024dc..bd7fead22 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,14 +5,14 @@ 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-storage-api-error-contracts` -- Baseline: `upstream/main` at `5fef10548477d9d25b0d391874f8280bf259d10e` -- PR type for this branch: `contract` +- Branch: `overtrue/arch-config-consumer-inventory` +- Baseline: `upstream/main` at `a73c90c813bba16e668be090c5c4ca22c765b81b` +- PR type for this branch: `docs-only` - Runtime behavior changes: none. -- Rust code changes: add the `rustfs-storage-api` error-code/result contract - and route ECStore storage error numeric conversion through that contract. +- Rust code changes: none. - CI/script changes: none -- Docs changes: record the API-002 first-slice contract boundary. +- Docs changes: add the CFG-002 config model boundary ADR and link it from the + architecture overview, crate-boundary guardrails, and this progress handoff. ## Phase 0 Tasks @@ -62,6 +62,27 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block when a register entry lacks a source marker, or when a source marker omits a removal condition. +## Phase 1a Config Model Tasks + +- [x] `CFG-001` Inventory `ecstore::config::{Config, KV, KVS}` consumers. + - Acceptance: + [`ecstore-config-consumer-inventory.md`](ecstore-config-consumer-inventory.md) + records the current definitions, persistence helpers, global accessors, + consumer groups, migration risks, and do-not-change contract. +- [x] `CFG-002` Decide model boundary. + - Acceptance: + [`config-model-boundary-adr.md`](config-model-boundary-adr.md) records + `rustfs-config` as the target package, `server_config` as the future model + module, allowed dependencies, forbidden dependencies, preserved shape, and + extraction verification gates. +- [ ] `CFG-003` Move pure model definitions. + - Next boundary: move only `Config`, `KV`, `KVS`, and default-registration + surface into `rustfs-config`; keep persistence helpers and global + server-config state in `ecstore`. +- [ ] `CFG-004` Keep old `ecstore::config::*` compatibility path. + - Required compatibility: source must contain `RUSTFS_COMPAT_TODO(CFG-004)` + and a matching cleanup-register entry. + ## Phase 1 Security Governance Tasks - [x] `S-001` Add `crates/security-governance`. @@ -166,45 +187,46 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block of rustfs-storage-api. 3. `test-only`: add focused compatibility checks before moving store traits or consumer imports. +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 + RUSTFS_COMPAT_TODO(CFG-004) and cleanup-register coverage. +6. `consumer-migration`: migrate external consumers one group at a time only + after the model path and compatibility shim are stable. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | rustfs-storage-api only adds `StorageErrorCode` and `StorageResult`; ECStore keeps `StorageError`, `DiskError`, filemeta, lock, and `std::io::Error` conversion ownership. | -| Migration preservation | pass | ECStore numeric conversion now consumes the shared code table while preserving old variant defaults, reserved gaps `0x2B/0x2C`, and existing display/conversion logic. | -| Testing/verification | pass | Focused storage-api and ECStore tests, cargo check, dependency guard, migration guards, Rust quality scan, `make pre-commit`, nextest, and doctests 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. | ## Verification Notes Passed: -- `cargo fmt --all` -- `cargo fmt --all --check` -- `cargo test -p rustfs-storage-api` -- `cargo test -p rustfs-ecstore error -- --nocapture` -- `cargo check -p rustfs-storage-api -p rustfs-ecstore` -- `cargo tree -p rustfs-storage-api --edges normal` - `./scripts/check_architecture_migration_rules.sh` - `./scripts/check_layer_dependencies.sh` - `./scripts/check_metrics_migration_refs.sh` - `git diff --check` -- Rust quality scan on changed Rust files. -- `make pre-commit` - - Full nextest: 5704 passed, 111 skipped. - - Workspace doctests passed. +- `git diff --name-only -- '*.rs' 'Cargo.toml' 'Cargo.lock' '.github/**' 'Makefile' 'Justfile'` Notes: -- This branch changes Rust storage contract code and ECStore contract mapping. -- Rust quality scan reported only existing patterns in `crates/ecstore/src/error.rs`: - a test-only `unwrap()` and the pre-existing `StorageError::other` boxed error - boundary. This PR does not add either pattern. +- 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. ## Handoff Notes -- Keep this branch as the API-002 first slice, not the full error enum move. -- Do not move `StorageError`, `DiskError`, filemeta conversion, lock conversion, - or `std::io::Error` downcast behavior out of ECStore in this PR. -- Do not add ECStore implementation, KMS/SSE reader logic, erasure logic, or - remote disk internals to `rustfs-storage-api`. +- Keep this CFG-002 branch as a focused `docs-only` PR. Do not move + `Config`, `KV`, `KVS`, persistence helpers, global server-config state, + Storage API code, startup code, or target/notify/audit/IAM consumers in this + branch. +- The next extraction PR must preserve the tuple-struct shape, serde fields, + `hiddenIfEmpty` alias, `Config::new` default behavior, marshal/unmarshal + behavior, and old `rustfs_ecstore::config::*` path. - Do not add temporary compatibility code without a matching `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry. +- Do not create a new config-model crate unless a later implementation attempt + proves `rustfs-config` cannot hold the pure model boundary. diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index c59d31149..dbd63b303 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -33,6 +33,9 @@ hot-path behavior must not drift during this migration. - [`ecstore-config-consumer-inventory.md`](ecstore-config-consumer-inventory.md): current `ecstore::config::{Config, KV, KVS}` definitions, consumers, migration risks, and do-not-change contract. +- [`config-model-boundary-adr.md`](config-model-boundary-adr.md): target crate, + module path, dependency rules, and verification gates for moving the pure + server-config model. - [`migration-progress.md`](migration-progress.md): current task state and context handoff. - [`compat-cleanup-register.md`](compat-cleanup-register.md): temporary @@ -71,3 +74,4 @@ The first implementation sequence is conservative: 2. Establish PR and compatibility rules. 3. Add dependency and loss-prevention checks in a separate `ci-gate` PR. 4. Inventory `ecstore::config::{Config, KV, KVS}` before moving any code. +5. Decide the config model boundary before extracting or migrating consumers.