docs: decide config model boundary (#3307)

2026-07-26 08:18:18 +00:00 · 2026-06-10 10:02:53 +08:00
parent 2eafd9c602
commit f80162b5c9
4 changed files with 223 additions and 28 deletions
@@ -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<KV>)`
+- `Config(pub HashMap<String, HashMap<String, KVS>>)`
+- `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.
@@ -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.
@@ -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(<task-id>)` 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.
@@ -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.