-
-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add SSL support with configuration updates for Apache
, FrankenPHP
and Nginx
, including certificate files.
#108
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
…enPHP` and `Nginx`, including certificate files.
WalkthroughThis change introduces explicit SSL/HTTPS support across Apache, Nginx, and FrankenPHP server configurations using mkcert-generated certificates. It updates Docker Compose files to map new HTTPS ports, revises server configuration files to enforce HTTP-to-HTTPS redirection, adds HSTS headers, and adjusts documentation and comments for accuracy and clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Server (Apache/Nginx/FrankenPHP)
participant App
User->>Browser: Enter http://localhost
Browser->>Server (HTTP): Request on port 80/8081/8082
Server (HTTP)->>Browser: Redirect to HTTPS (port 443/8443/8444/8445)
Browser->>Server (HTTPS): Request on HTTPS port
Server (HTTPS)->>App: Serve static or route to index.php as needed
App-->>Server (HTTPS): Response
Server (HTTPS)-->>Browser: Encrypted response (HSTS header set)
Possibly related PRs
Suggested labels
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #108 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 36 36
===========================================
Files 23 23
Lines 621 621
===========================================
Hits 621 621 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
docker-compose.frankenphp.yml (1)
14-16
: Consider exposing HTTPS port in documentation for discoverabilityThe new mappings (
8081:80
,8444:443
,8444:443/udp
) look fine and avoid collisions with Apache (8080/8443
) and Nginx (8082/8445
).
However, the README still lists only the HTTP endpoint for FrankenPHP (http://localhost:8081/
). Adding the HTTPS URL (https://localhost:8444/
) will save users a round-trip of guessing.No change to the Compose file itself—just a doc follow-up.
docker-compose.nginx.yml (1)
12-13
: Expose UDP 443 only if HTTP/3 is enabledYou expose TCP 443 but omit
8445:443/udp
.
If the bundled Nginx image is compiled with--with-http_v3_module
, consider adding the UDP mapping so QUIC/HTTP-3 works out of the box:- - '8445:443' + - '8445:443' + - '8445:443/udp'Otherwise, leaving it out is fine—just be explicit in the Nginx Dockerfile/README that HTTP-3 isn’t enabled.
docker/frankenphp/Dockerfile (1)
47-48
: Keep the queue worker line commented but add a clarifying noteThe comment capitalization change is harmless.
Given past confusion (see learning note), consider expanding the comment to state why it’s commented:-#COPY docker/frankenphp/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf +# Uncomment the following line **only** if you install yii2-queue +#COPY docker/frankenphp/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.confREADME.md (1)
124-128
: List HTTPS URLs alongside HTTP for consistencyUsers may miss that HTTPS is available because only HTTP endpoints are documented. Suggest adding twin lines:
-# For FrankenPHP -http://localhost:8081/ +# For FrankenPHP +http://localhost:8081/ +https://localhost:8444/ -# For Nginx -http://localhost:8082/ +# For Nginx +http://localhost:8082/ +https://localhost:8445/ + +# For Apache +https://localhost:8443/docker/frankenphp/Caddyfile (1)
7-9
: Consider parameterizing localhost references for production flexibility.The configuration hardcodes
localhost
references which may need adjustment for production environments with different hostnames.Consider using environment variables or template substitution for hostname configuration to improve deployment flexibility.
Also applies to: 61-61
docker/nginx/default.conf (1)
7-7
: Consider parameterizing hardcoded localhost references.The configuration hardcodes
localhost
references which may need adjustment for different deployment environments.Consider using environment variables or nginx variables to make the hostname configurable for different deployment scenarios.
Also applies to: 18-19
docker/apache/vhost.conf (3)
48-66
: Consider simplifying the complex rewrite rules for better maintainability.The rewrite rules are comprehensive but quite complex with 5 distinct steps. While functionally correct for Yii2, this complexity might make maintenance challenging.
Consider consolidating some of the rewrite conditions or adding more detailed comments explaining the purpose of each step to improve maintainability.
45-46
: Remove debug logging in production.The rewrite trace logging is enabled for debugging but should be disabled in production environments to avoid performance impact and log verbosity.
Consider making the LogLevel configurable or adding a comment reminder to disable debug logging in production.
26-26
: Consider parameterizing hardcoded localhost references.The configuration hardcodes
localhost
references which may need adjustment for different deployment environments.Consider using Apache variables or environment substitution to make the hostname configurable for different deployment scenarios.
Also applies to: 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
composer.lock
is excluded by!**/*.lock
docker/ssl/localhost-key.pem
is excluded by!**/*.pem
docker/ssl/localhost.pem
is excluded by!**/*.pem
📒 Files selected for processing (11)
README.md
(2 hunks)docker-compose.frankenphp.yml
(1 hunks)docker-compose.nginx.yml
(1 hunks)docker-compose.yml
(1 hunks)docker/apache/Dockerfile
(2 hunks)docker/apache/vhost.conf
(1 hunks)docker/frankenphp/Caddyfile
(3 hunks)docker/frankenphp/Dockerfile
(3 hunks)docker/nginx/Dockerfile
(2 hunks)docker/nginx/default.conf
(4 hunks)docker/nginx/nginx.conf
(0 hunks)
💤 Files with no reviewable changes (1)
- docker/nginx/nginx.conf
🧰 Additional context used
🧠 Learnings (5)
docker/nginx/Dockerfile (1)
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#102
File: docker/apache/Dockerfile:0-0
Timestamp: 2025-07-10T13:59:10.805Z
Learning: In docker/apache/Dockerfile, the queue.conf copy command is intentionally commented out because yii2-queue is optional. Users should uncomment the line `COPY docker/apache/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf` only if they install yii2-queue.
docker-compose.yml (1)
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#102
File: docker/apache/Dockerfile:0-0
Timestamp: 2025-07-10T13:59:10.805Z
Learning: In docker/apache/Dockerfile, the queue.conf copy command is intentionally commented out because yii2-queue is optional. Users should uncomment the line `COPY docker/apache/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf` only if they install yii2-queue.
docker/frankenphp/Dockerfile (1)
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#102
File: docker/apache/Dockerfile:0-0
Timestamp: 2025-07-10T13:59:10.805Z
Learning: In docker/apache/Dockerfile, the queue.conf copy command is intentionally commented out because yii2-queue is optional. Users should uncomment the line `COPY docker/apache/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf` only if they install yii2-queue.
docker/apache/Dockerfile (1)
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#102
File: docker/apache/Dockerfile:0-0
Timestamp: 2025-07-10T13:59:10.805Z
Learning: In docker/apache/Dockerfile, the queue.conf copy command is intentionally commented out because yii2-queue is optional. Users should uncomment the line `COPY docker/apache/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf` only if they install yii2-queue.
docker/apache/vhost.conf (1)
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#102
File: docker/apache/Dockerfile:0-0
Timestamp: 2025-07-10T13:59:10.805Z
Learning: In docker/apache/Dockerfile, the queue.conf copy command is intentionally commented out because yii2-queue is optional. Users should uncomment the line `COPY docker/apache/supervisord/conf.d/queue.conf /etc/supervisor/conf.d/queue.conf` only if they install yii2-queue.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: codeception / PHP 8.3-windows-latest
- GitHub Check: test-frankenphp
- GitHub Check: test-frankenphp
🔇 Additional comments (13)
docker-compose.yml (1)
14-14
: Verify that 8443 isn’t already bound by another service on host
'8443:443'
collides with nothing inside this repo, but many developers run local services (e.g., Traefik, Kind, minikube) that default to 8443.
If avoiding accidental clashes is a priority, consider bumping Apache to the next free slot (e.g., 8446) or documenting the assumption in the README.README.md (1)
41-41
: 👍 Good call-out for SSL supportThe new bullet clearly advertises HTTPS availability.
docker/nginx/Dockerfile (1)
3-31
: LGTM - Stylistic improvements enhance consistency.The comment capitalization updates improve code readability and maintain consistency across the Dockerfile without affecting functionality.
docker/apache/Dockerfile (2)
3-6
: LGTM - Proper SSL module enablement and port configuration.The SSL module enablement and port configuration are correctly implemented:
- Enables necessary Apache modules (ssl, rewrite, headers, mime)
- Configures ports.conf for both HTTP (80) and HTTPS (443) with SSL
- Follows Apache best practices for SSL setup
8-35
: LGTM - Comment improvements and proper configuration copying.The comment capitalization updates maintain consistency, and the configuration file copying and supervisord setup remain unchanged and appropriate.
docker/frankenphp/Caddyfile (2)
6-22
: LGTM - Well-configured HTTPS server with proper security headers.The HTTPS configuration is properly implemented:
- Correct TLS certificate specification for mkcert
- Appropriate HSTS header with 1-year max-age and includeSubDomains
- Maintains existing security headers and PHP processing
58-62
: LGTM - Proper HTTP to HTTPS redirect implementation.The HTTP redirect is correctly configured to permanently redirect all HTTP traffic to HTTPS, preserving the URI path.
docker/nginx/default.conf (3)
1-8
: LGTM - Proper HTTP to HTTPS redirect configuration.The HTTP server block correctly redirects all HTTP traffic to HTTPS, preserving the request URI and using a permanent redirect (301).
17-26
: Excellent SSL security configuration following modern best practices.The SSL configuration is well-implemented:
- Uses modern TLS protocols (1.2, 1.3)
- Employs secure cipher suites with forward secrecy
- Proper session caching and timeout settings
- Lets clients prefer cipher order (ssl_prefer_server_ciphers off) for better compatibility
36-36
: LGTM - Proper HSTS header and PHP SSL detection.The HSTS header with 1-year max-age and includeSubDomains enhances security, and setting
fastcgi_param HTTPS on
ensures PHP applications can properly detect SSL connections.Also applies to: 57-57
docker/apache/vhost.conf (3)
7-15
: LGTM - Excellent static file optimization.The FilesMatch directive properly optimizes static file serving by:
- Removing ETags to prevent unnecessary requests
- Disabling PHP processing for static assets
- Ensuring Apache serves these files directly for better performance
23-26
: LGTM - Proper HTTP to HTTPS redirect implementation.The redirect configuration correctly redirects all HTTP traffic to HTTPS while preserving the request URI.
40-43
: LGTM - Proper SSL configuration for Apache.The SSL configuration is correctly implemented with appropriate certificate file paths matching the mkcert certificate locations used across other services.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style