diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 4b9e90407..c97efac72 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -46,6 +46,9 @@ jobs: - name: Install cargo-machete run: cargo install cargo-machete + - name: Install cargo-deny (v0.19.9) + run: cargo install --locked cargo-deny@0.19.9 + - name: Install Git pre-commit hooks run: ./contrib/dev-tools/git/install-git-hooks.sh diff --git a/.github/workflows/testing.yaml b/.github/workflows/testing.yaml index 835e5f3ee..c1e84ec40 100644 --- a/.github/workflows/testing.yaml +++ b/.github/workflows/testing.yaml @@ -97,6 +97,29 @@ jobs: name: Run Unit Tests run: cargo test --tests --benches --examples --workspace --all-targets --all-features + layer-bans: + name: Layer Boundary Bans + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - id: checkout + name: Checkout Repository + uses: actions/checkout@v6 + + - id: setup + name: Setup Toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: stable + + - id: deny + name: Install cargo-deny (v0.19.9) + run: cargo install --locked cargo-deny@0.19.9 + + - id: deny-check + name: Check layer boundary bans + run: cargo deny check bans + docker-e2e: # Skip this job when container.yaml is also running for the same event — it builds # the same image and runs the same E2E tests. container.yaml triggers on pushes to diff --git a/contrib/dev-tools/git/hooks/pre-commit.sh b/contrib/dev-tools/git/hooks/pre-commit.sh index cdc397edd..477c46c19 100755 --- a/contrib/dev-tools/git/hooks/pre-commit.sh +++ b/contrib/dev-tools/git/hooks/pre-commit.sh @@ -25,6 +25,7 @@ set -uo pipefail declare -a STEPS=( "Checking for unused dependencies (cargo machete --with-metadata)|cargo machete --with-metadata" + "Checking workspace layer boundary bans (cargo deny check bans)|cargo deny check bans" "Running all linters|linter all" "Running documentation tests|cargo test --doc --workspace" ) diff --git a/deny.toml b/deny.toml new file mode 100644 index 000000000..5fe081234 --- /dev/null +++ b/deny.toml @@ -0,0 +1,85 @@ +# deny.toml +# Configuration for `cargo deny check bans` +# +# This file enforces the workspace's layered architecture rules by preventing +# accidental dependency edges between layers. Dependencies may only flow downward: +# servers may depend on core/protocol/domain, but inner layers must not depend on +# outer layers. +# +# See: +# - docs/packages.md for the layer architecture and forbidden edge table +# - packages/AGENTS.md for the package catalog + +[advisories] +# Advisory scanning is a separate concern and not configured here. + +[licenses] +# License checking is a separate concern and not configured here. + +[bans] +# `multiple-versions` is set to "warn" because the workspace has pre-existing +# duplicate external dependency versions (e.g. `block-buffer`, `sha1`, `toml`). +# Fixing those is out of scope for this layer enforcement configuration and +# would require a separate workspace-wide dependency audit. +multiple-versions = "warn" +wildcards = "deny" + +# Ban server-layer crates from being depended on by non-server packages. +# The `wrappers` list specifies which packages are allowed to use each +# server crate as a direct dependency. All other uses (direct or transitive) +# are denied. +deny = [ + # axum server crates — only the root binary and other axum servers may depend on them + { crate = "torrust-tracker-axum-health-check-api-server", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-health-check-api-server", + ] }, + { crate = "torrust-tracker-axum-http-server", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-health-check-api-server", + ] }, + { crate = "torrust-tracker-axum-rest-api-server", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-health-check-api-server", + ] }, + { crate = "torrust-tracker-axum-server", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-health-check-api-server", + "torrust-tracker-axum-http-server", + "torrust-tracker-axum-rest-api-server", + ] }, + + # udp server — only server-layer + root + rest-api-core (pending fix) may depend on it + { crate = "torrust-tracker-udp-server", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-health-check-api-server", + "torrust-tracker-axum-rest-api-server", + "torrust-tracker-rest-api-core", + ] }, + + # Protocol crates must not be used directly by torrust-tracker-core. + # Only servers and the respective protocol-specific *-core may depend on them. + { crate = "torrust-tracker-http-protocol", wrappers = [ + "torrust-tracker-axum-http-server", + "torrust-tracker-http-core", + ] }, + { crate = "torrust-tracker-udp-protocol", wrappers = [ + "torrust-tracker-client-lib", + "torrust-tracker-udp-core", + "torrust-tracker-udp-server", + ] }, + + # Core protocol-specific wrappers must not be depended on by tracker-core + { crate = "torrust-tracker-http-core", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-http-server", + "torrust-tracker-axum-rest-api-server", + "torrust-tracker-rest-api-core", + ] }, + { crate = "torrust-tracker-udp-core", wrappers = [ + "torrust-tracker", + "torrust-tracker-axum-rest-api-server", + "torrust-tracker-rest-api-core", + "torrust-tracker-udp-server", + ] }, +] diff --git a/docs/issues/open/1925-1669-si-31-configure-cargo-deny-for-layer-boundary-enforcement.md b/docs/issues/open/1925-1669-si-31-configure-cargo-deny-for-layer-boundary-enforcement.md index df2b6ab3e..0bbf87268 100644 --- a/docs/issues/open/1925-1669-si-31-configure-cargo-deny-for-layer-boundary-enforcement.md +++ b/docs/issues/open/1925-1669-si-31-configure-cargo-deny-for-layer-boundary-enforcement.md @@ -5,13 +5,17 @@ status: open priority: p2 github-issue: 1925 spec-path: docs/issues/open/1925-1669-si-31-configure-cargo-deny-for-layer-boundary-enforcement.md -branch: null -related-pr: null -last-updated-utc: 2026-06-20 +branch: 1925-configure-cargo-deny-for-layer-boundary-enforcement +related-pr: 1932 +last-updated-utc: 2026-06-22 semantic-links: skill-links: - create-issue related-artifacts: + - deny.toml + - .github/workflows/testing.yaml + - .github/workflows/copilot-setup-steps.yml + - contrib/dev-tools/git/hooks/pre-commit.sh - Cargo.toml - packages/AGENTS.md - docs/packages.md @@ -179,18 +183,16 @@ deny = [ "torrust-tracker", ] }, - # Protocol crates must not be used by tracker-core or core layers. - # Only server and the respective *-core should depend on them. + # Protocol crates must not be used directly by torrust-tracker-core. + # Only servers and the respective protocol-specific *-core may depend on them. { crate = "torrust-tracker-http-protocol", wrappers = [ "torrust-tracker-axum-http-server", "torrust-tracker-http-core", ] }, { crate = "torrust-tracker-udp-protocol", wrappers = [ + "torrust-tracker-client-lib", "torrust-tracker-udp-core", "torrust-tracker-udp-server", - "torrust-tracker-axum-http-server", - "torrust-tracker-client-lib", - "torrust-tracker-client", ] }, # Core protocol-specific wrappers must not be depended on by tracker-core @@ -213,23 +215,25 @@ deny = [ > SI-29 is already done, so no name updates are needed. > > **Cross-reference to client extraction**: The wrappers list for `torrust-tracker-udp-protocol` -> includes `torrust-tracker-client-lib` and `torrust-tracker-client` (console). When the -> client extraction ([`1669-extract-torrust-tracker-client-to-standalone-repo.md`](./1669-extract-torrust-tracker-client-to-standalone-repo.md)) -> removes both from the workspace, these wrapper entries must be removed from `deny.toml`. +> currently includes `torrust-tracker-client-lib`. The console binary crate `torrust-tracker-client` +> was initially included but removed from wrappers during implementation because it is not +> consumed as a dependency by any other crate (standalone binary). When the +> client extraction removes `torrust-tracker-client-lib` from the workspace, its wrapper entry +> must also be removed. ## Implementation Plan Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | -| T1 | TODO | Install `cargo deny` (or confirm it's available) | `cargo install --locked cargo-deny` or via system package manager | -| T2 | TODO | Create `deny.toml` at the workspace root with bans configuration | Configuration matching the proposed section above | -| T3 | TODO | Run `cargo deny check bans` and verify it passes | All dependency edges match the allowed wrappers | -| T4 | TODO | Add `cargo deny check bans` to CI testing workflow (GitHub Actions) | CI catches violations before merge | -| T5 | TODO | Add `cargo deny check bans` to pre-commit (fast) or pre-push (slow), per ongoing #1843 decision | Gated by performance; integrated with future hook orchestrator | -| T6 | TODO | Verify that adding a test `core -> server` dep triggers a deny error | Proof the enforcement works | -| T7 | TODO | Document the `deny.toml` configuration in `packages/AGENTS.md` | Future developers understand the rules | +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | +| T1 | DONE | Install `cargo deny` (or confirm it's available) | `cargo install --locked cargo-deny` (v0.19.9) | +| T2 | DONE | Create `deny.toml` at the workspace root with bans configuration | Created with minor adjustments (see final config below) | +| T3 | DONE | Run `cargo deny check bans` and verify it passes | `bans ok`, exit code 0, zero errors | +| T4 | DONE | Add `cargo deny check bans` to CI testing workflow (GitHub Actions) | Added to `testing.yaml` and `copilot-setup-steps.yml` | +| T5 | DONE | Add `cargo deny check bans` to pre-commit (fast) or pre-push (slow), per ongoing #1843 decision | Added to pre-commit (0.57s runtime — fast enough) | +| T6 | DONE | Verify that adding a test `core -> server` dep triggers a deny error | Verified: `http-core -> udp-server` triggers `error[banned]` | +| T7 | DONE | Document the `deny.toml` configuration in `packages/AGENTS.md` | Step 7 in "Adding or Modifying a Package" section | ## Progress Tracking @@ -239,25 +243,26 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [ ] Spec reviewed and approved by user/maintainer - [ ] GitHub issue created and issue number added to this spec - [ ] Spec moved to `docs/issues/open/` with issue number prefix -- [ ] Implementation completed -- [ ] Automatic verification completed (`cargo deny check bans`, `linter all`, `cargo test --workspace`) +- [x] Implementation completed +- [x] Automatic verification completed (`cargo deny check bans` ✓, `linter all` ✓, `cargo test --workspace` ✓) - [ ] Manual verification scenarios executed and recorded -- [ ] Acceptance criteria reviewed after implementation and updated with evidence +- [x] Acceptance criteria reviewed after implementation and updated with evidence - [ ] EPIC #1669 Active Subissues table updated to `DONE` - [ ] Issue closed and spec moved to `docs/issues/closed/` ### Progress Log - 2026-06-11 00:00 UTC - josecelano - Spec drafted as subissue of EPIC #1669 +- 2026-06-22 16:34 UTC - josecelano - Implementation completed on branch `1925-configure-cargo-deny-for-layer-boundary-enforcement` ## Acceptance Criteria -- [ ] `deny.toml` exists at the workspace root with bans configuration. -- [ ] `cargo deny check bans` passes (exit code 0) on the current workspace state. -- [ ] Adding a forbidden dependency edge (e.g., `core -> server`) causes `cargo deny check bans` to fail. -- [ ] CI (GitHub Actions testing workflow) runs `cargo deny check bans` and rejects changes with new banned edges. -- [ ] The pre-commit or pre-push hook (per performance and #1843 outcome) runs `cargo deny check bans`. -- [ ] `packages/AGENTS.md` references the `deny.toml` enforcement in its Adding/Modifying a Package section. +- [x] `deny.toml` exists at the workspace root with bans configuration. +- [x] `cargo deny check bans` passes (exit code 0) on the current workspace state. +- [x] Adding a forbidden dependency edge (e.g., `core -> server`) causes `cargo deny check bans` to fail. +- [x] CI (GitHub Actions testing workflow) runs `cargo deny check bans` and rejects changes with new banned edges. +- [x] The pre-commit hook runs `cargo deny check bans` (0.57s runtime). +- [x] `packages/AGENTS.md` references the `deny.toml` enforcement in its Adding/Modifying a Package section. ## Verification Plan @@ -269,9 +274,9 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. ### Manual Verification Scenarios -| ID | Scenario | Command / Steps | Expected Result | Status | -| --- | --------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | ------ | -| M1 | Baseline pass on current workspace | `cargo deny check bans` | Exit code 0 | TODO | -| M2 | Forbidden edge detected | Temporarily add `torrust-tracker-udp-server` to a core package's `Cargo.toml` deps, then `cargo deny check bans` | Exit code non-zero; error message about banned dep | TODO | -| M3 | Legitimate edge allowed | No action needed — current legitimate edges (e.g., `axum-rest-api-server -> udp-server`) pass | No errors on those edges | TODO | -| M4 | Pre-commit hooks pass after adding deny | `./contrib/dev-tools/git/hooks/pre-commit.sh` | Exit code 0 | TODO | +| ID | Scenario | Command / Steps | Expected Result | Status | +| --- | --------------------------------------- | ------------------------------------------------------------------------------------------------------------- | --------------------------------- | ------ | +| M1 | Baseline pass on current workspace | `cargo deny check bans` | Exit code 0 | PASS | +| M2 | Forbidden edge detected | Temporarily add `torrust-tracker-udp-server` to `packages/http-core/Cargo.toml`, then `cargo deny check bans` | `error[banned]` and `bans FAILED` | PASS | +| M3 | Legitimate edge allowed | No action needed — current legitimate edges (e.g., `axum-rest-api-server -> udp-server`) pass | No errors on those edges | PASS | +| M4 | Pre-commit hooks pass after adding deny | `./contrib/dev-tools/git/hooks/pre-commit.sh` | Exit code 0 | PASS | diff --git a/docs/packages.md b/docs/packages.md index ecaac9ad4..f4ddfb475 100644 --- a/docs/packages.md +++ b/docs/packages.md @@ -3,6 +3,7 @@ semantic-links: skill-links: - write-markdown-docs related-artifacts: + - deny.toml - docs/index.md - docs/adrs/20260429000000_keep_database_as_aggregate_supertrait.md - packages/ @@ -68,6 +69,78 @@ Key Architectural Principles: 2. **Protocol Compliance**: `*-protocol` packages strictly implement BEP specifications. 3. **Extensibility**: Core logic is framework-agnostic for easy protocol additions. +## Layer Boundary Enforcement + +Dependencies between layers are enforced programmatically via +[`cargo deny check bans`](https://embarkstudios.github.io/cargo-deny/) — configured in +[`deny.toml`](../deny.toml) at the workspace root. + +### Motivation + +The layered architecture (servers → core → protocol → domain) prevents +coupling between concerns. Without automated enforcement, a misplaced +dependency (e.g., a core crate importing a server crate) compiles and +passes CI silently. `cargo deny` prohibits these edges at the lockfile +level, catching violations in pre-commit hooks and CI before merge. + +### Forbidden edges + +| Edge | Description | Current violations | +| -------------------------- | -------------------------------------------------------------- | ----------------------------- | +| `core -> server` | Core must not depend on delivery-layer packages | `rest-api-core -> udp-server` | +| `tracker-core -> core` | Tracker core must not depend on its protocol-specific wrappers | None | +| `tracker-core -> protocol` | Tracker core must not depend on protocol parsing crates | None | +| `tracker-core -> server` | Tracker core must not depend on server crates | None | +| `protocol -> core` | Protocol crates must not depend on core logic | None | +| `protocol -> tracker-core` | Protocol crates must not depend on tracker core | None | +| `protocol -> server` | Protocol crates must not depend on server crates | None | +| `domain -> server` | Domain/shared packages must not depend on server crates | None | + +### How it works + +`cargo deny` uses a **bans with wrappers** mechanism. For each server-layer +or protocol crate that should be restricted, `deny.toml` lists: + +- The **banned crate** (the server/protocol package). +- A **wrappers list** — the set of packages that are legitimately allowed + to depend on that crate directly. Any direct dependency outside this + list, and any transitive dependency from a non-server package, is rejected. + +For example, `torrust-tracker-udp-server` can only be depended on by: + +- `torrust-tracker` (root binary) +- `torrust-tracker-axum-rest-api-server` +- `torrust-tracker-axum-health-check-api-server` +- `torrust-tracker-rest-api-core` (known violation — see below) + +A core package like `torrust-tracker-http-core` adding `udp-server` as a +dependency would be immediately rejected by `cargo deny check bans`. + +### Known exceptions + +- `torrust-tracker-rest-api-core` depends on `torrust-tracker-udp-server` + — a known violation tracked separately. Until it is fixed, the wrapper + list for `udp-server` includes `rest-api-core`. + +### Maintenance + +When adding a new dependency to a workspace package, run: + +```sh +cargo deny check bans +``` + +If it fails, either: + +1. The new dependency is on a restricted crate — check whether your + package belongs in that crate's wrappers list. +2. The dependency is legitimate — add your package to the appropriate + wrapper entry in `deny.toml`. + +Adding a package to a wrapper list should be a deliberate architectural +decision, reviewed with the same care as any layer-crossing dependency. +See `deny.toml` for the complete configuration. + ## Design Decisions - Persistence trait boundaries and the aggregate supertrait choice: @@ -83,8 +156,8 @@ Key Architectural Principles: | `axum-rest-api-server` | Management REST API | Tracker configuration & monitoring | | `axum-health-check-api-server` | Health monitoring endpoint | System health reporting | | **Core Components** | | | -| `http-core` | HTTP-specific implementation | Request validation, Response formatting | -| `udp-core` | UDP-specific implementation | Connectionless request handling | +| `http-core` | HTTP-specific implementation | Request validation, Response formatting | +| `udp-core` | UDP-specific implementation | Connectionless request handling | | `tracker-core` | Central tracker logic | Peer management | | **Protocols** | | | | `http-protocol` | HTTP tracker protocol (BEP 3/23) | Announce/scrape request parsing | diff --git a/packages/AGENTS.md b/packages/AGENTS.md index 2efdfd11c..9f89afa5a 100644 --- a/packages/AGENTS.md +++ b/packages/AGENTS.md @@ -122,6 +122,11 @@ Strict BEP implementations — parse and serialize wire formats only. No tracker - At minimum one unit test (doc-test acceptable for simple utility crates). 5. Run `cargo machete` after adding dependencies — unused deps must not be committed. 6. Run `linter all` before committing. +7. **Layer boundary enforcement**: `deny.toml` at the workspace root configures `cargo deny check bans` to + prevent cross-layer dependency violations. If you add a dependency on a server-layer or protocol crate + to a package that isn't listed in that crate's `wrappers` list, the check will fail. + See [`docs/packages.md`](../docs/packages.md) for the forbidden edge table and + [`deny.toml`](../deny.toml) for the full configuration. ## Testing Packages diff --git a/packages/persistence-benchmark/Cargo.toml b/packages/persistence-benchmark/Cargo.toml index 92b6ae454..0ff1bb03c 100644 --- a/packages/persistence-benchmark/Cargo.toml +++ b/packages/persistence-benchmark/Cargo.toml @@ -28,5 +28,5 @@ sqlx = { version = "0.8", features = [ "macros", "mysql", "postgres", "runtime-t testcontainers = "0" tokio = { version = "1", features = [ "macros", "rt-multi-thread" ] } torrust-tracker-configuration = { version = "3.0.0-develop", path = "../configuration" } -torrust-tracker-core = { path = "../tracker-core" } +torrust-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" } torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" } diff --git a/packages/udp-server/Cargo.toml b/packages/udp-server/Cargo.toml index 769399139..e239af5a2 100644 --- a/packages/udp-server/Cargo.toml +++ b/packages/udp-server/Cargo.toml @@ -14,7 +14,7 @@ rust-version.workspace = true version.workspace = true [dependencies] -torrust_tracker_udp_protocol = { package = "torrust-tracker-udp-protocol", path = "../udp-protocol" } +torrust_tracker_udp_protocol = { package = "torrust-tracker-udp-protocol", version = "3.0.0-develop", path = "../udp-protocol" } torrust-peer-id = "0.1.0" torrust-info-hash = "=0.2.0" torrust-tracker-client = { package = "torrust-tracker-client-lib", version = "3.0.0-develop", path = "../tracker-client" }