Skip to content

fix(mysql): [#410] Fix multiple MySQL configuration issues#424

Merged
josecelano merged 4 commits intotorrust:mainfrom
josecelano:410-fix-multiple-mysql-configuration-issues
Apr 8, 2026
Merged

fix(mysql): [#410] Fix multiple MySQL configuration issues#424
josecelano merged 4 commits intotorrust:mainfrom
josecelano:410-fix-multiple-mysql-configuration-issues

Conversation

@josecelano
Copy link
Copy Markdown
Member

@josecelano josecelano commented Apr 8, 2026

Summary

Fixes three related MySQL configuration bugs discovered during the Hetzner demo deployment (#405).

Closes #410

Bug 1 — MySQL DSN hardcoded in tracker.toml with no URL-encoding

Problem: The tracker config template interpolated raw credentials directly into the DSN URL. Passwords with URL-reserved characters (@, :, /, +) produced malformed DSNs and broke MySQL connectivity. The plaintext password also ended up in a mounted config file instead of an env var.

Fix:

  • Added percent-encoding crate; built a custom USERINFO_ENCODE AsciiSet that preserves RFC 3986 unreserved chars (so tracker_user is not over-encoded)
  • DSN is now percent-encoded in Rust (EnvContext::new_with_mysql) and stored as TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH in .env
  • docker-compose.yml.tera injects the new env var into the tracker container
  • tracker.toml.tera no longer contains the DSN; a static comment explains the injection
  • MysqlTemplateConfig removed entirely

Bug 2 — MySQL root password not configurable

Problem: Root password was unconditionally derived as {app_password}_root, giving users no way to supply their own value and making it entirely predictable.

Fix:

  • Added optional root_password field to the environment config JSON schema
  • Added generate_random_password() in src/shared/secrets/random.rs (32-char, mixed charset, satisfies MySQL MEDIUM policy)
  • Password is generated once at environment creation time (in TryFrom<DatabaseSection>), not at render time, so it stays stable across re-renders
  • create_mysql_contexts no longer contains any derivation logic

Bug 3 — No validation that MySQL app username is not "root"

Problem: The MySQL Docker image rejects MYSQL_USER=root, but the deployer only validated for an empty username, deferring the error to Docker startup time.

Fix:

  • Added ReservedUsername variant to MysqlConfigError with an actionable help() message
  • MysqlConfig::new() rejects "root" immediately with a clear error

Testing

  • Added unit tests for all three fixes
  • All 2314 tests pass (cargo test)
  • Full E2E LXD deployment verified with password tracker_p@ss!word#1 — confirmed @ → %40, # → %23 in the rendered .env
  • All linters pass (cargo run --bin linter all)
  • cargo machete reports no unused dependencies
  • Pre-commit checks pass (./scripts/pre-commit.sh)

Checklist

  • Pre-commit checks pass
  • MysqlConfig::new() rejects "root" with ReservedUsername error
  • root_password is configurable and generated at creation time when omitted
  • DSN is percent-encoded and injected via env var override
  • tracker.toml no longer contains the database password
  • cargo machete reports no unused dependencies

@josecelano
fix: [torrust#410] reject 'root' as MySQL app username (Bug 3)
2ec8f1e
@josecelano
feat(mysql): make root_password configurable, generate at creation time
81ed6d1 
Bug 2 fix from issue torrust#410.

- Add optional root_password field to environment config JSON schema
- MysqlConfig.root_password is Password (non-optional); domain always has a value
- MysqlConfigRaw.root_password is Option<Password> for backward compat with
  persisted environments that pre-date this field
- Add src/shared/secrets/random.rs: generate_random_password() -> Password
  using rand::rng(), mixed charset (lower+upper+digit+symbol), length 32,
  satisfies MySQL validate_password MEDIUM policy
- Generate root_password once in TryFrom<DatabaseSection> at environment
  creation time (not at render time) so it is stable across re-renders
- Remove format!("{password}_root") derivation from create_mysql_contexts
- Update spec docs/issues/410-bug-multiple-mysql-configuration-issues.md
  to reflect the actual implementation
@josecelano
feat(mysql): move DSN to env var override with percent-encoding (Bug 1)
d465611 
Phase 3 fix from issue torrust#410.

- Add percent-encoding = "2.0" to Cargo.toml
- TrackerServiceConfig: add optional database_path field (Some for MySQL, None
  for SQLite) with serde skip_serializing_if so it only appears in template
  context for MySQL
- EnvContext::new_with_mysql: percent-encode username and password using a custom
  USERINFO_ENCODE AsciiSet (encodes @, :, /, #, ?, %, + but NOT unreserved chars
  like _ so tracker_user stays tracker_user, not tracker%5Fuser)
- Build full MySQL DSN and store in tracker.database_path
- EnvContext::new_with_mysql: add mysql_host (String) and mysql_port (u16) params
  so the DSN can be constructed without needing domain types at the infra layer
- templates/docker-compose/.env.tera: add
  TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__PATH inside the Tracker Service
  Configuration section (inside {%- if mysql %} block); move before MySQL section
- templates/docker-compose/docker-compose.yml.tera: inject the new env var into
  the tracker service environment section, conditionally on {%- if mysql %}
- templates/tracker/tracker.toml.tera: remove mysql path = line; replace with a
  comment explaining the env var override (no Tera tag needed — comment is static)
- TrackerContext: remove MysqlTemplateConfig struct and mysql field entirely; the
  MySQL case only needs database_driver = mysql now
- Add tests: percent-encoded DSN, alphanumeric DSN, database_path None for SQLite
- Update project_generator.rs test: hardcoded template updated and assertions
  changed to confirm driver = mysql and absence of path/password lines
- Update issue doc: mark all Phase 1-4 items [x]; add Phase 5 status note
- 2314 tests pass
@josecelano
chore(mysql): fix linting issues from Phase 5
afabd0c 
- cspell: add USERINFO, userinfo, CSPRNG, plainpassword to project-words.txt
- clippy: use map_or_else instead of map().unwrap_or_else() in tracker_core_section.rs
- clippy: backtick MySQL and SQLite in doc comments (doc_markdown)
- clippy: add #[allow(clippy::too_many_arguments)] to new_with_mysql
- clippy: change mysql_host and host from String to &str (needless_pass_by_value)
- clippy: add # Panics section to generate_random_password docs
- clippy: replace redundant closures with char::is_uppercase/is_lowercase
- rustfmt: run cargo fmt
- docs: mark Phase 5 and all acceptance criteria as complete in issue torrust#410
@josecelano josecelano self-assigned this Apr 8, 2026
@josecelano
Copy link
Copy Markdown
Member Author

josecelano commented Apr 8, 2026

ACK afabd0c

@josecelano josecelano merged commit a962e4c into torrust:main Apr 8, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@josecelano josecelano

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Multiple MySQL Configuration Issues in Tracker Deployer

1 participant

@josecelano