-
Notifications
You must be signed in to change notification settings - Fork 0
feat: [#272] Add HTTPS support with Caddy for all HTTP services #273
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.
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
Conversation
- Add Context Data Preparation Pattern section to template architecture docs - Explain why templates receive pre-processed data instead of raw domain objects - Document port extraction example: handled in Rust, not Tera filters - Update issue spec to use port-as-integer pattern in Caddyfile template - Remove extract_port Tera filter from implementation plan (not needed) This follows existing patterns (e.g., PrometheusContext receives api_port: u16) and maintains consistency with the codebase's approach to template rendering.
- Add Caddyfile.tera template with conditional service blocks - Update docker-compose.yml.tera with Caddy service configuration - Add proxy_network and caddy volumes - Add caddy.md documentation for template usage - Update template-system-architecture.md with directory organization rule - Update issue progress tracking
- Create CaddyProjectGenerator following Project Generator pattern - Create CaddyContext with pre-processed data for template rendering - Create CaddyService struct with domain and port fields - Create CaddyfileRenderer for Caddyfile template processing - Add 14 unit tests covering all HTTPS scenarios - Update issue progress tracking (Phase 3 mostly complete)
Implement Caddy reverse proxy with TLS termination for automatic HTTPS: Phase 1 - Template Creation: - Create Caddyfile.tera template with conditional service blocks - Create caddy.md documentation for template variables - Update docker-compose.yml.tera with Caddy service block - Register templates in CaddyProjectGenerator with 14 unit tests Phase 2 - Configuration DTOs: - Add HttpsSection DTO with admin_email and use_staging fields - Add TlsSection DTO with domain field for service-specific TLS - Extend HttpApiSection, HttpTrackerSection, GrafanaSection with optional tls - Implement validation (has_any_tls_configured, https/tls consistency) - Add Email type in src/shared/email.rs for email format validation - Add DomainName type in src/shared/domain_name.rs for domain validation Phase 3 - Template Rendering Integration: - Create RenderCaddyTemplatesStep for template rendering - Create DeployCaddyConfigStep for Ansible deployment - Create deploy-caddy-config.yml Ansible playbook - Add RenderCaddyTemplates and DeployCaddyConfigToRemote to ReleaseStep enum - Integrate CaddyContext into Docker Compose template rendering - Add CaddyConfigDeployment error variant with actionable help text Manual E2E testing verified: - HTTPS endpoints working for API, Grafana, and HTTP trackers - HTTP→HTTPS redirect (308 Permanent Redirect) - HTTP/2 and HTTP/3 enabled - Caddy Local CA for .local domains Work in progress - remaining phases: - Phase 4: Security workflow updates - Phase 5: Documentation - Phase 6: Automated E2E tests - Phase 7: Schema generation - Phase 8: ADR creation
…S flags in Rust - Refactor TrackerPorts to use constructor with pre-computed flags - Remove HttpTrackerPort struct, use Vec<u16> for ports without TLS - Add needs_ports_section flag to simplify template conditionals - Add http_api_has_tls and grafana_has_tls flags - Filter TLS-enabled ports in Rust instead of Tera template - Simplify docker-compose.yml.tera with cleaner conditionals - Update all tests to use TrackerPorts::new() constructor - Document manual E2E testing procedure in issue spec
…config - Add x-defaults anchor with common service settings (tty, restart, logging) - Apply anchor to all services: caddy, tracker, prometheus, grafana, mysql - Remove ~30 lines of duplicated configuration
Move conditional network selection from Tera template to pre-computed Rust values. This follows the project pattern of keeping templates simple by computing all logic in Rust. Changes: - Rename TrackerPorts to TrackerServiceConfig (more descriptive) - Add networks: Vec<String> field to TrackerServiceConfig - Add compute_networks() method for metrics/database/proxy networks - Add has_prometheus, has_mysql, has_caddy parameters to constructor - Update template to iterate over tracker.networks list - Rename context field from 'ports' to 'tracker' - Keep TrackerPorts type alias for backward compatibility - Update all tests to use new 7-argument constructor
…pose templates - Create CaddyServiceConfig in docker-compose context module for docker-compose.yml.tera - CaddyContext remains in caddy module for Caddyfile.tera - Simplify builder to use boolean flag instead of full CaddyContext - Update docker-compose template to use new caddy variable with network iteration - Rename caddy_config volume to caddy_config_vol to avoid naming conflict
…figuration - Create MysqlServiceConfig in docker-compose context module - Follow same pattern as CaddyServiceConfig (networks field only) - Update docker-compose template to use mysql variable for network iteration - Ensures consistency across all service configurations
- Use Tera whitespace trimming ({%- and -%}) to reduce excessive blank lines
- Generated docker-compose.yml now has cleaner, more readable formatting
- Single blank lines between services, compact network/volume sections
Prevent accidental commits of Rust compiler ICE dump files
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
- Add caddy:2.10 to third-party images matrix in CI - Add SARIF upload step for Caddy vulnerability scanning - Create security scan documentation for Caddy image - Document 4 known vulnerabilities (3 HIGH, 1 CRITICAL) in Go dependencies
ef56958 to
704f153
Compare
- Show HTTPS URLs with configured domains for TLS-enabled services - Separate HTTP trackers into HTTPS (via Caddy) and direct access groups - Display API endpoint with HTTPS indicator when TLS is configured - Add /etc/hosts hint with all TLS domains and instance IP - Show note about internal ports not being directly accessible with TLS - Add Grafana HTTPS URL when TLS is configured
… handling - Task 7.3: Add TLS support for health check API - Task 7.4: Handle localhost-bound services (validation + show command) - Mark task 7.2 as complete
Task 7.3: Health Check API TLS Support - Add TLS configuration to HealthCheckApiConfig domain model - Add HealthCheckApiSection DTO with TLS support - Update ServiceInfo to include health_check_uses_https flag - Add health_check_api service to CaddyContext - Update Caddyfile.tera template for health check reverse proxy - Add TrackerContext health_check_api_bind_address field - Update tracker.toml.tera with [health_check_api] section - Update show command views to display HTTPS indicator for health check - Add tests for health check TLS configuration
- Validation occurs in domain layer during DTO-to-domain conversion - Grafana excluded (bind address hardcoded at port 3000) - Localhost detection: 127.0.0.1 and ::1 only - Add is_localhost_only field to ServiceInfo (not message in URL) - Show 'Internal only' for localhost services (never hide)
…dation (Task 7.4) - Add LocalhostWithTls error to reject localhost + TLS combinations in domain layer - Add is_localhost helper function for detecting 127.0.0.1 and ::1 - Add is_localhost_only fields to ServiceInfo for API, health check, HTTP trackers - Update show command to display "Internal only" for localhost services - Add SSH tunnel hint for localhost-only services - Validation logic in TrackerConfig::validate() to avoid duplication
e40e3ff to
5334429
Compare
- Add subtask 7.5 to fix on_reverse_proxy tracker config bug - Replace misleading 'tls' object with 'domain' + 'use_tls_proxy' fields - Document incremental implementation plan (one service at a time) - Add before/after examples using full manual-https-test.json - Document dependency rule: use_tls_proxy implies on_reverse_proxy - Document future compatibility with per-tracker on_reverse_proxy
- Document issue discovered during HTTPS/Caddy implementation - Explain why per-HTTP-tracker on_reverse_proxy is needed - Propose solution with backward-compatible configuration - Reference deployer workaround (all-or-nothing proxy rule)
- Add 'How to Reproduce' section to task 7.5 - Document step-by-step verification of the problem - Include actual error message from tracker - Link to upstream issue torrust/torrust-tracker#1640
…TTP trackers Step 7.5.1 of HTTPS support implementation: - Replace tls: Option<TlsSection> with domain: Option<String> + use_tls_proxy: Option<bool> in HttpTrackerSection DTO - Update HttpTrackerConfig domain type with domain: Option<DomainName> and use_tls_proxy: bool - Add validation: use_tls_proxy: true requires domain to be present (TlsProxyWithoutDomain error) - Add validation: use_tls_proxy: true with localhost bind address is rejected - Update tracker.toml.tera template to use dynamic on_reverse_proxy based on any_http_tracker_uses_tls_proxy() - Update Caddy template context to filter HTTP trackers by use_tls_proxy - Update show command ServiceInfo for HTTP tracker display - Update envs/manual-https-test.json for HTTP trackers only - Add unit tests for new validation logic The on_reverse_proxy setting is now conditional: true only when ANY HTTP tracker has use_tls_proxy: true, false otherwise. This fixes the hardcoded value that was always true.
Step 7.5.2: Apply the same TLS configuration pattern to the Tracker REST API (HttpApiSection/HttpApiConfig) as was done for HTTP trackers in Step 7.5.1. Changes: - Replace `tls: Option<TlsSection>` with `domain: Option<String>` and `use_tls_proxy: Option<bool>` in HttpApiSection DTO - Update HttpApiConfig domain type with `domain: Option<DomainName>` and `use_tls_proxy: bool` fields - Add `uses_tls_proxy()` and `tls_domain()` helper methods to HttpApiConfig - Update TrackerConfig::check_localhost_with_tls() validation - Update show command to use new fields for API endpoint display - Update all test code and doc comments to use new structure - Update envs/manual-https-test.json with new HTTP API configuration This is part of the incremental migration to remove the TlsSection type and use explicit domain + use_tls_proxy fields for clearer semantics.
…ck API This commit migrates the Health Check API from the old `tls: Option<TlsSection>` pattern to the new `domain: Option<String>` + `use_tls_proxy: Option<bool>` pattern, following the same approach used for HTTP trackers and HTTP API. Changes: - Update HealthCheckApiSection DTO with new fields - Update HealthCheckApiConfig domain type with domain and use_tls_proxy - Add validation that use_tls_proxy requires a domain - Update TrackerConfig localhost+TLS validation to use use_tls_proxy - Update show command to use tls_domain() method - Update Caddy template context documentation - Update all test fixtures and doc examples - Update envs/manual-https-test.json with new format This is part of the incremental migration to remove the TlsSection type and standardize on the simpler domain + use_tls_proxy pattern across all services. Next: Grafana (Step 7.5.4).
Step 7.5.4 of configuration restructuring: - Replace tls: Option<TlsSection> with domain: Option<String> + use_tls_proxy - Update GrafanaSection DTO with new fields and validation - Update GrafanaConfig domain type with new constructor signature - Update docker compose context builder to use use_tls_proxy() - Update show command tests for Grafana - Update envs/manual-https-test.json to use new configuration format - Update spec document marking Step 7.5.4 complete This change simplifies the configuration model for Grafana, making TLS proxy enablement explicit rather than inferred from the presence of a tls section. Note: Grafana has no configurable bind address, so no localhost+TLS validation is needed.
Step 7.5.5: Cleanup and Final Verification After migrating all services (HTTP Trackers, Tracker REST API, Health Check API, and Grafana) from tls: Option<TlsSection> to domain: Option<String> + use_tls_proxy: Option<bool>, the TlsSection type and domain::tls module are no longer used. This cleanup removes: - TlsSection struct and impl from application layer DTOs - TlsSection tests - domain::tls module (TlsConfig type) - TlsSection re-export from config module All 1848 tests pass after removal.
- Create ServiceEndpoint type for HTTP/HTTPS endpoint configuration - Update RunningServicesValidator to use ServiceEndpoint instead of ports - Add domain resolution for HTTPS (like curl --resolve) - Accept self-signed certificates for .local domains - Update test command handler to build endpoints from tracker config - Update E2E tests to use ServiceEndpoint pattern When TLS is enabled via Caddy, the test command now: - Uses HTTPS URLs with configured domains - Resolves domains locally to VM IP (no DNS needed) - Accepts self-signed certs for .local domains
…server IP - Store validated Url instead of port/path/tls fields - http() takes SocketAddr and returns Result for URL validation - https() takes (domain, path, server_ip) and returns Result - Add InvalidServiceEndpointUrl error type - url() returns &Url reference instead of building string - Add socket_addr() method for convenience - domain() returns Option<&str> instead of Option<&DomainName> - Remove TlsEndpointConfig struct (no longer needed) - Update RunningServicesValidator to get server_ip from endpoint - Update TestCommandHandler to pass server_ip when building endpoints
- Create docs/user-guide/services/https.md with complete HTTPS setup guide - Document domain + use_tls_proxy configuration pattern for each service - Add Let's Encrypt production vs staging documentation - Add configuration examples for various HTTPS patterns - Add troubleshooting section for common certificate issues - Update services README to include HTTPS as an available service - Update main user guide README with HTTPS service reference - Update grafana.md with TLS proxy configuration fields - Regenerate JSON schema to reflect current implementation
Phase 9: Documentation ADRs created: - caddy-for-tls-termination.md: Why Caddy v2.10 was chosen over Pingoo/nginx - per-service-tls-configuration.md: Why domain+use_tls_proxy pattern vs nested tls section - uniform-http-tracker-tls-requirement.md: Why all HTTP trackers must use same TLS setting Key rationale documented: - Caddy: WebSocket support (Pingoo failed), automatic HTTPS, simple config - use_tls_proxy naming: Avoids confusion with tracker's native TslConfig - Uniform TLS: Tracker's on_reverse_proxy is global, not per-tracker This completes issue #272 - all 9 phases are now done.
…roxy Add explicit header_up X-Forwarded-For configuration to Caddyfile.tera instead of relying on Caddy's default behavior. Rationale: - While Caddy sets X-Forwarded-For by default, we explicitly configure it to guard against future changes in Caddy's default behavior - This header is critical for the tracker's on_reverse_proxy mode to correctly identify client IPs for peer tracking in BitTorrent swarms - Explicit configuration makes the intent clear and self-documenting The X-Forwarded-For header is required by the tracker when running behind a reverse proxy to record the correct peer IP addresses.
The X-Forwarded-For header is critical only for HTTP trackers where the
tracker needs the real client IP to record correct peer addresses in
the swarm. Other services (API, health check, Grafana) don't require
explicit header configuration as Caddy's default forwarding is sufficient.
This change:
- Keeps explicit header_up X-Forwarded-For {remote_host} for HTTP trackers
- Removes explicit header for API, health check, and Grafana endpoints
- Updates header comment to clarify it's for HTTP trackers specifically
|
ACK 28336e0 |
Summary
Add HTTPS support with Caddy reverse proxy for automatic TLS termination on all HTTP services (Tracker API, HTTP Trackers, Grafana, Health Check API).
Closes #272
What's Implemented
Phase 1: Template Creation ✅
templates/caddy/Caddyfile.terawith conditional service blocksdocs/contributing/templates/caddy.mddocumenting template variablestemplates/docker-compose/docker-compose.yml.terawith Caddy service blockCaddyProjectGeneratorwith 14 unit testsPhase 2: Configuration DTOs ✅
HttpsSectionDTO withadmin_emailanduse_stagingfieldsdomainanduse_tls_proxyfields (replaced nestedtlssection)HttpApiSection,HttpTrackerSection,GrafanaSection,HealthCheckApiSectionwith TLS optionshas_any_tls_configured, https/tls consistency, uniform HTTP tracker TLS)Emailtype insrc/shared/email.rsfor email format validationDomainNametype insrc/shared/domain_name.rsfor domain validationPhase 3: Template Rendering Integration ✅
RenderCaddyTemplatesStepfor template renderingDeployCaddyConfigStepfor Ansible deploymentdeploy-caddy-config.ymlAnsible playbookRenderCaddyTemplatesandDeployCaddyConfigToRemotetoReleaseStepenumCaddyContextinto Docker Compose template renderingCaddyConfigDeploymenterror variant with actionable help textPhase 4: Security Workflow Updates ✅
caddy:2.10to Docker security scan workflow matrixPhase 5: Documentation ✅
docs/user-guide/services/https.mdwith complete HTTPS setup guidedocs/user-guide/services/README.mdwith HTTPS service entrydocs/user-guide/services/grafana.mdwith TLS proxy fields documentationschemas/environment-config.json)Phase 6: E2E Testing ✅ (Manual)
Manual E2E testing verified:
.localdomainsPhase 7: Schema Generation ✅
https,domain,use_tls_proxyfieldsPhase 8: ADRs (Decision Documentation) ✅
Three ADRs created:
docs/decisions/caddy-for-tls-termination.md) - Why Caddy was chosen over Pingoo/nginxdocs/decisions/per-service-tls-configuration.md) - Whydomain + use_tls_proxypattern was chosen, including naming rationaledocs/decisions/uniform-http-tracker-tls-requirement.md) - Why all HTTP trackers must use same TLS settingWhat's Remaining
Configuration Example
{ "https": { "admin_email": "admin@example.com", "use_staging": true }, "tracker": { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "secret", "domain": "api.tracker.local", "use_tls_proxy": true }, "http_trackers": [ { "bind_address": "0.0.0.0:7070", "domain": "http1.tracker.local", "use_tls_proxy": true } ] }, "grafana": { "admin_user": "admin", "admin_password": "admin", "domain": "grafana.tracker.local", "use_tls_proxy": true } }Testing
Key Design Decisions
X-Forwarded-For only for HTTP trackers: The explicit
header_up X-Forwarded-For {remote_host}is only set for HTTP trackers (not API, health check, or Grafana) because it's critical only for peer IP tracking in BitTorrent swarms.use_tls_proxynaming: Named to avoid confusion with tracker's native TLS support (TslConfig), reserving namespace for futureuse_native_tlsoption.Notes for Reviewers
Key files to review:
templates/caddy/Caddyfile.tera- Caddy template with explicit X-Forwarded-For for HTTP trackerssrc/application/command_handlers/create/config/https.rs- HTTPS configuration DTOssrc/application/command_handlers/release/handler.rs- Release workflow integrationsrc/infrastructure/templating/caddy/- Caddy template renderingdocs/decisions/- Three new ADRs documenting architectural decisions