Skip to content

feat(configuration): decompose v3 database configuration #2084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josecelano merged 5 commits into from
Aug 24, 2026
Merged

feat(configuration): decompose v3 database configuration #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7339c98
docs(issues): confirm database migration scope
josecelano Aug 24, 2026
0141f1e
feat(configuration): decompose v3 database configuration
josecelano Aug 24, 2026
165ac33
fix(configuration): resolve database review findings
josecelano Aug 24, 2026
887287a
docs(review): document PR #2084 copilot suggestions audit
josecelano Aug 24, 2026
19a2b9b
docs(review): align PR #2084 suggestions audit
josecelano Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 41 additions & 36 deletions docs/issues/open/1490-1978-decompose-database-configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ github-issue: 1490
spec-path: docs/issues/open/1490-1978-decompose-database-configuration.md
branch: "1490-decompose-database-configuration"
related-pr: null
last-updated-utc: 2026-08-21 16:45
last-updated-utc: 2026-08-24 11:00
semantic-links:
skill-links:
- create-issue
Expand Down Expand Up @@ -139,17 +139,17 @@ This is a **breaking v3 configuration-schema change** with no fallback for the l

## Implementation Plan

| ID | Status | Task | Notes |
| --- | ------ | ----------------------------------------- | ------------------------------------------------------------------------------------------- |
| T1 | TODO | Confirm secrecy prerequisite is merged | Use the established dependency, `Secret<String>` convention, and API-token changes. |
| T2 | TODO | Define `ConnectionInfo` and `Database` | Replace the v3 struct in `packages/configuration/src/v3_0_0/database.rs`. |
| T3 | TODO | Implement driver-specific deserialization | Use `driver` as the discriminant; reject incompatible and unknown fields. |
| T4 | TODO | Validate network connection values | Default omitted ports; reject omitted or empty passwords with clear configuration errors. |
| T5 | TODO | Protect the isolated v3 password | Use `Secret<String>` and remove v3 database `mask_secrets()`; do not change v2 URL masking. |
| T6 | TODO | Update v3 database setup | Dispatch from enum variants and expose the password only at the database boundary. |
| T7 | TODO | Update all v3 consumers | Update helpers, driver tests, examples, benchmarks, E2E builders, and fixtures. |
| T8 | TODO | Update user-facing configuration docs | Update defaults, inline TOML, `docs/containers.md`, and the v2-to-v3 migration guide. |
| T9 | TODO | Verify compatibility and quality | Run targeted tests, `linter all`, and the required full test suite. |
| ID | Status | Task | Notes |
| --- | -------- | ----------------------------------------- | ------------------------------------------------------------------------------------------------------------ |
| T1 | DONE | Confirm secrecy prerequisite is merged | Use the established dependency, `Secret<String>` convention, and API-token changes. |
| T2 | DONE | Define `ConnectionInfo` and `Database` | Replaced the v3 struct in `packages/configuration/src/v3_0_0/database.rs`. |
| T3 | DONE | Implement driver-specific deserialization | `driver` selects the variant; incompatible and unknown fields are rejected. |
| T4 | DONE | Validate network connection values | Omitted ports default; omitted or blank passwords are rejected with safe errors. |
| T5 | DONE | Protect the isolated v3 password | Uses `SecretString`; v3 database masking is removed; v2 URL masking is unchanged. |
| T6 | DEFERRED | Update v3 database setup | Deferred to #1980, which migrates active runtime consumers to v3. |
| T7 | DEFERRED | Update all v3 consumers | Active helpers, examples, benchmarks, E2E builders, and fixtures use v2 aliases; #1980 owns their migration. |
| T8 | DONE | Update user-facing configuration docs | Updated the v2-to-v3 migration guide; active v2 defaults and operational docs are deferred to #1980. |
| T9 | DONE | Verify compatibility and quality | Stable-toolchain `linter all` and `cargo test --workspace` pass. |

## Progress Tracking

Expand All @@ -158,8 +158,8 @@ This is a **breaking v3 configuration-schema change** with no fallback for the l
- [x] Spec drafted in `docs/issues/drafts/`
- [ ] Spec reviewed and approved by user/maintainer
- [x] GitHub issue created and issue number added to this spec
- [ ] Implementation completed
- [ ] Automatic verification completed (`linter all`, relevant tests)
- [x] Implementation completed
- [x] Automatic verification completed (`linter all`, relevant tests)
- [ ] Manual verification scenarios executed and recorded
- [ ] Acceptance criteria reviewed after implementation
- [ ] Issue closed and spec moved to `docs/issues/closed/`
Expand All @@ -170,17 +170,22 @@ This is a **breaking v3 configuration-schema change** with no fallback for the l
- 2026-07-14 00:00 UTC - josecelano - Reworked the proposal around a `Database` enum and `ConnectionInfo`; documented the consumer impact.
- 2026-08-21 00:00 UTC - josecelano - Initially replanned #1490 as v3 database-schema decomposition only, with secret typing, API tokens, and manual-redaction policy moved to a separate secrecy effort. Superseded by the later ordering decision for the isolated v3 database password.
- 2026-08-21 16:45 UTC - josecelano - Reordered the work: implement the smaller secrecy refactor first for API tokens in v2 and v3, retaining v2 database URLs and their masking. #1490 follows and uses `Secret<String>` for its new isolated v3 database password.
- 2026-08-24 00:00 UTC - josecelano - Confirmed that #2079 is merged into the implementation base. Confirmed that this is a v3-only breaking-schema migration: v2 remains unchanged because v3.0.0 migration is imminent and changing v2 would introduce an unnecessary breaking change.
- 2026-08-24 00:30 UTC - josecelano - Confirmed that active runtime consumers, defaults, examples, benchmarks, and E2E configuration remain on v2 aliases and are deferred to #1980, which performs the explicit v3 consumer migration. #1490 implements only the isolated v3 schema, validation, secret handling, tests, and migration guide.
- 2026-08-24 01:00 UTC - agent - Implemented the isolated v3 database enum and `ConnectionInfo`, driver-specific TOML validation and port defaults, `SecretString` redaction and authorized persistence serialization, and migration-guide examples. `cargo test -p torrust-tracker-configuration` (111 tests) and package Clippy passed. `linter all` was blocked by a Rust nightly Clippy internal compiler error in the unrelated `swarm-coordination-registry` crate.
- 2026-08-24 11:00 UTC - agent - Resolved an implementation-specific Clippy `needless_pass_by_value` diagnostic. The nightly-only Clippy ICE was reported upstream as rust-lang/rust-clippy#17622. Stable Rust 1.98.0 completed `linter all` successfully; final workspace tests remain.
- 2026-08-24 11:15 UTC - agent - Completed final verification using stable Rust 1.98.0: `cargo test --workspace` and `linter all` passed.

## Acceptance Criteria

- [ ] AC1: v3 `Database` is an enum with `Sqlite3`, `MySQL(ConnectionInfo)`, and `PostgreSQL(ConnectionInfo)` variants.
- [ ] AC2: v3 TOML accepts the documented fields for each driver and rejects fields that do not apply to its selected driver.
- [ ] AC3: Omitted MySQL/PostgreSQL ports default to `3306`/`5432`; omitted or empty network database passwords are rejected.
- [ ] AC4: `ConnectionInfo.password` uses `Secret<String>` and formats as `Secret([REDACTED])`; the v3 database `mask_secrets()` implementation is removed.
- [ ] AC5: All affected v3 consumers compile and pass tests with the new enum.
- [ ] AC6: Default config files, inline TOML, container documentation, and the v2-to-v3 migration guide use the new per-driver format.
- [ ] `linter all` exits with code `0`.
- [ ] Relevant tests pass.
- [x] AC1: v3 `Database` is an enum with `Sqlite3`, `MySQL(ConnectionInfo)`, and `PostgreSQL(ConnectionInfo)` variants.
- [x] AC2: v3 TOML accepts the documented fields for each driver and rejects fields that do not apply to its selected driver.
- [x] AC3: Omitted MySQL/PostgreSQL ports default to `3306`/`5432`; omitted or empty network database passwords are rejected.
- [x] AC4: `ConnectionInfo.password` uses `SecretString` and generic serialization emits `"***"`; the v3 database `mask_secrets()` implementation is removed.
- [x] AC5: Isolated v3 configuration consumers compile and pass tests with the new enum; active runtime consumer migration is deferred to #1980.
- [x] AC6: The v2-to-v3 migration guide uses the new per-driver format; active v2 default files, inline TOML, and operational documentation are deferred to #1980.
- [x] `linter all` exits with code `0` (stable Rust 1.98.0).
- [x] Relevant tests pass (`cargo test -p torrust-tracker-configuration` and `cargo test --workspace`).

## Verification Plan

Expand All @@ -193,23 +198,23 @@ This is a **breaking v3 configuration-schema change** with no fallback for the l

### Manual Verification Scenarios

| ID | Scenario | Command/Steps | Expected Result | Status | Evidence |
| --- | ----------------------------------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------ | ------ | -------- |
| M1 | Parse SQLite configuration | Load a v3 TOML file with `driver = "sqlite3"` and `path`. | The configuration loads and uses the supplied filesystem path. | TODO | |
| M2 | Parse MySQL and PostgreSQL configurations | Load v3 TOML for each network driver with and without `port`. | Explicit ports are retained; omitted ports become `3306`/`5432`. | TODO | |
| M3 | Reject invalid network credentials | Load v3 TOML with missing and empty `password` values. | Loading fails with a clear validation error and does not print credentials. | TODO | |
| M4 | Verify database redaction | Format a v3 MySQL/PostgreSQL config containing a test password. | The actual password is absent and the exact literal `Secret([REDACTED])` is present. | TODO | |
| ID | Scenario | Command/Steps | Expected Result | Status | Evidence |
| --- | ----------------------------------------- | ---------------------------------------------------------------- | ------------------------------------------------------------------ | ------ | ------------------------------------------------------- |
| M1 | Parse SQLite configuration | Deserialize v3 SQLite TOML with `driver = "sqlite3"` and `path`. | The configuration loads and uses the supplied filesystem path. | PASS | Configuration tests cover SQLite default-path override. |
| M2 | Parse MySQL and PostgreSQL configurations | Deserialize v3 TOML for each network driver without `port`. | Omitted ports become `3306`/`5432`. | PASS | Dedicated configuration tests. |
| M3 | Reject invalid network credentials | Deserialize v3 TOML with missing and blank `password` values. | Loading fails with a safe validation error. | PASS | Dedicated configuration test. |
| M4 | Verify database redaction | Serialize a v3 MySQL configuration containing a test password. | The password is absent and generic serialization contains `"***"`. | PASS | Dedicated configuration test. |

### Acceptance Verification

| AC ID | Status | Evidence |
| ----- | ------ | -------- |
| AC1 | TODO | |
| AC2 | TODO | |
| AC3 | TODO | |
| AC4 | TODO | |
| AC5 | TODO | |
| AC6 | TODO | |
| AC ID | Status | Evidence |
| ----- | ------ | ------------------------------------------------------------------------ |
| AC1 | PASS | `Database` enum implementation and focused tests. |
| AC2 | PASS | Driver-specific and unknown-field rejection tests. |
| AC3 | PASS | MySQL/PostgreSQL port-default and password-validation tests. |
| AC4 | PASS | `SecretString` type and redacted generic-serialization test. |
| AC5 | PASS | `cargo test -p torrust-tracker-configuration` (111 tests). |
| AC6 | PASS | Updated v2-to-v3 migration guide; active v2 artifacts deferred to #1980. |

## Risks and Trade-offs

Expand Down
21 changes: 20 additions & 1 deletion ...ssues/open/1978-configuration-overhaul-epic/configuration-v2-to-v3-migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ update, and why.
| `threshold` | `trace_filter` | #889 | DONE |
| No connection ID policy | `[udp_tracker_server] connection_id_validation` | #1136 | DONE |
| Hardcoded IP bans interval | `[udp_tracker_server] ip_bans_reset_interval_in_secs` | #1453 | IN_REVIEW |
| Flat `[core.database]` | Database enum with per-driver config | #1490 | TODO |
| Flat `[core.database]` | Database enum with per-driver config | #1490 | DONE |
| No announce `ip` opt-in | Per-HTTP-tracker opt-in field (TBD) | #1987 | TODO |

## Step 1: Update the schema version
Expand Down Expand Up @@ -226,6 +226,24 @@ password = "db_user_password" # mandatory and non-empty
database = "torrust_tracker"
```

PostgreSQL uses the same component fields and defaults an omitted `port` to
`5432`:

```toml
# v2
[core.database]
driver = "postgresql"
path = "postgresql://postgres:postgres_password@postgres:5432/torrust_tracker"

# v3
[core.database]
driver = "postgresql"
host = "postgres"
user = "postgres"
password = "postgres_password"
database = "torrust_tracker"
```

SQLite retains its filesystem `path`:

```toml
Expand Down Expand Up @@ -283,6 +301,7 @@ Use this checklist to verify your configuration is ready for v3:
- [ ] `public_url` added to trackers and API (optional, recommended for reverse proxies)
- [ ] `connection_id_validation` reviewed in `[udp_tracker_server]` (optional, defaults to `"strict"`)
- [ ] `ip_bans_reset_interval_in_secs` reviewed in `[udp_tracker_server]` (optional, defaults to `86400`)
- [ ] Network database URLs replaced with component fields; database passwords are non-empty

## References

Expand Down
53 changes: 53 additions & 0 deletions docs/pr-reviews/pr-2084-copilot-suggestions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
semantic-links:
skill-links:
- process-copilot-suggestions
related-artifacts:
- .github/skills/dev/pr-reviews/process-copilot-suggestions/SKILL.md
---

<!-- cspell:disable -->

<!-- skill-link: process-copilot-suggestions -->

# PR #2084 Copilot Suggestions Tracking

Source: Copilot PR review threads for https://github.com/torrust/torrust-tracker/pull/2084

Status legend:

- `action`: code/docs change applied
- `no-action`: suggestion reviewed; no code change needed
- `resolved`: thread resolved in PR

## Workflow

1. Download all review threads (including resolved/outdated state and thread IDs).
2. Add one row per thread in the Suggestions table.
3. Process suggestions one by one:
- decide `action` or `no-action`
- if `action`, apply change and validate
- if needed, commit changes
- reply on the PR thread with the fix commit and outcome, or the no-action rationale
- resolve the PR thread

4. Set `Thread State` to `resolved` once resolved in PR.

## Processing Log

- 2026-08-24: Started processing suggestions.
- 2026-08-24: Resolved both suggestions after validation and documented the outcomes below.

## Suggestions

| # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State |
| --- | ----------------------- | ----------------------------------------------- | ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------- | ------ | ------------ |
| 1 | `PRRT_kwDOGp2yqc6brCbf` | `packages/configuration/src/v3_0_0/database.rs` | <https://github.com/torrust/torrust-tracker/pull/2084#discussion_r3842996405> | Prevent Figment from merging the SQLite default path into network database configuration. | action — fixed in `165ac333` with MySQL/PostgreSQL regression coverage. | <https://github.com/torrust/torrust-tracker/pull/2084#discussion_r3843905646> | DONE | RESOLVED |
| 2 | `PRRT_kwDOGp2yqc6brCb-` | `packages/configuration/src/v3_0_0/database.rs` | <https://github.com/torrust/torrust-tracker/pull/2084#discussion_r3842996448> | Make the public SQLite database path constructible and inspectable. | no-action — fields in public enum variants inherit public visibility; `pub` is invalid here. | <https://github.com/torrust/torrust-tracker/pull/2084#discussion_r3843907340> | DONE | RESOLVED |

## Notes

- Keep this file as an audit log of review handling for the PR.
- Prefer concise decisions with explicit rationale.
- If no code changes are needed, explain why in `Decision`.
- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers.
Loading
Loading