From 94a61d789aadf83154fafbe83be4537c5080ac40 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 27 Aug 2026 03:49:29 +0800 Subject: [PATCH] fix(guards): catch dotted-form leaf deps, pin madmin to rustfs-signer (#6692) fix(guards): catch dotted-form leaf deps; pin madmin to rustfs-signer --- ARCHITECTURE.md | 12 ++++--- docs/architecture/crate-boundaries.md | 18 ++++++---- scripts/check_architecture_migration_rules.sh | 34 ++++++++++++------- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 541e9d892..eb30d58e6 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -115,11 +115,13 @@ default build (lifecycle: 1. **Layers flow downward.** Server → Admin/App → Storage → ecstore → rio/io-core. No upward imports. -2. **Leaf crates depend only on external crates, with one adjudicated exception.** - `config`, `credentials`, `crypto`, `io-metrics`, and `madmin` take no internal - dependency except `io-metrics → rustfs-s3-ops` (transitively `rustfs-s3-types`), - a pure contract crate with no I/O and no global state. Adjudicated in - rustfs/backlog#1834 and pinned by the leaf allowlist in +2. **Leaf crates depend only on external crates, with adjudicated exceptions + pinned by a guard.** `config`, `credentials`, and `crypto` take no internal + dependency. `io-metrics` takes exactly `rustfs-s3-ops` (transitively + `rustfs-s3-types`), a pure contract crate with no I/O and no global state — + adjudicated in rustfs/backlog#1834. `madmin` left the leaf set when #6166 made + it the SigV4-signed admin SDK client; its internal dependency surface is pinned + to exactly `rustfs-signer`. Both pins live in the leaf allowlist in `scripts/check_architecture_migration_rules.sh`; any other internal dependency fails the guard ([crate boundaries](docs/architecture/crate-boundaries.md)). - ✅ RESOLVED: the historical `utils → config` and `common → filemeta`/`madmin` diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index a8851caa2..909305556 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -39,14 +39,18 @@ Leaf crates carry exactly one adjudicated allowed edge: `io-metrics -> rustfs-s3-ops` (transitively `rustfs-s3-types`). Both are pure contract crates — types and enums only, no I/O, no global state, no non-contract internal dependencies — so `io-metrics` reuses the `S3Operation` vocabulary -instead of copying it. The allowance covers that edge and nothing else: the -leaf-crate allowlist in `scripts/check_architecture_migration_rules.sh` fails any -other `rustfs-*` dependency in `config`, `credentials`, `crypto`, `io-metrics`, -or `madmin`. Adjudicated in +instead of copying it. `madmin` is no longer counted a leaf: since #6166 it is +the SigV4-signed admin SDK client and deliberately depends on `rustfs-signer`; +the guard pins its internal dependency surface to exactly that edge so it cannot +quietly grow storage-side dependencies. The leaf-crate allowlist in +`scripts/check_architecture_migration_rules.sh` fails any other `rustfs-*` +dependency in `config`, `credentials`, `crypto`, `io-metrics`, or `madmin`, in +either TOML spelling (`rustfs-x = ...` or `rustfs-x.workspace = true`). +Adjudicated in [`rustfs/backlog#1834`](https://github.com/rustfs/backlog/issues/1834); a further -exception must meet the same criterion — pure contract crate, no I/O, no globals, -no non-contract internal dependencies — and land its guard allowlist entry -alongside the dependency. +leaf exception must meet the pure-contract criterion — types and enums only, no +I/O, no globals, no non-contract internal dependencies — and land its guard +allowlist entry alongside the dependency. Dependency direction also applies to compile-time source reads: `include_str!`/`include!` of a `.rs` file must not resolve outside the diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index 99c1d87e1..cbc3ccfac 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -5452,13 +5452,17 @@ require_source_contains \ "SetDisks storage-api HealOperations compile-time coverage test" # --- Leaf crates must stay free of internal dependencies (backlog#1834) --- -# ARCHITECTURE.md invariant 2 names config, credentials, crypto, io-metrics, -# and madmin as leaf crates that depend only on external crates. Allowlist: +# ARCHITECTURE.md invariant 2 names config, credentials, crypto, and io-metrics +# as leaf crates that depend only on external crates. Allowlist: # io-metrics -> rustfs-s3-ops. That edge is DECIDED in backlog#1834: allowed as a # pure-contract-crate exception (types/enums only, no I/O, no globals, no -# non-contract internal deps), narrowed to exactly this edge. Adding any other -# rustfs-* dependency to a leaf crate needs its own adjudication, not a quiet -# Cargo.toml edit. +# non-contract internal deps), narrowed to exactly this edge. +# madmin left the leaf set when #6166 made it the SigV4-signed admin SDK client; +# its internal dependency surface is pinned to exactly rustfs-signer so it cannot +# quietly grow storage-side dependencies. Adding any other rustfs-* dependency to +# any of these five crates needs its own adjudication, not a quiet Cargo.toml edit. +# The pattern matches both TOML dependency spellings: `rustfs-x = ...` and the +# dotted `rustfs-x.workspace = true` form (which previously escaped this guard). LEAF_CRATE_DEP_HITS_FILE="${TMP_DIR}/leaf_crate_dep_hits.txt" : >"$LEAF_CRATE_DEP_HITS_FILE" ( @@ -5467,20 +5471,26 @@ LEAF_CRATE_DEP_HITS_FILE="${TMP_DIR}/leaf_crate_dep_hits.txt" manifest="crates/${leaf}/Cargo.toml" [[ -f "$manifest" ]] || continue leaf_dep_status=0 - rg -n --with-filename '^rustfs-[a-z0-9-]+ *=' "$manifest" >"${TMP_DIR}/leaf_dep_raw.txt" || leaf_dep_status=$? + rg -n --with-filename '^rustfs-[a-z0-9-]+(\.[a-zA-Z_-]+)* *=' "$manifest" >"${TMP_DIR}/leaf_dep_raw.txt" || leaf_dep_status=$? if [[ "$leaf_dep_status" -ne 0 && "$leaf_dep_status" -ne 1 ]]; then exit "$leaf_dep_status" fi - if [[ "$leaf" == "io-metrics" ]]; then - rg -v '^[^:]*:[0-9]+:rustfs-s3-ops *=' "${TMP_DIR}/leaf_dep_raw.txt" >>"$LEAF_CRATE_DEP_HITS_FILE" || true - else - cat "${TMP_DIR}/leaf_dep_raw.txt" >>"$LEAF_CRATE_DEP_HITS_FILE" - fi + case "$leaf" in + io-metrics) + rg -v '^[^:]*:[0-9]+:rustfs-s3-ops(\.[a-zA-Z_-]+)* *=' "${TMP_DIR}/leaf_dep_raw.txt" >>"$LEAF_CRATE_DEP_HITS_FILE" || true + ;; + madmin) + rg -v '^[^:]*:[0-9]+:rustfs-signer(\.[a-zA-Z_-]+)* *=' "${TMP_DIR}/leaf_dep_raw.txt" >>"$LEAF_CRATE_DEP_HITS_FILE" || true + ;; + *) + cat "${TMP_DIR}/leaf_dep_raw.txt" >>"$LEAF_CRATE_DEP_HITS_FILE" + ;; + esac done ) if [[ -s "$LEAF_CRATE_DEP_HITS_FILE" ]]; then - report_failure "leaf crates (config/credentials/crypto/io-metrics/madmin) must not depend on internal rustfs-* crates (sole adjudicated exception, decided in backlog#1834: io-metrics -> rustfs-s3-ops, a pure contract crate; any new edge needs its own adjudication): $(paste -sd '; ' "$LEAF_CRATE_DEP_HITS_FILE")" + report_failure "leaf/pinned-dep crates: config/credentials/crypto take no internal rustfs-* dependency; io-metrics only rustfs-s3-ops (pure contract crate, backlog#1834); madmin only rustfs-signer (admin SDK SigV4 client, #6166); any new edge needs its own adjudication: $(paste -sd '; ' "$LEAF_CRATE_DEP_HITS_FILE")" fi # --- ecstore module-level lint blankets (backlog#1823 step 9) ---