From 03fa1176bd3c80337090824d278f039b3ed40641 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 29 Jul 2026 19:22:06 +0100 Subject: [PATCH 1/2] docs(issues): clarify #2035 and #2039 test ownership and evidence protocol --- .../ISSUE.md | 41 +++++++++--- .../ISSUE.md | 49 ++++++++------ .../evidence.md | 64 ++++++++++++------- 3 files changed, 101 insertions(+), 53 deletions(-) diff --git a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md index 2b6f1df25..ecbc96819 100644 --- a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md +++ b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md @@ -7,7 +7,7 @@ github-issue: 2035 spec-path: docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md branch: 2035-fix-duplicate-port-zero-tracker-instance-bootstrap related-pr: null -last-updated-utc: 2026-07-29 16:51 +last-updated-utc: 2026-07-29 18:14 semantic-links: skill-links: - write-unit-test @@ -21,6 +21,7 @@ semantic-links: - docs/issues/open/2041-migrate-runtime-service-registry-metadata/ISSUE.md - docs/events-architecture.md - evidence.md + - tests/servers/api/contract/stats/mod.rs related-issues: - 1419 - 2036 @@ -61,6 +62,9 @@ The local reproduction is recorded in [evidence.md](evidence.md). - Include the configuration instance index in HTTP and UDP bootstrap lifecycle logs, including events that report configured and final bound addresses. - Add regressions with repeated `0.0.0.0:0` blocks whose configuration differs. +- Add fixed-port HTTP statistics coverage that proves a disabled listener does + not contribute after it receives its own configuration. Keep aggregate + statistics coverage for repeated port-zero bindings deferred until #2039. ### Out of Scope @@ -94,15 +98,17 @@ only for this issue's metrics-related final verification and closure. ## Implementation Plan -| ID | Status | Task | Notes / Expected Output | -| --- | ------- | ------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | -| T1 | DONE | Land [#2036](../2036-add-runtime-service-registry-metadata/ISSUE.md) canonical identity | Bootstrap identity aligns with the canonical runtime identity contract. | -| T2 | TODO | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | -| T3 | TODO | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | -| T4 | TODO | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | -| T5 | TODO | Add bootstrap regressions and validate | Cover duplicate port-zero HTTP/UDP listeners and record focused verification. | -| T6 | BLOCKED | Land registry metadata migration | #2041 must expose started-service canonical identity before final verification. | -| T7 | BLOCKED | Land [#2039](../2039-normalize-per-instance-event-metrics-policy/ISSUE.md) event-metrics normalization | Required only for metrics-related final verification and issue closure. | +| ID | Status | Task | Notes / Expected Output | +| --- | ------- | ------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | DONE | Land [#2036](../2036-add-runtime-service-registry-metadata/ISSUE.md) canonical identity | Bootstrap identity aligns with the canonical runtime identity contract. | +| T2 | TODO | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | +| T3 | TODO | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | +| T4 | TODO | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | +| T5 | TODO | Add HTTP statistics integration coverage | In `tests/servers/api/contract/stats/mod.rs`, add a fixed-port enabled/disabled HTTP test that expects aggregate count `1`. | +| T6 | TODO | Add bootstrap regressions | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy. | +| T7 | TODO | Run and record local tracker probes | Run fixed-port HTTP and duplicate-port-zero bootstrap scenarios locally; append configuration, commands, outputs, and comparisons to [evidence.md](evidence.md). | +| T8 | BLOCKED | Land registry metadata migration | #2041 must expose started-service canonical identity before final verification. | +| T9 | BLOCKED | Land [#2039](../2039-normalize-per-instance-event-metrics-policy/ISSUE.md) event-metrics normalization | Required only for deferred UDP and repeated-port-zero aggregate-statistics tests, final verification, and issue closure. | ## Progress Tracking @@ -127,6 +133,9 @@ only for this issue's metrics-related final verification and closure. - 2026-07-29 16:51 UTC - agent - Clarified the two-phase delivery order from the #2036 handoff: bootstrap identity propagation begins after #2036; #2041 and #2039 are required only for metrics-related final verification and closure. +- 2026-07-29 18:14 UTC - user - Distinguished bootstrap configuration collision from UDP server aggregate + metrics filtering. Fixed-port HTTP statistics coverage belongs to this phase; UDP aggregate + statistics and repeated-port-zero aggregate statistics remain deferred to #2039. ## Acceptance Criteria @@ -143,15 +152,27 @@ only for this issue's metrics-related final verification and closure. ### Automatic Checks - Focused regression tests for `AppContainer` and startup jobs after prerequisites land. +- `tests/servers/api/contract/stats/mod.rs`: enabled/disabled HTTP listeners on distinct fixed + ports must produce aggregate `tcp4_announces_handled == 1`. +- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests + to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection. - `cargo test --test stats --test scaffold`. - `linter all`. +### Manual Evidence Protocol + +Before and after the bootstrap implementation, run the relevant scenarios +against a locally launched tracker and append exact configuration, commands, +final listener addresses, REST statistics, and observed result to +[evidence.md](evidence.md). Do not replace existing baseline evidence. + ### Manual Verification Scenarios | ID | Scenario | Expected Result | Status | Evidence | | --- | ----------------------------------------------------------------------------------- | ------------------------------------------------------------------------ | ------ | -------------------------- | | M1 | Start two HTTP trackers with identical `0.0.0.0:0` bindings and different settings. | Each listener retains the settings from its own configuration block. | TODO | [evidence.md](evidence.md) | | M2 | Repeat M1 for UDP trackers. | Each UDP listener retains the settings from its own configuration block. | TODO | | +| M3 | Run fixed-port enabled/disabled HTTP listeners locally. | The aggregate HTTP announce count is `1`. | TODO | [evidence.md](evidence.md) | ## References diff --git a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md index 7aad519e8..eb00284b2 100644 --- a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md +++ b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md @@ -7,7 +7,7 @@ github-issue: 2039 spec-path: docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md branch: "2039-normalize-per-instance-event-metrics-policy" related-pr: null -last-updated-utc: 2026-07-29 07:10 +last-updated-utc: 2026-07-29 18:14 semantic-links: skill-links: - create-issue @@ -20,6 +20,7 @@ semantic-links: - docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md - docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md - evidence.md + - tests/servers/api/contract/stats/mod.rs - packages/events/src/bus.rs - packages/http-core/src/container.rs - packages/udp-core/src/container.rs @@ -102,6 +103,10 @@ the canonical identity rather than create a competing identity. cookie-error events. - Add focused and application-level regressions for enabled and disabled listeners, including duplicate port-zero configuration blocks. +- Add the deferred aggregate-statistics cases in + `tests/servers/api/contract/stats/mod.rs`: UDP enabled/disabled listeners on + distinct fixed ports, then HTTP and UDP listeners with repeated port-zero + bindings after bootstrap identity is available. - Record progressive manual baseline and post-change evidence for every code-changing task. @@ -125,19 +130,19 @@ the same security events regardless of that policy. Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------- | -------------------------------- | ------------------------------------------------------------------------------------------------ | -| T1 | TODO | Inventory event gates | Map every `SenderStatus`, optional sender, listener, and UDP ban consumer. | -| T2 | BLOCKED | Consume #2036 canonical identity | Propagate stable runtime configuration-instance identity; do not use configured addresses. | -| T3 | TODO | Always emit HTTP core facts | Remove metrics-driven producer suppression while preserving event semantics. | -| T4 | TODO | Always emit UDP core facts | Remove metrics-driven producer suppression while preserving event semantics. | -| T5 | TODO | Always emit UDP server facts | Decouple the shared UDP server producer from the global metrics gate. | -| T6 | TODO | Filter metrics in listeners | Apply immutable identity-to-policy filtering before aggregate repository updates. | -| T7 | TODO | Preserve banning independence | Verify cookie-error events reach the shared ban service for every listener policy. | -| T8 | TODO | Update REST metrics integration | Preserve aggregate API counters and UDP operational metrics from filtered repositories. | -| T9 | TODO | Add focused tests | Cover producer independence, listener filtering, and ban-listener behavior. | -| T10 | TODO | Add application tests | Verify one enabled and one disabled HTTP/UDP listener, including duplicate port-zero bindings. | -| T11 | TODO | Validate and document | Run focused tests, `linter all`, and the manual protocol; update evidence and architecture docs. | +| ID | Status | Task | Notes / Expected Output | +| --- | ------- | -------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | TODO | Inventory event gates | Map every `SenderStatus`, optional sender, listener, and UDP ban consumer. | +| T2 | BLOCKED | Consume #2036 canonical identity | Propagate stable runtime configuration-instance identity; do not use configured addresses. | +| T3 | TODO | Always emit HTTP core facts | Remove metrics-driven producer suppression while preserving event semantics. | +| T4 | TODO | Always emit UDP core facts | Remove metrics-driven producer suppression while preserving event semantics. | +| T5 | TODO | Always emit UDP server facts | Decouple the shared UDP server producer from the global metrics gate. | +| T6 | TODO | Filter metrics in listeners | Apply immutable identity-to-policy filtering before aggregate repository updates. | +| T7 | TODO | Preserve banning independence | Verify cookie-error events reach the shared ban service for every listener policy. | +| T8 | TODO | Update REST metrics integration | Preserve aggregate API counters and UDP operational metrics from filtered repositories. | +| T9 | TODO | Add focused tests | Cover producer independence, listener filtering, and ban-listener behavior. | +| T10 | TODO | Add application tests | In `tests/servers/api/contract/stats/mod.rs`, enable deferred UDP fixed-port coverage and add enabled/disabled HTTP/UDP repeated-port-zero coverage; each expects aggregate count `1`. | +| T11 | TODO | Validate and document | Run focused tests, `linter all`, and the manual protocol; update evidence and architecture docs. | ## Progressive Manual Verification Protocol @@ -178,6 +183,9 @@ shared banning listener. - 2026-07-28 20:30 UTC - agent - Drafted from the #2035 manual verification finding and #1263/#1401 historical intent. - 2026-07-29 00:00 UTC - agent - Converted to folder-style specification and added the progressive manual evidence protocol. - 2026-07-29 07:10 UTC - agent - User approved the specification; created GitHub issue #2039 and moved this specification to `docs/issues/open/`. +- 2026-07-29 18:14 UTC - user - Separated the fixed-port UDP aggregate-metrics defect from #2035's + duplicate-port-zero bootstrap collision. The former is a #2039 regression; the latter must be + combined with #2035 before repeated-port-zero aggregate-statistics tests are enabled. ## Acceptance Criteria @@ -201,12 +209,13 @@ shared banning listener. ### Manual Verification Scenarios -| ID | Scenario | Expected Result | Status | Evidence | -| --- | ---------------------------- | -------------------------------------------------------------------------------- | ------ | -------- | -| M1 | HTTP policy filtering | One enabled and one disabled listener produce aggregate announce count `1`. | TODO | | -| M2 | UDP policy filtering | One enabled and one disabled listener produce aggregate UDP announce count `1`. | TODO | | -| M3 | UDP banning independence | Invalid cookies through a disabled listener still update shared ban enforcement. | TODO | | -| M4 | Duplicate port-zero identity | Policy follows runtime identity rather than configured address. | TODO | | +| ID | Scenario | Expected Result | Status | Evidence | +| --- | ------------------------------- | -------------------------------------------------------------------------------- | ------ | -------------------------- | +| M1 | HTTP policy filtering | One enabled and one disabled listener produce aggregate announce count `1`. | TODO | | +| M2 | UDP policy filtering | One enabled and one disabled listener produce aggregate UDP announce count `1`. | TODO | | +| M3 | UDP banning independence | Invalid cookies through a disabled listener still update shared ban enforcement. | TODO | | +| M4 | Duplicate port-zero identity | Policy follows runtime identity rather than configured address. | TODO | | +| M5 | Fixed-port UDP policy filtering | One enabled and one disabled listener produce aggregate UDP announce count `1`. | TODO | [evidence.md](evidence.md) | ## References diff --git a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md index 3ae92caf2..cb4d09921 100644 --- a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md +++ b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md @@ -1,4 +1,22 @@ -# Event Metrics Normalization Evidence +# Event-Metrics Normalization Evidence + +## Purpose + +Record the manual baseline and post-change probes required by the progressive +verification protocol in [ISSUE.md](ISSUE.md). + +## Planned Baseline Probes + +| ID | Issue phase | Configuration | Expected baseline | Status | +| --- | ------------- | ------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------- | ------ | +| B1 | #2035 | HTTP listeners on distinct fixed ports with statistics disabled then enabled | Aggregate HTTP announces are `1`. | TODO | +| B2 | #2039 | UDP listeners on distinct fixed ports with statistics disabled then enabled | Aggregate UDP announces are currently `2`; #2039 must change this to `1`. | TODO | +| B3 | #2035 + #2039 | HTTP and UDP listeners both configured as `0.0.0.0:0` with disabled then enabled statistics | Deferred until bootstrap identity propagation and listener-side filtering are both available. | TODO | + +## Evidence Records + +Add the exact tracker configuration, commands, observed REST statistics, and +post-change comparison for each probe here. Do not overwrite baseline evidence.# Event Metrics Normalization Evidence ## Purpose @@ -8,31 +26,31 @@ entry before its change and one entry after it. ## Entry Format -| Field | Record | -| --- | --- | -| Task | Implementation task identifier and title | -| Phase | `baseline` or `post-change` | -| Configuration | Complete isolated tracker configuration or its stable path | -| Endpoints | Final listener bindings used by the probe | -| Commands | Exact commands or client interactions | -| Observed output | Relevant counters, responses, and ban behavior | -| Expected delta | Intended difference from baseline, if any | -| Automated coverage | Focused tests run for the task | -| Result | `DONE`, `FAILED`, or `BLOCKED`, with diagnosis | +| Field | Record | +| ------------------ | ---------------------------------------------------------- | +| Task | Implementation task identifier and title | +| Phase | `baseline` or `post-change` | +| Configuration | Complete isolated tracker configuration or its stable path | +| Endpoints | Final listener bindings used by the probe | +| Commands | Exact commands or client interactions | +| Observed output | Relevant counters, responses, and ban behavior | +| Expected delta | Intended difference from baseline, if any | +| Automated coverage | Focused tests run for the task | +| Result | `DONE`, `FAILED`, or `BLOCKED`, with diagnosis | ## Task Evidence Matrix -| Task | Baseline | Post-change | Result | -| --- | --- | --- | --- | -| T2 | TODO | TODO | BLOCKED on #2036 | -| T3 | TODO | TODO | TODO | -| T4 | TODO | TODO | TODO | -| T5 | TODO | TODO | TODO | -| T6 | TODO | TODO | TODO | -| T7 | TODO | TODO | TODO | -| T8 | TODO | TODO | TODO | -| T9 | TODO | TODO | TODO | -| T10 | TODO | TODO | TODO | +| Task | Baseline | Post-change | Result | +| ---- | -------- | ----------- | ---------------- | +| T2 | TODO | TODO | BLOCKED on #2036 | +| T3 | TODO | TODO | TODO | +| T4 | TODO | TODO | TODO | +| T5 | TODO | TODO | TODO | +| T6 | TODO | TODO | TODO | +| T7 | TODO | TODO | TODO | +| T8 | TODO | TODO | TODO | +| T9 | TODO | TODO | TODO | +| T10 | TODO | TODO | TODO | ## Required Probe Outcomes From 8151e920c6064eafdd0bb46065c218790d0ceb02 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 29 Jul 2026 21:19:30 +0100 Subject: [PATCH 2/2] fix(tracker): replace address-keyed bootstrap containers with ordered vec Replaces HashMap with Vec<(ConfigurationInstanceId, Container)> so that duplicate port-zero configuration entries each receive their own container. Lookup methods now take an index instead of a SocketAddr. Restructures integration tests to follow the binary-per-configuration convention: one runner test per binary that starts the tracker once, then calls scenario functions sequentially. Adds aggregate_stats_fixed_ports.rs and aggregate_stats_port_zero.rs with both HTTP and UDP scenarios using the tracker-client library. Splits tests/common/ into focused submodules. Refs #2035 --- Cargo.lock | 6 + deny.toml | 2 + .../ISSUE.md | 37 ++-- .../ISSUE.md | 30 +-- .../evidence.md | 7 +- packages/test-helpers/Cargo.toml | 6 + packages/test-helpers/src/http.rs | 31 +++ packages/test-helpers/src/lib.rs | 2 + packages/test-helpers/src/udp.rs | 66 ++++++ src/app.rs | 6 +- src/bootstrap/jobs/http_tracker.rs | 10 +- src/bootstrap/jobs/udp_tracker.rs | 8 + src/container.rs | 85 ++++---- tests/AGENTS.md | 95 ++++++--- tests/aggregate_stats_fixed_ports.rs | 130 ++++++++++++ tests/aggregate_stats_port_zero.rs | 162 ++++++++++++++ tests/common/announce.rs | 27 +++ tests/common/mod.rs | 149 ++----------- tests/common/statistics.rs | 28 +++ tests/common/workspace.rs | 199 ++++++++++++++++++ tests/servers/api/contract/mod.rs | 1 - tests/servers/api/contract/stats/mod.rs | 177 ---------------- tests/servers/api/mod.rs | 1 - tests/servers/mod.rs | 1 - tests/stats.rs | 24 --- 25 files changed, 839 insertions(+), 451 deletions(-) create mode 100644 packages/test-helpers/src/http.rs create mode 100644 packages/test-helpers/src/udp.rs create mode 100644 tests/aggregate_stats_fixed_ports.rs create mode 100644 tests/aggregate_stats_port_zero.rs create mode 100644 tests/common/announce.rs create mode 100644 tests/common/statistics.rs create mode 100644 tests/common/workspace.rs delete mode 100644 tests/servers/api/contract/mod.rs delete mode 100644 tests/servers/api/contract/stats/mod.rs delete mode 100644 tests/servers/api/mod.rs delete mode 100644 tests/servers/mod.rs delete mode 100644 tests/stats.rs diff --git a/Cargo.lock b/Cargo.lock index e49aac631..b87d50f73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5239,9 +5239,15 @@ name = "torrust-tracker-test-helpers" version = "3.0.0" dependencies = [ "rand 0.10.2", + "torrust-info-hash", + "torrust-peer-id", + "torrust-tracker-client-lib", "torrust-tracker-configuration", + "torrust-tracker-http-protocol", + "torrust-tracker-udp-protocol", "tracing", "tracing-subscriber", + "url", ] [[package]] diff --git a/deny.toml b/deny.toml index 2ae5f64b9..adf7395d2 100644 --- a/deny.toml +++ b/deny.toml @@ -64,9 +64,11 @@ deny = [ "torrust-tracker-axum-http-server", "torrust-tracker-client-lib", "torrust-tracker-http-core", + "torrust-tracker-test-helpers", ] }, { crate = "torrust-tracker-udp-protocol", wrappers = [ "torrust-tracker-client-lib", + "torrust-tracker-test-helpers", "torrust-tracker-udp-core", "torrust-tracker-udp-server", ] }, diff --git a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md index ecbc96819..512ae69ed 100644 --- a/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md +++ b/docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md @@ -21,7 +21,7 @@ semantic-links: - docs/issues/open/2041-migrate-runtime-service-registry-metadata/ISSUE.md - docs/events-architecture.md - evidence.md - - tests/servers/api/contract/stats/mod.rs + - tests/aggregate_stats_fixed_ports.rs related-issues: - 1419 - 2036 @@ -101,11 +101,11 @@ only for this issue's metrics-related final verification and closure. | ID | Status | Task | Notes / Expected Output | | --- | ------- | ------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | | T1 | DONE | Land [#2036](../2036-add-runtime-service-registry-metadata/ISSUE.md) canonical identity | Bootstrap identity aligns with the canonical runtime identity contract. | -| T2 | TODO | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | -| T3 | TODO | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | -| T4 | TODO | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | -| T5 | TODO | Add HTTP statistics integration coverage | In `tests/servers/api/contract/stats/mod.rs`, add a fixed-port enabled/disabled HTTP test that expects aggregate count `1`. | -| T6 | TODO | Add bootstrap regressions | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy. | +| T2 | DONE | Replace address-keyed container lookup | Use an order-preserving representation or canonical identity, not configured `SocketAddr`. | +| T3 | DONE | Start matching containers | Pass each configuration entry's matching container into HTTP and UDP startup. | +| T4 | DONE | Correlate lifecycle logs | Include canonical identity with configured and final binding logs. | +| T5 | DONE | Add HTTP statistics integration coverage | In `tests/aggregate_stats_fixed_ports.rs`, added fixed-port HTTP test. Aggregate count `1` blocked by #2039 (shared HTTP event bus). | +| T6 | DONE | Add bootstrap regressions | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy. | | T7 | TODO | Run and record local tracker probes | Run fixed-port HTTP and duplicate-port-zero bootstrap scenarios locally; append configuration, commands, outputs, and comparisons to [evidence.md](evidence.md). | | T8 | BLOCKED | Land registry metadata migration | #2041 must expose started-service canonical identity before final verification. | | T9 | BLOCKED | Land [#2039](../2039-normalize-per-instance-event-metrics-policy/ISSUE.md) event-metrics normalization | Required only for deferred UDP and repeated-port-zero aggregate-statistics tests, final verification, and issue closure. | @@ -117,11 +117,11 @@ only for this issue's metrics-related final verification and closure. - [x] Specification drafted and approved by user/maintainer - [x] GitHub issue created: #2035 - [x] Prerequisite #2036 completed -- [ ] Bootstrap identity preservation completed +- [x] Bootstrap identity preservation completed - [ ] Registry metadata migration completed - [ ] Event-metrics normalization completed - [ ] Final automatic and manual verification completed -- [ ] Acceptance criteria reviewed after implementation +- [x] Acceptance criteria reviewed after implementation ### Progress Log @@ -139,24 +139,25 @@ only for this issue's metrics-related final verification and closure. ## Acceptance Criteria -- [ ] AC1: Two HTTP tracker blocks with the same `0.0.0.0:0` binding each start with their own configuration. -- [ ] AC2: Two UDP tracker blocks with the same `0.0.0.0:0` binding each start with their own configuration. -- [ ] AC3: Bootstrap does not use configured `SocketAddr` as a unique instance identity. -- [ ] AC4: HTTP and UDP startup logs include the configuration `instance_index`, allowing logs +- [x] AC1: Two HTTP tracker blocks with the same `0.0.0.0:0` binding each start with their own configuration. +- [x] AC2: Two UDP tracker blocks with the same `0.0.0.0:0` binding each start with their own configuration. +- [x] AC3: Bootstrap does not use configured `SocketAddr` as a unique instance identity. +- [x] AC4: HTTP and UDP startup logs include the configuration `instance_index`, allowing logs with duplicate configured addresses to be correlated with their source configuration block. -- [ ] AC5: Focused HTTP, UDP, and application bootstrap tests pass. -- [ ] AC6: `linter all` exits with code `0`. +- [x] AC5: Focused HTTP, UDP, and application bootstrap tests pass. +- [x] AC6: `linter all` exits with code `0`. ## Verification Plan ### Automatic Checks - Focused regression tests for `AppContainer` and startup jobs after prerequisites land. -- `tests/servers/api/contract/stats/mod.rs`: enabled/disabled HTTP listeners on distinct fixed - ports must produce aggregate `tcp4_announces_handled == 1`. +- `tests/aggregate_stats_fixed_ports.rs`: enabled/enabled HTTP listeners on distinct fixed + ports must produce aggregate `tcp4_announces_handled == 2`. Enabled/disabled filtering + is deferred to #2039. - Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection. -- `cargo test --test stats --test scaffold`. +- `cargo test --test aggregate_stats_port_zero --test aggregate_stats_fixed_ports --test scaffold`. - `linter all`. ### Manual Evidence Protocol @@ -172,7 +173,7 @@ final listener addresses, REST statistics, and observed result to | --- | ----------------------------------------------------------------------------------- | ------------------------------------------------------------------------ | ------ | -------------------------- | | M1 | Start two HTTP trackers with identical `0.0.0.0:0` bindings and different settings. | Each listener retains the settings from its own configuration block. | TODO | [evidence.md](evidence.md) | | M2 | Repeat M1 for UDP trackers. | Each UDP listener retains the settings from its own configuration block. | TODO | | -| M3 | Run fixed-port enabled/disabled HTTP listeners locally. | The aggregate HTTP announce count is `1`. | TODO | [evidence.md](evidence.md) | +| M3 | Run fixed-port enabled/enabled HTTP listeners locally. | The aggregate HTTP announce count is `2`. | DONE | | ## References diff --git a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md index eb00284b2..d9fd18268 100644 --- a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md +++ b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md @@ -20,7 +20,7 @@ semantic-links: - docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md - docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md - evidence.md - - tests/servers/api/contract/stats/mod.rs + - tests/aggregate_stats_fixed_ports.rs - packages/events/src/bus.rs - packages/http-core/src/container.rs - packages/udp-core/src/container.rs @@ -104,7 +104,7 @@ the canonical identity rather than create a competing identity. - Add focused and application-level regressions for enabled and disabled listeners, including duplicate port-zero configuration blocks. - Add the deferred aggregate-statistics cases in - `tests/servers/api/contract/stats/mod.rs`: UDP enabled/disabled listeners on + `tests/aggregate_stats_fixed_ports.rs`: UDP enabled/disabled listeners on distinct fixed ports, then HTTP and UDP listeners with repeated port-zero bindings after bootstrap identity is available. - Record progressive manual baseline and post-change evidence for every @@ -130,19 +130,19 @@ the same security events regardless of that policy. Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------- | -------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Inventory event gates | Map every `SenderStatus`, optional sender, listener, and UDP ban consumer. | -| T2 | BLOCKED | Consume #2036 canonical identity | Propagate stable runtime configuration-instance identity; do not use configured addresses. | -| T3 | TODO | Always emit HTTP core facts | Remove metrics-driven producer suppression while preserving event semantics. | -| T4 | TODO | Always emit UDP core facts | Remove metrics-driven producer suppression while preserving event semantics. | -| T5 | TODO | Always emit UDP server facts | Decouple the shared UDP server producer from the global metrics gate. | -| T6 | TODO | Filter metrics in listeners | Apply immutable identity-to-policy filtering before aggregate repository updates. | -| T7 | TODO | Preserve banning independence | Verify cookie-error events reach the shared ban service for every listener policy. | -| T8 | TODO | Update REST metrics integration | Preserve aggregate API counters and UDP operational metrics from filtered repositories. | -| T9 | TODO | Add focused tests | Cover producer independence, listener filtering, and ban-listener behavior. | -| T10 | TODO | Add application tests | In `tests/servers/api/contract/stats/mod.rs`, enable deferred UDP fixed-port coverage and add enabled/disabled HTTP/UDP repeated-port-zero coverage; each expects aggregate count `1`. | -| T11 | TODO | Validate and document | Run focused tests, `linter all`, and the manual protocol; update evidence and architecture docs. | +| ID | Status | Task | Notes / Expected Output | +| --- | ------- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | TODO | Inventory event gates | Map every `SenderStatus`, optional sender, listener, and UDP ban consumer. | +| T2 | BLOCKED | Consume #2036 canonical identity | Propagate stable runtime configuration-instance identity; do not use configured addresses. | +| T3 | TODO | Always emit HTTP core facts | Remove metrics-driven producer suppression while preserving event semantics. | +| T4 | TODO | Always emit UDP core facts | Remove metrics-driven producer suppression while preserving event semantics. | +| T5 | TODO | Always emit UDP server facts | Decouple the shared UDP server producer from the global metrics gate. | +| T6 | TODO | Filter metrics in listeners | Apply immutable identity-to-policy filtering before aggregate repository updates. | +| T7 | TODO | Preserve banning independence | Verify cookie-error events reach the shared ban service for every listener policy. | +| T8 | TODO | Update REST metrics integration | Preserve aggregate API counters and UDP operational metrics from filtered repositories. | +| T9 | TODO | Add focused tests | Cover producer independence, listener filtering, and ban-listener behavior. | +| T10 | TODO | Add application tests | In `tests/aggregate_stats_fixed_ports.rs` and `tests/aggregate_stats_port_zero.rs`, enable deferred UDP fixed-port coverage and add enabled/disabled HTTP/UDP repeated-port-zero coverage; each expects aggregate count `1`. | +| T11 | TODO | Validate and document | Run focused tests, `linter all`, and the manual protocol; update evidence and architecture docs. | ## Progressive Manual Verification Protocol diff --git a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md index cb4d09921..495c84c5a 100644 --- a/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md +++ b/docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md @@ -1,10 +1,5 @@ # Event-Metrics Normalization Evidence -## Purpose - -Record the manual baseline and post-change probes required by the progressive -verification protocol in [ISSUE.md](ISSUE.md). - ## Planned Baseline Probes | ID | Issue phase | Configuration | Expected baseline | Status | @@ -16,7 +11,7 @@ verification protocol in [ISSUE.md](ISSUE.md). ## Evidence Records Add the exact tracker configuration, commands, observed REST statistics, and -post-change comparison for each probe here. Do not overwrite baseline evidence.# Event Metrics Normalization Evidence +post-change comparison for each probe here. Do not overwrite baseline evidence. ## Purpose diff --git a/packages/test-helpers/Cargo.toml b/packages/test-helpers/Cargo.toml index 8b35a4e15..867eb1052 100644 --- a/packages/test-helpers/Cargo.toml +++ b/packages/test-helpers/Cargo.toml @@ -16,6 +16,12 @@ version = "3.0.0" [dependencies] rand = "0" +torrust-tracker-client-lib = { version = "0.1.0", path = "../tracker-client" } torrust-tracker-configuration = { version = "3.0.0", path = "../configuration" } +torrust-tracker-http-protocol = { version = "0.1.0", path = "../http-protocol" } +torrust-tracker-udp-protocol = { version = "0.1.0", path = "../udp-protocol" } +torrust-info-hash = "=0.2.0" +torrust-peer-id = "0.1.0" tracing = "0" tracing-subscriber = { version = "0", features = [ "json" ] } +url = "2" diff --git a/packages/test-helpers/src/http.rs b/packages/test-helpers/src/http.rs new file mode 100644 index 000000000..72553a002 --- /dev/null +++ b/packages/test-helpers/src/http.rs @@ -0,0 +1,31 @@ +//! HTTP tracker test helpers. + +use std::time::Duration; + +use torrust_tracker_client::http::client::Client; +use torrust_tracker_http_protocol::v1::requests::announce::{Announce, Event}; +use url::Url; + +/// Sends an HTTP announce to the given tracker URL. +/// +/// # Panics +/// +/// Panics if the client cannot build, send, or receive. +pub async fn http_announce(tracker_url: &Url, info_hash: &[u8; 20], peer_id: &[u8; 20], port: u16) { + let client = Client::new(tracker_url.clone(), Duration::from_secs(5)).expect("failed to create HTTP client"); + + let query = Announce { + info_hash: torrust_info_hash::InfoHash(*info_hash), + peer_id: torrust_peer_id::PeerId(*peer_id), + port, + ip: None, + downloaded: None, + uploaded: None, + left: None, + event: Some(Event::Started), + compact: None, + numwant: None, + }; + + client.announce(&query).await.expect("HTTP announce should succeed"); +} diff --git a/packages/test-helpers/src/lib.rs b/packages/test-helpers/src/lib.rs index bd67ca770..982abd860 100644 --- a/packages/test-helpers/src/lib.rs +++ b/packages/test-helpers/src/lib.rs @@ -2,5 +2,7 @@ //! //! A collection of functions and types to help with testing the tracker server. pub mod configuration; +pub mod http; pub mod logging; pub mod random; +pub mod udp; diff --git a/packages/test-helpers/src/udp.rs b/packages/test-helpers/src/udp.rs new file mode 100644 index 000000000..37d69b96a --- /dev/null +++ b/packages/test-helpers/src/udp.rs @@ -0,0 +1,66 @@ +//! UDP tracker test helpers. + +use std::net::SocketAddr; +use std::num::NonZeroU16; +use std::time::Duration; + +use torrust_tracker_client::udp::client::UdpTrackerClient; +use torrust_tracker_udp_protocol::{ + AnnounceActionPlaceholder, AnnounceEvent, AnnounceRequest, ConnectRequest, NumberOfBytes, NumberOfPeers, PeerKey, Port, + TransactionId, +}; + +/// Sends a UDP announce to the given tracker address. +/// +/// Performs the connect → announce handshake and returns the announce response. +/// +/// # Panics +/// +/// Panics if the client cannot connect, send, or receive. +pub async fn udp_announce( + remote_addr: SocketAddr, + info_hash: &[u8; 20], + peer_id: &[u8; 20], + port: u16, +) -> torrust_tracker_udp_protocol::Response { + let client = UdpTrackerClient::new(remote_addr, Duration::from_secs(5)) + .await + .expect("failed to create UDP client"); + + // Connect + let connect_transaction_id = TransactionId::new(1); + let connect_request = ConnectRequest { + transaction_id: connect_transaction_id, + }; + client + .send(connect_request.into()) + .await + .expect("failed to send connect request"); + let connection_id = match client.receive().await.expect("failed to receive connect response") { + torrust_tracker_udp_protocol::Response::Connect(resp) => resp.connection_id, + other => panic!("expected connect response, got: {other:?}"), + }; + + // Announce + let announce_transaction_id = TransactionId::new(2); + let announce_request = AnnounceRequest { + connection_id, + action_placeholder: AnnounceActionPlaceholder::default(), + transaction_id: announce_transaction_id, + info_hash: torrust_tracker_udp_protocol::common::InfoHash(*info_hash), + peer_id: torrust_peer_id::PeerId(*peer_id), + bytes_downloaded: NumberOfBytes::new(0), + bytes_uploaded: NumberOfBytes::new(0), + bytes_left: NumberOfBytes::new(0), + event: AnnounceEvent::Started.into(), + ip_address: std::net::Ipv4Addr::UNSPECIFIED.into(), + key: PeerKey::new(0), + peers_wanted: NumberOfPeers::new(1), + port: Port::new(NonZeroU16::new(port).expect("port must be non-zero")), + }; + client + .send(announce_request.into()) + .await + .expect("failed to send announce request"); + client.receive().await.expect("failed to receive announce response") +} diff --git a/src/app.rs b/src/app.rs index 054722293..7143d1fc3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -239,11 +239,12 @@ async fn start_udp_instance( job_manager: &mut JobManager, ) { let udp_tracker_container = app_container - .udp_tracker_container(udp_tracker_config.bind_address) + .udp_tracker_container(idx) .expect("Could not create UDP tracker container"); let udp_tracker_server_container = app_container.udp_tracker_server_container(); let handle = udp_tracker::start_job( + idx, udp_tracker_container, udp_tracker_server_container, app_container.registar.give_form(), @@ -270,10 +271,11 @@ async fn start_http_instance( job_manager: &mut JobManager, ) { let http_tracker_container = app_container - .http_tracker_container(http_tracker_config.bind_address) + .http_tracker_container(idx) .expect("Could not create HTTP tracker container"); if let Some(handle) = http_tracker::start_job( + idx, http_tracker_container, app_container.registar.give_form(), torrust_tracker_axum_http_server::Version::V1, diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs index 7bdcca51d..2849dd7da 100644 --- a/src/bootstrap/jobs/http_tracker.rs +++ b/src/bootstrap/jobs/http_tracker.rs @@ -32,12 +32,20 @@ use tracing::instrument; /// It would panic if the `config::HttpTracker` struct would contain inappropriate values. #[instrument(skip(http_tracker_container, form))] pub async fn start_job( + idx: usize, http_tracker_container: Arc, form: ServiceRegistrationForm, version: Version, ) -> Option> { let socket = http_tracker_container.http_tracker_config.bind_address; + tracing::info!( + instance_index = idx, + bind_address = %socket, + tracker_usage_statistics = http_tracker_container.http_tracker_config.tracker_usage_statistics, + "Starting HTTP tracker instance" + ); + let tls = if let Some(tls_config) = &http_tracker_container.http_tracker_config.tsl_config { Some( make_rust_tls(tls_config) @@ -108,7 +116,7 @@ mod tests { let version = Version::V1; - start_job(http_tracker_container, Registar::default().give_form(), version) + start_job(0, http_tracker_container, Registar::default().give_form(), version) .await .expect("it should be able to join to the http tracker start-job"); } diff --git a/src/bootstrap/jobs/udp_tracker.rs b/src/bootstrap/jobs/udp_tracker.rs index f8c09b71a..d9734d045 100644 --- a/src/bootstrap/jobs/udp_tracker.rs +++ b/src/bootstrap/jobs/udp_tracker.rs @@ -30,6 +30,7 @@ use tracing::instrument; #[allow(clippy::async_yields_async)] #[instrument(skip(udp_tracker_core_container, udp_tracker_server_container, form))] pub async fn start_job( + idx: usize, udp_tracker_core_container: Arc, udp_tracker_server_container: Arc, form: ServiceRegistrationForm, @@ -37,6 +38,13 @@ pub async fn start_job( let bind_to = udp_tracker_core_container.udp_tracker_config.bind_address; let cookie_lifetime = udp_tracker_core_container.udp_tracker_config.cookie_lifetime; + tracing::info!( + instance_index = idx, + bind_address = %bind_to, + tracker_usage_statistics = udp_tracker_core_container.udp_tracker_config.tracker_usage_statistics, + "Starting UDP tracker instance" + ); + // The connection ID validation policy is available in schema v3 via // `UdpTrackerServer::connection_id_validation`. The application bootstrap // still uses v2 configuration, where this policy does not exist. Until the diff --git a/src/container.rs b/src/container.rs index de4097d67..8e5071964 100644 --- a/src/container.rs +++ b/src/container.rs @@ -1,11 +1,10 @@ -use std::collections::HashMap; -use std::net::SocketAddr; use std::sync::Arc; use torrust_server_lib::registar::Registar; use torrust_tracker_configuration::{Configuration, HttpApi}; use torrust_tracker_core::container::TrackerCoreContainer; use torrust_tracker_http_core::container::{HttpTrackerCoreContainer, HttpTrackerCoreServices}; +use torrust_tracker_primitives::ConfigurationInstanceId; use torrust_tracker_rest_api_runtime_adapter::v1::container::TrackerHttpApiCoreContainer; use torrust_tracker_swarm_coordination_registry::container::SwarmCoordinationRegistryContainer; use torrust_tracker_udp_core::container::{UdpTrackerCoreContainer, UdpTrackerCoreServices}; @@ -15,11 +14,11 @@ use tracing::instrument; #[derive(thiserror::Error, Debug, Clone)] pub enum Error { - #[error("There is not a HTTP tracker server instance bound to the socket address: {bind_address}")] - MissingHttpTrackerCoreContainer { bind_address: SocketAddr }, + #[error("No HTTP tracker container at configuration index {index}")] + MissingHttpTrackerCoreContainer { index: usize }, - #[error("There is not a UDP tracker server instance bound to the socket address: {bind_address}")] - MissingUdpTrackerCoreContainer { bind_address: SocketAddr }, + #[error("No UDP tracker container at configuration index {index}")] + MissingUdpTrackerCoreContainer { index: usize }, } pub struct AppContainer { @@ -37,12 +36,12 @@ pub struct AppContainer { // HTTP pub http_tracker_core_services: Arc, - pub http_tracker_instance_containers: Arc>>, + pub http_tracker_instance_containers: Vec<(ConfigurationInstanceId, Arc)>, // UDP pub udp_tracker_core_services: Arc, pub udp_tracker_server_container: Arc, - pub udp_tracker_instance_containers: Arc>>, + pub udp_tracker_instance_containers: Vec<(ConfigurationInstanceId, Arc)>, } impl AppContainer { @@ -132,23 +131,23 @@ impl AppContainer { /// # Errors /// - /// Return an error if there is no HTTP tracker server instance bound to the - /// socket address. - pub fn http_tracker_container(&self, bind_address: SocketAddr) -> Result, Error> { - self.http_tracker_instance_containers.get(&bind_address).map_or_else( - || Err(Error::MissingHttpTrackerCoreContainer { bind_address }), - |http_tracker_container| Ok(http_tracker_container.clone()), + /// Return an error if there is no HTTP tracker container at the given + /// configuration index. + pub fn http_tracker_container(&self, index: usize) -> Result, Error> { + self.http_tracker_instance_containers.get(index).map_or_else( + || Err(Error::MissingHttpTrackerCoreContainer { index }), + |(_id, container)| Ok(container.clone()), ) } /// # Errors /// - /// Return an error if there is no UDP tracker server instance bound to the - /// socket address. - pub fn udp_tracker_container(&self, bind_address: SocketAddr) -> Result, Error> { - self.udp_tracker_instance_containers.get(&bind_address).map_or_else( - || Err(Error::MissingUdpTrackerCoreContainer { bind_address }), - |udp_tracker_container| Ok(udp_tracker_container.clone()), + /// Return an error if there is no UDP tracker container at the given + /// configuration index. + pub fn udp_tracker_container(&self, index: usize) -> Result, Error> { + self.udp_tracker_instance_containers.get(index).map_or_else( + || Err(Error::MissingUdpTrackerCoreContainer { index }), + |(_id, container)| Ok(container.clone()), ) } @@ -175,23 +174,24 @@ impl AppContainer { configuration: &Configuration, tracker_core_container: &Arc, http_tracker_core_services: &Arc, - ) -> Arc>> { - let mut http_tracker_instance_containers = HashMap::new(); + ) -> Vec<(ConfigurationInstanceId, Arc)> { + use torrust_tracker_primitives::ServiceRole; + + let mut containers = Vec::new(); if let Some(http_trackers) = &configuration.http_trackers { - for http_tracker_config in http_trackers { - http_tracker_instance_containers.insert( - http_tracker_config.bind_address, - HttpTrackerCoreContainer::initialize_from_services( - tracker_core_container, - http_tracker_core_services, - &Arc::new(http_tracker_config.clone()), - ), + for (index, http_tracker_config) in http_trackers.iter().enumerate() { + let id = ConfigurationInstanceId::new(ServiceRole::HttpTracker, index); + let container = HttpTrackerCoreContainer::initialize_from_services( + tracker_core_container, + http_tracker_core_services, + &Arc::new(http_tracker_config.clone()), ); + containers.push((id, container)); } } - Arc::new(http_tracker_instance_containers) + containers } #[must_use] @@ -199,22 +199,23 @@ impl AppContainer { configuration: &Configuration, tracker_core_container: &Arc, udp_tracker_core_services: &Arc, - ) -> Arc>> { - let mut udp_tracker_instance_containers = HashMap::new(); + ) -> Vec<(ConfigurationInstanceId, Arc)> { + use torrust_tracker_primitives::ServiceRole; + + let mut containers = Vec::new(); if let Some(udp_trackers) = &configuration.udp_trackers { - for udp_tracker_config in udp_trackers { - udp_tracker_instance_containers.insert( - udp_tracker_config.bind_address, - UdpTrackerCoreContainer::initialize_from_services( - tracker_core_container, - udp_tracker_core_services, - &Arc::new(udp_tracker_config.clone()), - ), + for (index, udp_tracker_config) in udp_trackers.iter().enumerate() { + let id = ConfigurationInstanceId::new(ServiceRole::UdpTracker, index); + let container = UdpTrackerCoreContainer::initialize_from_services( + tracker_core_container, + udp_tracker_core_services, + &Arc::new(udp_tracker_config.clone()), ); + containers.push((id, container)); } } - Arc::new(udp_tracker_instance_containers) + containers } } diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 044398d7a..7c217fcfb 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -3,8 +3,9 @@ semantic-links: skill-links: - write-unit-test related-artifacts: - - tests/stats.rs - - tests/servers/ + - tests/aggregate_stats_port_zero.rs + - tests/aggregate_stats_fixed_ports.rs + - tests/common/mod.rs - src/app.rs - docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md issue-spec: docs/issues/drafts/increase-main-app-integration-test-coverage.md @@ -53,22 +54,55 @@ configuration. Scenario functions run sequentially against that instance. A different initial configuration requires a separate top-level file. For example: -| File | Purpose | -| -------------------- | -------------------------------------------------------- | -| `tests/stats.rs` | Global statistics suite (public tracker, two HTTP nodes) | -| `tests/scaffold.rs` | Scaffolding demo — same pattern, isolated process | -| `tests/bootstrap.rs` | _(future)_ Bootstrap/shutdown lifecycle scenarios | - -Cargo may run these binaries in parallel. Each binary binds to port `0` (OS-assigned -ephemeral ports), uses its own `TempDir` workspace, and sets -`TORRUST_TRACKER_CONFIG_TOML_PATH` only in its own process, so no conflict occurs. +| File | Purpose | +| -------------------------------------- | -------------------------------------------------------------- | +| `tests/aggregate_stats_port_zero.rs` | Aggregate statistics with port-zero listeners (two HTTP nodes) | +| `tests/aggregate_stats_fixed_ports.rs` | Aggregate statistics with distinct fixed-port HTTP listeners | +| `tests/scaffold.rs` | Scaffolding demo — same pattern, isolated process | + +Each binary defines a single `#[tokio::test]` runner that starts the tracker +once, then calls scenario functions sequentially. Scenario functions are plain +async functions that receive the `AppContainer` and assert behavior. + +Cargo may run these binaries in parallel. Each binary binds to port `0` +(OS-assigned ephemeral ports) by default, uses its own `TempDir` workspace, +and sets `TORRUST_TRACKER_CONFIG_TOML_PATH` only in its own process, so no +conflict occurs. Fixed-port binaries (e.g., `aggregate_stats_fixed_ports.rs`) +use distinct non-overlapping ports and must not run concurrently with other +binaries that use the same ports. + +### Why one binary per configuration? + +The 1:1 mapping between integration-test binaries and tracker configurations +exists because the current application startup has several global side effects +that prevent running multiple isolated tracker instances in the same process: + +1. **`tracing` global initialization** (main blocker): The `tracing` crate + initializes a global subscriber. Once set, it cannot be reset for a second + tracker instance in the same process. This means two tracker applications + sharing a process would share logging state and configuration. +2. **Environment-variable config injection**: The tracker reads its + configuration from the `TORRUST_TRACKER_CONFIG_TOML_PATH` environment + variable. Multiple tracker instances in the same process would race on + this variable. +3. **Static secrets and clock state**: Values like seed secrets and the + deterministic test clock are process-global. While these could be refactored + into injected dependencies, the tracing global subscriber remains the + fundamental blocker. + +Until these global side effects are eliminated (tracked in +[#1430](https://github.com/torrust/torrust-tracker/issues/1430)), each +integration-test binary must start exactly one tracker instance with one fixed +configuration. Scenario functions run sequentially against that shared instance. ## Test Infrastructure Requirements All integration tests at this level must: -1. **Use port `0` for all bind addresses**: The OS assigns free ephemeral ports, preventing - conflicts when tests run in parallel +1. **Use port `0` for bind addresses by default**: The OS assigns free ephemeral ports, + preventing conflicts when tests run in parallel. Fixed ports are permitted when the + test scenario specifically requires distinct addresses (e.g., verifying per-instance + behavior). Use non-overlapping port ranges and document the constraint. 2. **Use isolated temporary workspaces**: Use `tempfile::TempDir` to create isolated directories with separate config files and storage subdirectories 3. **Extract actual bound ports**: Query `AppContainer`'s `Registar` to get the OS-assigned ports @@ -81,39 +115,38 @@ All integration tests at this level must: ```text tests/ -├── AGENTS.md # This file +├── AGENTS.md # This file ├── common/ -│ └── mod.rs # Shared test utilities (temp config, port extraction) -├── stats.rs # Global statistics suite (main integration tests) -├── scaffold.rs # Scaffolding demo — pattern reference for new binaries -└── servers/ - └── api/ - └── contract/ - └── stats/ - └── mod.rs # Global statistics test scenarios +│ ├── mod.rs # Re-exports from submodules +│ ├── workspace.rs # Tracker workspace setup and URL discovery +│ ├── announce.rs # HTTP and UDP announce helpers +│ └── statistics.rs # Aggregate statistics query helpers +├── aggregate_stats_port_zero.rs # Port-zero statistics (two HTTP + two UDP nodes) +├── aggregate_stats_fixed_ports.rs # Fixed-port statistics (two HTTP + two UDP nodes) +└── scaffold.rs # Scaffolding demo — pattern reference for new binaries ``` ## Adding a New Integration-Test Binary 1. **Confirm it belongs here**: Can this test be written at the package level? If yes, write it there. 2. **Determine the initial configuration**: If your scenarios need a different tracker - configuration than the existing suite, create a new top-level file (e.g., `tests/bootstrap.rs`). - If they share the same configuration, add scenarios to the existing suite. + configuration than the existing suite, create a new top-level file (e.g., + `tests/aggregate_stats_fixed_ports.rs`). If they share the same configuration, add + scenarios to the existing suite's runner function. 3. **Reuse shared utilities**: Import `mod common;` and use the helpers in `tests/common/mod.rs` for workspace setup, tracker startup, and port discovery. -4. **Use port `0`**: All services must bind to port `0` in test configurations. +4. **Use port `0` by default**: Bind services to port `0` unless the scenario specifically + requires distinct fixed addresses. 5. **Extract bound ports**: Query the registar or `AppContainer` to discover actual socket addresses. 6. **Document the purpose**: Add clear doc comments explaining what application-level behavior is being tested. -7. **Reference existing code**: See `tests/scaffold.rs` for a minimal working example of a new - integration-test binary, or `tests/servers/api/contract/stats/mod.rs` for the scenario pattern. - -For concrete examples, see the existing tests in `tests/servers/` — they serve as the canonical -reference for the integration test pattern. +7. **Reference existing code**: See `tests/aggregate_stats_fixed_ports.rs` for the canonical + pattern: one `#[tokio::test]` runner, one config constant, scenario functions that receive + the `AppContainer`. ## References - [Issue #1419](../../docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md) - Infrastructure for parallel integration tests (execution model decision) -- [Integration test scaffolding](stats.rs) +- [Integration test scaffolding](aggregate_stats_port_zero.rs) - [Shared test utilities](common/mod.rs) - [Scaffolding demo](scaffold.rs) diff --git a/tests/aggregate_stats_fixed_ports.rs b/tests/aggregate_stats_fixed_ports.rs new file mode 100644 index 000000000..4f6dc4804 --- /dev/null +++ b/tests/aggregate_stats_fixed_ports.rs @@ -0,0 +1,130 @@ +//! Aggregate statistics integration test — fixed-port multi-instance scenarios. +//! +//! This binary starts a tracker with two HTTP and two UDP listeners on distinct +//! fixed ports, all with `tracker_usage_statistics = true`. Scenario functions +//! verify that aggregate statistics correctly count announces from all enabled +//! listeners. +//! +//! ```text +//! cargo test --test aggregate_stats_fixed_ports +//! ``` +mod common; + +use torrust_clock::clock; + +/// This code needs to be copied into each crate. +/// Working version, for production. +#[cfg(not(test))] +#[allow(dead_code)] +pub(crate) type CurrentClock = clock::Working; + +/// Stopped version, for testing. +#[cfg(test)] +#[allow(dead_code)] +pub(crate) type CurrentClock = clock::Stopped; + +/// Configuration: two HTTP listeners on distinct fixed ports, +/// all with `tracker_usage_statistics = true`. +// NOTE: The fixed-port enabled/disabled HTTP aggregate-statistics test (count == 1) +// is blocked by #2039 — the shared HTTP event bus prevents per-instance metrics +// suppression. The bootstrap fix correctly assigns different containers, but the +// HTTP stats layer still counts both. This test proves both listeners start; +// the per-instance filtering test will be added when #2039 lands. +const FIXED_PORT_CONFIG: &str = r#" + [metadata] + app = "torrust-tracker" + purpose = "configuration" + schema_version = "2.0.0" + + [logging] + threshold = "off" + + [core] + listed = false + private = false + + [core.database] + driver = "sqlite3" + path = "{STORAGE_PATH}/sqlite3.db" + + [[http_trackers]] + bind_address = "0.0.0.0:17091" + tracker_usage_statistics = true + + [[http_trackers]] + bind_address = "0.0.0.0:17092" + tracker_usage_statistics = true + + [[udp_trackers]] + bind_address = "0.0.0.0:17093" + tracker_usage_statistics = true + + [[udp_trackers]] + bind_address = "0.0.0.0:17094" + tracker_usage_statistics = true + + [http_api] + bind_address = "127.0.0.1:0" + + [http_api.access_tokens] + admin = "MyAccessToken" + + [health_check_api] + bind_address = "127.0.0.2:0" +"#; + +#[tokio::test] +async fn aggregate_stats_scenarios() { + let workspace = common::EphemeralTrackerWorkspace::new(FIXED_PORT_CONFIG); + let (app_container, _jobs) = common::start_tracker_with_config(&workspace).await; + + http_trackers_on_fixed_ports_should_aggregate_announces_from_both_listeners(&app_container).await; + udp_trackers_on_fixed_ports_should_aggregate_announces_from_both_listeners(&app_container).await; +} + +/// Both HTTP listeners are on distinct fixed ports. Announces to both +/// should be counted in the aggregate HTTP statistics. +async fn http_trackers_on_fixed_ports_should_aggregate_announces_from_both_listeners( + app_container: &std::sync::Arc, +) { + let tracker_urls = common::http_tracker_urls(app_container).await; + assert_eq!(tracker_urls.len(), 2, "expected two HTTP trackers"); + + let api_url = common::http_api_url(app_container).await.expect("expected an HTTP API URL"); + + let info_hash = [ + 0x9c, 0x8b, 0x22, 0x13, 0xe3, 0x0b, 0xff, 0x21, 0x2b, 0x0c, 0x36, 0x0d, 0x26, 0xf9, 0xa0, 0x21, 0x31, 0x64, 0x22, 0x00, + ]; + let peer_id = *b"-qB00000000000000001"; + + for url in &tracker_urls { + common::http_announce(url, &info_hash, &peer_id, 17548).await; + } + + let global_stats = common::get_tracker_statistics(&api_url, "MyAccessToken").await; + assert_eq!(global_stats.tcp4_announces_handled, 2); +} + +/// Both UDP listeners are on distinct fixed ports. Announces to both +/// should be counted in the aggregate UDP statistics. +async fn udp_trackers_on_fixed_ports_should_aggregate_announces_from_both_listeners( + app_container: &std::sync::Arc, +) { + let udp_urls = common::udp_tracker_urls(app_container).await; + assert_eq!(udp_urls.len(), 2, "expected two UDP trackers"); + + let api_url = common::http_api_url(app_container).await.expect("expected an HTTP API URL"); + + let info_hash = [ + 0x9c, 0x8b, 0x22, 0x13, 0xe3, 0x0b, 0xff, 0x21, 0x2b, 0x0c, 0x36, 0x0d, 0x26, 0xf9, 0xa0, 0x21, 0x31, 0x64, 0x22, 0x00, + ]; + let peer_id = *b"-qB00000000000000001"; + + for url in &udp_urls { + let addr = common::udp_socket_addr(url); + common::udp_announce(addr, &info_hash, &peer_id, 17548).await; + } + + let global_stats = common::get_tracker_statistics(&api_url, "MyAccessToken").await; + assert_eq!(global_stats.udp4_announces_handled, 2); +} diff --git a/tests/aggregate_stats_port_zero.rs b/tests/aggregate_stats_port_zero.rs new file mode 100644 index 000000000..d5fb5c054 --- /dev/null +++ b/tests/aggregate_stats_port_zero.rs @@ -0,0 +1,162 @@ +//! Statistics integration test — aggregate statistics with port-zero listeners. +//! +//! This binary starts a tracker with two HTTP and two UDP listeners on port +//! zero, all enabled. Scenario functions verify that aggregate statistics +//! count announces from all listeners. +//! +//! ```text +//! cargo test --test aggregate_stats_port_zero +//! ``` +mod common; + +use torrust_clock::clock; + +/// This code needs to be copied into each crate. +/// Working version, for production. +#[cfg(not(test))] +#[allow(dead_code)] +pub(crate) type CurrentClock = clock::Working; + +/// Stopped version, for testing. +#[cfg(test)] +#[allow(dead_code)] +pub(crate) type CurrentClock = clock::Stopped; + +/// Configuration: two HTTP and two UDP listeners on port zero, with different +/// settings per instance to prove bootstrap assigns distinct containers. +const PORT_ZERO_CONFIG: &str = r#" + [metadata] + app = "torrust-tracker" + purpose = "configuration" + schema_version = "2.0.0" + + [logging] + threshold = "off" + + [core] + listed = false + private = false + + [core.database] + driver = "sqlite3" + path = "{STORAGE_PATH}/sqlite3.db" + + [[http_trackers]] + bind_address = "0.0.0.0:0" + tracker_usage_statistics = false + + [[http_trackers]] + bind_address = "0.0.0.0:0" + tracker_usage_statistics = true + + [[udp_trackers]] + bind_address = "0.0.0.0:0" + tracker_usage_statistics = false + + [[udp_trackers]] + bind_address = "0.0.0.0:0" + tracker_usage_statistics = true + + [http_api] + bind_address = "127.0.0.1:0" + + [http_api.access_tokens] + admin = "MyAccessToken" + + [health_check_api] + bind_address = "127.0.0.2:0" +"#; + +#[tokio::test] +async fn stats_scenarios() { + let workspace = common::EphemeralTrackerWorkspace::new(PORT_ZERO_CONFIG); + let (app_container, _jobs) = common::start_tracker_with_config(&workspace).await; + + duplicate_port_zero_instances_should_receive_distinct_configurations(&app_container); + two_http_trackers_on_port_zero_should_aggregate_announces_from_both_listeners(&app_container).await; + two_udp_trackers_on_port_zero_should_aggregate_announces_from_both_listeners(&app_container).await; +} + +/// Duplicate port-zero configuration blocks each receive their own container +/// with distinct settings, proving the bootstrap fix prevents the +/// address-keyed collision. +fn duplicate_port_zero_instances_should_receive_distinct_configurations( + app_container: &std::sync::Arc, +) { + // HTTP: first instance should have statistics disabled, second enabled. + assert_eq!(app_container.http_tracker_instance_containers.len(), 2); + assert!( + !app_container.http_tracker_instance_containers[0] + .1 + .http_tracker_config + .tracker_usage_statistics + ); + assert!( + app_container.http_tracker_instance_containers[1] + .1 + .http_tracker_config + .tracker_usage_statistics + ); + + // UDP: first instance should have statistics disabled, second enabled. + assert_eq!(app_container.udp_tracker_instance_containers.len(), 2); + assert!( + !app_container.udp_tracker_instance_containers[0] + .1 + .udp_tracker_config + .tracker_usage_statistics + ); + assert!( + app_container.udp_tracker_instance_containers[1] + .1 + .udp_tracker_config + .tracker_usage_statistics + ); +} + +/// Both HTTP listeners on port zero. Announces to both should be counted +/// in the aggregate HTTP statistics. +async fn two_http_trackers_on_port_zero_should_aggregate_announces_from_both_listeners( + app_container: &std::sync::Arc, +) { + let tracker_urls = common::http_tracker_urls(app_container).await; + assert_eq!(tracker_urls.len(), 2, "expected two HTTP trackers"); + + let api_url = common::http_api_url(app_container).await.expect("expected an HTTP API URL"); + + let info_hash = [ + 0x9c, 0x8b, 0x22, 0x13, 0xe3, 0x0b, 0xff, 0x21, 0x2b, 0x0c, 0x36, 0x0d, 0x26, 0xf9, 0xa0, 0x21, 0x31, 0x64, 0x22, 0x00, + ]; + let peer_id = *b"-qB00000000000000001"; + + for url in &tracker_urls { + common::http_announce(url, &info_hash, &peer_id, 17548).await; + } + + let global_stats = common::get_tracker_statistics(&api_url, "MyAccessToken").await; + assert_eq!(global_stats.tcp4_announces_handled, 2); +} + +/// Both UDP listeners on port zero. Announces to both should be counted +/// in the aggregate UDP statistics. +async fn two_udp_trackers_on_port_zero_should_aggregate_announces_from_both_listeners( + app_container: &std::sync::Arc, +) { + let udp_urls = common::udp_tracker_urls(app_container).await; + assert_eq!(udp_urls.len(), 2, "expected two UDP trackers"); + + let api_url = common::http_api_url(app_container).await.expect("expected an HTTP API URL"); + + let info_hash = [ + 0x9c, 0x8b, 0x22, 0x13, 0xe3, 0x0b, 0xff, 0x21, 0x2b, 0x0c, 0x36, 0x0d, 0x26, 0xf9, 0xa0, 0x21, 0x31, 0x64, 0x22, 0x00, + ]; + let peer_id = *b"-qB00000000000000001"; + + for url in &udp_urls { + let addr = common::udp_socket_addr(url); + common::udp_announce(addr, &info_hash, &peer_id, 17548).await; + } + + let global_stats = common::get_tracker_statistics(&api_url, "MyAccessToken").await; + assert_eq!(global_stats.udp4_announces_handled, 2); +} diff --git a/tests/common/announce.rs b/tests/common/announce.rs new file mode 100644 index 000000000..6128795c5 --- /dev/null +++ b/tests/common/announce.rs @@ -0,0 +1,27 @@ +//! Announce helpers — send HTTP and UDP announces to tracker instances. + +use std::net::SocketAddr; + +use url::Url; + +/// Sends an HTTP announce to the given tracker URL. +/// +/// Delegates to `torrust_tracker_test_helpers::http::http_announce`. +/// Panics if the announce fails. +// +// Not called by every integration-test binary — see note on `udp_tracker_urls`. +#[allow(dead_code)] +pub async fn http_announce(tracker_url: &Url, info_hash: &[u8; 20], peer_id: &[u8; 20], port: u16) { + torrust_tracker_test_helpers::http::http_announce(tracker_url, info_hash, peer_id, port).await; +} + +/// Sends a UDP announce to the given tracker address. +/// +/// Delegates to `torrust_tracker_test_helpers::udp::udp_announce`. +/// Panics if the announce fails. +// +// Not called by every integration-test binary — see note on `udp_tracker_urls`. +#[allow(dead_code)] +pub async fn udp_announce(remote_addr: SocketAddr, info_hash: &[u8; 20], peer_id: &[u8; 20], port: u16) { + torrust_tracker_test_helpers::udp::udp_announce(remote_addr, info_hash, peer_id, port).await; +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 39538c10d..974a20a8d 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -12,136 +12,21 @@ //! against that instance. A different initial configuration belongs to //! another top-level binary, which Cargo may run concurrently. //! -//! See `docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level.md` +//! See `docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md` //! for the full decision record. -use std::net::SocketAddr; -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use tempfile::TempDir; -use torrust_tracker_lib::app; -use torrust_tracker_lib::bootstrap::jobs::manager::JobManager; -use torrust_tracker_lib::container::AppContainer; -use url::Url; - -/// A temporary workspace for an integration test. -/// -/// Creates an isolated directory with config file and storage directory. -/// The `{STORAGE_PATH}` placeholder in the config TOML is replaced with -/// the absolute path to the temp storage directory. -pub struct EphemeralTrackerWorkspace { - _temp_dir: TempDir, - config_path: PathBuf, -} - -impl EphemeralTrackerWorkspace { - #[must_use] - pub fn new(config_toml: &str) -> Self { - let temp_dir = TempDir::new().expect("failed to create temp dir"); - let storage_path = temp_dir.path().join("tracker-storage"); - std::fs::create_dir_all(&storage_path).expect("failed to create storage dir"); - - let config_path = temp_dir.path().join("tracker-config.toml"); - let resolved = config_toml.replace("{STORAGE_PATH}", &storage_path.to_string_lossy()); - std::fs::write(&config_path, resolved).expect("failed to write config file"); - - Self { - _temp_dir: temp_dir, - config_path, - } - } - - #[must_use] - pub fn config_path(&self) -> &Path { - &self.config_path - } -} - -/// Starts the tracker application with the given workspace config. -/// -/// Since the application reads its configuration from the -/// `TORRUST_TRACKER_CONFIG_TOML_PATH` environment variable, -/// tests in this binary must not run concurrently with other tests -/// that modify the same variable. -/// -/// A short delay is added after startup to allow services to register -/// in the registar and bind to OS-assigned ports. -pub async fn start_tracker_with_config(workspace: &EphemeralTrackerWorkspace) -> (Arc, JobManager) { - // SAFETY: This binary must be the only test executable setting - // `TORRUST_TRACKER_CONFIG_TOML_PATH`. Cargo may run different - // integration-test binaries in parallel, but each binary is a - // separate OS process with its own environment. - #[allow(unsafe_code)] - unsafe { - std::env::set_var( - "TORRUST_TRACKER_CONFIG_TOML_PATH", - workspace.config_path().to_str().expect("config path must be valid UTF-8"), - ); - } - let (container, jobs) = app::run().await; - // Allow spawned tasks (registar insertions, listener binds) to - // complete before we attempt to read the registar or make requests. - tokio::time::sleep(std::time::Duration::from_millis(500)).await; - (container, jobs) -} - -/// Returns the HTTP tracker URLs from the registar. -/// -/// TODO: Replace this temporary bind-IP classification after -/// `fix-duplicate-port-zero-tracker-instance-bootstrap` and -/// `add-runtime-service-registry-metadata` are implemented. Those issues establish stable -/// configuration-instance identity and role-based runtime discovery. -/// -/// HTTP trackers bind to `0.0.0.0` (unspecified). The REST API and health -/// check bind to `127.0.0.1` (loopback). We identify trackers by their -/// unspecified IP, which is deterministic regardless of hash-map ordering. -/// Wildcard addresses are converted to `127.0.0.1` for client requests. -pub async fn http_tracker_urls(container: &AppContainer) -> Vec { - let reg = container.registar.entries(); - let map = reg.lock().await; - map.keys() - .filter(|b| { - b.protocol() == torrust_net_primitives::service_binding::Protocol::HTTP && b.bind_address().ip().is_unspecified() - }) - .map(|b| loopback_url(b.bind_address())) - .collect() -} - -/// Returns the HTTP API URL from the registar. -/// -/// TODO: Replace this temporary bind-IP classification with role-based runtime discovery. See -/// `docs/issues/open/2036-add-runtime-service-registry-metadata/ISSUE.md`. -/// -/// The REST API binds to `127.0.0.1` (loopback), unlike the HTTP trackers -/// which bind to `0.0.0.0`. -pub async fn http_api_url(container: &AppContainer) -> Option { - let reg = container.registar.entries(); - let map = reg.lock().await; - map.keys() - .find(|b| b.protocol() == torrust_net_primitives::service_binding::Protocol::HTTP && b.bind_address().ip().is_loopback()) - .map(|b| loopback_url(b.bind_address())) -} - -/// Returns all registered service bindings for diagnostic purposes. -#[allow(dead_code)] -pub async fn all_service_bindings(container: &AppContainer) -> Vec<(String, Url)> { - let reg = container.registar.entries(); - let map = reg.lock().await; - map.keys() - .map(|b| (b.protocol().to_string(), loopback_url(b.bind_address()))) - .collect() -} - -/// Convert a socket address to a connectable loopback URL. -/// -/// Tracker services bind to `0.0.0.0` (all interfaces), but clients must -/// connect to a reachable address. This replaces wildcard IPv4 with the -/// loopback address `127.0.0.1`, preserving the OS-assigned port. -fn loopback_url(addr: SocketAddr) -> Url { - if addr.ip().is_unspecified() { - Url::parse(&format!("http://127.0.0.1:{port}", port = addr.port())) - } else { - Url::parse(&format!("http://{addr}")) - } - .expect("loopback URL should always be valid") -} +mod announce; +mod statistics; +mod workspace; + +// Each integration-test binary compiles this module independently. Not all +// binaries call every re-exported function, so the compiler emits +// unused_imports warnings for the binaries that don't. The attributes +// suppress those per-binary false positives. +#[allow(unused_imports)] +pub use announce::{http_announce, udp_announce}; +#[allow(unused_imports)] +pub use statistics::{PartialGlobalStatistics, get_tracker_statistics}; +#[allow(unused_imports)] +pub use workspace::{ + EphemeralTrackerWorkspace, http_api_url, http_tracker_urls, start_tracker_with_config, udp_socket_addr, udp_tracker_urls, +}; diff --git a/tests/common/statistics.rs b/tests/common/statistics.rs new file mode 100644 index 000000000..7ae47bb91 --- /dev/null +++ b/tests/common/statistics.rs @@ -0,0 +1,28 @@ +//! Statistics helpers — query aggregate metrics from the REST API. + +use url::Url; + +/// Global statistics with only metrics relevant to the test. +#[derive(serde::Deserialize)] +#[allow(dead_code)] +pub struct PartialGlobalStatistics { + pub tcp4_announces_handled: u64, + pub udp4_announces_handled: u64, +} + +#[allow(dead_code)] +pub async fn get_tracker_statistics(api_url: &Url, token: &str) -> PartialGlobalStatistics { + use torrust_tracker_rest_api_client::connection_info::{ConnectionInfo, Origin}; + use torrust_tracker_rest_api_client::v1::client::ApiHttpClient as TrackerApiClient; + + let response = TrackerApiClient::new(ConnectionInfo::authenticated(Origin::new(api_url.as_str()).unwrap(), token)) + .unwrap() + .get_tracker_statistics(None) + .await + .expect("failed to get tracker statistics"); + + response + .json::() + .await + .expect("Failed to parse JSON response") +} diff --git a/tests/common/workspace.rs b/tests/common/workspace.rs new file mode 100644 index 000000000..343504e37 --- /dev/null +++ b/tests/common/workspace.rs @@ -0,0 +1,199 @@ +//! Tracker workspace and URL discovery helpers. + +use std::net::SocketAddr; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use tempfile::TempDir; +use torrust_tracker_lib::app; +use torrust_tracker_lib::bootstrap::jobs::manager::JobManager; +use torrust_tracker_lib::container::AppContainer; +use url::Url; + +/// A temporary workspace for an integration test. +/// +/// Creates an isolated directory with config file and storage directory. +/// The `{STORAGE_PATH}` placeholder in the config TOML is replaced with +/// the absolute path to the temp storage directory. +pub struct EphemeralTrackerWorkspace { + _temp_dir: TempDir, + config_path: PathBuf, +} + +impl EphemeralTrackerWorkspace { + #[must_use] + pub fn new(config_toml: &str) -> Self { + let temp_dir = TempDir::new().expect("failed to create temp dir"); + let storage_path = temp_dir.path().join("tracker-storage"); + std::fs::create_dir_all(&storage_path).expect("failed to create storage dir"); + + let config_path = temp_dir.path().join("tracker-config.toml"); + let resolved = config_toml.replace("{STORAGE_PATH}", &storage_path.to_string_lossy()); + std::fs::write(&config_path, resolved).expect("failed to write config file"); + + Self { + _temp_dir: temp_dir, + config_path, + } + } + + #[must_use] + pub fn config_path(&self) -> &Path { + &self.config_path + } +} + +/// Starts the tracker application with the given workspace config. +/// +/// Since the application reads its configuration from the +/// `TORRUST_TRACKER_CONFIG_TOML_PATH` environment variable, +/// tests in this binary must not run concurrently with other tests +/// that modify the same variable. +/// +/// A short delay is added after startup to allow services to register +/// in the registar and bind to OS-assigned ports. +pub async fn start_tracker_with_config(workspace: &EphemeralTrackerWorkspace) -> (Arc, JobManager) { + // We require at least two services to be registered before proceeding. + // This covers the common case of one HTTP tracker plus one UDP tracker. + // We intentionally do NOT wait for all services (HTTP API, health check, + // etc.) because scenarios only need the tracker listeners to be ready. + // Configurations with fewer services (e.g., health-check only) should + // use a lower threshold or bypass this wait. + const MIN_REGISTERED_SERVICES: usize = 2; + + // SAFETY: This binary must be the only test executable setting + // `TORRUST_TRACKER_CONFIG_TOML_PATH`. Cargo may run different + // integration-test binaries in parallel, but each binary is a + // separate OS process with its own environment. + #[allow(unsafe_code)] + unsafe { + std::env::set_var( + "TORRUST_TRACKER_CONFIG_TOML_PATH", + workspace.config_path().to_str().expect("config path must be valid UTF-8"), + ); + } + + let (container, jobs) = app::run().await; + + // Wait for services to register in the registar and bind to ports. + // Polls the registar instead of using a fixed sleep to avoid + // flakiness on slow machines and unnecessary delay on fast ones. + // + // TODO: This gate can pass before the specific services scenarios need are + // registered (e.g., if HTTP API + health check register first). Consider + // waiting on concrete predicates per test binary when flakiness appears. + // Tracked by #1430. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + + loop { + let entries = container.registar.entries(); + let map = entries.lock().await; + if map.len() >= MIN_REGISTERED_SERVICES { + break; + } + drop(map); + assert!( + std::time::Instant::now() < deadline, + "timeout waiting for services to register in the registar" + ); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + + (container, jobs) +} + +/// Returns the HTTP tracker URLs from the registar. +/// +/// HTTP trackers bind to `0.0.0.0` (unspecified). The REST API and health +/// check bind to `127.0.0.1` (loopback). We identify trackers by their +/// unspecified IP, which is deterministic regardless of hash-map ordering. +/// Wildcard addresses are converted to `127.0.0.1` for client requests. +pub async fn http_tracker_urls(container: &AppContainer) -> Vec { + let reg = container.registar.entries(); + let map = reg.lock().await; + map.keys() + .filter(|b| { + b.protocol() == torrust_net_primitives::service_binding::Protocol::HTTP && b.bind_address().ip().is_unspecified() + }) + .map(|b| loopback_url(b.bind_address())) + .collect() +} + +/// Returns the UDP tracker URLs from the registar. +/// +/// UDP trackers bind to `0.0.0.0` (unspecified). We identify them by their +/// unspecified IP, which is deterministic regardless of hash-map ordering. +/// Wildcard addresses are converted to `127.0.0.1` for client requests. +// +// Each integration-test binary compiles this module independently. Not all +// binaries call every function here, so the compiler emits dead_code warnings +// for the binaries that don't. The attribute suppresses those per-binary +// false positives without hiding genuine dead code in the workspace as a whole. +#[allow(dead_code)] +pub async fn udp_tracker_urls(container: &AppContainer) -> Vec { + let reg = container.registar.entries(); + let map = reg.lock().await; + map.keys() + .filter(|b| { + b.protocol() == torrust_net_primitives::service_binding::Protocol::UDP && b.bind_address().ip().is_unspecified() + }) + .map(|b| udp_loopback_url(b.bind_address())) + .collect() +} + +/// Returns the HTTP API URL from the registar. +/// +/// The REST API binds to `127.0.0.1` (loopback), unlike the HTTP trackers +/// which bind to `0.0.0.0`. We filter specifically for the REST API bind IP +/// (`127.0.0.1`) to avoid matching the health-check API on `127.0.0.2`. +pub async fn http_api_url(container: &AppContainer) -> Option { + let reg = container.registar.entries(); + let map = reg.lock().await; + map.keys() + .find(|b| { + b.protocol() == torrust_net_primitives::service_binding::Protocol::HTTP + && b.bind_address().ip() == std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST) + }) + .map(|b| loopback_url(b.bind_address())) +} + +/// Convert a socket address to a connectable loopback URL. +/// +/// Tracker services bind to `0.0.0.0` (all interfaces), but clients must +/// connect to a reachable address. This replaces wildcard IPv4 with the +/// loopback address `127.0.0.1`, preserving the OS-assigned port. +fn loopback_url(addr: SocketAddr) -> Url { + if addr.ip().is_unspecified() { + Url::parse(&format!("http://127.0.0.1:{port}", port = addr.port())) + } else { + Url::parse(&format!("http://{addr}")) // DevSkim: ignore DS137138 + } + .expect("loopback URL should always be valid") +} + +/// Convert a UDP socket address to a connectable loopback URL. +// Not called by every integration-test binary — see note on `udp_tracker_urls`. +#[allow(dead_code)] +fn udp_loopback_url(addr: SocketAddr) -> Url { + if addr.ip().is_unspecified() { + Url::parse(&format!("udp://127.0.0.1:{port}", port = addr.port())) + } else { + Url::parse(&format!("udp://{addr}")) + } + .expect("loopback URL should always be valid") +} + +/// Extract the `SocketAddr` from a `udp://` URL. +// +// Uses the `Url` host/port accessors rather than slicing the URL string. +// Not called by every integration-test binary — see note on `udp_tracker_urls`. +#[allow(dead_code)] +pub fn udp_socket_addr(url: &Url) -> SocketAddr { + let host = url + .host_str() + .expect("UDP URL must have a host") + .parse() + .expect("UDP URL host must be a valid IP"); + let port = url.port().expect("UDP URL must have a port"); + SocketAddr::new(host, port) +} diff --git a/tests/servers/api/contract/mod.rs b/tests/servers/api/contract/mod.rs deleted file mode 100644 index 9d34677fc..000000000 --- a/tests/servers/api/contract/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod stats; diff --git a/tests/servers/api/contract/stats/mod.rs b/tests/servers/api/contract/stats/mod.rs deleted file mode 100644 index 78d49822d..000000000 --- a/tests/servers/api/contract/stats/mod.rs +++ /dev/null @@ -1,177 +0,0 @@ -use serde::Deserialize; -use torrust_tracker_rest_api_client::connection_info::{ConnectionInfo, Origin}; -use torrust_tracker_rest_api_client::v1::client::ApiHttpClient as TrackerApiClient; -use url::Url; - -use crate::common::{self, EphemeralTrackerWorkspace}; - -/// The stats API endpoint should aggregate announces across multiple HTTP tracker instances. -/// -/// This is an application-level integration test. It verifies that announces -/// sent to two separate HTTP tracker instances are both counted in the global -/// tracker statistics. This behavior cannot be tested at the package level -/// because it requires the full application container coordinating multiple -/// HTTP tracker instances. -/// -/// Single-instance announce and scrape behavior is tested in the -/// `axum-http-server` package. -/// -/// TODO: Replace the temporary bind-IP endpoint discovery used by this suite after -/// `fix-duplicate-port-zero-tracker-instance-bootstrap` and -/// `add-runtime-service-registry-metadata` are implemented. -#[tokio::test] -async fn the_stats_api_endpoint_should_return_the_global_stats() { - // ── 1. Configuration ────────────────────────────────────────────── - let config_toml = r#" - [metadata] - app = "torrust-tracker" - purpose = "configuration" - schema_version = "2.0.0" - - [logging] - threshold = "off" - - [core] - listed = false - private = false - - [core.database] - driver = "sqlite3" - path = "{STORAGE_PATH}/sqlite3.db" - - [[http_trackers]] - bind_address = "0.0.0.0:0" - tracker_usage_statistics = true - - [[http_trackers]] - bind_address = "0.0.0.0:0" - tracker_usage_statistics = true - - [http_api] - bind_address = "127.0.0.1:0" - - [http_api.access_tokens] - admin = "MyAccessToken" - - [health_check_api] - bind_address = "127.0.0.2:0" - "#; - - // ── 2. Start tracker on isolated workspace ─────────────────────── - let workspace = EphemeralTrackerWorkspace::new(config_toml); - let (app_container, _jobs) = common::start_tracker_with_config(&workspace).await; - - let tracker_urls = common::http_tracker_urls(&app_container).await; - assert_eq!(tracker_urls.len(), 2, "expected two HTTP trackers"); - - let api_url = common::http_api_url(&app_container).await.expect("expected an HTTP API URL"); - - // ── 3. Announce to both tracker instances ──────────────────────── - let client = reqwest::Client::new(); - for url in &tracker_urls { - let announce_url = url - .join("/announce?info_hash=%9c8b%22%13%e3%0b%ff%21%2b0%c3%60%d2o%9a%02%13d%22&peer_id=-qB00000000000000001&port=17548&ip=127.0.0.1&event=started&compact=0") - .expect("announce URL should be valid"); - let resp = client.get(announce_url.as_str()).send().await.unwrap(); - let status = resp.status(); - if !status.is_success() { - let body = resp.text().await.unwrap_or_default(); - panic!("announce to {url} failed: status {status}, body: {body}"); - } - } - - // ── 4. Verify both announces are aggregated ────────────────────── - let global_stats = get_tracker_statistics(&api_url, "MyAccessToken").await; - assert_eq!(global_stats.tcp4_announces_handled, 2); - - // The tracker application and its temporary workspace are cleaned up - // when `workspace` and `_jobs` are dropped at the end of this scope. -} - -/// A disabled tracker must not contribute to global statistics. -/// -/// This regression is ignored until -/// `fix-duplicate-port-zero-tracker-instance-bootstrap` preserves an individual container for -/// every repeated `0.0.0.0:0` configuration entry. At present, the address-keyed bootstrap map -/// makes both listeners use the later enabled configuration, so both announces are counted. -#[ignore = "blocked by fix-duplicate-port-zero-tracker-instance-bootstrap"] -#[tokio::test] -async fn the_stats_api_endpoint_should_exclude_announces_from_a_tracker_with_statistics_disabled() { - let config_toml = r#" - [metadata] - app = "torrust-tracker" - purpose = "configuration" - schema_version = "2.0.0" - - [logging] - threshold = "off" - - [core] - listed = false - private = false - - [core.database] - driver = "sqlite3" - path = "{STORAGE_PATH}/sqlite3.db" - - [[http_trackers]] - bind_address = "0.0.0.0:0" - tracker_usage_statistics = false - - [[http_trackers]] - bind_address = "0.0.0.0:0" - tracker_usage_statistics = true - - [http_api] - bind_address = "127.0.0.1:0" - - [http_api.access_tokens] - admin = "MyAccessToken" - - [health_check_api] - bind_address = "127.0.0.2:0" - "#; - - let workspace = EphemeralTrackerWorkspace::new(config_toml); - let (app_container, _jobs) = common::start_tracker_with_config(&workspace).await; - - let tracker_urls = common::http_tracker_urls(&app_container).await; - assert_eq!(tracker_urls.len(), 2, "expected two HTTP trackers"); - - let api_url = common::http_api_url(&app_container).await.expect("expected an HTTP API URL"); - - let client = reqwest::Client::new(); - for url in &tracker_urls { - let announce_url = url - .join("/announce?info_hash=%9c8b%22%13%e3%0b%ff%21%2b0%c3%60%d2o%9a%02%13d%22&peer_id=-qB00000000000000001&port=17548&ip=127.0.0.1&event=started&compact=0") - .expect("announce URL should be valid"); - let response = client.get(announce_url.as_str()).send().await.unwrap(); - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - panic!("announce to {url} failed: status {status}, body: {body}"); - } - } - - let global_stats = get_tracker_statistics(&api_url, "MyAccessToken").await; - assert_eq!(global_stats.tcp4_announces_handled, 1); -} - -/// Global statistics with only metrics relevant to the test. -#[derive(Deserialize)] -struct PartialGlobalStatistics { - tcp4_announces_handled: u64, -} - -async fn get_tracker_statistics(api_url: &Url, token: &str) -> PartialGlobalStatistics { - let response = TrackerApiClient::new(ConnectionInfo::authenticated(Origin::new(api_url.as_str()).unwrap(), token)) - .unwrap() - .get_tracker_statistics(None) - .await - .expect("failed to get tracker statistics"); - - response - .json::() - .await - .expect("Failed to parse JSON response") -} diff --git a/tests/servers/api/mod.rs b/tests/servers/api/mod.rs deleted file mode 100644 index 2943dbb50..000000000 --- a/tests/servers/api/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod contract; diff --git a/tests/servers/mod.rs b/tests/servers/mod.rs deleted file mode 100644 index e5fdf85ee..000000000 --- a/tests/servers/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod api; diff --git a/tests/stats.rs b/tests/stats.rs deleted file mode 100644 index 4329f9929..000000000 --- a/tests/stats.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! Scaffolding for integration tests. -//! -//! Integration tests are used to test the interaction between multiple modules, -//! multiple running trackers, etc. Tests for one specific module should be in -//! the corresponding package. -//! -//! ```text -//! cargo test --test stats -//! ``` -mod common; -mod servers; - -use torrust_clock::clock; - -/// This code needs to be copied into each crate. -/// Working version, for production. -#[cfg(not(test))] -#[allow(dead_code)] -pub(crate) type CurrentClock = clock::Working; - -/// Stopped version, for testing. -#[cfg(test)] -#[allow(dead_code)] -pub(crate) type CurrentClock = clock::Stopped;