fix(docker): fix 5 bugs in Shevek's PHP 8.2 Docker setup - #470
Merged
Conversation
added 3 commits
August 5, 2026 17:14
- Remove DB credential echo from entrypoint (password was printed to docker logs on every container start) - Add env-var fallbacks after .env grep so prod containers can start: composer dump-env prod deletes .env, causing the PDO wait loop to fail for 300s then exit 1 with empty credentials - Fix migrations path: src/Migrations/ does not exist; actual dir is Migrations/ at repo root (migrations were silently skipped) - Add COPY Migrations Migrations/ to Dockerfile (files were absent from the production image entirely) - Update CI from PHP 7.4 to 8.2 and MariaDB 10.1.41 to 12.0.2
- actions/checkout v2 → v4 - actions/cache v1 → v4 (was blocking all jobs on run) - actions/upload-artifact v2 → v4 - actions/setup-node v2-beta → v4 - Replace deprecated set-output syntax with GITHUB_OUTPUT
Following the pattern already used in develop's build-image.yml. SHA pins prevent supply chain attacks via tag mutation. - actions/checkout v4.2.2 → 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 - actions/cache v4 → 0057852bfaa89a56745cba8c7296529d2fc39830 - actions/upload-artifact v4.6.2 → 043fb46d1a93c77aae656e7c1c64a875d1fc6a0a - actions/setup-node v4 → 49933ea5288caeca8642d1e84afbd3f7d6820020 - shivammathur/setup-php v2 → f3e473d116dcccaddc5834248c87452386958240
Neophytis
force-pushed
the
fix/master-docker-entrypoint-bugs
branch
from
August 5, 2026 15:44
7e6f125 to
8faecb5
Compare
thisismeonmounteverest
approved these changes
Aug 5, 2026
| database_name=$(grep '^DB_NAME=' .env | cut -f 2 -d '=') | ||
| database_user=$(grep '^DB_USER=' .env | cut -f 2 -d '=') | ||
| database_password=$(grep '^DB_PASS=' .env | cut -f 2 -d '=') | ||
| database_host="${database_host:-${DB_HOST:-db}}" |
Contributor
There was a problem hiding this comment.
This overrides the information grepped from the .env file just above, doesn't it?
| mysql $database_name -u $database_user -p$database_password -h $database_host < docker/db/geonamesadminunits.sql | ||
| fi | ||
| elif ls -A src/Migrations/*.php > /dev/null 2>&1; then | ||
| elif ls -A Migrations/*.php > /dev/null 2>&1; then |
Contributor
There was a problem hiding this comment.
Master will use the production database on deployments no migrations needed at all.
That also means for master we might want to have a docker setup that stops when APP_ENV isn't set to prod.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 5 bugs introduced in commit 901b23a ("Make master work on php8.2 and update docker setup accordingly"):
echo "new PDO(...)"line printed the full database password todocker logson every container start.envgrep —composer dump-env proddeletes.envin the production image, causing all five credential variables to be empty strings; the PDO wait loop then fails for 300s and exits 1, making production containers unbootablesrc/Migrations/→Migrations/in entrypoint —src/Migrations/does not exist; the condition always evaluated falseCOPY Migrations Migrations/to Dockerfile — migration files were not copied into the production image at allphp-version: 7.4to8.2, and the Behat MariaDB service from10.1.41to12.0.2to match the DockerfileTest plan
docker build --target bewelcome_php -t bewelcome/app .docker logs <container>should not contain any PDO connection stringsAPP_ENV=prodand DB credentials passed as env vars (not via.env)Migrations/directory is present in the built image:docker run --rm --entrypoint sh bewelcome/app -c 'ls Migrations/'