refactor: Add environment configuration and initialization script for Docker setup, including user permissions and Composer dependency management. #109
Conversation
… Docker setup, including user permissions and Composer dependency management.
WalkthroughThe changes introduce enhanced environment variable management, improved Docker build configurability, and refined directory handling for a Yii2 FrankenPHP setup. Key updates include a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DockerContainer
participant InitScript
participant Composer
participant Supervisord
User->>DockerContainer: Start container
DockerContainer->>InitScript: Run /usr/local/bin/init.sh
InitScript->>InitScript: Check for composer.json and vendor/
alt Needs Composer install
InitScript->>Composer: Install dependencies (prod/dev based on YII_ENV)
Composer-->>InitScript: Installation complete
InitScript->>InitScript: Set ownership of /app/vendor
end
InitScript->>Supervisord: Start supervisord
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (17)
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 #109 +/- ##
===========================================
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docker-compose.frankenphp.yml (1)
4-9: Build args won’t propagate without matchingARGlines in the Dockerfile
USER_ID,GROUP_ID,USER_NAME,GROUP_NAMEare forwarded here, but the current Dockerfile lacks correspondingARGdeclarations.
Unless they are added (see Dockerfile comment), the values will be ignored and the build will still default to root/www-data.No change needed in Compose once the Dockerfile is fixed.
🧹 Nitpick comments (6)
.env (1)
1-9: Commit a template, not the real.envShipping a concrete
.envin VCS invites accidental production misuse and UID/GID clashes on contributors’ machines. Best practice is to:
- rename this file to
.env.example(documenting default values);- add
.envto.gitignore;- instruct users to copy & adjust locally.
Would you consider that adjustment?
docker/init.sh (3)
6-13: Run Composer as the target user to avoid post-installchownInstead of running
composeras root and then recursivelychown-ing:su -s /bin/bash "$USER_NAME" -c "composer install …"This preserves file permissions, is faster for large vendor trees, and removes the need for the separate
chown.
14-16: Quote variable expansions inchownIf you keep the current approach, quote variables to avoid word-splitting:
- chown -R $USER_NAME:$GROUP_NAME /app/vendor + chown -R "${USER_NAME}:${GROUP_NAME}" /app/vendor
20-21: Considerexec "$@"pattern for extensibilityHard-coding
supervisordmakes it impossible to override the entrypoint easily. A more flexible pattern:exec "$@"and set
CMD ["supervisord","-c","/etc/supervisor/supervisord.conf"]in the Dockerfile.docker/frankenphp/Dockerfile (1)
51-57: Duplicatemkdir -p /var/run& doublechown– consolidate the layers
/var/runis created and chowned in two consecutive layers (lines 51 and 53-56). This burns an extra layer and adds a few MB to the final image.-RUN mkdir -p /var/run && chown -R www-data:www-data /var/run - -RUN mkdir -p /var/run /app/runtime /app/web/assets /app/runtime/cache /app/runtime/logs && \ +RUN mkdir -p /var/run /app/runtime /app/web/assets /app/runtime/cache /app/runtime/logs && \ - chown -R ${USER_NAME}:${GROUP_NAME} /var/run /app && \ + chown -R ${USER_NAME}:${GROUP_NAME} /var/run /app && \One
RUNis enough; drop the first or merge them.
Minor, but keeps the image slimmer.docker-compose.frankenphp.yml (1)
27-28: Quote style for boolean envs
YII_DEBUGis a boolean flag but is quoted as a string.
Docker passes everything as strings anyway, but if some shell scripts expecttrue/falsewithout quotes, consider:YII_DEBUG: ${YII_DEBUG:-false} YII_ENV: ${YII_ENV:-prod}Purely stylistic – up to you.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env(1 hunks)codeception.yml(1 hunks)docker-compose.frankenphp.yml(2 hunks)docker/frankenphp/Dockerfile(1 hunks)docker/init.sh(1 hunks)docker/php/php.ini(1 hunks)phpstan.neon(1 hunks)runtime/.gitignore(0 hunks)
💤 Files with no reviewable changes (1)
- runtime/.gitignore
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#97
File: docs/installation.md:5-8
Timestamp: 2025-07-09T14:52:41.210Z
Learning: The Yii2 framework has multiple active branches including the 2.0.x branch (maintenance/LTS) and the 22.0 branch (development). When specifying version requirements, "2.0.53+ or 22.x" is accurate and should not be changed to only reference the 2.x branch.
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#97
File: docs/installation.md:5-8
Timestamp: 2025-07-09T14:52:41.210Z
Learning: The Yii2 framework has multiple active branches including the 2.0.x branch (maintenance/LTS) and the 22.0 branch (development). When specifying version requirements, "2.0.53+ or 22.x" is accurate and should not be changed to only reference the 2.x branch.
.env (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/php/php.ini (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/init.sh (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.frankenphp.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.
🪛 Shellcheck (0.10.0)
docker/init.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
⏰ 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 (3)
codeception.yml (1)
6-9: Test artefacts now land inpublic/– double-check exposureMoving
outputand_envsunderpublic/runtimemeans HTML coverage reports, failure screenshots, etc., are potentially reachable over HTTP. If that is not intentional, point them back to a private folder or deny access in your web server configuration.docker/php/php.ini (1)
12-24: Validate JIT & memory settings for your base image
opcache.jit=tracingand the 256 M OPCache allocation only make sense on PHP ≥ 8.1 compiled with JIT. If the FrankenPHP image is built without JIT support these directives are silently ignored. Confirm the image capabilities and trim unused directives to keepphpinfo()tidy.docker/frankenphp/Dockerfile (1)
58-65: Ensureinit.shhas proper signal forwardingSwitching the container’s entrypoint from
supervisordto a shell script is fine, but make sureinit.sh:
- Starts with
#!/usr/bin/env bash(orsh) and is executable (chmod already done ✔).- Uses
exec supervisord …so that PID 1 issupervisord, ensuring proper signal handling (SIGTERM,SIGQUIT) duringdocker stop.Please verify the script; otherwise containers may hang on shutdown.
| - tests/Unit | ||
|
|
||
| tmpDir: %currentWorkingDirectory%/runtime | ||
| tmpDir: %currentWorkingDirectory%/public/runtime |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid storing PHPStan cache inside the web-root
public/runtime is typically web-accessible (the document root for FrankenPHP). Exposing PHPStan’s .tmp artefacts may leak class names, stack traces or other internal information if the web server is misconfigured. A safer location would be outside public, e.g.:
- tmpDir: %currentWorkingDirectory%/public/runtime
+ tmpDir: %currentWorkingDirectory%/runtime/phpstanPlease relocate or ensure the directory is not served.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tmpDir: %currentWorkingDirectory%/public/runtime | |
| tmpDir: %currentWorkingDirectory%/runtime/phpstan |
🤖 Prompt for AI Agents
In phpstan.neon at line 17, the tmpDir is set to a path inside the web-root
(public/runtime), which risks exposing PHPStan cache files publicly. Change the
tmpDir setting to a directory outside the public folder, such as a sibling
directory to public, to prevent accidental web access to these internal files.
| # Global PHP configuration for the Docker container | ||
| date.timezone = UTC | ||
| display_errors = Off | ||
| expose_php = Off | ||
| memory_limit = 512M | ||
| post_max_size = 150M | ||
| session.auto_start = Off |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Configuration clash: post_max_size 150 M vs upload_max_filesize 15 M
upload_max_filesize is capped at 15 M, effectively nullifying the 150 M post_max_size. Either raise upload_max_filesize or drop the oversized post limit to avoid confusion.
🤖 Prompt for AI Agents
In docker/php/php.ini lines 1 to 7, there is a mismatch between post_max_size
set to 150M and upload_max_filesize which defaults to 15M, limiting uploads to
15M despite the larger post_max_size. To fix this, explicitly set
upload_max_filesize to a value equal to or greater than 150M to match
post_max_size, or reduce post_max_size to 15M or less to align with
upload_max_filesize and avoid configuration confusion.
| # Check if composer.json exists and vendor directory doesn't exist | ||
| if [ -f "/app/composer.json" ] && [ ! -d "/app/vendor" ]; then | ||
| echo "Installing Composer dependencies..." | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing shebang breaks shell-lint and may execute under /bin/sh unexpectedly
Add an explicit shebang and fail-fast flags:
+#/usr/bin/env bash
+set -euo pipefailThis satisfies SC2148 and aborts on errors.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
In docker/init.sh at the beginning of the file (lines 1 to 4), add a shebang
line such as #!/bin/bash -e to explicitly specify the shell interpreter and
enable fail-fast behavior. This will prevent the script from running under
/bin/sh unexpectedly and ensure it aborts on errors, satisfying shell-lint
SC2148.
… for better visibility.
…oved structure and clarity.
1 participant
Summary by CodeRabbit
New Features
Configuration
Chores