From 86314aad87efc11644a20e5e1a97cab3409a5e5a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sat, 22 Aug 2026 10:39:09 +0100 Subject: [PATCH 1/7] docs(issues): clarify flat service analysis roadmap --- .../1978-configuration-overhaul-epic/EPIC.md | 22 ++++---- .../ISSUE.md | 53 ++++++++++++------- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md b/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md index 233d12f2b..376eb1e67 100644 --- a/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md +++ b/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md @@ -4,7 +4,7 @@ status: open github-issue: 1978 spec-path: docs/issues/open/1978-configuration-overhaul-epic/EPIC.md epic-owner: josecelano -last-updated-utc: 2026-08-20 16:44 +last-updated-utc: 2026-08-22 00:00 semantic-links: skill-links: - create-issue @@ -99,7 +99,7 @@ Status values: `TODO`, `IN_PROGRESS`, `IN_REVIEW`, `BLOCKED`, `DONE`. | 10 | [#1987](../../issues/1987) — Use peer IP from the HTTP announce `ip` parameter when configured | `docs/issues/open/1987-add-config-option-to-use-ip-from-announce-query-string/ISSUE.md` | TODO | After #3 and external prerequisite #1985; per-HTTP-tracker opt-in policy | | 11 | [#1980](../../issues/1980) — Final cleanup: remove global re-exports, migrate consumers to explicit v3 imports | `docs/issues/open/1980-1978-configuration-overhaul-final-cleanup.md` | TODO | Must precede #12; depends on all other existing subissues | | 12 | [#2023](../../issues/2023) — Expose configured public URLs in runtime observability | `docs/issues/open/2023-1978-expose-configured-public-urls-in-runtime-observability.md` | TODO | Must follow #1417 and #1980; adds `public_url` to health checks, metrics, and logs without replacing ServiceBinding | -| 13 | [#2067](../../issues/2067) — Analyze a flat heterogeneous service configuration | `docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md` | TODO | Non-blocking analysis only; any recommended implementation follows #1980 and accounts for #1490 | +| 13 | [#2067](../../issues/2067) — Analyze a flat heterogeneous service configuration | `docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md` | TODO | Analysis only. If approved, a new implementation sub-issue follows #1490 and precedes #1980 so it ships in schema v3.0.0. | ## Delivery Strategy @@ -177,12 +177,13 @@ These can run in any order or in parallel branches: metrics, and logs. Preserve the distinction between configured bind address, post-bind `ServiceBinding`, and `public_url`; do not implement `internal_service_url`. -### Phase 4: Post-v3 Research +### Phase 4: Final v3 Design Decision -- **Subissue #13** (#2067) — Analyze a possible successor schema that represents heterogeneous - listener services in one ordered collection. This non-blocking research does not implement a - schema or runtime change and must not delay #1980. Any implementation recommendation must be - tracked separately and account for #1490. +- **Subissue #13** (#2067) — Analyze whether v3 should retain role-specific operator-facing TOML, + introduce a heterogeneous listener collection, or retain the TOML layout while normalizing to an + internal polymorphic service inventory. This analysis does not implement a schema or runtime + change. If approved, its separately created implementation sub-issue follows #1490 and precedes + #1980 so that the final consumer migration targets the selected v3 model. For each subissue implementation in this EPIC, the default completion policy is: @@ -262,10 +263,13 @@ For each subissue implementation in this EPIC, the default completion policy is: Runtime consumption of the configured value remains assigned to #1980. - 2026-08-20 16:36 UTC - Copilot/User - Restored #2023 as the twelfth native GitHub sub-issue, resolving the discrepancy with this specification. Created approved Task #2067 as the thirteenth - native sub-issue for non-blocking research into a possible post-v3 flat heterogeneous service - configuration; any implementation remains separate from this EPIC delivery. + native sub-issue for analysis into a possible flat heterogeneous service configuration. - 2026-08-20 16:44 UTC - Copilot - Renamed #2067's folder-based subissue specification to include the parent EPIC number, following the open-issues naming convention. +- 2026-08-22 UTC - Copilot/User - Corrected #2067's roadmap placement: if its analysis recommends + implementation, a new sub-issue must complete after #1490 and before #1980 so the chosen model + is part of schema v3.0.0. The analysis also compares operator-facing TOML design independently + from a possible internal polymorphic service inventory. ## Acceptance Criteria diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md index 651c88ab8..c0332d8be 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md @@ -9,7 +9,7 @@ branch: "2067-analyze-flat-service-configuration" related-pr: 2068 depends-on: null blocks: null -last-updated-utc: 2026-08-20 16:51 +last-updated-utc: 2026-08-22 00:00 semantic-links: skill-links: - create-issue @@ -62,13 +62,13 @@ During weekly planning, Cameron proposed representing the listener services as a The current v3 configuration module still uses the existing split structure, while the application remains on the v2 public aliases pending #1980. This analysis must distinguish an immediately feasible schema representation from the proper delivery point in the configuration-overhaul roadmap. -This is a non-blocking research sub-issue of #1978. It may inform a later schema version, but it must not delay the v3.0.0 delivery or expand #1978's implementation scope. Any implementation recommended by this analysis must be tracked in a new issue and scheduled after #1980; the analysis must also account for the #1490 secrets work that #1980 depends on. +This is an analysis-only sub-issue of #1978. If the final recommendation is to implement a configuration-schema change, maintainers must first approve and create a new #1978 sub-issue. That implementation must complete after #1490 and before #1980 so that it is included in the v3.0.0 configuration release. The analysis itself must not implement the schema, migration tool, or runtime changes. ## Illustrative Configuration Outcome The following comparison deliberately starts from the v3 configuration schema, not the current v2 runtime configuration shown in `tests/common/configuration.rs`. The v2-to-v3 changes are independently planned under the Configuration Overhaul EPIC and #1980. This issue would be a later, separate breaking schema change built on top of v3: it changes only how v3's already-defined service configurations are organized at the root level. -Consequently, the two examples use the same service-specific fields, nested structures, and shared `udp_tracker_server` policy. Their only intentional difference is the root-level representation: v3 uses role-specific sections; the illustrative successor uses a heterogeneous `services` list. The successor is a design example only, not a selected representation or a commitment to use the exact field names below. This analysis must validate its TOML and Serde feasibility and may recommend rejecting or changing the proposed form. +Consequently, the two examples use the same service-specific fields, nested structures, and shared `udp_tracker_server` policy. Their only intentional difference is the root-level representation: v3 uses role-specific sections; the illustrative alternative uses a heterogeneous `services` list. The alternative is a design example only, not a selected representation or a commitment to use the exact field names below. This analysis must validate its TOML and Serde feasibility and may recommend rejecting or changing the proposed form. ### Before: v3 Role-Specific Service Sections @@ -152,7 +152,7 @@ ip_bans_reset_interval_in_secs = 86400 connection_id_validation = "strict" ``` -### After: Illustrative Flat Heterogeneous Service Collection +### Alternative: Illustrative Flat Heterogeneous Service Collection The example uses an **adjacently tagged** representation: every list item has a `kind` discriminator and a nested `configuration` table. It models a Rust `Vec`, where `Service` is an enum with one variant per service type, and each variant wraps the corresponding v3 role-specific configuration type. This avoids requiring all service variants to share the same fields. @@ -160,7 +160,7 @@ The example uses an **adjacently tagged** representation: every list item has a [metadata] app = "torrust-tracker" purpose = "configuration" -schema_version = "4.0.0" +schema_version = "3.0.0" [logging] trace_filter = "info" @@ -254,9 +254,21 @@ ip_bans_reset_interval_in_secs = 86400 connection_id_validation = "strict" ``` -TOML attaches each `[services.configuration]` table and its nested tables to the immediately preceding `[[services]]` entry. `udp_tracker_server` remains top-level because it configures policy shared by all UDP listeners rather than one listener instance. The illustrative schema therefore requires a new schema version beyond the current v3 model; `4.0.0` is only a placeholder, not a release decision. +TOML attaches each `[services.configuration]` table and its nested tables to the immediately preceding `[[services]]` entry. `udp_tracker_server` remains top-level because it configures policy shared by all UDP listeners rather than one listener instance. The illustrative schema would replace the existing v3 root-level role-specific layout before the v3.0.0 release; it does not imply a second successor schema version. + +In this illustration, declaration order represents the configuration's service inventory only. It must not acquire startup-order semantics: startup remains dependency-driven and role-grouped. A recommended design must define validation for singleton service kinds and clarify whether `ConfigurationInstanceId` continues to use role-local ordinals while scanning this list or adopts global list positions. + +## Maintainer Direction + +The final decision must remain evidence-led: decide whether the change should be implemented, deferred, or rejected. The following approved direction constrains the analysis but does not predetermine its recommendation: -In this illustration, declaration order represents the configuration's service inventory; the analysis must determine whether it would also carry startup-order semantics. A recommended design must define validation for singleton service kinds and clarify whether `ConfigurationInstanceId` continues to use role-local ordinals while scanning this list or adopts global list positions. +- The operator-facing TOML experience is the primary configuration-design concern. Names, explicit structure, readability, and the ability to build a correct configuration without explanatory comments are more important than mirroring internal runtime types. +- The configuration representation and the internal runtime representation may differ. The analysis must compare retaining role-specific TOML while normalizing it into a polymorphic internal service inventory against exposing a flat polymorphic `services` list in TOML. +- The internal inventory must be evaluated as a possible way to manage running services, handles, jobs, threads, registration, and metrics. It must remain distinct from the broader job collection, which also contains non-listener tasks such as cleanup jobs. +- If a flat `services` TOML collection is selected, declaration order is presentation/configuration order only; startup remains dependency-driven and role-grouped. +- `http_api` and `health_check_api` are singleton kinds: each may occur at most once. `http_api` remains optional. A missing `health_check_api` entry preserves the existing implicit/default health-check behavior. `http_tracker` and `udp_tracker` remain multi-instance kinds. +- If a v2-to-v3 migration needs to materialize a flat collection, use the canonical order HTTP trackers, UDP trackers, HTTP API, then health-check API. +- If implementation is recommended and approved, create a separate #1978 sub-issue after #1490 and before #1980, so the selected configuration model is included in the v3.0.0 release. ## Analysis Deliverables @@ -278,7 +290,7 @@ This open issue is stored at `docs/issues/open/2067-1978-analyze-flat-service-co 4. **Feasibility Results**: TOML parsing, Serde serialization round-trip, Figment defaulting and environment overrides, unknown/discriminator errors, and constraints discovered by prototypes. 5. **Runtime and Normalization Model**: recommended single owner for normalization, role-specific views, service startup dependencies, singleton/default behavior, and preservation of existing health/metrics/registration contracts. 6. **Identity, Ordering, and Migration**: `ServiceKind` to `ServiceRole` mapping, `ConfigurationInstanceId` behavior, loss of cross-role ordering during v3-to-v4 migration, and a canonical migration-order rule if implementation is recommended. -7. **Schema Lifecycle, Security, and Compatibility**: v3/v4 loading and transition policy, #1980/#1490 relationship, secret redaction, external configuration consumers, and observability compatibility. +7. **Schema Lifecycle, Security, and Compatibility**: v3 loading and transition policy, the #1490 → implementation → #1980 relationship, secret redaction, external configuration consumers, and observability compatibility. 8. **Cost, Risks, and Recommendation**: affected modules, high-level effort, unresolved risks, decision rationale, and exact scope for any follow-up implementation issue. ### Required `evidence.md` Record Format @@ -309,6 +321,7 @@ For an experiment, preserve the exact TOML input and command in the record. Test - an internally tagged/flattened representation, including whether it requires duplicated fields or custom deserialization; - an externally tagged or equivalent representation where relevant. - Evaluate configuration usability, readability, validation, environment-variable overrides, default configuration generation, and serialization/round-trip behavior for each viable representation. +- Compare the operator-facing role-specific TOML model plus a normalized internal polymorphic service inventory with a TOML-level heterogeneous `services` collection. Treat configuration UX and internal runtime organization as separate design decisions. - Identify the required semantic rules that are currently structural, including singleton handling for the REST API and health-check API and the current always-started/defaulted health-check behavior. Define expected behavior for an omitted `services` list, an empty list, no health-check entry, duplicate singleton entries, and UDP entries in private mode. - Analyze whether `udp_tracker_server` remains a top-level shared support-service configuration or belongs in a flat listener list. - Inventory configuration values that look per-listener but are consumed through shared runtime services, including `max_connection_id_errors_per_ip`. Recommend whether each must become shared, be validated as consistent, or be redesigned in a separate implementation issue; do not make that runtime change here. @@ -318,7 +331,7 @@ For an experiment, preserve the exact TOML input and command in the record. Test - describe the consequences of instead using the global list position. - Define a typed `ServiceKind` to `ServiceRole` mapping, including the distinction between the configuration-facing `http_api` kind and the existing `RestApi` runtime role. - Treat `ConfigurationInstanceId` as an existing constraint. Do **not** explore alternative identifier schemes such as explicit user-provided IDs, socket addresses after binding, or configuration hashes. -- Identify migration, documentation, test, and consumer impacts, including the dependency/order relationship with #1980 and schema-versioning implications. Decide whether a future application accepts only the successor schema, dispatches among schema versions, or requires an external migration; state that v3 cannot express a cross-role service order and define any canonical migration order. +- Identify migration, documentation, test, and consumer impacts, including the #1490 → implementation → #1980 dependency/order relationship and v3 release implications. Decide whether the selected v3 model accepts only one final shape or requires an external migration; state that the current split layout cannot express a cross-role service order and define any canonical migration order. - Analyze the effect of moving `HttpApi` inside a service enum on configuration logging, JSON serialization, and redaction of API tokens, including compatibility with #1490's planned secret types. - Preserve existing post-bind `ServiceBinding`, health-check registration, and metrics behavior as compatibility invariants, even though changing those public contracts is out of scope. - Provide a high-level implementation estimate, dependency plan, risks, and a recommended next step: reject, defer, or create a separate implementation issue. @@ -346,7 +359,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. | T3 | TODO | Compare configuration representations | Record readability, ergonomics, validation, environment override, round-trip serialization, and backwards-migration trade-offs for each option in `analysis.md`. | | T4 | TODO | Analyze runtime integration | Define a conceptual single normalization owner, role-specific views, startup dependencies, shared UDP policies, singleton/default behavior, and compatibility invariants without changing production. | | T5 | TODO | Analyze identity compatibility | Compare role-local ordinals with global positions; define `ServiceKind` to `ServiceRole` mapping and show how one normalizer keeps IDs, containers, jobs, and registry metadata aligned. | -| T6 | TODO | Define migration and schema lifecycle | Decide v3-to-successor ordering rules, schema loading/transition strategy, v3 compatibility policy, #1980/#1490 prerequisites, and the non-blocking relationship to the v3 EPIC. | +| T6 | TODO | Define migration and schema lifecycle | Decide current-layout-to-final-v3 ordering rules, loading/transition strategy, #1490 → implementation → #1980 prerequisites, and the effect on the v3.0.0 delivery. | | T7 | TODO | Analyze security and operator impact | Document redaction, configuration logging/serialization, external configuration consumers, deployment overrides, and post-bind observability compatibility. | | T8 | TODO | Write the final analysis deliverables | Complete `analysis.md` and `evidence.md`; ensure every recommendation is traceable to evidence and no production implementation is included. | | T9 | TODO | Run automatic checks | Run `linter all` and relevant focused tests for any analysis fixtures or documentation tooling changes. | @@ -369,7 +382,7 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [ ] Manual verification scenarios executed and recorded (status + evidence) - [ ] Acceptance criteria reviewed after analysis and updated with evidence - [ ] Reviewer validated acceptance criteria and updated checkboxes -- [ ] Committer verified spec progress is up to date before commit +- [x] Committer verified spec progress is up to date before commit - [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` ### Progress Log @@ -379,6 +392,8 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - 2026-08-20 16:36 UTC - Copilot/User - User approved the draft. Created GitHub Task #2067 and linked it as the thirteenth native sub-issue of #1978 after restoring #2023's missing native parent relationship. - 2026-08-20 16:44 UTC - Copilot - Renamed the folder to include the parent EPIC number, as required for folder-based subissue specifications. - 2026-08-20 16:51 UTC - Copilot/User - Opened spec-only PR #2068 against `develop`, linked it as related to #2067, and requested review from @da2ce7 because the proposal originated with Cameron. +- 2026-08-22 UTC - Copilot/User - Clarified that the analysis must decide whether to implement the change in schema v3.0.0, not a later successor version. If approved after the analysis, a new implementation sub-issue must follow #1490 and precede #1980. Added operator-focused configuration UX and the independent internal-normalization alternative as explicit evaluation criteria. Confirmed role-grouped, dependency-driven startup; singleton HTTP API and health-check kinds; implicit default health-check behavior; and the canonical migration order. +- 2026-08-22 UTC - Copilot - Reviewed the updated issue and EPIC roadmap specifications before committing. `git diff --check` passed; the repository `linter` executable was unavailable in this environment. ## Acceptance Criteria @@ -412,13 +427,13 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. -| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | -| --- | -------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | --------------------------------------------------------- | -| M1 | Review current port-zero fixture | Compare `tests/common/configuration.rs` with configuration structs, bootstrap, containers, shared UDP services, registry, and redaction paths. | Evidence explains role-local IDs, post-bind identities, shared policy behavior, and current compatibility constraints. | TODO | `evidence.md#e1-current-state-baseline` | -| M2 | Review candidate TOML files | Parse and serialize interleaved entries for each viable form. Exercise unknown kinds, numeric environment overrides, omitted/empty lists, missing health entries, and duplicate singletons. | Each result records syntax, readability, round-trip behavior, defaulting, error quality, and compatibility with nested TLS/network/access-token settings. | TODO | `evidence.md#e2-configuration-representation-feasibility` | -| M3 | Review normalization plan | Trace a representative interleaved list through conceptual normalization, role-local ID allocation, container lookup, startup phases, registration, and metrics without changing production code. | The analysis identifies one consistent normalization boundary and proves whether source list order affects startup or presentation only. | TODO | `evidence.md#e3-runtime-and-identity-model` | -| M4 | Review migration and transition | Compare the recommended form with v3/default configs, environment overrides, docs, integration fixtures, #1980, and #1490. Define a canonical migration order and version-loading policy. | The impact inventory, compatibility policy, prerequisites, and implementation estimate are complete; unresolved constraints are explicit. | TODO | `evidence.md#e4-migration-schema-lifecycle-and-security` | -| M5 | Review final reports | Check every conclusion in `analysis.md` against the linked record in `evidence.md`; confirm the recommendation does not include implementation work. | The decision record is complete, traceable, and limited to analysis plus a proposed follow-up scope when warranted. | TODO | `evidence.md#e5-report-review` | +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | +| --- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | --------------------------------------------------------- | +| M1 | Review current port-zero fixture | Compare `tests/common/configuration.rs` with configuration structs, bootstrap, containers, shared UDP services, registry, and redaction paths. | Evidence explains role-local IDs, post-bind identities, shared policy behavior, and current compatibility constraints. | TODO | `evidence.md#e1-current-state-baseline` | +| M2 | Review candidate TOML files | Parse and serialize interleaved entries for each viable form. Exercise unknown kinds, numeric environment overrides, omitted/empty lists, missing health entries, and duplicate singletons. | Each result records syntax, readability, round-trip behavior, defaulting, error quality, and compatibility with nested TLS/network/access-token settings. | TODO | `evidence.md#e2-configuration-representation-feasibility` | +| M3 | Review normalization plan | Trace a representative interleaved list through conceptual normalization, role-local ID allocation, container lookup, startup phases, registration, and metrics without changing production code. | The analysis identifies one consistent normalization boundary and proves whether source list order affects startup or presentation only. | TODO | `evidence.md#e3-runtime-and-identity-model` | +| M4 | Review migration and transition | Compare the recommended final-v3 form with the current split layout/default configs, environment overrides, docs, integration fixtures, #1490, and #1980. Define a canonical migration order and loading policy. | The impact inventory, compatibility policy, prerequisites, and implementation estimate are complete; unresolved constraints are explicit. | TODO | `evidence.md#e4-migration-schema-lifecycle-and-security` | +| M5 | Review final reports | Check every conclusion in `analysis.md` against the linked record in `evidence.md`; confirm the recommendation does not include implementation work. | The decision record is complete, traceable, and limited to analysis plus a proposed follow-up scope when warranted. | TODO | `evidence.md#e5-report-review` | ### Acceptance Verification @@ -454,7 +469,7 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. - **Hidden shared UDP policy:** A field placed on a UDP listener can still configure one shared runtime service. The analysis must expose and resolve that semantic mismatch before a flat list makes ordering effects less visible. - **Schema lifecycle ambiguity:** A v4 representation requires an explicit transition, compatibility, or migration strategy because a versioned configuration loader accepts one schema shape at a time. - **Secret exposure:** Nesting API configuration in an enum can bypass current redaction paths unless serialization/logging behavior is explicitly tested and coordinated with #1490. -- **Roadmap conflict:** Implementing the change before #1980 would create parallel v3 schema work while the application still consumes v2 aliases. This analysis is non-blocking; any implementation must be separately scheduled after #1980 and its prerequisites. +- **Roadmap integration:** If approved, the configuration change must be a separately scoped sub-issue after #1490 and before #1980. It must resolve its schema shape before #1980 performs the final v3 consumer migration, avoiding a second migration of runtime consumers. ## References From ee5aa855dbd044bc4922aeb20211e76fe4604173 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 09:03:07 +0100 Subject: [PATCH 2/7] docs(configuration): analyze flat service configuration --- .../ISSUE.md | 122 ++++--- .../analysis.md | 158 ++++++-- .../evidence.md | 118 ++++-- packages/configuration/src/v3_0_0/mod.rs | 341 ++++++++++++++++++ 4 files changed, 636 insertions(+), 103 deletions(-) diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md index c0332d8be..5d45af244 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md @@ -9,7 +9,7 @@ branch: "2067-analyze-flat-service-configuration" related-pr: 2068 depends-on: null blocks: null -last-updated-utc: 2026-08-22 00:00 +last-updated-utc: 2026-08-22 02:00 semantic-links: skill-links: - create-issue @@ -263,6 +263,8 @@ In this illustration, declaration order represents the configuration's service i The final decision must remain evidence-led: decide whether the change should be implemented, deferred, or rejected. The following approved direction constrains the analysis but does not predetermine its recommendation: - The operator-facing TOML experience is the primary configuration-design concern. Names, explicit structure, readability, and the ability to build a correct configuration without explanatory comments are more important than mirroring internal runtime types. +- Treat the current role-specific TOML layout as the operator baseline. It keeps each service type's fields close together, avoids a per-entry discriminator, and makes a known service type easy to locate. The analysis must independently test this view against the flat-list alternative rather than assuming it is correct. +- Prioritize the common deployment: one public listener of one tracker protocol, normally either a single HTTP tracker or a single UDP tracker. Also evaluate the less common one-listener-per-kind deployment. Do not optimize the primary configuration experience for uncommon multi-instance, mixed-protocol inventories without demonstrated operator value. - The configuration representation and the internal runtime representation may differ. The analysis must compare retaining role-specific TOML while normalizing it into a polymorphic internal service inventory against exposing a flat polymorphic `services` list in TOML. - The internal inventory must be evaluated as a possible way to manage running services, handles, jobs, threads, registration, and metrics. It must remain distinct from the broader job collection, which also contains non-listener tasks such as cleanup jobs. - If a flat `services` TOML collection is selected, declaration order is presentation/configuration order only; startup remains dependency-driven and role-grouped. @@ -352,19 +354,19 @@ For an experiment, preserve the exact TOML input and command in the record. Test Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Capture the current model | Record v3 shape, cardinality/defaulting, startup phases, identity, role mappings, shared UDP state, service registration, observability contracts, and secret-redaction paths in `evidence.md`. | -| T2 | TODO | Prototype schema representations | Add isolated, non-production TOML/Serde/Figment experiments for viable enum forms, including round-trip behavior, numeric environment overrides, unknown kinds, and default/empty-list cases. | -| T3 | TODO | Compare configuration representations | Record readability, ergonomics, validation, environment override, round-trip serialization, and backwards-migration trade-offs for each option in `analysis.md`. | -| T4 | TODO | Analyze runtime integration | Define a conceptual single normalization owner, role-specific views, startup dependencies, shared UDP policies, singleton/default behavior, and compatibility invariants without changing production. | -| T5 | TODO | Analyze identity compatibility | Compare role-local ordinals with global positions; define `ServiceKind` to `ServiceRole` mapping and show how one normalizer keeps IDs, containers, jobs, and registry metadata aligned. | -| T6 | TODO | Define migration and schema lifecycle | Decide current-layout-to-final-v3 ordering rules, loading/transition strategy, #1490 → implementation → #1980 prerequisites, and the effect on the v3.0.0 delivery. | -| T7 | TODO | Analyze security and operator impact | Document redaction, configuration logging/serialization, external configuration consumers, deployment overrides, and post-bind observability compatibility. | -| T8 | TODO | Write the final analysis deliverables | Complete `analysis.md` and `evidence.md`; ensure every recommendation is traceable to evidence and no production implementation is included. | -| T9 | TODO | Run automatic checks | Run `linter all` and relevant focused tests for any analysis fixtures or documentation tooling changes. | -| T10 | TODO | Perform manual review | Review candidate TOML, normalizer pseudocode, migration rules, the report/evidence cross-links, and the impact inventory; record evidence. | -| T11 | TODO | Re-review acceptance criteria | Update evidence after the analysis and recommendation are complete. | +| ID | Status | Task | Notes / Expected Output | +| --- | ----------- | ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | DONE | Capture the current model | Recorded the v3/v2 boundary, cardinality/defaulting, startup, identity, shared UDP state, registration, observability, and redaction evidence in `evidence.md#e1-current-state-baseline`. | +| T2 | DONE | Prototype schema representations | Added isolated test-only TOML/Serde/Figment experiments. Numeric list overrides fail with the current Figment provider; see `evidence.md#e2-configuration-representation-feasibility`. | +| T3 | DONE | Compare configuration representations | Compared split TOML, adjacent, flattened, and externally tagged forms in `analysis.md#candidate-representations`. | +| T4 | DONE | Analyze runtime integration | Defined the conditional single-normalizer model and preserved dependency-grouped startup in `analysis.md#runtime-and-normalization-model`. | +| T5 | DONE | Analyze identity compatibility | Documented role-local ordinal preservation, global-position consequences, and `ServiceKind` mapping in `analysis.md#identity-ordering-and-migration`. | +| T6 | DONE | Define migration and schema lifecycle | Rejected the schema transition; documented canonical export ordering and #1490/#1980 constraints in `analysis.md#schema-lifecycle-security-and-compatibility`. | +| T7 | DONE | Analyze security and operator impact | Documented redaction, logging, override, and post-bind compatibility constraints in `analysis.md`. | +| T8 | DONE | Write the final analysis deliverables | Completed `analysis.md` and `evidence.md` with an analysis-only rejection recommendation. | +| T9 | DONE | Run automatic checks | `cargo test -p torrust-tracker-configuration` and the mandatory pre-commit gate passed using the installed stable toolchain. | +| T10 | DONE | Perform manual review | Reviewed candidate presentation, report/evidence links, migration rule, and impact inventory; see M5 and `evidence.md#e5-final-report-review`. | +| T11 | DONE | Re-review acceptance criteria | Acceptance criteria reviewed against E1–E5 and the completed validation results. | ## Progress Tracking @@ -375,13 +377,13 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - [x] GitHub issue created and issue number added to this spec (#2067) - [x] Linked as a sub-issue of #1978 in GitHub and in the EPIC specification - [ ] (Optional, recommended for complex issues) Spec-only PR merged into `develop` before analysis work -- [ ] Analysis completed; no production schema change included -- [ ] `analysis.md` completed with an explicit recommendation -- [ ] `evidence.md` completed with reproducible evidence for each material conclusion -- [ ] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks) -- [ ] Manual verification scenarios executed and recorded (status + evidence) -- [ ] Acceptance criteria reviewed after analysis and updated with evidence -- [ ] Reviewer validated acceptance criteria and updated checkboxes +- [x] Analysis completed; no production schema change included +- [x] `analysis.md` completed with an explicit recommendation +- [x] `evidence.md` completed with reproducible evidence for each material conclusion +- [x] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks) +- [x] Manual verification scenarios executed and recorded (status + evidence) +- [x] Acceptance criteria reviewed after analysis and updated with evidence +- [x] Reviewer validated acceptance criteria and updated checkboxes - [x] Committer verified spec progress is up to date before commit - [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` @@ -394,25 +396,29 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. - 2026-08-20 16:51 UTC - Copilot/User - Opened spec-only PR #2068 against `develop`, linked it as related to #2067, and requested review from @da2ce7 because the proposal originated with Cameron. - 2026-08-22 UTC - Copilot/User - Clarified that the analysis must decide whether to implement the change in schema v3.0.0, not a later successor version. If approved after the analysis, a new implementation sub-issue must follow #1490 and precede #1980. Added operator-focused configuration UX and the independent internal-normalization alternative as explicit evaluation criteria. Confirmed role-grouped, dependency-driven startup; singleton HTTP API and health-check kinds; implicit default health-check behavior; and the canonical migration order. - 2026-08-22 UTC - Copilot - Reviewed the updated issue and EPIC roadmap specifications before committing. `git diff --check` passed; the repository `linter` executable was unavailable in this environment. +- 2026-08-22 UTC - Copilot/User - Recorded the operator baseline and deployment priorities: role-specific sections are provisionally clearer because related fields remain together, no discriminator must be read, and roles are easy to locate. The analysis must assess this against a flat list while prioritizing the common single-HTTP-or-single-UDP deployment rather than uncommon multi-instance inventories. +- 2026-08-22 UTC - Copilot - Completed source tracing and isolated TOML/Serde/Figment prototypes. The adjacent, flattened, and externally tagged forms round-trip, but numeric Figment overrides for list entries fail. Drafted the evidence-backed analysis recommending rejection of a flat TOML schema and deferral of any internal normalizer until it has a concrete consumer. +- 2026-08-22 UTC - Copilot - Completed the final manual report review and acceptance-criteria re-review. The configuration package tests and final mandatory pre-commit gate passed all checks, including `linter all` and workspace documentation tests. +- 2026-08-22 UTC - Task Reviewer - Independently reviewed the final analysis. Confirmed the flat-versus-split Figment comparison, evidence traceability, analysis-only scope, and synchronized acceptance verification. Approved the analysis as commit-ready. ## Acceptance Criteria -- [ ] AC1: The analysis describes the current v2/v3 service configuration, cardinality rules, startup sequence, shared runtime state, secret-redaction boundary, and the role of `ConfigurationInstanceId`, using concrete source references. -- [ ] AC2: At least two viable TOML/Rust representations for a heterogeneous ordered service list are compared, with an explicit recommendation or rejection rationale. -- [ ] AC3: Feasibility is demonstrated or disproved with focused deserialization, serialization, defaulting, and environment-override evidence using the repository's supported configuration stack; no production schema change is made. -- [ ] AC4: The analysis states whether list order controls startup order, configuration presentation order, both, or neither; explains the loss of cross-role order when migrating v3; and identifies the necessary runtime constraints. -- [ ] AC5: The analysis explicitly evaluates omitted/empty lists, duplicate/absence validation for REST API and health-check API entries, private-mode UDP behavior, and the placement of shared `udp_tracker_server` configuration. -- [ ] AC6: The analysis documents the consequences of preserving role-local `ConfigurationInstanceId` ordinals versus using global list positions; defines the `ServiceKind` to `ServiceRole` mapping; and recommends a single normalization boundary consistent with the existing identifier contract. -- [ ] AC7: The analysis inventories shared UDP behavior, including `max_connection_id_errors_per_ip`, and recommends a future policy without changing current runtime behavior. -- [ ] AC8: The analysis identifies schema migration/versioning and transition requirements, #1980/#1490 dependency implications, affected configuration consumers, documentation, defaults, test fixtures, and a high-level implementation estimate. -- [ ] AC9: The analysis documents secret-redaction, configuration logging/serialization, and post-bind health/metrics/registration compatibility constraints. -- [ ] AC10: `analysis.md` contains every required section, makes one explicit recommendation, and identifies a precise follow-up implementation issue or rejection/defer rationale. -- [ ] AC11: `evidence.md` contains reproducible evidence records for every material conclusion in `analysis.md`. -- [ ] AC12: `linter all` exits with code `0` for all changes made by this analysis task. -- [ ] AC13: Relevant focused tests pass for any experiment or analysis fixture added by this task. -- [ ] AC14: Manual verification scenarios are executed and documented with status and evidence. -- [ ] AC15: Acceptance criteria are re-reviewed after analysis and reflect actual evidence. -- [ ] AC16: Documentation is updated when the analysis changes the configuration roadmap or governance artifacts. +- [x] AC1: Current-state analysis is traceable to E1. +- [x] AC2: Candidate representations and rejection rationale are documented in `analysis.md` and E2. +- [x] AC3: Test-only feasibility experiments and results are recorded in E2. +- [x] AC4: Order semantics and lifecycle constraints are documented in E3. +- [x] AC5: List, singleton, private-mode, and UDP policy behavior is documented in E1–E2. +- [x] AC6: Identity compatibility, mapping, and normalization boundary are documented in E3. +- [x] AC7: Shared UDP behavior and future policy are documented in E1. +- [x] AC8: Lifecycle, dependencies, consumers, and estimate are documented in E4. +- [x] AC9: Redaction and observability constraints are documented in E1 and E4. +- [x] AC10: `analysis.md` gives the explicit rejection rationale. +- [x] AC11: `evidence.md` contains E1–E5. +- [x] AC12: The 2026-08-22 pre-commit gate passed `linter all`. +- [x] AC13: `cargo test -p torrust-tracker-configuration` passed 96 tests. +- [x] AC14: M1–M5 are recorded as complete below. +- [x] AC15: Acceptance criteria were re-reviewed on 2026-08-22. +- [x] AC16: Issue decision artifacts were updated. ## Verification Plan @@ -429,32 +435,32 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. | ID | Scenario | Command/Steps | Expected Result | Status | Evidence | | --- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | --------------------------------------------------------- | -| M1 | Review current port-zero fixture | Compare `tests/common/configuration.rs` with configuration structs, bootstrap, containers, shared UDP services, registry, and redaction paths. | Evidence explains role-local IDs, post-bind identities, shared policy behavior, and current compatibility constraints. | TODO | `evidence.md#e1-current-state-baseline` | -| M2 | Review candidate TOML files | Parse and serialize interleaved entries for each viable form. Exercise unknown kinds, numeric environment overrides, omitted/empty lists, missing health entries, and duplicate singletons. | Each result records syntax, readability, round-trip behavior, defaulting, error quality, and compatibility with nested TLS/network/access-token settings. | TODO | `evidence.md#e2-configuration-representation-feasibility` | -| M3 | Review normalization plan | Trace a representative interleaved list through conceptual normalization, role-local ID allocation, container lookup, startup phases, registration, and metrics without changing production code. | The analysis identifies one consistent normalization boundary and proves whether source list order affects startup or presentation only. | TODO | `evidence.md#e3-runtime-and-identity-model` | -| M4 | Review migration and transition | Compare the recommended final-v3 form with the current split layout/default configs, environment overrides, docs, integration fixtures, #1490, and #1980. Define a canonical migration order and loading policy. | The impact inventory, compatibility policy, prerequisites, and implementation estimate are complete; unresolved constraints are explicit. | TODO | `evidence.md#e4-migration-schema-lifecycle-and-security` | -| M5 | Review final reports | Check every conclusion in `analysis.md` against the linked record in `evidence.md`; confirm the recommendation does not include implementation work. | The decision record is complete, traceable, and limited to analysis plus a proposed follow-up scope when warranted. | TODO | `evidence.md#e5-report-review` | +| M1 | Review current port-zero fixture | Compare `tests/common/configuration.rs` with configuration structs, bootstrap, containers, shared UDP services, registry, and redaction paths. | Evidence explains role-local IDs, post-bind identities, shared policy behavior, and current compatibility constraints. | DONE | `evidence.md#e1-current-state-baseline` | +| M2 | Review candidate TOML files | Parse and serialize interleaved entries for each viable form. Exercise unknown kinds, numeric environment overrides, omitted/empty lists, missing health entries, and duplicate singletons. | Each result records syntax, readability, round-trip behavior, defaulting, error quality, and compatibility with nested TLS/network/access-token settings. | DONE | `evidence.md#e2-configuration-representation-feasibility` | +| M3 | Review normalization plan | Trace a representative interleaved list through conceptual normalization, role-local ID allocation, container lookup, startup phases, registration, and metrics without changing production code. | The analysis identifies one consistent normalization boundary and proves whether source list order affects startup or presentation only. | DONE | `evidence.md#e3-runtime-and-identity-model` | +| M4 | Review migration and transition | Compare the recommended final-v3 form with the current split layout/default configs, environment overrides, docs, integration fixtures, #1490, and #1980. Define a canonical migration order and loading policy. | The impact inventory, compatibility policy, prerequisites, and implementation estimate are complete; unresolved constraints are explicit. | DONE | `evidence.md#e4-migration-schema-lifecycle-and-security` | +| M5 | Review final reports | Check every conclusion in `analysis.md` against the linked record in `evidence.md`; confirm the recommendation does not include implementation work. | The decision record is complete, traceable, and limited to analysis plus a proposed follow-up scope when warranted. | DONE | `evidence.md#e5-report-review` | ### Acceptance Verification | AC ID | Status (`TODO`/`DONE`) | Evidence | | ----- | ---------------------- | -------- | -| AC1 | TODO | | -| AC2 | TODO | | -| AC3 | TODO | | -| AC4 | TODO | | -| AC5 | TODO | | -| AC6 | TODO | | -| AC7 | TODO | | -| AC8 | TODO | | -| AC9 | TODO | | -| AC10 | TODO | | -| AC11 | TODO | | -| AC12 | TODO | | -| AC13 | TODO | | -| AC14 | TODO | | -| AC15 | TODO | | -| AC16 | TODO | | +| AC1 | DONE | `evidence.md#e1-current-state-baseline` | +| AC2 | DONE | `analysis.md#candidate-representations`, `evidence.md#e2-configuration-representation-feasibility` | +| AC3 | DONE | `evidence.md#e2-configuration-representation-feasibility` | +| AC4 | DONE | `analysis.md#runtime-and-normalization-model`, `evidence.md#e3-runtime-and-identity-model` | +| AC5 | DONE | `analysis.md#feasibility-results`, E1–E2 | +| AC6 | DONE | `analysis.md#identity-ordering-and-migration`, `evidence.md#e3-runtime-and-identity-model` | +| AC7 | DONE | `analysis.md#current-state-baseline`, `evidence.md#e1-current-state-baseline` | +| AC8 | DONE | `analysis.md#schema-lifecycle-security-and-compatibility`, `evidence.md#e4-migration-schema-lifecycle-and-security` | +| AC9 | DONE | E1 and E4 | +| AC10 | DONE | `analysis.md`, `evidence.md#e5-final-report-review` | +| AC11 | DONE | `evidence.md#e1-current-state-baseline` through `evidence.md#e5-final-report-review` | +| AC12 | DONE | 2026-08-22 pre-commit gate (`linter all`) | +| AC13 | DONE | Focused prototype tests (9 passed) and final pre-commit gate | +| AC14 | DONE | M1–M5 and E1–E5 | +| AC15 | DONE | 2026-08-22 acceptance review | +| AC16 | DONE | `ISSUE.md`, `analysis.md`, and `evidence.md` | ## Risks and Trade-offs diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md index af1e59534..be2be19c5 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md @@ -1,6 +1,6 @@ # Analysis Report: Flat Heterogeneous Service Configuration -> **Status:** Planned +> **Status:** Complete — recommendation: reject the flat TOML schema change > > **Issue contract:** [ISSUE.md](ISSUE.md) > @@ -12,50 +12,160 @@ not describe unapproved production work as implemented. ## Executive Decision -| Field | Result | -| ---------------------- | ------ | -| Recommendation | TODO | -| Decision status | TODO | -| Rationale | TODO | -| Required prerequisites | TODO | -| Proposed follow-up | TODO | +| Field | Result | +| ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Recommendation | **Reject** a flat heterogeneous `[[services]]` TOML collection for schema v3.0.0. | +| Decision status | Ready for maintainer review. | +| Rationale | The split layout is clearer for the common one-HTTP-or-one-UDP deployment, preserves structural cardinality, and avoids a breaking migration. The flat form supplies no demonstrated operator benefit that offsets those costs. | +| Required prerequisites | None for this rejection. Complete #1490 and #1980 under their existing plans. | +| Proposed follow-up | Do not create the proposed configuration-schema implementation issue. Defer any internal normalized listener inventory until a concrete lifecycle consumer cannot use the existing registry and role-specific container views. | + +The rejection is limited to changing the **operator-facing TOML shape**. It does not prohibit a +future internal service inventory when it is justified independently of the configuration schema. +The existing `Registar` already provides an inventory of successfully +started listeners, while the job manager intentionally also contains non-listener work. See +[E1](evidence.md#e1-current-state-baseline) and [E3](evidence.md#e3-runtime-and-identity-model). ## Current-State Baseline -TODO: Describe the v3 configuration shape, cardinality/defaulting, startup phases, container and -registry behavior, `ConfigurationInstanceId`, shared UDP behavior, and configuration redaction. +Schema v3 currently has separate root fields: optional `Vec` and `Vec`, +an optional `HttpApi`, a defaulted `HealthCheckApi`, and defaulted shared +`UdpTrackerServer` policy. Consequently, trackers are $0..N$, the REST API is structurally +$0..1$, and health checking has exactly one effective configuration even when no TOML health +section is supplied. The health listener is always started; a missing REST API is not. [E1](evidence.md#e1-current-state-baseline) + +The application currently imports the v2 public aliases. The v3 module is the appropriate +analysis target, but no production v3 consumer migration is valid before #1980. Runtime startup +is role- and dependency-grouped: shared UDP support work precedes UDP listeners, then HTTP +listeners, optional REST API, and unconditional health API. Therefore source declaration order +has no current startup meaning and must not acquire one. [E1](evidence.md#e1-current-state-baseline) + +`ConfigurationInstanceId` is the established runtime identity: `(ServiceRole, role-local index)`. +It deliberately excludes both configured and bound addresses, which is required for valid +port-zero listeners. REST and health already register as `RestApi(0)` and `HealthCheckApi(0)`. +Registration instead records the final post-bind `ServiceBinding`, preserving metrics and health +contracts. [E1](evidence.md#e1-current-state-baseline) + +The primary baseline defect is unrelated to TOML layout: each `UdpTracker` exposes +`max_connection_id_errors_per_ip`, yet container construction reads only the first UDP entry to +initialize one shared ban service. This must not be perpetuated as flat-list order dependence. +It belongs either in shared UDP policy, behind consistency validation, or in separately approved +ban-service redesign work. [E1](evidence.md#e1-current-state-baseline) ## Candidate Representations -TODO: Compare at least two TOML/Rust representations. Include the adjacent-tagged form and the -rejected or less-preferred alternatives with their operator-facing trade-offs. +| Representation | TOML and Rust shape | Advantages | Costs and decision | +| ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Current split TOML plus optional internal normalization** | `[[http_trackers]]`, `[[udp_trackers]]`, optional `[http_api]`, defaulted `[health_check_api]`; normalize role-specific views only inside a lifecycle boundary if later needed. | Names and role-specific fields remain adjacent; common single-service files require no type discriminator; singleton cardinality is structural; named nested Figment overrides remain supported; no migration. | Cross-role source order cannot be expressed; numeric overrides of any list entry are unsupported by the current Figment provider. **Recommended.** | +| **Adjacent-tagged list** | `Vec` with `#[serde(tag = "kind", content = "configuration")]`. Each list item has `kind` plus a nested configuration table. | The most viable flat representation: preserves per-kind typed configuration, TOML order, Serde round trips, and clear unknown-kind rejection. | Adds a discriminator and nesting before each service's fields; duplicate singleton rules move to custom validation; omitted health needs normalizer defaulting; numeric list environment overrides fail; v2-to-v3 migration invents an order. **Rejected for TOML.** | +| **Internally tagged flattened list** | `#[serde(tag = "kind")]` plus `#[serde(flatten)]` wrapped role configuration. | Removes one TOML nesting level and round-trips. | Mixes discriminators with fields whose meaning varies by type, makes field discovery less local, and has no compensating benefit for common deployments. **Not recommended.** | +| **Externally tagged list** | `Vec` such as `[services.http_tracker]`. | Round-trips and has no explicit discriminator field. | Adds a role-named wrapper table, duplicates the role grouping at per-item granularity, and is less discoverable than current sections. **Not recommended.** | + +For an operator with one HTTP or one UDP listener—the expected primary deployment—the split form +has a direct path from service purpose to its fields. A flat list imposes the extra steps “find +the list entry” and “interpret its kind” before fields can be evaluated. Interleaving service +types is only valuable when it represents an operational ordering, but ordering must not control +startup and the current model has no demonstrated operator workflow requiring it. [E2](evidence.md#e2-configuration-representation-feasibility) ## Feasibility Results -TODO: Summarize TOML parsing, Serde round-trip behavior, Figment defaults and environment -overrides, discriminator validation, and any prototype constraints. Link each conclusion to an -evidence record. +Isolated tests using the repository's `toml`, Serde, and Figment versions confirm that adjacent, +flattened, and externally tagged enum forms parse and serialize an interleaved service document. +Adjacent tagging rejects an unknown `kind`. It is therefore technically feasible, but feasibility +does not make it an appropriate operator schema. [E2](evidence.md#e2-configuration-representation-feasibility) + +The prototype establishes a provider limitation, not a flat-list regression: Figment's environment +provider merges a numeric path such as `SERVICES__0__CONFIGURATION__BIND_ADDRESS` as a map, not a +sequence item, so extraction fails with `InvalidType(Map, "a sequence")`. The equivalent current +split-list override also fails. Named nested overrides such as `HTTP_API__ACCESS_TOKENS__ADMIN` +remain supported. A flat representation would inherit this existing list-override limitation; +it would need a separate provider solution only if indexed listener overrides become a requirement. +[E2](evidence.md#e2-configuration-representation-feasibility) + +An omitted or empty prototype list deserializes as empty. That alone does **not** preserve the +current default health listener: normalization would need to materialize `HealthCheckApi::default` +when no health entry exists. Duplicate `http_api` and `health_check_api` entries also require +explicit semantic diagnostics, whereas the split TOML form makes duplicates structurally +impossible. [E2](evidence.md#e2-configuration-representation-feasibility) ## Runtime and Normalization Model -TODO: Explain the proposed normalization boundary, role-specific views, startup dependencies, -singleton/default behavior, shared UDP policies, and observability compatibility invariants. +Do not implement a normalization layer now. If a concrete internal consumer later needs one, it +must be the sole boundary between parsed configuration and runtime assembly. It would scan the +chosen configuration representation once, assign role-local IDs, validate singleton and shared +policy rules, materialize the default health configuration, and expose role-specific ordered views +to existing container and job startup code. No container, job, metrics collector, or registry +consumer should independently translate a list position into a role-local identity. [E3](evidence.md#e3-runtime-and-identity-model) + +The normalized output must retain dependency grouping: initialize core and shared UDP services; +start prerequisite UDP event/cleanup jobs before UDP listeners; then HTTP listeners; then optional +REST API and health API. Declaration order is presentation order only. It must preserve +post-bind `ServiceBinding`, `RuntimeServiceMetadata`, registration, and metrics behavior. For +UDP entries in private mode, current semantics are retained: configuration is accepted, startup +skips UDP and logs a warning; this is not a schema validation failure. [E1](evidence.md#e1-current-state-baseline) ## Identity, Ordering, and Migration -TODO: Define the `ServiceKind` to `ServiceRole` mapping, compare role-local and global positions, -document the v3 cross-role ordering limitation, and state any canonical migration-order rule. +If a future typed configuration enum is ever justified, its configuration-facing mapping must be: + +| `ServiceKind` | Runtime role | +| ------------------ | ----------------------------- | +| `http_tracker` | `ServiceRole::HttpTracker` | +| `udp_tracker` | `ServiceRole::UdpTracker` | +| `http_api` | `ServiceRole::RestApi` | +| `health_check_api` | `ServiceRole::HealthCheckApi` | + +`http_api` intentionally does not expose the runtime serialization name `tracker_rest_api` to +operators. A single scanner can preserve IDs by incrementing a separate ordinal for each mapped +role; the prototype proves this for interleaved HTTP and UDP entries. Using global list positions +would renumber an HTTP entry when an unrelated earlier UDP entry is added, contradicting the +existing identity contract and destabilizing metrics/container lookups. [E3](evidence.md#e3-runtime-and-identity-model) + +The present split layout records ordering only within each role; it cannot recover cross-role +order. If an external migration ever has to materialize a flat list, it must document the approved +synthetic order: HTTP trackers, UDP trackers, HTTP API, health-check API. That rule is a +deterministic export convention, not recovery of historical startup or operator order. Since the +flat TOML proposal is rejected, no migration tool or dual loader is proposed. [E4](evidence.md#e4-migration-schema-lifecycle-and-security) ## Schema Lifecycle, Security, and Compatibility -TODO: State the future version-loading/transition recommendation, #1980/#1490 relationship, -redaction requirements, affected external consumers, and compatibility constraints. +Keep one final v3 schema shape: the current role-specific layout. Do not introduce a dual layout +or migration tool. This avoids an otherwise unnecessary compatibility surface for default TOML, +deployment overrides, documentation, fixtures, and external configuration consumers. #1490 still +must replace manual token masking with secret types before #1980 moves application consumers to +v3. [E4](evidence.md#e4-migration-schema-lifecycle-and-security) + +The current v3 root `mask_secrets` explicitly reaches `http_api`; nesting that configuration in +an enum would require exhaustive redaction traversal and tests. #1490 makes that concern more +important because it changes both API-token and database-password representations. Retaining the +split root prevents new redaction traversal risk while #1490 performs the planned security +overhaul. [E1](evidence.md#e1-current-state-baseline) ## Cost, Risks, and Recommendation -TODO: Estimate the affected modules and effort, list remaining risks, and provide the final -recommendation with exact proposed scope for any follow-up implementation issue. +Implementing flat TOML would change at least `packages/configuration` loading/defaulting/ +serialization/validation/redaction, default configuration files, migration documentation, +fixtures, configuration consumers, containers, bootstrap jobs, registration/metrics tests, and +environment override behavior. It also collides with #1490 and #1980, which already make a broad +v3 consumer migration. The hidden UDP shared-policy defect would need an explicit decision rather +than preservation of first-entry-wins behavior. [E1](evidence.md#e1-current-state-baseline) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) + +The adjacent enum is feasible, but its only distinct benefit—cross-role presentation order—does +not improve the primary operator workflows and cannot influence lifecycle startup. Its costs are +concrete: less local TOML, semantic singleton/default rules, unsupported indexed overrides, +breaking migration, and redaction changes. **Reject the flat TOML implementation and do not +create a new #1978 implementation sub-issue.** + +The remaining opportunity is deliberately deferred, not committed: if a future runtime feature +needs a complete configuration-derived listener inventory beyond the existing registry, create a +separate issue for an internal normalizer while retaining the role-specific TOML model. It must +first define a consumer, shared UDP policy handling, and role-local ID ownership. [E3](evidence.md#e3-runtime-and-identity-model) ## Evidence Index -TODO: Link each material conclusion above to the relevant section of [evidence.md](evidence.md). +| Report area | Evidence | +| --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Executive decision and current-state baseline | [E1](evidence.md#e1-current-state-baseline), [E3](evidence.md#e3-runtime-and-identity-model), [E4](evidence.md#e4-migration-schema-lifecycle-and-security) | +| Candidate representations and feasibility | [E2](evidence.md#e2-configuration-representation-feasibility) | +| Runtime/normalization and identity | [E1](evidence.md#e1-current-state-baseline), [E3](evidence.md#e3-runtime-and-identity-model) | +| Migration, lifecycle, security, cost | [E1](evidence.md#e1-current-state-baseline), [E4](evidence.md#e4-migration-schema-lifecycle-and-security) | diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md index 81661b52d..c3168e4fa 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md @@ -1,6 +1,6 @@ # Evidence Ledger: Flat Heterogeneous Service Configuration -> **Status:** Planned +> **Status:** Complete > > **Issue contract:** [ISSUE.md](ISSUE.md) > @@ -14,48 +14,124 @@ or runtime change was implemented. - **Question:** What current configuration, runtime, identity, shared-state, and redaction contracts constrain the analysis? -- **Status:** TODO -- **Method:** TODO -- **Observation:** TODO -- **Conclusion:** TODO +- **Status:** PASS +- **Method:** Reviewed `packages/configuration/src/v3_0_0/mod.rs`, `http_tracker.rs`, + `udp_tracker.rs`, `tracker_api.rs`, `health_check_api.rs`, and `udp_tracker_server.rs`; + `packages/configuration/src/lib.rs`; `src/bootstrap/app.rs`, `src/app.rs`, and + `src/container.rs`; `packages/primitives/src/configuration_instance_id.rs`, + `service_role.rs`, and `runtime_service_metadata.rs`; `packages/udp-core/src/container.rs`; + and `tests/common/configuration.rs`. +- **Observation:** V3 has optional HTTP/UDP vectors and HTTP API, but defaulted health and shared + UDP server sections. Production global aliases still select v2 until #1980. Startup groups + shared UDP work before UDP instances, then HTTP instances, optional REST, and health. IDs are + role-local; REST and health use ordinal zero. The registry records final post-bind bindings. + The shared UDP ban service takes `max_connection_id_errors_per_ip` from only the first configured + UDP listener. V3 manual `mask_secrets` explicitly descends into the root `http_api`. +- **Conclusion:** The split schema encodes cardinality/defaulting structurally and is distinct from + the existing role-grouped runtime lifecycle. Any future normalizer needs one ownership point for + ID allocation, health defaulting, singleton validation, and shared UDP policy. It must retain + post-bind registration and redaction behavior. - **Report Links:** `analysis.md` sections "Current-State Baseline" and "Runtime and Normalization Model". ## E2: Configuration Representation Feasibility - **Question:** Which TOML/Rust enum representations parse, serialize, validate, and support required configuration-source behavior? -- **Status:** TODO -- **Method:** TODO -- **Observation:** TODO -- **Conclusion:** TODO +- **Status:** PASS +- **Method:** Added test-only local types under + `packages/configuration/src/v3_0_0/mod.rs::tests::flat_service_configuration_prototype`. + Ran: + + `cargo test -p torrust-tracker-configuration flat_service_configuration_prototype -- --nocapture` + + Adjacent-tagged TOML input: + + ```toml + [[services]] + kind = "http_tracker" + [services.configuration] + bind_address = "127.0.0.1:17070" + + [[services]] + kind = "udp_tracker" + [services.configuration] + bind_address = "127.0.0.1:16969" + ``` + + Flat-list indexed override input: + + ```text + TORRUST_TRACKER_CONFIG_OVERRIDE_SERVICES__0__CONFIGURATION__BIND_ADDRESS=127.0.0.1:18080 + ``` + + Equivalent split-list indexed override input: + + ```text + TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_TRACKERS__0__BIND_ADDRESS=127.0.0.1:18080 + ``` + +- **Observation:** All nine focused tests passed. Adjacent, flattened/internal-tagged, and + externally tagged forms round-trip through TOML and Serde. Adjacent tagging rejects an unknown + kind. Omitted and empty lists deserialize as empty; duplicate singleton kinds need semantic + validation. Both flat and equivalent split-list indexed Figment overrides fail extraction with + `InvalidType(Map, "a sequence")`; the current named nested HTTP API override remains covered by + an existing test. +- **Conclusion:** An adjacent enum is technically feasible and shares the current Figment + limitation for indexed listener overrides. It transfers singleton/default behavior from + structure to custom normalization/validation. Flattened and external forms are feasible but less + operator-friendly. - **Report Links:** `analysis.md` sections "Candidate Representations" and "Feasibility Results". ## E3: Runtime and Identity Model - **Question:** Can one normalization model preserve role-local IDs, container lookups, startup dependencies, registration, and metrics behavior for interleaved services? -- **Status:** TODO -- **Method:** TODO -- **Observation:** TODO -- **Conclusion:** TODO +- **Status:** PASS +- **Method:** Traced `src/container.rs::{initialize, +initialize_http_tracker_instance_containers,initialize_udp_tracker_instance_containers}` and + `src/app.rs::{start_jobs,start_udp_tracker_services,start_the_http_instances,start_the_http_api}`. + Reviewed prototype test `role_local_ids_remain_stable_when_another_role_precedes_a_service`. +- **Observation:** Existing container construction assigns IDs beside per-role containers, and + jobs retrieve those containers by role-local index. The prototype scans interleaved services and + yields `UdpTracker(0)`, `HttpTracker(0)`, `UdpTracker(1)`, `HttpTracker(1)`. Startup is grouped + by dependency rather than declaration order. `Registar` already inventories started listeners; + the job manager includes both listeners and non-listener jobs. +- **Conclusion:** A flat source list can preserve existing IDs only through one scanner with + role-specific counters. Global list positions are incompatible. An internal normalizer is + possible without a flat TOML schema, but no current consumer demonstrates that it is required. - **Report Links:** `analysis.md` sections "Runtime and Normalization Model" and "Identity, Ordering, and Migration". ## E4: Migration, Schema Lifecycle, and Security - **Question:** What migration order, schema transition policy, dependency order, and redaction constraints would a successor schema require? -- **Status:** TODO -- **Method:** TODO -- **Observation:** TODO -- **Conclusion:** TODO +- **Status:** PASS +- **Method:** Reviewed `packages/configuration/src/lib.rs`, v3 load/default/version checks, + `src/bootstrap/app.rs`, #1490 at + `docs/issues/open/1490-1978-decompose-database-config-and-overhaul-secrets.md`, and #1978 at + `docs/issues/open/1978-configuration-overhaul-epic/EPIC.md`. +- **Observation:** Current v3 loading accepts a single exact schema version, while production + consumers remain v2 until #1980. #1490 replaces manual secret masking and must precede #1980. + The split layout has no cross-role ordering; a flat migration would fabricate the approved + HTTP, UDP, REST, health order. A flat enum would require new redaction traversal, migration + guidance, default files, fixture updates, and an override solution. +- **Conclusion:** A dual loader or migration tool adds cost without an operator benefit. Retaining + the split layout lets #1490 and #1980 proceed without redoing their consumer migration. No + schema implementation follow-up is warranted. - **Report Links:** `analysis.md` sections "Identity, Ordering, and Migration" and "Schema Lifecycle, Security, and Compatibility". ## E5: Final Report Review - **Question:** Does every material recommendation in `analysis.md` have sufficient evidence, and does the recommendation remain analysis-only? -- **Status:** TODO -- **Method:** TODO -- **Observation:** TODO -- **Conclusion:** TODO +- **Status:** PASS +- **Method:** Checked every required `analysis.md` section against E1–E4 and confirmed the + prototype is restricted to `#[cfg(test)]` test-local types with no production schema/runtime + behavior changes. +- **Observation:** The report compares three TOML representations plus internal normalization, + provides reproducible test input and commands, identifies the Figment limit, preserves the + identity/lifecycle constraints, and issues one recommendation. +- **Conclusion:** The decision record is traceable and remains analysis-only. The recommendation + is to reject a flat TOML schema change and defer any internal normalizer until a real consumer + need exists. - **Report Links:** `analysis.md` section "Executive Decision" and "Cost, Risks, and Recommendation". diff --git a/packages/configuration/src/v3_0_0/mod.rs b/packages/configuration/src/v3_0_0/mod.rs index cc08754d2..2bdee3e92 100644 --- a/packages/configuration/src/v3_0_0/mod.rs +++ b/packages/configuration/src/v3_0_0/mod.rs @@ -1100,4 +1100,345 @@ mod tests { }); } } + + /// Test-only feasibility experiments for issue #2067. + /// + /// These local types deliberately do not change the public v3 schema. They + /// exercise the TOML, Serde, and Figment behavior a separately approved + /// schema implementation would rely on. + mod flat_service_configuration_prototype { + use figment::Figment; + use figment::providers::{Env, Format, Toml}; + use serde::{Deserialize, Serialize}; + use torrust_tracker_primitives::{ConfigurationInstanceId, ServiceRole}; + + use crate::v3_0_0::health_check_api::HealthCheckApi; + use crate::v3_0_0::http_tracker::HttpTracker; + use crate::v3_0_0::tracker_api::HttpApi; + use crate::v3_0_0::udp_tracker::UdpTracker; + + const OVERRIDE_PREFIX: &str = "TORRUST_TRACKER_CONFIG_OVERRIDE_"; + const OVERRIDE_SEPARATOR: &str = "__"; + + #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] + #[serde(tag = "kind", content = "configuration", rename_all = "snake_case", deny_unknown_fields)] + enum AdjacentService { + HttpTracker(HttpTracker), + UdpTracker(UdpTracker), + HttpApi(HttpApi), + HealthCheckApi(HealthCheckApi), + } + + #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] + #[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] + enum FlattenedService { + HttpTracker { + #[serde(flatten)] + configuration: HttpTracker, + }, + UdpTracker { + #[serde(flatten)] + configuration: UdpTracker, + }, + HttpApi { + #[serde(flatten)] + configuration: HttpApi, + }, + HealthCheckApi { + #[serde(flatten)] + configuration: HealthCheckApi, + }, + } + + #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] + #[serde(rename_all = "snake_case", deny_unknown_fields)] + enum ExternallyTaggedService { + HttpTracker(HttpTracker), + UdpTracker(UdpTracker), + HttpApi(HttpApi), + HealthCheckApi(HealthCheckApi), + } + + #[derive(Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + struct AdjacentServicesDocument { + #[serde(default)] + services: Vec, + } + + #[derive(Serialize, Deserialize)] + struct FlattenedServicesDocument { + services: Vec, + } + + #[derive(Serialize, Deserialize)] + struct ExternallyTaggedServicesDocument { + services: Vec, + } + + #[allow(dead_code)] + #[derive(Deserialize)] + struct SplitServicesDocument { + #[serde(default)] + http_trackers: Vec, + } + + fn role_and_configuration(service: &AdjacentService) -> (ServiceRole, usize) { + match service { + AdjacentService::HttpTracker(_) => (ServiceRole::HttpTracker, 0), + AdjacentService::UdpTracker(_) => (ServiceRole::UdpTracker, 0), + AdjacentService::HttpApi(_) => (ServiceRole::RestApi, 0), + AdjacentService::HealthCheckApi(_) => (ServiceRole::HealthCheckApi, 0), + } + } + + fn role_local_ids(services: &[AdjacentService]) -> Vec { + let mut http_trackers = 0; + let mut udp_trackers = 0; + let mut http_apis = 0; + let mut health_check_apis = 0; + + services + .iter() + .map(|service| { + let (role, _) = role_and_configuration(service); + let index = match role { + ServiceRole::HttpTracker => { + let index = http_trackers; + http_trackers += 1; + index + } + ServiceRole::UdpTracker => { + let index = udp_trackers; + udp_trackers += 1; + index + } + ServiceRole::RestApi => { + let index = http_apis; + http_apis += 1; + index + } + ServiceRole::HealthCheckApi => { + let index = health_check_apis; + health_check_apis += 1; + index + } + }; + + ConfigurationInstanceId::new(role, index) + }) + .collect() + } + + fn validate_singletons(services: &[AdjacentService]) -> Result<(), &'static str> { + let http_apis = services + .iter() + .filter(|service| matches!(service, AdjacentService::HttpApi(_))) + .count(); + let health_check_apis = services + .iter() + .filter(|service| matches!(service, AdjacentService::HealthCheckApi(_))) + .count(); + + if http_apis > 1 { + return Err("services may contain at most one http_api entry"); + } + if health_check_apis > 1 { + return Err("services may contain at most one health_check_api entry"); + } + + Ok(()) + } + + #[test] + fn adjacent_tagged_services_round_trip_and_preserve_interleaved_order() { + let input = r#" + [[services]] + kind = "http_tracker" + [services.configuration] + bind_address = "127.0.0.1:17070" + + [[services]] + kind = "udp_tracker" + [services.configuration] + bind_address = "127.0.0.1:16969" + + [[services]] + kind = "http_api" + [services.configuration] + bind_address = "127.0.0.1:1212" + "#; + + let document: AdjacentServicesDocument = toml::from_str(input).expect("adjacent-tagged TOML should deserialize"); + let serialized = toml::to_string(&document).expect("services document should serialize"); + let round_tripped: AdjacentServicesDocument = + toml::from_str(&serialized).expect("serialized services document should deserialize"); + + assert_eq!(document.services, round_tripped.services); + assert!(matches!(document.services[0], AdjacentService::HttpTracker(_))); + assert!(matches!(document.services[1], AdjacentService::UdpTracker(_))); + assert!(matches!(document.services[2], AdjacentService::HttpApi(_))); + } + + #[test] + fn flattened_services_round_trip_but_mix_kind_and_service_specific_fields() { + let input = r#" + [[services]] + kind = "http_tracker" + bind_address = "127.0.0.1:17070" + tracker_usage_statistics = true + + [[services]] + kind = "udp_tracker" + bind_address = "127.0.0.1:16969" + cookie_lifetime = { secs = 120, nanos = 0 } + "#; + + let document: FlattenedServicesDocument = toml::from_str(input).expect("flattened TOML should deserialize"); + let serialized = toml::to_string(&document).expect("flattened services document should serialize"); + let round_tripped: FlattenedServicesDocument = + toml::from_str(&serialized).expect("serialized flattened services document should deserialize"); + + assert_eq!(document.services, round_tripped.services); + } + + #[test] + fn externally_tagged_services_round_trip_but_add_a_role_named_wrapper_table() { + let input = r#" + [[services]] + [services.http_tracker] + bind_address = "127.0.0.1:17070" + "#; + + let document: ExternallyTaggedServicesDocument = + toml::from_str(input).expect("externally-tagged TOML should deserialize"); + let serialized = toml::to_string(&document).expect("externally-tagged services document should serialize"); + let round_tripped: ExternallyTaggedServicesDocument = + toml::from_str(&serialized).expect("serialized externally-tagged services document should deserialize"); + + assert_eq!(document.services, round_tripped.services); + } + + #[test] + fn adjacent_tagged_services_reject_unknown_kinds() { + let input = r#" + [[services]] + kind = "smtp_tracker" + [services.configuration] + bind_address = "127.0.0.1:25" + "#; + + let result = toml::from_str::(input); + + assert!(result.is_err(), "unknown service kinds must be rejected"); + } + + #[test] + fn omitted_or_empty_services_are_valid_and_health_defaults_are_not_materialized_by_deserialization() { + let omitted: AdjacentServicesDocument = toml::from_str("").expect("omitted services should deserialize"); + let empty: AdjacentServicesDocument = toml::from_str("services = []").expect("empty services should deserialize"); + + assert!(omitted.services.is_empty()); + assert!(empty.services.is_empty()); + assert!(validate_singletons(&omitted.services).is_ok()); + assert!(validate_singletons(&empty.services).is_ok()); + } + + #[test] + fn singleton_validation_rejects_duplicate_http_api_and_health_check_api_entries() { + let duplicate_http_api = vec![ + AdjacentService::HttpApi(HttpApi::default()), + AdjacentService::HttpApi(HttpApi::default()), + ]; + let duplicate_health_check_api = vec![ + AdjacentService::HealthCheckApi(HealthCheckApi::default()), + AdjacentService::HealthCheckApi(HealthCheckApi::default()), + ]; + + assert_eq!( + validate_singletons(&duplicate_http_api), + Err("services may contain at most one http_api entry") + ); + assert_eq!( + validate_singletons(&duplicate_health_check_api), + Err("services may contain at most one health_check_api entry") + ); + } + + #[test] + fn role_local_ids_remain_stable_when_another_role_precedes_a_service() { + let services = vec![ + AdjacentService::UdpTracker(UdpTracker::default()), + AdjacentService::HttpTracker(HttpTracker::default()), + AdjacentService::UdpTracker(UdpTracker::default()), + AdjacentService::HttpTracker(HttpTracker::default()), + ]; + + assert_eq!( + role_local_ids(&services), + vec![ + ConfigurationInstanceId::new(ServiceRole::UdpTracker, 0), + ConfigurationInstanceId::new(ServiceRole::HttpTracker, 0), + ConfigurationInstanceId::new(ServiceRole::UdpTracker, 1), + ConfigurationInstanceId::new(ServiceRole::HttpTracker, 1), + ] + ); + } + + #[allow(clippy::result_large_err)] + #[test] + fn figment_rejects_numeric_adjacent_service_index_overrides() { + figment::Jail::expect_with(|jail| { + jail.set_env( + "TORRUST_TRACKER_CONFIG_OVERRIDE_SERVICES__0__CONFIGURATION__BIND_ADDRESS", + "127.0.0.1:18080", + ); + + let figment = Figment::from(Toml::string( + r#" + [[services]] + kind = "http_tracker" + [services.configuration] + bind_address = "127.0.0.1:17070" + "#, + )) + .merge(Env::prefixed(OVERRIDE_PREFIX).split(OVERRIDE_SEPARATOR)); + let result = figment.extract::(); + + assert!( + result.is_err(), + "Figment environment maps cannot override a sequence item by numeric index" + ); + + Ok(()) + }); + } + + #[allow(clippy::result_large_err)] + #[test] + fn figment_also_rejects_numeric_split_service_index_overrides() { + figment::Jail::expect_with(|jail| { + jail.set_env( + "TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_TRACKERS__0__BIND_ADDRESS", + "127.0.0.1:18080", + ); + + let figment = Figment::from(Toml::string( + r#" + [[http_trackers]] + bind_address = "127.0.0.1:17070" + "#, + )) + .merge(Env::prefixed(OVERRIDE_PREFIX).split(OVERRIDE_SEPARATOR)); + let result = figment.extract::(); + + assert!( + result.is_err(), + "Figment environment maps cannot override a split sequence item by numeric index" + ); + + Ok(()) + }); + } + } } From 4871fb0e278499f6c53ef89fe8818b14289b026d Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 09:31:53 +0100 Subject: [PATCH 3/7] docs(configuration): clarify configuration findings --- .../ISSUE.md | 61 ++++++------- .../analysis.md | 64 +++++++++---- .../max-connection-id-errors-per-ip-bug.md | 90 +++++++++++++++++++ 3 files changed, 168 insertions(+), 47 deletions(-) create mode 100644 docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md index 5d45af244..41c285b80 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md @@ -20,6 +20,7 @@ semantic-links: - docs/issues/open/1490-1978-decompose-database-config-and-overhaul-secrets.md - docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md - docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md + - docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md - docs/issues/open/2067-1978-analyze-flat-service-configuration/first-impressions.md - packages/configuration/src/lib.rs - packages/configuration/src/v2_0_0/mod.rs @@ -354,19 +355,19 @@ For an experiment, preserve the exact TOML input and command in the record. Test Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ----------- | ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | DONE | Capture the current model | Recorded the v3/v2 boundary, cardinality/defaulting, startup, identity, shared UDP state, registration, observability, and redaction evidence in `evidence.md#e1-current-state-baseline`. | -| T2 | DONE | Prototype schema representations | Added isolated test-only TOML/Serde/Figment experiments. Numeric list overrides fail with the current Figment provider; see `evidence.md#e2-configuration-representation-feasibility`. | -| T3 | DONE | Compare configuration representations | Compared split TOML, adjacent, flattened, and externally tagged forms in `analysis.md#candidate-representations`. | -| T4 | DONE | Analyze runtime integration | Defined the conditional single-normalizer model and preserved dependency-grouped startup in `analysis.md#runtime-and-normalization-model`. | -| T5 | DONE | Analyze identity compatibility | Documented role-local ordinal preservation, global-position consequences, and `ServiceKind` mapping in `analysis.md#identity-ordering-and-migration`. | -| T6 | DONE | Define migration and schema lifecycle | Rejected the schema transition; documented canonical export ordering and #1490/#1980 constraints in `analysis.md#schema-lifecycle-security-and-compatibility`. | -| T7 | DONE | Analyze security and operator impact | Documented redaction, logging, override, and post-bind compatibility constraints in `analysis.md`. | -| T8 | DONE | Write the final analysis deliverables | Completed `analysis.md` and `evidence.md` with an analysis-only rejection recommendation. | -| T9 | DONE | Run automatic checks | `cargo test -p torrust-tracker-configuration` and the mandatory pre-commit gate passed using the installed stable toolchain. | -| T10 | DONE | Perform manual review | Reviewed candidate presentation, report/evidence links, migration rule, and impact inventory; see M5 and `evidence.md#e5-final-report-review`. | -| T11 | DONE | Re-review acceptance criteria | Acceptance criteria reviewed against E1–E5 and the completed validation results. | +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| T1 | DONE | Capture the current model | Recorded the v3/v2 boundary, cardinality/defaulting, startup, identity, shared UDP state, registration, observability, and redaction evidence in `evidence.md#e1-current-state-baseline`. | +| T2 | DONE | Prototype schema representations | Added isolated test-only TOML/Serde/Figment experiments. Numeric list overrides fail with the current Figment provider; see `evidence.md#e2-configuration-representation-feasibility`. | +| T3 | DONE | Compare configuration representations | Compared split TOML, adjacent, flattened, and externally tagged forms in `analysis.md#candidate-representations`. | +| T4 | DONE | Analyze runtime integration | Defined the conditional single-normalizer model and preserved dependency-grouped startup in `analysis.md#runtime-and-normalization-model`. | +| T5 | DONE | Analyze identity compatibility | Documented role-local ordinal preservation, global-position consequences, and `ServiceKind` mapping in `analysis.md#identity-ordering-and-migration`. | +| T6 | DONE | Define migration and schema lifecycle | Rejected the schema transition; documented canonical export ordering and #1490/#1980 constraints in `analysis.md#schema-lifecycle-security-and-compatibility`. | +| T7 | DONE | Analyze security and operator impact | Documented redaction, logging, override, and post-bind compatibility constraints in `analysis.md`. | +| T8 | DONE | Write the final analysis deliverables | Completed `analysis.md` and `evidence.md` with an analysis-only rejection recommendation. | +| T9 | DONE | Run automatic checks | `cargo test -p torrust-tracker-configuration` and the mandatory pre-commit gate passed using the installed stable toolchain. | +| T10 | DONE | Perform manual review | Reviewed candidate presentation, report/evidence links, migration rule, and impact inventory; see M5 and `evidence.md#e5-final-report-review`. | +| T11 | DONE | Re-review acceptance criteria | Acceptance criteria reviewed against E1–E5 and the completed validation results. | ## Progress Tracking @@ -443,24 +444,24 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. ### Acceptance Verification -| AC ID | Status (`TODO`/`DONE`) | Evidence | -| ----- | ---------------------- | -------- | -| AC1 | DONE | `evidence.md#e1-current-state-baseline` | -| AC2 | DONE | `analysis.md#candidate-representations`, `evidence.md#e2-configuration-representation-feasibility` | -| AC3 | DONE | `evidence.md#e2-configuration-representation-feasibility` | -| AC4 | DONE | `analysis.md#runtime-and-normalization-model`, `evidence.md#e3-runtime-and-identity-model` | -| AC5 | DONE | `analysis.md#feasibility-results`, E1–E2 | -| AC6 | DONE | `analysis.md#identity-ordering-and-migration`, `evidence.md#e3-runtime-and-identity-model` | -| AC7 | DONE | `analysis.md#current-state-baseline`, `evidence.md#e1-current-state-baseline` | +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | ------------------------------------------------------------------------------------------------------------------- | +| AC1 | DONE | `evidence.md#e1-current-state-baseline` | +| AC2 | DONE | `analysis.md#candidate-representations`, `evidence.md#e2-configuration-representation-feasibility` | +| AC3 | DONE | `evidence.md#e2-configuration-representation-feasibility` | +| AC4 | DONE | `analysis.md#runtime-and-normalization-model`, `evidence.md#e3-runtime-and-identity-model` | +| AC5 | DONE | `analysis.md#feasibility-results`, E1–E2 | +| AC6 | DONE | `analysis.md#identity-ordering-and-migration`, `evidence.md#e3-runtime-and-identity-model` | +| AC7 | DONE | `analysis.md#current-state-baseline`, `evidence.md#e1-current-state-baseline` | | AC8 | DONE | `analysis.md#schema-lifecycle-security-and-compatibility`, `evidence.md#e4-migration-schema-lifecycle-and-security` | -| AC9 | DONE | E1 and E4 | -| AC10 | DONE | `analysis.md`, `evidence.md#e5-final-report-review` | -| AC11 | DONE | `evidence.md#e1-current-state-baseline` through `evidence.md#e5-final-report-review` | -| AC12 | DONE | 2026-08-22 pre-commit gate (`linter all`) | -| AC13 | DONE | Focused prototype tests (9 passed) and final pre-commit gate | -| AC14 | DONE | M1–M5 and E1–E5 | -| AC15 | DONE | 2026-08-22 acceptance review | -| AC16 | DONE | `ISSUE.md`, `analysis.md`, and `evidence.md` | +| AC9 | DONE | E1 and E4 | +| AC10 | DONE | `analysis.md`, `evidence.md#e5-final-report-review` | +| AC11 | DONE | `evidence.md#e1-current-state-baseline` through `evidence.md#e5-final-report-review` | +| AC12 | DONE | 2026-08-22 pre-commit gate (`linter all`) | +| AC13 | DONE | Focused prototype tests (9 passed) and final pre-commit gate | +| AC14 | DONE | M1–M5 and E1–E5 | +| AC15 | DONE | 2026-08-22 acceptance review | +| AC16 | DONE | `ISSUE.md`, `analysis.md`, and `evidence.md` | ## Risks and Trade-offs diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md index be2be19c5..2d2784d8d 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md @@ -12,13 +12,13 @@ not describe unapproved production work as implemented. ## Executive Decision -| Field | Result | -| ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Recommendation | **Reject** a flat heterogeneous `[[services]]` TOML collection for schema v3.0.0. | -| Decision status | Ready for maintainer review. | +| Field | Result | +| ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Recommendation | **Reject** a flat heterogeneous `[[services]]` TOML collection for schema v3.0.0. | +| Decision status | Ready for maintainer review. | | Rationale | The split layout is clearer for the common one-HTTP-or-one-UDP deployment, preserves structural cardinality, and avoids a breaking migration. The flat form supplies no demonstrated operator benefit that offsets those costs. | -| Required prerequisites | None for this rejection. Complete #1490 and #1980 under their existing plans. | -| Proposed follow-up | Do not create the proposed configuration-schema implementation issue. Defer any internal normalized listener inventory until a concrete lifecycle consumer cannot use the existing registry and role-specific container views. | +| Required prerequisites | None for this rejection. Complete #1490 and #1980 under their existing plans. | +| Proposed follow-up | Do not create the proposed configuration-schema implementation issue. Defer any internal normalized listener inventory until a concrete lifecycle consumer cannot use the existing registry and role-specific container views. | The rejection is limited to changing the **operator-facing TOML shape**. It does not prohibit a future internal service inventory when it is justified independently of the configuration schema. @@ -48,18 +48,20 @@ contracts. [E1](evidence.md#e1-current-state-baseline) The primary baseline defect is unrelated to TOML layout: each `UdpTracker` exposes `max_connection_id_errors_per_ip`, yet container construction reads only the first UDP entry to -initialize one shared ban service. This must not be perpetuated as flat-list order dependence. -It belongs either in shared UDP policy, behind consistency validation, or in separately approved -ban-service redesign work. [E1](evidence.md#e1-current-state-baseline) +initialize one shared ban service. This is a confirmed configuration-model bug, not merely an +open design choice: a setting consumed by one shared service must be global/shared, or the runtime +must construct genuinely independent per-instance services. The shared-services ADR requires the +former for the ban service. The separately tracked bug record defines the correction boundary; +this analysis does not implement it. [E1](evidence.md#e1-current-state-baseline) ## Candidate Representations -| Representation | TOML and Rust shape | Advantages | Costs and decision | -| ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Current split TOML plus optional internal normalization** | `[[http_trackers]]`, `[[udp_trackers]]`, optional `[http_api]`, defaulted `[health_check_api]`; normalize role-specific views only inside a lifecycle boundary if later needed. | Names and role-specific fields remain adjacent; common single-service files require no type discriminator; singleton cardinality is structural; named nested Figment overrides remain supported; no migration. | Cross-role source order cannot be expressed; numeric overrides of any list entry are unsupported by the current Figment provider. **Recommended.** | -| **Adjacent-tagged list** | `Vec` with `#[serde(tag = "kind", content = "configuration")]`. Each list item has `kind` plus a nested configuration table. | The most viable flat representation: preserves per-kind typed configuration, TOML order, Serde round trips, and clear unknown-kind rejection. | Adds a discriminator and nesting before each service's fields; duplicate singleton rules move to custom validation; omitted health needs normalizer defaulting; numeric list environment overrides fail; v2-to-v3 migration invents an order. **Rejected for TOML.** | -| **Internally tagged flattened list** | `#[serde(tag = "kind")]` plus `#[serde(flatten)]` wrapped role configuration. | Removes one TOML nesting level and round-trips. | Mixes discriminators with fields whose meaning varies by type, makes field discovery less local, and has no compensating benefit for common deployments. **Not recommended.** | -| **Externally tagged list** | `Vec` such as `[services.http_tracker]`. | Round-trips and has no explicit discriminator field. | Adds a role-named wrapper table, duplicates the role grouping at per-item granularity, and is less discoverable than current sections. **Not recommended.** | +| Representation | TOML and Rust shape | Advantages | Costs and decision | +| ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Current split TOML plus optional internal normalization** | `[[http_trackers]]`, `[[udp_trackers]]`, optional `[http_api]`, defaulted `[health_check_api]`; normalize role-specific views only inside a lifecycle boundary if later needed. | Names and role-specific fields remain adjacent; common single-service files require no type discriminator; singleton cardinality is structural; named nested Figment overrides remain supported; no migration. | Cross-role source order cannot be expressed; numeric overrides of any list entry are unsupported by the current Figment provider. **Recommended.** | +| **Adjacent-tagged list** | `Vec` with `#[serde(tag = "kind", content = "configuration")]`. Each list item has `kind` plus a nested configuration table. | The most viable flat representation: preserves per-kind typed configuration, TOML order, Serde round trips, and clear unknown-kind rejection. | Adds a discriminator and nesting before each service's fields; duplicate singleton rules move to custom validation; omitted health needs normalizer defaulting; numeric list environment overrides fail; v2-to-v3 migration invents an order. **Rejected for TOML.** | +| **Internally tagged flattened list** | `#[serde(tag = "kind")]` plus `#[serde(flatten)]` wrapped role configuration. | Removes one TOML nesting level and round-trips. | Mixes discriminators with fields whose meaning varies by type, makes field discovery less local, and has no compensating benefit for common deployments. **Not recommended.** | +| **Externally tagged list** | `Vec` such as `[services.http_tracker]`. | Round-trips and has no explicit discriminator field. | Adds a role-named wrapper table, duplicates the role grouping at per-item granularity, and is less discoverable than current sections. **Not recommended.** | For an operator with one HTTP or one UDP listener—the expected primary deployment—the split form has a direct path from service purpose to its fields. A flat list imposes the extra steps “find @@ -82,6 +84,34 @@ remain supported. A flat representation would inherit this existing list-overrid it would need a separate provider solution only if indexed listener overrides become a requirement. [E2](evidence.md#e2-configuration-representation-feasibility) +For example, an operator may expect this current split-list configuration and override to change +the listener's bind address: + +```toml +[[http_trackers]] +bind_address = "127.0.0.1:7070" +``` + +```text +TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_TRACKERS__0__BIND_ADDRESS=127.0.0.1:17070 +``` + +Instead, Figment merges the environment path as a table/map and fails because `http_trackers` +must deserialize as a sequence. The adjacent flat-list equivalent fails for the same reason: + +```text +TORRUST_TRACKER_CONFIG_OVERRIDE_SERVICES__0__CONFIGURATION__BIND_ADDRESS=127.0.0.1:17070 +``` + +Do not add an alternative canonical configuration layout solely to solve this unproven deployment +need. A map keyed by operator-chosen listener names could make an override path such as +`HTTP_TRACKERS__PUBLIC__BIND_ADDRESS` feasible, but it would replace ordering with naming, +introduce an additional schema and migration decision, and make the common single-listener TOML +less direct. If deployments demonstrate a need for per-listener environment overrides, investigate +that option or a configuration-provider capability in a separate issue. Until then, operators can +provide the complete listener configuration through `TORRUST_TRACKER_CONFIG_TOML` or use a mounted +TOML file. [E2](evidence.md#e2-configuration-representation-feasibility) + An omitted or empty prototype list deserializes as empty. That alone does **not** preserve the current default health listener: normalization would need to materialize `HealthCheckApi::default` when no health entry exists. Duplicate `http_api` and `health_check_api` entries also require @@ -147,8 +177,8 @@ Implementing flat TOML would change at least `packages/configuration` loading/de serialization/validation/redaction, default configuration files, migration documentation, fixtures, configuration consumers, containers, bootstrap jobs, registration/metrics tests, and environment override behavior. It also collides with #1490 and #1980, which already make a broad -v3 consumer migration. The hidden UDP shared-policy defect would need an explicit decision rather -than preservation of first-entry-wins behavior. [E1](evidence.md#e1-current-state-baseline) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) +v3 consumer migration. The confirmed UDP shared-policy bug must be fixed independently rather +than preserving first-entry-wins behavior. [E1](evidence.md#e1-current-state-baseline) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) The adjacent enum is feasible, but its only distinct benefit—cross-role presentation order—does not improve the primary operator workflows and cannot influence lifecycle startup. Its costs are diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md new file mode 100644 index 000000000..666918670 --- /dev/null +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md @@ -0,0 +1,90 @@ +# Confirmed Bug: UDP Connection-ID Error Limit Is Mis-scoped + +> **Status:** Confirmed during issue #2067 analysis; no fix is included here. +> +> **Parent analysis:** [ISSUE.md](ISSUE.md) +> +> **Decision record:** [analysis.md](analysis.md) + +## Summary + +`max_connection_id_errors_per_ip` is declared on every UDP listener configuration, implying that +each `[[udp_trackers]]` entry can control its own connection-ID error limit. The runtime does not +honor that meaning. It reads only the first configured UDP listener's value, then constructs one +shared `BanService` used by every UDP listener in the process. + +This is a configuration-model bug: either a value is listener-specific and every listener must +receive an independent service configured with its own value, or it controls a shared service and +must be represented once as shared/global configuration. The current first-entry-wins behavior is +neither model and makes security behavior depend silently on configuration order. + +## Reproduction + +The following values imply two different listener policies: + +```toml +[[udp_trackers]] +bind_address = "127.0.0.1:6969" +max_connection_id_errors_per_ip = 1 + +[[udp_trackers]] +bind_address = "127.0.0.1:6970" +max_connection_id_errors_per_ip = 100 +``` + +`src/container.rs` selects the first entry's value: + +```rust +let max_connection_id_errors = configuration + .udp_trackers + .as_ref() + .and_then(|trackers| trackers.first()) + .map_or(default_max_connection_id_errors, |config| { + config.max_connection_id_errors_per_ip + }); +``` + +It passes that one value to `UdpTrackerCoreServices::initialize_from`. That function creates one +`Arc>`, and each `UdpTrackerCoreContainer` receives a clone of the same arc. +Consequently both listeners use the limit `1`; the second listener's configured `100` is ignored. +Reordering the TOML entries changes the application-wide limit without changing the shared-service +design. + +## Evidence + +| Fact | Source | +| --- | --- | +| Field is placed on each listener | `packages/configuration/src/v2_0_0/udp_tracker.rs`, `packages/configuration/src/v3_0_0/udp_tracker.rs` | +| First UDP listener value is selected | `src/container.rs::AppContainer::initialize` | +| One shared ban service is created | `packages/udp-core/src/container.rs::UdpTrackerCoreServices::initialize_from` | +| All UDP containers clone that service | `packages/udp-core/src/container.rs::UdpTrackerCoreContainer::initialize_from_services` | +| Shared ban state is intentional security design | `docs/adrs/20260727180000_shared_services_across_tracker_instances.md` | + +The ADR explicitly states that settings affecting shared services must themselves be global and +uses global `connection_id_validation` as its example. The same reasoning applies to the error +limit held by the shared `BanService`. + +## Recommended Follow-up Scope + +Create a separate bug sub-issue of EPIC #1978. Its preferred correction is: + +1. Move `max_connection_id_errors_per_ip` from `UdpTracker` to the shared + `UdpTrackerServer` configuration. +2. Remove the per-listener field from the final v3 schema, defaults, fixtures, documentation, and + constructors. +3. Make `AppContainer` pass the one shared `udp_tracker_server` value to + `UdpTrackerCoreServices::initialize_from`. +4. Add tests proving that multiple UDP listeners use the same declared global limit and that + configuration order cannot change it. +5. Update the v2-to-v3 migration guidance because the field moves from each listener to the shared + section. + +Do not implement this bug fix as part of #2067. The next step is to draft and review a dedicated +sub-issue specification before creating its GitHub issue. + +## Rejected Interim Option + +Validating that every listener repeats the same value would prevent inconsistent input but would +still duplicate one global policy in every listener block. It is an inferior schema because it +retains ambiguity and raises maintenance cost. The field should be represented once where the +shared service is configured. From abbf39f96dd74b0ec6520d2c4fca5f3eccebc9af Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 09:32:21 +0100 Subject: [PATCH 4/7] docs(configuration): format UDP policy bug record --- .../max-connection-id-errors-per-ip-bug.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md index 666918670..2fdf0ef4f 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/max-connection-id-errors-per-ip-bug.md @@ -52,13 +52,13 @@ design. ## Evidence -| Fact | Source | -| --- | --- | -| Field is placed on each listener | `packages/configuration/src/v2_0_0/udp_tracker.rs`, `packages/configuration/src/v3_0_0/udp_tracker.rs` | -| First UDP listener value is selected | `src/container.rs::AppContainer::initialize` | -| One shared ban service is created | `packages/udp-core/src/container.rs::UdpTrackerCoreServices::initialize_from` | -| All UDP containers clone that service | `packages/udp-core/src/container.rs::UdpTrackerCoreContainer::initialize_from_services` | -| Shared ban state is intentional security design | `docs/adrs/20260727180000_shared_services_across_tracker_instances.md` | +| Fact | Source | +| ----------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| Field is placed on each listener | `packages/configuration/src/v2_0_0/udp_tracker.rs`, `packages/configuration/src/v3_0_0/udp_tracker.rs` | +| First UDP listener value is selected | `src/container.rs::AppContainer::initialize` | +| One shared ban service is created | `packages/udp-core/src/container.rs::UdpTrackerCoreServices::initialize_from` | +| All UDP containers clone that service | `packages/udp-core/src/container.rs::UdpTrackerCoreContainer::initialize_from_services` | +| Shared ban state is intentional security design | `docs/adrs/20260727180000_shared_services_across_tracker_instances.md` | The ADR explicitly states that settings affecting shared services must themselves be global and uses global `connection_id_validation` as its example. The same reasoning applies to the error From 79cd5f8230bc0967a492151b8d3201bbc5de73ab Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 16:28:22 +0100 Subject: [PATCH 5/7] docs(configuration): address flat schema review --- .../1978-configuration-overhaul-epic/EPIC.md | 10 +- .../ISSUE.md | 35 ++++--- .../analysis.md | 24 ++++- .../evidence.md | 37 +++++--- .../first-impressions.md | 2 +- packages/configuration/src/v3_0_0/mod.rs | 92 ++++++++++++++++--- 6 files changed, 148 insertions(+), 52 deletions(-) diff --git a/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md b/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md index 376eb1e67..99df79f76 100644 --- a/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md +++ b/docs/issues/open/1978-configuration-overhaul-epic/EPIC.md @@ -4,7 +4,7 @@ status: open github-issue: 1978 spec-path: docs/issues/open/1978-configuration-overhaul-epic/EPIC.md epic-owner: josecelano -last-updated-utc: 2026-08-22 00:00 +last-updated-utc: 2026-08-23 semantic-links: skill-links: - create-issue @@ -267,9 +267,11 @@ For each subissue implementation in this EPIC, the default completion policy is: - 2026-08-20 16:44 UTC - Copilot - Renamed #2067's folder-based subissue specification to include the parent EPIC number, following the open-issues naming convention. - 2026-08-22 UTC - Copilot/User - Corrected #2067's roadmap placement: if its analysis recommends - implementation, a new sub-issue must complete after #1490 and before #1980 so the chosen model - is part of schema v3.0.0. The analysis also compares operator-facing TOML design independently - from a possible internal polymorphic service inventory. + implementation, a new sub-issue must complete after #1490 and before #1980 so the selected final + v3 model is in place before production consumers migrate. The analysis also compares operator-facing + TOML design independently from a possible internal polymorphic service inventory. +- 2026-08-23 UTC - Copilot - Reconfirmed the #2067 policy during PR #2082 review remediation: it is + a decision about the final schema v3.0.0 shape before #1980, not a later-schema transition. ## Acceptance Criteria diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md index 41c285b80..8e6642617 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md @@ -6,10 +6,10 @@ priority: p2 github-issue: 2067 spec-path: docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md branch: "2067-analyze-flat-service-configuration" -related-pr: 2068 +related-pr: 2082 depends-on: null blocks: null -last-updated-utc: 2026-08-22 02:00 +last-updated-utc: 2026-08-23 semantic-links: skill-links: - create-issue @@ -44,7 +44,7 @@ semantic-links: Determine whether a future version of the Torrust Tracker configuration schema can represent all listener/service instances in one ordered, heterogeneous `services` collection instead of separate `http_trackers`, `udp_trackers`, `http_api`, and `health_check_api` sections. -Produce a decision-ready analysis covering viable TOML and Rust representations, benefits, costs, compatibility and migration implications, service lifecycle effects, the relationship with `ConfigurationInstanceId`, and a high-level implementation estimate. The output is a recommendation to reject, defer, or create a separate implementation issue. This is an analysis-only task; it must not implement a schema change, a v4 configuration loader, a migration tool, or production runtime changes. +Produce a decision-ready analysis covering viable TOML and Rust representations, benefits, costs, compatibility and migration implications, service lifecycle effects, the relationship with `ConfigurationInstanceId`, and a high-level implementation estimate. The output is a recommendation to reject, defer, or create a separate implementation issue. This is an analysis-only task; it must not implement a schema change, a flat-v3 loader, a migration tool, or production runtime changes. ## Background @@ -63,11 +63,11 @@ During weekly planning, Cameron proposed representing the listener services as a The current v3 configuration module still uses the existing split structure, while the application remains on the v2 public aliases pending #1980. This analysis must distinguish an immediately feasible schema representation from the proper delivery point in the configuration-overhaul roadmap. -This is an analysis-only sub-issue of #1978. If the final recommendation is to implement a configuration-schema change, maintainers must first approve and create a new #1978 sub-issue. That implementation must complete after #1490 and before #1980 so that it is included in the v3.0.0 configuration release. The analysis itself must not implement the schema, migration tool, or runtime changes. +This is an analysis-only sub-issue of #1978. If the final recommendation is to implement a configuration-schema change, maintainers must first approve and create a new #1978 sub-issue. That implementation must complete after #1490 and before #1980 so that the selected shape is included in the v3.0.0 configuration release. The analysis itself must not implement the schema, migration tool, or runtime changes. ## Illustrative Configuration Outcome -The following comparison deliberately starts from the v3 configuration schema, not the current v2 runtime configuration shown in `tests/common/configuration.rs`. The v2-to-v3 changes are independently planned under the Configuration Overhaul EPIC and #1980. This issue would be a later, separate breaking schema change built on top of v3: it changes only how v3's already-defined service configurations are organized at the root level. +The following comparison deliberately starts from the v3 configuration schema, not the current v2 runtime configuration shown in `tests/common/configuration.rs`. The v2-to-v3 changes are independently planned under the Configuration Overhaul EPIC and #1980. If approved, this issue's separately scoped implementation would select the final v3 root-level organization before #1980 migrates production consumers; it is not a post-v3 schema change. Consequently, the two examples use the same service-specific fields, nested structures, and shared `udp_tracker_server` policy. Their only intentional difference is the root-level representation: v3 uses role-specific sections; the illustrative alternative uses a heterogeneous `services` list. The alternative is a design example only, not a selected representation or a commitment to use the exact field names below. This analysis must validate its TOML and Serde feasibility and may recommend rejecting or changing the proposed form. @@ -255,7 +255,7 @@ ip_bans_reset_interval_in_secs = 86400 connection_id_validation = "strict" ``` -TOML attaches each `[services.configuration]` table and its nested tables to the immediately preceding `[[services]]` entry. `udp_tracker_server` remains top-level because it configures policy shared by all UDP listeners rather than one listener instance. The illustrative schema would replace the existing v3 root-level role-specific layout before the v3.0.0 release; it does not imply a second successor schema version. +TOML attaches each `[services.configuration]` table and its nested tables to the immediately preceding `[[services]]` entry. `udp_tracker_server` remains top-level because it configures policy shared by all UDP listeners rather than one listener instance. The illustrative schema would replace the existing v3 root-level role-specific layout before #1980 migrates production consumers; it does not imply an additional schema version. In this illustration, declaration order represents the configuration's service inventory only. It must not acquire startup-order semantics: startup remains dependency-driven and role-grouped. A recommended design must define validation for singleton service kinds and clarify whether `ConfigurationInstanceId` continues to use role-local ordinals while scanning this list or adopts global list positions. @@ -292,7 +292,7 @@ This open issue is stored at `docs/issues/open/2067-1978-analyze-flat-service-co 3. **Candidate Representations**: at least two TOML/Rust shapes, including the adjacent-tagged candidate; operator ergonomics and validation consequences for each. 4. **Feasibility Results**: TOML parsing, Serde serialization round-trip, Figment defaulting and environment overrides, unknown/discriminator errors, and constraints discovered by prototypes. 5. **Runtime and Normalization Model**: recommended single owner for normalization, role-specific views, service startup dependencies, singleton/default behavior, and preservation of existing health/metrics/registration contracts. -6. **Identity, Ordering, and Migration**: `ServiceKind` to `ServiceRole` mapping, `ConfigurationInstanceId` behavior, loss of cross-role ordering during v3-to-v4 migration, and a canonical migration-order rule if implementation is recommended. +6. **Identity, Ordering, and Migration**: `ServiceKind` to `ServiceRole` mapping, `ConfigurationInstanceId` behavior, loss of cross-role ordering when materializing a flat v3 collection from the current split layout, and a canonical migration-order rule if implementation is recommended. 7. **Schema Lifecycle, Security, and Compatibility**: v3 loading and transition policy, the #1490 → implementation → #1980 relationship, secret redaction, external configuration consumers, and observability compatibility. 8. **Cost, Risks, and Recommendation**: affected modules, high-level effort, unresolved risks, decision rationale, and exact scope for any follow-up implementation issue. @@ -347,7 +347,7 @@ For an experiment, preserve the exact TOML input and command in the record. Test - Making the REST API or health-check API multi-instance unless the analysis identifies that as a necessary consequence requiring a separately approved decision. - Replacing the global `udp_tracker_server` policy with per-listener configuration. - Changing the active v2 runtime configuration or completing #1980. -- Implementing a successor schema parser, dual-version dispatcher, configuration migration tool, normalizer, or production container/job changes. +- Implementing a flat-v3 schema parser, dual-layout dispatcher, configuration migration tool, normalizer, or production container/job changes. - Changing secret storage, secret types, or redaction policy; those remain owned by #1490. - Creating any implementation issue before the final analysis recommendation is reviewed and approved. @@ -391,21 +391,30 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. ### Progress Log - 2026-08-20 UTC - Copilot/User - Drafted an analysis-only sub-issue after weekly planning discussion. The proposed scope evaluates a heterogeneous listener-service list while explicitly retaining the existing `ConfigurationInstanceId` strategy as a constraint. -- 2026-08-20 UTC - Copilot/User - Converted the draft to a folder-style analysis issue. Added the final report and evidence-ledger contract, clarified that the work is non-blocking research for a potential post-v3 successor, and expanded the analysis scope around migration, normalization, shared UDP state, defaults, security, and compatibility. +- 2026-08-20 UTC - Copilot/User - Converted the draft to a folder-style analysis issue. Added the final report and evidence-ledger contract, and expanded the analysis scope around migration, normalization, shared UDP state, defaults, security, and compatibility. This initially described a later schema change; the 2026-08-22 policy superseded that premise. - 2026-08-20 16:36 UTC - Copilot/User - User approved the draft. Created GitHub Task #2067 and linked it as the thirteenth native sub-issue of #1978 after restoring #2023's missing native parent relationship. - 2026-08-20 16:44 UTC - Copilot - Renamed the folder to include the parent EPIC number, as required for folder-based subissue specifications. - 2026-08-20 16:51 UTC - Copilot/User - Opened spec-only PR #2068 against `develop`, linked it as related to #2067, and requested review from @da2ce7 because the proposal originated with Cameron. -- 2026-08-22 UTC - Copilot/User - Clarified that the analysis must decide whether to implement the change in schema v3.0.0, not a later successor version. If approved after the analysis, a new implementation sub-issue must follow #1490 and precede #1980. Added operator-focused configuration UX and the independent internal-normalization alternative as explicit evaluation criteria. Confirmed role-grouped, dependency-driven startup; singleton HTTP API and health-check kinds; implicit default health-check behavior; and the canonical migration order. +- 2026-08-22 UTC - Copilot/User - Clarified that the analysis must decide the final schema v3.0.0 shape before #1980. If approved after the analysis, a new implementation sub-issue must follow #1490 and precede #1980. Added operator-focused configuration UX and the independent internal-normalization alternative as explicit evaluation criteria. Confirmed role-grouped, dependency-driven startup; singleton HTTP API and health-check kinds; implicit default health-check behavior; and the canonical migration order. - 2026-08-22 UTC - Copilot - Reviewed the updated issue and EPIC roadmap specifications before committing. `git diff --check` passed; the repository `linter` executable was unavailable in this environment. - 2026-08-22 UTC - Copilot/User - Recorded the operator baseline and deployment priorities: role-specific sections are provisionally clearer because related fields remain together, no discriminator must be read, and roles are easy to locate. The analysis must assess this against a flat list while prioritizing the common single-HTTP-or-single-UDP deployment rather than uncommon multi-instance inventories. - 2026-08-22 UTC - Copilot - Completed source tracing and isolated TOML/Serde/Figment prototypes. The adjacent, flattened, and externally tagged forms round-trip, but numeric Figment overrides for list entries fail. Drafted the evidence-backed analysis recommending rejection of a flat TOML schema and deferral of any internal normalizer until it has a concrete consumer. - 2026-08-22 UTC - Copilot - Completed the final manual report review and acceptance-criteria re-review. The configuration package tests and final mandatory pre-commit gate passed all checks, including `linter all` and workspace documentation tests. - 2026-08-22 UTC - Task Reviewer - Independently reviewed the final analysis. Confirmed the flat-versus-split Figment comparison, evidence traceability, analysis-only scope, and synchronized acceptance verification. Approved the analysis as commit-ready. +- 2026-08-23 UTC - Copilot - Remediated the five open Copilot review findings for PR #2082 locally: reconciled the v3-before-#1980 policy, traced the masked JSON log boundary, added qualitative effort estimates, extended nested-field feasibility coverage, and asserted the concrete Figment error representation. Per user instruction, this local remediation does not commit, push, reply to, or resolve threads. + +### PR #2082 Copilot Review Remediation Checklist + +| Thread ID | Finding | Local remediation | Validation | Publish | Reply and resolution | +| ----------------------- | ------------------------------------- | -------------------------------------------------------------------------------------------- | ---------------------- | ------------------- | -------------------- | +| `PRRT_kwDOGp2yqc6beOKU` | Stale post-v3/v3-to-v4 language | Reconciled this spec and the #1978 EPIC with the approved v3-before-#1980 policy. | Pre-commit gate passed | Pending commit/push | Pending | +| `PRRT_kwDOGp2yqc6beOKf` | Missing logged-JSON redaction trace | Added source trace and enum redaction-before-JSON prototype evidence. | Pre-commit gate passed | Pending commit/push | Pending | +| `PRRT_kwDOGp2yqc6beOKk` | Missing effort estimate | Added qualitative estimates for the rejected flat TOML and deferred normalizer alternatives. | Pre-commit gate passed | Pending commit/push | Pending | +| `PRRT_kwDOGp2yqc6beOKr` | Missing nested-field round trip | Added adjacent-enum round-trip coverage for `network`, `tls_config`, and `access_tokens`. | Pre-commit gate passed | Pending commit/push | Pending | +| `PRRT_kwDOGp2yqc6beOKx` | Weak numeric-override error assertion | Both numeric override tests now match Figment `InvalidType(Map, "a sequence")`. | Pre-commit gate passed | Pending commit/push | Pending | ## Acceptance Criteria -- [x] AC1: Current-state analysis is traceable to E1. -- [x] AC2: Candidate representations and rejection rationale are documented in `analysis.md` and E2. - [x] AC3: Test-only feasibility experiments and results are recorded in E2. - [x] AC4: Order semantics and lifecycle constraints are documented in E3. - [x] AC5: List, singleton, private-mode, and UDP policy behavior is documented in E1–E2. @@ -474,7 +483,7 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. - **Environment override uncertainty:** Numeric paths for list entries may not work with current Figment override behavior. This must be verified before recommending the schema. - **Unrecoverable migration order:** V3 stores role-local order but not a cross-role order. A migration cannot reconstruct a desired interleaving; the analysis must recommend a canonical order or require explicit operator reordering. - **Hidden shared UDP policy:** A field placed on a UDP listener can still configure one shared runtime service. The analysis must expose and resolve that semantic mismatch before a flat list makes ordering effects less visible. -- **Schema lifecycle ambiguity:** A v4 representation requires an explicit transition, compatibility, or migration strategy because a versioned configuration loader accepts one schema shape at a time. +- **Schema lifecycle ambiguity:** Selecting a different final v3 representation before #1980 would require an explicit transition, compatibility, or migration strategy because a versioned configuration loader accepts one schema shape at a time. - **Secret exposure:** Nesting API configuration in an enum can bypass current redaction paths unless serialization/logging behavior is explicitly tested and coordinated with #1490. - **Roadmap integration:** If approved, the configuration change must be a separately scoped sub-issue after #1490 and before #1980. It must resolve its schema shape before #1980 performs the final v3 consumer migration, avoiding a second migration of runtime consumers. diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md index 2d2784d8d..feca39646 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md @@ -165,11 +165,16 @@ deployment overrides, documentation, fixtures, and external configuration consum must replace manual token masking with secret types before #1980 moves application consumers to v3. [E4](evidence.md#e4-migration-schema-lifecycle-and-security) -The current v3 root `mask_secrets` explicitly reaches `http_api`; nesting that configuration in -an enum would require exhaustive redaction traversal and tests. #1490 makes that concern more -important because it changes both API-token and database-password representations. Retaining the -split root prevents new redaction traversal risk while #1490 performs the planned security -overhaul. [E1](evidence.md#e1-current-state-baseline) +The current bootstrap boundary is concrete: `src/bootstrap/app.rs::setup` logs +`configuration.clone().mask_secrets().to_json()` through `tracing::info!`. `Configuration::mask_secrets` +first masks the database and then explicitly descends into root `http_api`; only the resulting clone +is JSON serialized and logged. A hypothetical `Vec` enum must preserve that exact ordering: +clone the complete configuration, exhaustively traverse every secret-carrying enum variant (currently +the `HttpApi` variant) to mask it, and only then call `to_json` for the log. A test-only enum prototype +confirms that traversal removes an API token from serialized JSON; it must be extended for every future +secret-bearing variant. #1490 makes this boundary more important because it changes both API-token and +database-password representations. Retaining the split root prevents new traversal risk while #1490 +performs the planned security overhaul. [E1](evidence.md#e1-current-state-baseline) [E2](evidence.md#e2-configuration-representation-feasibility) ## Cost, Risks, and Recommendation @@ -180,6 +185,15 @@ environment override behavior. It also collides with #1490 and #1980, which alre v3 consumer migration. The confirmed UDP shared-policy bug must be fixed independently rather than preserving first-entry-wins behavior. [E1](evidence.md#e1-current-state-baseline) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) +The estimates are deliberately qualitative because the flat schema is rejected before an approved +implementation design exists. A complete flat TOML delivery is **large, multi-week work**: it spans +public schema and default/migration surfaces, semantic validation and secret-redaction traversal, +then the already-planned #1980 consumer migration and cross-package lifecycle regression coverage. +An internal normalizer that retains split TOML is **medium, multi-day to small multi-week work** if a +concrete consumer justifies it; its size is driven by establishing one ID/default/shared-policy owner +and adapting its consumers, rather than by external migration. Neither estimate authorizes work; +both exclude the separately required correction for the shared UDP error-limit bug. [E1](evidence.md#e1-current-state-baseline) [E3](evidence.md#e3-runtime-and-identity-model) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) + The adjacent enum is feasible, but its only distinct benefit—cross-role presentation order—does not improve the primary operator workflows and cannot influence lifecycle startup. Its costs are concrete: less local TOML, semantic singleton/default rules, unsupported indexed overrides, diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md index c3168e4fa..0c57a0301 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/evidence.md @@ -26,11 +26,14 @@ or runtime change was implemented. shared UDP work before UDP instances, then HTTP instances, optional REST, and health. IDs are role-local; REST and health use ordinal zero. The registry records final post-bind bindings. The shared UDP ban service takes `max_connection_id_errors_per_ip` from only the first configured - UDP listener. V3 manual `mask_secrets` explicitly descends into the root `http_api`. + UDP listener. In `src/bootstrap/app.rs::setup`, the exact log expression is + `configuration.clone().mask_secrets().to_json()`: V3 `mask_secrets` masks the database, then + explicitly descends into root `http_api`, and `to_json` serializes only that masked clone. - **Conclusion:** The split schema encodes cardinality/defaulting structurally and is distinct from the existing role-grouped runtime lifecycle. Any future normalizer needs one ownership point for ID allocation, health defaulting, singleton validation, and shared UDP policy. It must retain - post-bind registration and redaction behavior. + post-bind registration and redaction behavior. Any enum-based schema must exhaustively traverse + secret-bearing variants before JSON serialization at this existing log boundary. - **Report Links:** `analysis.md` sections "Current-State Baseline" and "Runtime and Normalization Model". ## E2: Configuration Representation Feasibility @@ -70,16 +73,20 @@ or runtime change was implemented. TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_TRACKERS__0__BIND_ADDRESS=127.0.0.1:18080 ``` -- **Observation:** All nine focused tests passed. Adjacent, flattened/internal-tagged, and - externally tagged forms round-trip through TOML and Serde. Adjacent tagging rejects an unknown - kind. Omitted and empty lists deserialize as empty; duplicate singleton kinds need semantic - validation. Both flat and equivalent split-list indexed Figment overrides fail extraction with - `InvalidType(Map, "a sequence")`; the current named nested HTTP API override remains covered by - an existing test. -- **Conclusion:** An adjacent enum is technically feasible and shares the current Figment - limitation for indexed listener overrides. It transfers singleton/default behavior from - structure to custom normalization/validation. Flattened and external forms are feasible but less - operator-friendly. +- **Observation:** The focused prototype suite covers ten tests. Adjacent, flattened/internal-tagged, + and externally tagged forms round-trip through TOML and Serde. The adjacent fixture round-trips a + nested HTTP tracker `network` block and `tls_config`, plus HTTP API `access_tokens` and `tls_config`. + A separate enum traversal prototype masks the HTTP API token before JSON serialization, proving the + required redaction ordering for that variant. Adjacent tagging rejects an unknown kind. Omitted and + empty lists deserialize as empty; duplicate singleton kinds need semantic validation. Both flat and + equivalent split-list indexed Figment overrides fail extraction with a matched Figment + `InvalidType(Map, "a sequence")`; the current named nested HTTP API override remains covered by an + existing test. +- **Conclusion:** An adjacent enum is technically feasible for the nested v3 fields exercised and + shares the current Figment limitation for indexed listener overrides. Complete secret redaction + remains feasible only with an exhaustive enum traversal before the existing JSON logging boundary. + It transfers singleton/default behavior from structure to custom normalization/validation. + Flattened and external forms are feasible but less operator-friendly. - **Report Links:** `analysis.md` sections "Candidate Representations" and "Feasibility Results". ## E3: Runtime and Identity Model @@ -101,10 +108,10 @@ initialize_http_tracker_instance_containers,initialize_udp_tracker_instance_cont possible without a flat TOML schema, but no current consumer demonstrates that it is required. - **Report Links:** `analysis.md` sections "Runtime and Normalization Model" and "Identity, Ordering, and Migration". -## E4: Migration, Schema Lifecycle, and Security +## E4: Migration, Final-v3 Lifecycle, and Security -- **Question:** What migration order, schema transition policy, dependency order, and redaction - constraints would a successor schema require? +- **Question:** What migration order, final-v3 transition policy, dependency order, and redaction + constraints would a selected flat layout require before #1980? - **Status:** PASS - **Method:** Reviewed `packages/configuration/src/lib.rs`, v3 load/default/version checks, `src/bootstrap/app.rs`, #1490 at diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/first-impressions.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/first-impressions.md index cd6e2b9e2..8f164b69f 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/first-impressions.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/first-impressions.md @@ -45,7 +45,7 @@ not yet demonstrated, while the migration and runtime integration cost is alread - Current UDP configuration exposes an important semantic mismatch: the shared `BanService` is initialized from one UDP listener's `max_connection_id_errors_per_ip` value. A flat list could make this policy less visible without resolving it. -- A v3-to-successor migration cannot recover a meaningful cross-role order, because v3 stores +- Materializing a flat v3 collection from the current split layout cannot recover a meaningful cross-role order, because the split layout stores independent HTTP and UDP lists rather than one interleaved inventory. Any migration must impose a canonical order or require operator intervention. - The change must retain defaulting, environment overrides, configuration serialization, and diff --git a/packages/configuration/src/v3_0_0/mod.rs b/packages/configuration/src/v3_0_0/mod.rs index 2bdee3e92..f897f79ed 100644 --- a/packages/configuration/src/v3_0_0/mod.rs +++ b/packages/configuration/src/v3_0_0/mod.rs @@ -1159,7 +1159,7 @@ mod tests { HealthCheckApi(HealthCheckApi), } - #[derive(Serialize, Deserialize)] + #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] #[serde(deny_unknown_fields)] struct AdjacentServicesDocument { #[serde(default)] @@ -1177,7 +1177,7 @@ mod tests { } #[allow(dead_code)] - #[derive(Deserialize)] + #[derive(Debug, Deserialize)] struct SplitServicesDocument { #[serde(default)] http_trackers: Vec, @@ -1250,6 +1250,14 @@ mod tests { Ok(()) } + fn mask_service_secrets(services: &mut [AdjacentService]) { + for service in services { + if let AdjacentService::HttpApi(configuration) = service { + configuration.mask_secrets(); + } + } + } + #[test] fn adjacent_tagged_services_round_trip_and_preserve_interleaved_order() { let input = r#" @@ -1258,6 +1266,15 @@ mod tests { [services.configuration] bind_address = "127.0.0.1:17070" + [services.configuration.network] + external_ip = "203.0.113.5" + on_reverse_proxy = true + ipv6_v6only = true + + [services.configuration.tls_config] + ssl_cert_path = "tracker.crt" + ssl_key_path = "tracker.key" + [[services]] kind = "udp_tracker" [services.configuration] @@ -1267,6 +1284,13 @@ mod tests { kind = "http_api" [services.configuration] bind_address = "127.0.0.1:1212" + + [services.configuration.access_tokens] + admin = "ExampleSecretToken" + + [services.configuration.tls_config] + ssl_cert_path = "api.crt" + ssl_key_path = "api.key" "#; let document: AdjacentServicesDocument = toml::from_str(input).expect("adjacent-tagged TOML should deserialize"); @@ -1278,6 +1302,36 @@ mod tests { assert!(matches!(document.services[0], AdjacentService::HttpTracker(_))); assert!(matches!(document.services[1], AdjacentService::UdpTracker(_))); assert!(matches!(document.services[2], AdjacentService::HttpApi(_))); + + let AdjacentService::HttpTracker(http_tracker) = &document.services[0] else { + panic!("first service should be an HTTP tracker"); + }; + assert_eq!(http_tracker.network.external_ip, Some("203.0.113.5".parse().unwrap())); + assert!(http_tracker.network.on_reverse_proxy); + assert!(http_tracker.network.ipv6_v6only); + assert!(http_tracker.tls_config.is_some()); + + let AdjacentService::HttpApi(http_api) = &document.services[2] else { + panic!("third service should be an HTTP API"); + }; + assert_eq!(http_api.access_tokens.get("admin"), Some(&"ExampleSecretToken".to_string())); + assert!(http_api.tls_config.is_some()); + } + + #[test] + fn enum_redaction_must_traverse_http_api_before_json_serialization() { + let mut document = AdjacentServicesDocument { + services: vec![AdjacentService::HttpApi(HttpApi { + access_tokens: [("admin".to_string(), "ExampleSecretToken".to_string())].into(), + ..HttpApi::default() + })], + }; + + mask_service_secrets(&mut document.services); + let json = serde_json::to_string(&document).expect("masked services should serialize to JSON"); + + assert!(json.contains("***")); + assert!(!json.contains("ExampleSecretToken")); } #[test] @@ -1403,12 +1457,17 @@ mod tests { "#, )) .merge(Env::prefixed(OVERRIDE_PREFIX).split(OVERRIDE_SEPARATOR)); - let result = figment.extract::(); - - assert!( - result.is_err(), - "Figment environment maps cannot override a sequence item by numeric index" - ); + let errors = figment + .extract::() + .expect_err("Figment environment maps cannot override a sequence item by numeric index"); + + assert!(errors.into_iter().any(|error| { + matches!( + error.kind, + figment::error::Kind::InvalidType(figment::error::Actual::Map, expected) + if expected == "a sequence" + ) + })); Ok(()) }); @@ -1430,12 +1489,17 @@ mod tests { "#, )) .merge(Env::prefixed(OVERRIDE_PREFIX).split(OVERRIDE_SEPARATOR)); - let result = figment.extract::(); - - assert!( - result.is_err(), - "Figment environment maps cannot override a split sequence item by numeric index" - ); + let errors = figment + .extract::() + .expect_err("Figment environment maps cannot override a split sequence item by numeric index"); + + assert!(errors.into_iter().any(|error| { + matches!( + error.kind, + figment::error::Kind::InvalidType(figment::error::Actual::Map, expected) + if expected == "a sequence" + ) + })); Ok(()) }); From bb4ad46fd92ff07291bd6aa4cd65fa9a75fa3fdf Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 16:43:45 +0100 Subject: [PATCH 6/7] docs(configuration): update review remediation status --- .../ISSUE.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md index 8e6642617..496df05c5 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/ISSUE.md @@ -407,15 +407,14 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. | Thread ID | Finding | Local remediation | Validation | Publish | Reply and resolution | | ----------------------- | ------------------------------------- | -------------------------------------------------------------------------------------------- | ---------------------- | ------------------- | -------------------- | -| `PRRT_kwDOGp2yqc6beOKU` | Stale post-v3/v3-to-v4 language | Reconciled this spec and the #1978 EPIC with the approved v3-before-#1980 policy. | Pre-commit gate passed | Pending commit/push | Pending | -| `PRRT_kwDOGp2yqc6beOKf` | Missing logged-JSON redaction trace | Added source trace and enum redaction-before-JSON prototype evidence. | Pre-commit gate passed | Pending commit/push | Pending | -| `PRRT_kwDOGp2yqc6beOKk` | Missing effort estimate | Added qualitative estimates for the rejected flat TOML and deferred normalizer alternatives. | Pre-commit gate passed | Pending commit/push | Pending | -| `PRRT_kwDOGp2yqc6beOKr` | Missing nested-field round trip | Added adjacent-enum round-trip coverage for `network`, `tls_config`, and `access_tokens`. | Pre-commit gate passed | Pending commit/push | Pending | -| `PRRT_kwDOGp2yqc6beOKx` | Weak numeric-override error assertion | Both numeric override tests now match Figment `InvalidType(Map, "a sequence")`. | Pre-commit gate passed | Pending commit/push | Pending | +| `PRRT_kwDOGp2yqc6beOKU` | Stale post-v3/v3-to-v4 language | Reconciled this spec and the #1978 EPIC with the approved v3-before-#1980 policy. | Pre-commit gate passed | Published in `79cd5f82` | Summary posted; resolution pending | +| `PRRT_kwDOGp2yqc6beOKf` | Missing logged-JSON redaction trace | Added source trace and enum redaction-before-JSON prototype evidence. | Pre-commit gate passed | Published in `79cd5f82` | Summary posted; resolution pending | +| `PRRT_kwDOGp2yqc6beOKk` | Missing effort estimate | Added qualitative estimates for the rejected flat TOML and deferred normalizer alternatives. | Pre-commit gate passed | Published in `79cd5f82` | Summary posted; resolution pending | +| `PRRT_kwDOGp2yqc6beOKr` | Missing nested-field round trip | Added adjacent-enum round-trip coverage for `network`, `tls_config`, and `access_tokens`. | Pre-commit gate passed | Published in `79cd5f82` | Summary posted; resolution pending | +| `PRRT_kwDOGp2yqc6beOKx` | Weak numeric-override error assertion | Both numeric override tests now match Figment `InvalidType(Map, "a sequence")`. | Pre-commit gate passed | Published in `79cd5f82` | Summary posted; resolution pending | ## Acceptance Criteria -- [x] AC3: Test-only feasibility experiments and results are recorded in E2. - [x] AC4: Order semantics and lifecycle constraints are documented in E3. - [x] AC5: List, singleton, private-mode, and UDP policy behavior is documented in E1–E2. - [x] AC6: Identity compatibility, mapping, and normalization boundary are documented in E3. From 11ec94f5dcc3a70b851a39b89878e08f7ade871f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 23 Aug 2026 17:19:13 +0100 Subject: [PATCH 7/7] docs(configuration): fix analysis markdown lint --- .../2067-1978-analyze-flat-service-configuration/analysis.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md index 40e37bb91..0568dfa97 100644 --- a/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md +++ b/docs/issues/open/2067-1978-analyze-flat-service-configuration/analysis.md @@ -182,7 +182,7 @@ Implementing flat TOML would change at least `packages/configuration` loading/de serialization/validation/redaction, default configuration files, migration documentation, fixtures, configuration consumers, containers, bootstrap jobs, registration/metrics tests, and environment override behavior. It would also require a later successor-schema migration after -#2079, #1490, and #1980 complete. The confirmed UDP shared-policy bug must be fixed independently +issues #2079, #1490, and #1980 complete. The confirmed UDP shared-policy bug must be fixed independently rather than preserving first-entry-wins behavior. [E1](evidence.md#e1-current-state-baseline) [E4](evidence.md#e4-migration-schema-lifecycle-and-security) The estimates are deliberately qualitative because the flat schema is rejected before an approved