From dee550a831cbfc24aa5cecd60f6f3a4cfe1a2e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Mon, 8 Jun 2026 00:23:38 +0800 Subject: [PATCH] ci: add architecture migration rule checks (#3264) Co-authored-by: houseme --- .config/make/lint-fmt.mak | 5 + .config/make/pre-commit.mak | 2 +- .github/AGENTS.md | 1 + .../architecture-migration-rules.yml | 43 +++++ .github/workflows/ci.yml | 3 + docs/architecture/crate-boundaries.md | 8 +- docs/architecture/migration-progress.md | 71 ++++---- scripts/check_architecture_migration_rules.sh | 160 ++++++++++++++++++ 8 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 .github/workflows/architecture-migration-rules.yml create mode 100755 scripts/check_architecture_migration_rules.sh diff --git a/.config/make/lint-fmt.mak b/.config/make/lint-fmt.mak index a342d1522..9e857783a 100644 --- a/.config/make/lint-fmt.mak +++ b/.config/make/lint-fmt.mak @@ -21,6 +21,11 @@ unsafe-code-check: ## Check unsafe_code allowances have SAFETY comments @echo "🔒 Checking unsafe_code allowances..." ./scripts/check_unsafe_code_allowances.sh +.PHONY: architecture-migration-check +architecture-migration-check: ## Check architecture migration guardrails + @echo "🏗️ Checking architecture migration guardrails..." + ./scripts/check_architecture_migration_rules.sh + .PHONY: compilation-check compilation-check: core-deps ## Run compilation check @echo "🔨 Running compilation check..." diff --git a/.config/make/pre-commit.mak b/.config/make/pre-commit.mak index 23a66d0cc..38a0012aa 100644 --- a/.config/make/pre-commit.mak +++ b/.config/make/pre-commit.mak @@ -7,5 +7,5 @@ setup-hooks: ## Set up git hooks @echo "✅ Git hooks setup complete!" .PHONY: pre-commit -pre-commit: fmt unsafe-code-check clippy-check compilation-check test ## Run pre-commit checks +pre-commit: fmt unsafe-code-check architecture-migration-check clippy-check compilation-check test ## Run pre-commit checks @echo "✅ All pre-commit checks passed!" diff --git a/.github/AGENTS.md b/.github/AGENTS.md index 3d637371a..6c8195515 100644 --- a/.github/AGENTS.md +++ b/.github/AGENTS.md @@ -34,3 +34,4 @@ Current `test-and-lint` gate includes: - `cargo fmt --all --check` - `cargo clippy --all-targets --all-features -- -D warnings` - `./scripts/check_layer_dependencies.sh` +- `./scripts/check_architecture_migration_rules.sh` diff --git a/.github/workflows/architecture-migration-rules.yml b/.github/workflows/architecture-migration-rules.yml new file mode 100644 index 000000000..69f0e7288 --- /dev/null +++ b/.github/workflows/architecture-migration-rules.yml @@ -0,0 +1,43 @@ +# 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. + +name: Architecture Migration Rules + +on: + pull_request: + branches: [ main ] + paths: + - "ARCHITECTURE.md" + - "docs/architecture/**" + - "scripts/check_architecture_migration_rules.sh" + - ".github/workflows/architecture-migration-rules.yml" + workflow_dispatch: + +permissions: + contents: read + +jobs: + architecture-migration-rules: + name: Architecture Migration Rules + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install ripgrep + run: | + sudo apt-get update + sudo apt-get install -y ripgrep + + - name: Check architecture migration rules + run: ./scripts/check_architecture_migration_rules.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6391c2d7..cfc05e246 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,6 +139,9 @@ jobs: - name: Check layered dependencies run: ./scripts/check_layer_dependencies.sh + - name: Check architecture migration rules + run: ./scripts/check_architecture_migration_rules.sh + build-rustfs-debug-binary: name: Build RustFS Debug Binary needs: skip-check diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index 73a8f8b66..bbd1745de 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -29,8 +29,12 @@ Contract crates must stay below implementation crates. Initial forbidden edges: - `extension-schema -> rustfs` - `extension-schema -> ecstore` -Existing layer checks live in `scripts/check_layer_dependencies.sh`. The next -`ci-gate` PR should extend existing guardrails instead of adding a parallel system. +Existing migration checks live in: + +- `scripts/check_layer_dependencies.sh` +- `scripts/check_architecture_migration_rules.sh` + +Extend these guardrails instead of adding a parallel system. ## Pre-Push Expert Review diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index d062c68f0..cefdb0dac 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,14 +5,15 @@ 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-config-helper-inventory-followup` -- Baseline: `upstream/main` at `3c71d5ef1c63bec63780d48caa2a9fe880ea0c64` -- PR type for this branch: `docs-only` +- Branch: `overtrue/arch-migration-ci-rules` +- Baseline: `upstream/main` at `6f4d0b54a171ff1560b5d892378d2ed407411fb9` +- PR type for this branch: `ci-gate` - Runtime behavior changes: none - Rust code changes: none -- Docs changes: add PR-review-raised adjacent config-object helper users for - module-switch and IAM persistence without claiming a complete `com.rs` move - inventory. +- CI/script changes: add a migration rule check for PR type vocabulary and + temporary compatibility marker/register consistency, plus a lightweight + docs-only workflow so architecture documentation PRs still run the same guard. +- Docs changes: record the new guardrail in the migration handoff. ## Phase 0 Tasks @@ -30,9 +31,12 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Acceptance: `./scripts/check_layer_dependencies.sh` passes on current `upstream/main` while still rejecting new unaccepted layer dependencies. - [~] `G-006` Create migration loss-prevention checks. - - Current branch: not in scope. - - Next PR: add checks for public re-export, route matrix, and storage trait - coverage before pure moves. + - Current branch: add a migration rule check for PR type vocabulary and + temporary compatibility marker/register consistency, with a dedicated + architecture-doc trigger that covers `ARCHITECTURE.md` and + `docs/architecture/**` docs-only PRs. + - Remaining follow-up: add checks for public re-export, route matrix, and + storage trait coverage before pure moves. - [x] `G-007` Create startup timeline table. - Acceptance: [`startup-timeline.md`](startup-timeline.md) records current binary startup order, side effects, fatal boundaries, and readiness stages. @@ -50,50 +54,53 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block [`ecstore-config-consumer-inventory.md`](ecstore-config-consumer-inventory.md) records the current model definitions, global accessors, persistence helpers, consumer groups, migration risks, and do-not-change contract. -- [~] `TEST-PRTYPE-001` Check PR type enum consistency. - - Current branch: not in scope. - - Next PR: add a mechanical check that all migration docs use the same PR type - vocabulary. +- [x] `TEST-PRTYPE-001` Check PR type enum consistency. + - Acceptance: `./scripts/check_architecture_migration_rules.sh` parses the + allowed PR types from [`crate-boundaries.md`](crate-boundaries.md) and fails + when `ARCHITECTURE.md` or architecture docs reference an unknown PR type. +- [x] `COMPAT-REG-001` Check temporary compatibility cleanup consistency. + - Acceptance: `./scripts/check_architecture_migration_rules.sh` fails when a + source `RUSTFS_COMPAT_TODO()` marker lacks a cleanup-register entry, + when a register entry lacks a source marker, or when a source marker omits a + removal condition. ## Next PRs -1. `ci-gate`: add focused checks for PR type vocabulary and temporary - compatibility marker/register consistency. -2. `test-only`: add a mechanical admin route matrix guard from the current +1. `test-only`: add a mechanical admin route matrix guard from the current snapshot and `route_registration_test.rs`. -3. `contract`: define the config-model contract surface while preserving the +2. `contract`: define the config-model contract surface while preserving the existing `Config`, `KV`, and `KVS` behavior. +3. `ci-gate`: add focused checks for public re-export and storage trait coverage + before pure moves. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Re-review confirmed dependency/call direction arrows, scanner global readers, selected adjacent scanner/module-switch/IAM persistence helpers, notify config-manager persistence, narrowed test wording, and `Config model contract` wording are source-backed | -| Migration preservation | pass | Confirmed this branch is docs-only, aligned with `rustfs/backlog#660`, and does not touch runtime logic, storage hot paths, global state implementation, compatibility code, scripts, or crate boundaries | -| Testing/verification | pass | Confirmed docs-only verification is sufficient after wording was narrowed and the final staged diff check covers all docs | +| Quality/architecture | pass | Confirmed the guard script is narrow/readable, fixes single-token PR type detection, and reuses one script across CI/make/docs triggers rather than adding a parallel rule system | +| Migration preservation | pass | Confirmed this is a `ci-gate` PR with only CI/config/docs/script changes and no runtime logic, storage hot-path, global-state, compatibility implementation, or crate-split changes | +| Testing/verification | pass | Confirmed positive checks, staged diff check, and temporary negative checks cover unknown PR type and missing cleanup-register entry failure modes | ## Verification Notes -Passed locally (docs-only): - +Passed: +- `./scripts/check_architecture_migration_rules.sh` - `./scripts/check_layer_dependencies.sh` - `./scripts/check_metrics_migration_refs.sh` - `git diff --check` - -Final pre-push after staging all docs: - - `git diff --cached --check` -- focused source review of `crates/ecstore/src/config/mod.rs`, - `crates/ecstore/src/config/com.rs`, `rustfs/src/app/context.rs`, - `rustfs/src/server/{event,audit}.rs`, `rustfs/src/admin/**/*.rs`, - `crates/{notify,audit,targets,iam,scanner}/**/*.rs` -- three-expert review: quality/architecture, migration preservation, and - testing/verification +- `bash -n scripts/check_architecture_migration_rules.sh` +- `make architecture-migration-check` +- `ruby -e "require 'yaml'; YAML.load_file('.github/workflows/architecture-migration-rules.yml')"` +- temporary negative check for unknown single-token PR type +- temporary negative check for unknown PR type in `ARCHITECTURE.md` +- temporary negative check for unknown PR type in nested `docs/architecture/**` +- temporary negative check for source compatibility marker without a register entry ## Handoff Notes - Keep Phase 0 PRs small. Do not move Config, Storage API, Runtime, or ECStore - code inside this `docs-only` branch. + code inside this `ci-gate` branch. - Keep CI checks in a separate `ci-gate` PR so the PR type rule remains enforceable. - Do not add temporary compatibility code without a matching `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry. diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh new file mode 100755 index 000000000..c7334f02e --- /dev/null +++ b/scripts/check_architecture_migration_rules.sh @@ -0,0 +1,160 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +ARCH_DOC_DIR="${ROOT_DIR}/docs/architecture" +BOUNDARY_DOC="${ARCH_DOC_DIR}/crate-boundaries.md" +REGISTER_DOC="${ARCH_DOC_DIR}/compat-cleanup-register.md" + +FAILURES=0 + +report_failure() { + printf 'Architecture migration rule failed: %s\n' "$1" >&2 + FAILURES=$((FAILURES + 1)) +} + +contains_line() { + local needle="$1" + local file="$2" + + grep -qxF "$needle" "$file" +} + +TMP_DIR="$(mktemp -d)" +trap 'rm -rf "$TMP_DIR"' EXIT + +PR_TYPES_FILE="${TMP_DIR}/pr_types.txt" +PR_TYPE_HITS_FILE="${TMP_DIR}/pr_type_hits.txt" +SOURCE_MARKERS_FILE="${TMP_DIR}/source_markers.txt" +SOURCE_IDS_FILE="${TMP_DIR}/source_ids.txt" +REGISTER_IDS_FILE="${TMP_DIR}/register_ids.txt" + +awk ' + /^## PR Types$/ { + in_section = 1 + next + } + in_section && /^## / { + exit + } + in_section && /^- `/ { + line = $0 + sub(/^- `/, "", line) + sub(/`.*/, "", line) + print line + } +' "$BOUNDARY_DOC" | sort >"$PR_TYPES_FILE" + +if [[ ! -s "$PR_TYPES_FILE" ]]; then + report_failure "no PR types found in docs/architecture/crate-boundaries.md" +fi + +while IFS= read -r pr_type; do + if [[ ! "$pr_type" =~ ^[a-z][a-z0-9]*(-[a-z0-9]+)*$ ]]; then + report_failure "invalid PR type spelling in crate-boundaries.md: ${pr_type}" + fi +done <"$PR_TYPES_FILE" + +if duplicates="$(uniq -d "$PR_TYPES_FILE")" && [[ -n "$duplicates" ]]; then + report_failure "duplicate PR types in crate-boundaries.md: ${duplicates//$'\n'/, }" +fi + +( + cd "$ROOT_DIR" + { + printf '%s\0' ARCHITECTURE.md + find docs/architecture -type f -name '*.md' -print0 + } | xargs -0 perl -ne ' + if (!defined $current_file || $ARGV ne $current_file) { + $current_file = $ARGV; + $line = 0; + $in_next_prs = 0; + } + $line++; + + if (/^##\s+Next PRs\b/) { + $in_next_prs = 1; + } elsif (/^##\s+/) { + $in_next_prs = 0; + } + + if ($in_next_prs || /\bPR(?:s| type| Types)?\b/) { + while (/`([a-z][a-z0-9]*(?:-[a-z0-9]+)*)`/g) { + print "$ARGV:$line:$1\n"; + } + } + ' +) >"$PR_TYPE_HITS_FILE" + +while IFS=: read -r file line token; do + [[ -z "${token:-}" ]] && continue + if ! contains_line "$token" "$PR_TYPES_FILE"; then + report_failure "${file}:${line} references unknown PR type '${token}'" + fi +done <"$PR_TYPE_HITS_FILE" + +( + cd "$ROOT_DIR" + rg -n --no-heading \ + 'RUSTFS_COMPAT_TODO\([A-Za-z0-9][A-Za-z0-9_-]*\)' \ + --glob '!docs/**' \ + --glob '!target/**' \ + --glob '!scripts/check_architecture_migration_rules.sh' \ + . || true +) >"$SOURCE_MARKERS_FILE" + +if [[ -s "$SOURCE_MARKERS_FILE" ]]; then + while IFS= read -r hit; do + if [[ "$hit" != *"Remove after "* ]]; then + report_failure "compat marker must state a removal condition: ${hit}" + fi + done <"$SOURCE_MARKERS_FILE" +fi + +sed -E 's/.*RUSTFS_COMPAT_TODO\(([A-Za-z0-9][A-Za-z0-9_-]*)\).*/\1/' "$SOURCE_MARKERS_FILE" | + sort -u >"$SOURCE_IDS_FILE" + +awk ' + /^## Open Items$/ { + in_section = 1 + next + } + in_section && /^## / { + exit + } + in_section { + print + } +' "$REGISTER_DOC" | + perl -ne ' + while (/RUSTFS_COMPAT_TODO\(([A-Za-z0-9][A-Za-z0-9_-]*)\)|`([A-Za-z0-9][A-Za-z0-9_-]*)`/g) { + my $id = defined $1 ? $1 : $2; + print "$id\n" unless $id eq "task-id"; + } + ' | + sort -u >"$REGISTER_IDS_FILE" + +if [[ -s "$SOURCE_IDS_FILE" ]] && awk '/^## Open Items$/ { in_section = 1; next } in_section && /^## / { exit } in_section && /No compatibility code/ { found = 1 } END { exit found ? 0 : 1 }' "$REGISTER_DOC"; then + report_failure "compat cleanup register still says no compatibility code while source markers exist" +fi + +while IFS= read -r source_id; do + [[ -z "$source_id" ]] && continue + if ! contains_line "$source_id" "$REGISTER_IDS_FILE"; then + report_failure "source marker RUSTFS_COMPAT_TODO(${source_id}) has no cleanup-register entry" + fi +done <"$SOURCE_IDS_FILE" + +while IFS= read -r register_id; do + [[ -z "$register_id" ]] && continue + if ! contains_line "$register_id" "$SOURCE_IDS_FILE"; then + report_failure "cleanup-register entry ${register_id} has no source marker" + fi +done <"$REGISTER_IDS_FILE" + +if (( FAILURES > 0 )); then + exit 1 +fi + +echo "Architecture migration rules passed."