diff --git a/.config/s3s-edge-allowlist.txt b/.config/s3s-edge-allowlist.txt new file mode 100644 index 000000000..929320242 --- /dev/null +++ b/.config/s3s-edge-allowlist.txt @@ -0,0 +1,37 @@ +# Files that may still reference s3s (`s3s::` or `s3s_sigv4`) when Phase 1 of +# the gateway migration ends (rustfs/backlog#2734 task T0.6, rustfs/backlog#2741). +# Consumed only by `scripts/check_s3s_footprint.sh --mode allowlist`. +# +# One entry per line: a repository-relative file path or a git `:(glob)` +# pathspec. `**` matches across directories, `*` stays within one path +# component, and a directory entry matches everything under it. Blank lines +# and lines starting with `#` are ignored; an entry may not contain whitespace, +# so a note belongs on its own comment line. +# +# Every Rust file listed by `git grep -l '\bs3s\(::\|_sigv4\)' -- '*.rs'` that +# no entry matches fails the allowlist mode. Task T1.9 switches CI to that +# mode; until then `--dry-run` prints the files still outside this list, which +# is the Phase 1 tracking list. +# +# Never add crates/** other than crates/e2e_test: the engine crates leave s3s +# entirely, and the script rejects such an entry. Only the s3s-facing edge of +# the rustfs binary and the e2e oracle keep it. + +# rustfs binary: the s3s-facing edge that survives Phase 1 +rustfs/src/storage/ecfs.rs +rustfs/src/storage/s3_compat/** +rustfs/src/storage/access.rs +rustfs/src/auth.rs +rustfs/src/error.rs +rustfs/src/storage_api.rs +rustfs/src/app/storage_api.rs +rustfs/src/admin/router.rs +rustfs/src/app/metadata_route.rs +rustfs/src/server/http.rs +rustfs/src/server/layer.rs +rustfs/src/server/hybrid.rs +rustfs/src/protocols/client.rs + +# e2e oracle: the test target that verifies S3 behaviour against s3s +crates/e2e_test/src/fake_s3_target/** +crates/e2e_test/src/** diff --git a/.config/s3s-footprint-baseline.txt b/.config/s3s-footprint-baseline.txt new file mode 100644 index 000000000..89617cbab --- /dev/null +++ b/.config/s3s-footprint-baseline.txt @@ -0,0 +1,16 @@ +# Exact baseline for the direct s3s footprint, consumed only by +# scripts/check_s3s_footprint.sh (rustfs/backlog#1677; rustfs/backlog#2734 T0.4). +# +# Each value is the exact measurement of the tree, not a ceiling. CI fails when a +# measured counter differs from its line here in either direction: +# - a larger measurement means new code widened the s3s surface: route it +# through the gateway abstractions instead; never raise a value here; +# - a smaller measurement means the PR shrank the surface: lower the matching +# value to the measured number in the same PR. +# +# files Rust files referencing s3s paths, repo-wide minus crates/e2e_test +# s3_error_lines s3_error! invocation lines, same scope, summed +# ecstore_files Rust files referencing s3s paths under crates/ecstore/src +files=209 +s3_error_lines=1585 +ecstore_files=36 diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 189de36bb..8c3c75326 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -155,7 +155,7 @@ default build (lifecycle: - ⚠️ PARTIALLY VIOLATED: serving-side `s3s` references remain in ecstore (bucket metadata/replication/lifecycle DTOs and error mapping). The count is ratcheted shrink-only by `scripts/check_s3s_footprint.sh` - (`S3S_ECSTORE_FILES_BASELINE`; the `object_lock` module was converted to + (the `ecstore_files` line of `.config/s3s-footprint-baseline.txt`; the `object_lock` module was converted to storage-level types as the first ratchet step). Target state: the baseline reaches zero and ecstore's `Cargo.toml` drops `s3s`. diff --git a/docs/architecture/ecstore-module-split-plan.md b/docs/architecture/ecstore-module-split-plan.md index dd9cd8d84..d6eba81c9 100644 --- a/docs/architecture/ecstore-module-split-plan.md +++ b/docs/architecture/ecstore-module-split-plan.md @@ -22,7 +22,7 @@ find crates/ecstore/src/bucket/replication -name '*.rs' | xargs wc -l | tail -1 Rule for new code: in a domain that already has a contract crate, new logic that does not need ECStore runtime state lands in that crate (`rustfs-lifecycle`, `rustfs-replication`), not under `crates/ecstore/src/bucket/`. -The S3 client extraction is complete: the former `client/` directory moved to `crates/s3-client`, its two server-side modules moved to `crates/ecstore/src/object_api/object_api_utils.rs` and `crates/ecstore/src/bucket/lifecycle/object_handlers_common.rs`, and the remaining serving-side `s3s` references in ECStore are ratcheted shrink-only by `S3S_ECSTORE_FILES_BASELINE` in `scripts/check_s3s_footprint.sh`. +The S3 client extraction is complete: the former `client/` directory moved to `crates/s3-client`, its two server-side modules moved to `crates/ecstore/src/object_api/object_api_utils.rs` and `crates/ecstore/src/bucket/lifecycle/object_handlers_common.rs`, and the remaining serving-side `s3s` references in ECStore are ratcheted shrink-only by the `ecstore_files` line of `.config/s3s-footprint-baseline.txt`, enforced by `scripts/check_s3s_footprint.sh`. ## Non-Negotiable Rules diff --git a/scripts/README.md b/scripts/README.md index 2dab2e007..ba636d0eb 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -35,7 +35,7 @@ their issue closes. | `check_migration_gate_count.sh` | ci-gate | Migration-critical test gate with committed count floor (`.config/migration-gate-floor.txt`) | ci.yml Test and Lint; `docs/testing/README.md` | | `check_no_planning_docs.sh` | ci-gate | Blocks committed planning-type documents | ci.yml Quick Checks; `make pre-commit` | | `check_no_tokio_io_uring.sh` | ci-gate | Keeps tokio's io-uring backend disabled | ci.yml Quick Checks | -| `check_s3s_footprint.sh` | ci-gate | Lower-only ratchet freezing the direct s3s surface ahead of the s3gate migration | ci.yml Quick Checks; `make pre-commit` | +| `check_s3s_footprint.sh` | ci-gate | Exact, lower-only baseline on the direct s3s surface ahead of the gateway migration (reads `.config/s3s-footprint-baseline.txt`); `--mode allowlist` fails on any Rust file referencing s3s outside `.config/s3s-edge-allowlist.txt`, `--dry-run` lists them | ci.yml Quick Checks; `make pre-commit` | | `check_test_count_ratchet.sh` | ci-gate | Exact per-crate test-count and `#[ignore]` ratchet against `.config/test-count-baseline.txt`; no crate may lose tests or gain ignores | ci.yml Quick Checks; `make pre-commit` | | `check_unsafe_code_allowances.sh` | ci-gate | Unsafe-code allowance ledger guard | ci.yml Quick Checks | | `layer-dependency-baseline.txt` | ci-gate (data) | Committed baseline consumed by `check_layer_dependencies.sh` | [Architecture guard troubleshooting](../docs/operations/architecture-guard-troubleshooting.md) | diff --git a/scripts/check_s3s_footprint.sh b/scripts/check_s3s_footprint.sh index acbdb1f48..e0d266672 100755 --- a/scripts/check_s3s_footprint.sh +++ b/scripts/check_s3s_footprint.sh @@ -1,14 +1,51 @@ #!/usr/bin/env bash -# Ratchet guard freezing the s3s dependency footprint ahead of the -# s3gate/gateway migration (rustfs/backlog#1677, review finding F1; -# acceptance criteria recorded in rustfs/backlog#1733). +# Guard on the direct s3s dependency footprint during the gateway migration +# (rustfs/backlog#1677 review finding F1, acceptance in rustfs/backlog#1733; +# made exact by rustfs/backlog#2734 task T0.4, rustfs/backlog#2739; allowlist +# mode added by task T0.6, rustfs/backlog#2741). # -# The migration's goal is to shrink the direct s3s surface, so new code must -# not grow it. Two counters are ratcheted (verification dates live next to the -# baseline values below): +# Usage: scripts/check_s3s_footprint.sh [--mode baseline|allowlist] [--dry-run] # -# - files referencing s3s paths: rg -l "$S3S_PATH_PATTERN" --type rust . (files) -# - s3_error! invocation lines: rg -c 's3_error!' --type rust . (summed) +# The two modes are independent: each reads its own input file and runs its own +# search, so a malformed baseline file cannot fail (or pass) the allowlist mode +# and vice versa. CI runs the mode it wants; running both means two invocations. +# Any other argument, an unknown mode, a repeated --mode, or --dry-run outside +# the allowlist mode is rejected with exit 2 rather than ignored, so a typo in a +# CI step cannot silently run a different check. +# +# --------------------------------------------------------------------------- +# --mode baseline (the default) +# +# Three counters are measured and compared against the committed values in +# .config/s3s-footprint-baseline.txt, the only place the numbers live: +# +# files rg -l "$S3S_PATH_PATTERN" --type rust --glob='!crates/e2e_test/**' . +# s3_error_lines rg -c 's3_error!' --type rust --glob='!crates/e2e_test/**' . (summed) +# ecstore_files rg -l "$S3S_PATH_PATTERN" --type rust crates/ecstore/src +# +# crates/e2e_test/ is excluded from the repo-wide counters: test infrastructure +# legitimately uses s3s to verify S3 behaviour and does not widen the +# production surface. The ecstore counter (rustfs/backlog#1842) ratchets the +# serving-side s3s references out of the storage engine (ARCHITECTURE.md +# invariant 4); the S3-consuming client already moved to crates/s3-client. +# +# The comparison is EXACT and the baseline is LOWER-ONLY: +# +# measured > baseline fails with "s3s footprint grew: > ". +# Route the new code through the gateway abstractions +# instead of importing s3s. Raising a baseline value is +# never the fix: rustfs/rustfs#5964 raised the s3_error! +# ceiling to go green, and a ceiling with slack is what +# let that drift accumulate unnoticed in the first place. +# measured < baseline fails with "lower the baseline to in this PR". +# A baseline above the tree is slack, and slack is where +# the next growth hides, so the PR that shrinks the +# footprint also lowers the value. +# measured = baseline passes. +# +# End state: when all three values reach 0 the baseline file is retired and +# task T4.2 of rustfs/backlog#2734 switches this script to "any occurrence of +# s3s fails"; until then every PR leaves the baseline equal to its own tree. # # Every rg invocation MUST pass an explicit path ('.' for repo-wide): without # one, rg searches stdin instead of the tree whenever stdin is a readable @@ -17,72 +54,244 @@ # counters read 0 and were waved through as "shrank"). The sanity assertions # below fail hard if that ever regresses. # -# Either count exceeding its baseline fails the check with the offending -# delta. Baselines are LOWER-ONLY: when a PR shrinks the footprint, lower the -# matching baseline in the same PR so the ratchet stays tight. Never raise a -# baseline to get green (AGENTS.md, Verification Before PR) — route new S3 -# API code through the gateway abstractions instead of importing s3s -# directly. +# --------------------------------------------------------------------------- +# --mode allowlist # -# Usage: scripts/check_s3s_footprint.sh +# Every Rust file listed by +# +# git grep -l '\bs3s\(::\|_sigv4\)' -- '*.rs' +# +# must be matched by an entry of .config/s3s-edge-allowlist.txt: the files that +# may still reference s3s when Phase 1 of rustfs/backlog#2734 ends. A file +# outside the allowlist fails the check and is listed; with --dry-run the list +# is printed and the exit status is 0, which is the Phase 1 tracking view. +# Task T1.9 switches the CI step to this mode. The search deliberately differs +# from the baseline counters: it is the spec's git grep pattern, which also +# catches s3s_sigv4, and it covers crates/e2e_test, which the allowlist admits +# explicitly as the e2e oracle. git grep searches tracked files, as a CI +# checkout is. +# +# Output contract: stdout carries only the files outside the allowlist, one +# per line, so the dry run can be redirected into a tracking list; every +# summary, error and verdict line goes to stderr. +# +# The allowlist file is validated strictly, and --dry-run downgrades none of +# this: a missing file, an entry containing whitespace, an entry that is not a +# valid pathspec, and an entry that names or matches anything under crates/ +# other than crates/e2e_test each fail hard. The engine crates leave s3s +# entirely; rustfs/backlog#2741 forbids allowlisting them, and a wildcard that +# reaches into crates/ is the same thing spelled differently. +# +# Before the tree search the pattern is run, through git grep itself with the +# same dialect flag and in this repository's config context, against a control +# file holding two lines it must match and three it must not. A dialect that +# matches nothing would otherwise read as "no s3s outside the allowlist", which +# is the one wrong answer this mode must never give once CI depends on it. set -euo pipefail cd "$(dirname "$0")/.." -# Baselines verified on 2026-08-26. Lower-only; see header. -# Excludes crates/e2e_test/ — test infrastructure legitimately uses s3s -# to verify S3 behavior and does not widen the production s3s surface. -# 208 → 215 on 2026-08-26: PR #6670 mechanically split -# rustfs/src/app/object_usecase.rs into 8 per-operation modules (net +7 -# files, zero new s3s code — the same handler-layer surface redistributed). -# The file counter is split-sensitive; the s3_error! line counter confirms -# no growth (unchanged at 1620). -# 1620 → 1616 on 2026-08-27: backlog#1840 moved the site-replication service -# subsystem to rustfs/src/site_replication/ (s3s access funneled through the -# root storage facade's s3 shim, keeping the file count at 215). The move -# inlined one s3_error! call in transport.rs (+1); measured 1615 on the -# pre-move main (after #6694) and 1616 after, so the slack 1620 baseline is -# retightened to the measured 1616. -# 215 → 213 on 2026-08-28: multipart foreground admission cleanup moved the -# new usecase dependency behind the app object-domain facade while main had -# already shed two direct s3s-importing files. Retighten the file counter only; -# s3_error! stays flat at 1616. -# 1616 → 1613 on 2026-09-02: dependency refresh verified the current tree has -# already shed three s3_error! invocation lines; retighten the line counter. -# 1613 → 1589 on 2026-09-06: rustfs/backlog#2309 and rustfs/rustfs#7225 both -# folded the ten per-config arms of ExportBucketMetadata into one helper, which -# now reports an unreadable configuration as a plain string instead of raising -# an S3 error per arm (24 invocation lines removed from -# rustfs/src/admin/handlers/bucket_meta.rs; measured after merging the two). -# 1589 -> 1588 on 2026-09-08: the GA blocker set (rustfs/backlog#2366) added -# three invocation lines to the endpoint-refresh paths and folded the five -# copies of the concurrent-change error into one constructor, netting -1. -# 1588 -> 1586 on 2026-09-10: the release merge no longer introduces direct -# s3_error! constructors for heal percent-decoding or tagging not-found errors. -# 2026-09-15 main -> release integration retains the reviewed diagnostics -# handlers from main (+3 invocation lines relative to release) and release's -# other reductions. The merged tree measures 209 import files and 1586 -# invocation lines, tightening main's reviewed 1592-line baseline by six. -# 209 -> 208 and 1586 -> 1585 on 2026-10-08: rustfs/backlog#2787 moved the -# s3s-compat tower layers out of rustfs/src/server/layer.rs, the only file of -# them that imported s3s, and funneled the two error types the moved layers -# need through the storage_api shim (no new importing file). The line counter -# is the tree's measured value after merging main. -S3S_IMPORT_FILES_BASELINE=208 -S3_ERROR_LINES_BASELINE=1585 -# 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=36 +usage() { + echo "usage: scripts/check_s3s_footprint.sh [--mode baseline|allowlist] [--dry-run]" >&2 +} + +MODE='' +DRY_RUN=0 +while (($# > 0)); do + case "$1" in + --mode) + if (($# < 2)); then + echo "error: '--mode' needs a value: baseline or allowlist" >&2 + usage + exit 2 + fi + if [[ -n "$MODE" ]]; then + echo "error: '--mode' given more than once" >&2 + usage + exit 2 + fi + case "$2" in + baseline | allowlist) MODE="$2" ;; + *) + echo "error: unknown mode '$2'; expected baseline or allowlist" >&2 + usage + exit 2 + ;; + esac + shift 2 + ;; + --dry-run) + DRY_RUN=1 + shift + ;; + *) + echo "error: unexpected argument '$1'" >&2 + usage + exit 2 + ;; + esac +done +MODE="${MODE:-baseline}" +if ((DRY_RUN)) && [[ "$MODE" != allowlist ]]; then + echo "error: '--dry-run' is only valid with '--mode allowlist'" >&2 + usage + exit 2 +fi + +BASELINE_FILE='.config/s3s-footprint-baseline.txt' +ALLOWLIST_FILE='.config/s3s-edge-allowlist.txt' S3S_PATH_PATTERN='(^|[^"[:alnum:]_])s3s::' +# The allowlist search pattern, verbatim from rustfs/backlog#2741: a git grep +# basic regular expression (-G below pins the dialect against grep.patternType). +S3S_EDGE_PATTERN='\bs3s\(::\|_sigv4\)' E2E_TEST_GLOB='--glob=!crates/e2e_test/**' TMP_DIR="$(mktemp -d)" trap 'rm -rf "$TMP_DIR"' EXIT +# Returns 0 when every s3s-referencing Rust file is allowlisted (or on a dry +# run), 1 when files remain outside the allowlist; exits 1 on a broken input. +run_allowlist_mode() { + local git_dir control grep_status line offender total allowed outside + + if [[ ! -f "$ALLOWLIST_FILE" ]]; then + echo "error: allowlist file '$ALLOWLIST_FILE' is missing" >&2 + exit 1 + fi + + # Two-directional regex control (see header): the positive lines must + # match and the negative ones must not, through the same engine and flag. + # --git-dir keeps the control in this repository's config context, so a + # grep.patternType set here reaches the control as well as the measurement. + git_dir="$(git rev-parse --absolute-git-dir)" + printf '%s\n' 'use s3s::Body;' 'use s3s_sigv4::Sig;' \ + 'use ms3s::X;' 'use s3sx::Y;' 'use s3s_other::Z;' >"$TMP_DIR/control.rs" + control="$(git -C "$TMP_DIR" --git-dir="$git_dir" grep --no-index -G -c -e "$S3S_EDGE_PATTERN" -- control.rs &2 + echo " 'control.rs:2'; git grep or its regex dialect is broken, so a tree result would be" >&2 + echo " meaningless." >&2 + exit 1 + fi + + # Exit 1 is "no match", a legitimate end state once every s3s reference is + # gone; only a larger status is an error. + grep_status=0 + git grep -l -G -e "$S3S_EDGE_PATTERN" -- '*.rs' >"$TMP_DIR/s3s_files" 1)); then + echo "error: \"git grep -l -G -e '$S3S_EDGE_PATTERN' -- '*.rs'\" failed with status $grep_status" >&2 + exit 1 + fi + LC_ALL=C sort -u -o "$TMP_DIR/s3s_files" "$TMP_DIR/s3s_files" + + : >"$TMP_DIR/allowed_files" + while IFS= read -r line || [[ -n "$line" ]]; do + case "$line" in + '' | '#'*) continue ;; + esac + if [[ "$line" =~ [[:space:]] ]]; then + echo "error: '$ALLOWLIST_FILE' entry '$line' contains whitespace; one path or glob per" >&2 + echo " line, and a note goes on its own '#' line" >&2 + exit 1 + fi + case "$line" in + crates/e2e_test | crates/e2e_test/*) ;; + crates | crates/*) + echo "error: '$ALLOWLIST_FILE' entry '$line' allowlists crates/ outside crates/e2e_test;" >&2 + echo " the engine crates leave s3s entirely (rustfs/backlog#2741)" >&2 + exit 1 + ;; + esac + if ! git ls-files -- ":(glob)$line" >"$TMP_DIR/entry_files" &2 + exit 1 + fi + # The textual check above catches the spelled-out case; this one + # catches a wildcard whose matches reach into crates/ (e.g. '**/mod.rs'). + offender="$(awk '/^crates\// && !/^crates\/e2e_test\// { print; exit }' "$TMP_DIR/entry_files")" + if [[ -n "$offender" ]]; then + echo "error: '$ALLOWLIST_FILE' entry '$line' matches '$offender', which is under crates/" >&2 + echo " outside crates/e2e_test; the engine crates leave s3s entirely (rustfs/backlog#2741)" >&2 + exit 1 + fi + cat "$TMP_DIR/entry_files" >>"$TMP_DIR/allowed_files" + done <"$ALLOWLIST_FILE" + LC_ALL=C sort -u -o "$TMP_DIR/allowed_files" "$TMP_DIR/allowed_files" + + LC_ALL=C comm -12 "$TMP_DIR/s3s_files" "$TMP_DIR/allowed_files" >"$TMP_DIR/allowed_hits" + LC_ALL=C comm -23 "$TMP_DIR/s3s_files" "$TMP_DIR/allowed_files" >"$TMP_DIR/outside" + total="$(grep -c . "$TMP_DIR/s3s_files" || true)" + allowed="$(grep -c . "$TMP_DIR/allowed_hits" || true)" + outside="$(grep -c . "$TMP_DIR/outside" || true)" + if ((allowed + outside != total)); then + echo "error: allowlist partition does not add up: $allowed allowlisted + $outside outside != $total total" >&2 + exit 1 + fi + + echo "s3s edge allowlist: $total Rust files reference s3s, $allowed allowlisted, $outside outside the allowlist" >&2 + cat "$TMP_DIR/outside" + if ((outside == 0)); then + echo "✅ s3s edge allowlist check passed: no s3s reference outside $ALLOWLIST_FILE" >&2 + return 0 + fi + if ((DRY_RUN)); then + echo "s3s edge allowlist dry run: $outside files still reference s3s outside $ALLOWLIST_FILE (listed on stdout; not a failure)" >&2 + return 0 + fi + echo "❌ s3s edge allowlist: $outside files reference s3s outside $ALLOWLIST_FILE (listed on stdout)" >&2 + echo " Route the code through the gateway abstractions instead of importing s3s. The" >&2 + echo " allowlist names the edge that keeps s3s and is never widened to go green." >&2 + return 1 +} + +# Dispatched before the baseline inputs are read: see the header. +if [[ "$MODE" == allowlist ]]; then + if run_allowlist_mode; then + exit 0 + fi + exit 1 +fi + +# The baseline file is the guard's only input and always exists in a checkout; +# a missing or malformed file is a broken guard, never a pass. +if [[ ! -f "$BASELINE_FILE" ]]; then + echo "error: baseline file '$BASELINE_FILE' is missing" >&2 + exit 1 +fi + +baseline_value() { + local key="$1" lines value + lines="$(grep -E "^${key}=" "$BASELINE_FILE" || true)" + if [[ -z "$lines" ]]; then + echo "error: '$BASELINE_FILE' has no '${key}=' line" >&2 + exit 1 + fi + if (($(printf '%s\n' "$lines" | wc -l) != 1)); then + echo "error: '$BASELINE_FILE' defines '${key}' more than once" >&2 + exit 1 + fi + value="${lines#*=}" + if ! [[ "$value" =~ ^[0-9]+$ ]]; then + echo "error: '$BASELINE_FILE' value for '${key}' is not a non-negative integer: '$value'" >&2 + exit 1 + fi + printf '%s\n' "$value" +} + +# Any non-comment line that is not one of the three keys is a typo that would +# otherwise read as "missing key" or be ignored outright. +if grep -Ev '^[[:space:]]*(#|$)' "$BASELINE_FILE" \ + | grep -Ev '^(files|s3_error_lines|ecstore_files)=' >/dev/null; then + echo "error: '$BASELINE_FILE' contains a line that is neither a comment nor one of" >&2 + echo " files=, s3_error_lines=, ecstore_files=" >&2 + exit 1 +fi + +files_baseline="$(baseline_value files)" +s3_error_lines_baseline="$(baseline_value s3_error_lines)" +ecstore_files_baseline="$(baseline_value ecstore_files)" + # rg exits 1 on zero matches (a legitimate count of 0 at the end of the # migration) and >1 on real errors; only the latter may abort the check. run_rg_to() { @@ -115,8 +324,8 @@ done # the repo-wide file count dropping below the ecstore-scoped one (a strict # subset of it), means the counter itself broke — most likely rg searching # stdin instead of the tree (see header) — not that the footprint shrank. -# Fail hard rather than waving the ratchet through. If the footprint ever -# genuinely reaches zero, lower the baseline to 0 in the same PR. +# Fail hard rather than telling the author to lower the baseline to 0. If the +# footprint ever genuinely reaches zero, lower the baseline to 0 in the same PR. sanity_nonzero() { local label="$1" count="$2" baseline="$3" if ((count == 0 && baseline > 0)); then @@ -125,8 +334,8 @@ sanity_nonzero() { exit 1 fi } -sanity_nonzero "files importing s3s" "$s3s_import_files" "$S3S_IMPORT_FILES_BASELINE" -sanity_nonzero "s3_error! invocation lines" "$s3_error_lines" "$S3_ERROR_LINES_BASELINE" +sanity_nonzero "files importing s3s" "$s3s_import_files" "$files_baseline" +sanity_nonzero "s3_error! invocation lines" "$s3_error_lines" "$s3_error_lines_baseline" if ((s3s_import_files < s3s_ecstore_files)); then echo "error: repo-wide s3s file count ($s3s_import_files) is below the ecstore-scoped" >&2 echo " count ($s3s_ecstore_files); the repo-wide counter is broken (see header)." >&2 @@ -135,33 +344,40 @@ fi status=0 -check_ratchet() { - local label="$1" count="$2" baseline="$3" inspect_cmd="$4" +# Exact comparison of one counter against its baseline line. Reports every +# counter before failing so one run shows all three deltas. The allowlist mode +# lives in run_allowlist_mode above, dispatched before any baseline input is +# read, not inside this function. +check_exact() { + local key="$1" label="$2" count="$3" baseline="$4" inspect_cmd="$5" if ((count > baseline)); then - echo "❌ s3s footprint ratchet violation: $label is $count, baseline is $baseline (+$((count - baseline)))" >&2 - echo " New code must not widen the s3s surface being removed by the s3gate migration" >&2 - echo " (rustfs/backlog#1677 F1, rustfs/backlog#1733). Use the gateway abstractions" >&2 - echo " instead of importing s3s directly. To find the offenders, compare" >&2 - echo " '$inspect_cmd' against origin/main." >&2 + echo "❌ s3s footprint grew: $count > $baseline ($label, baseline key '$key')" >&2 + echo " New code must not widen the s3s surface being removed by the gateway" >&2 + echo " migration (rustfs/backlog#1677 F1, rustfs/backlog#2734). Use the gateway" >&2 + echo " abstractions instead of importing s3s directly; do not raise '$key' in" >&2 + echo " $BASELINE_FILE. To find the offenders, compare" >&2 + echo " '$inspect_cmd' against the PR base branch." >&2 status=1 elif ((count < baseline)); then - echo "ℹ️ s3s footprint shrank: $label is $count, baseline is $baseline ($((count - baseline)))." >&2 - echo " Lower the baseline in scripts/check_s3s_footprint.sh in this PR to keep the ratchet tight." >&2 + echo "❌ s3s footprint shrank: $count < $baseline ($label, baseline key '$key')" >&2 + echo " lower the baseline to $count in this PR: set '$key=$count' in $BASELINE_FILE" >&2 + echo " so the baseline stays equal to the tree." >&2 + status=1 else - echo "s3s footprint OK: $label is $count (baseline: $baseline)" + echo "s3s footprint OK: $label is $count (baseline $key=$baseline)" fi } -check_ratchet "files importing s3s" "$s3s_import_files" "$S3S_IMPORT_FILES_BASELINE" \ +check_exact files "files importing s3s" "$s3s_import_files" "$files_baseline" \ "rg -l '$S3S_PATH_PATTERN' --type rust $E2E_TEST_GLOB ." -check_ratchet "s3_error! invocation lines" "$s3_error_lines" "$S3_ERROR_LINES_BASELINE" \ +check_exact s3_error_lines "s3_error! invocation lines" "$s3_error_lines" "$s3_error_lines_baseline" \ "rg -c 's3_error!' --type rust $E2E_TEST_GLOB ." -check_ratchet "ecstore files referencing s3s" "$s3s_ecstore_files" "$S3S_ECSTORE_FILES_BASELINE" \ +check_exact ecstore_files "ecstore files referencing s3s" "$s3s_ecstore_files" "$ecstore_files_baseline" \ "rg -l '$S3S_PATH_PATTERN' --type rust crates/ecstore/src" if ((status != 0)); then exit 1 fi -echo "✅ s3s footprint ratchet check passed" +echo "✅ s3s footprint baseline check passed"