Fixed #8884: Fixed alpine image build #8885
Conversation
…ror in vendor folder and laravel.log file
|
💖 Thanks for this pull request! 💖 We use semantic commit messages to streamline the release process and easily generate changelogs between versions. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix if it doesn't have one already. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
There was a problem hiding this comment.
This looks good, thank you for the fix! I had a few small questions I wanted to make sure I understood before I merge your PR though - once you can answer those, I'll make sure to get this merged and released! In the meantime, I'm going to try to do a docker build against this PR and see if it gets any further than the previous one.
| @@ -23,6 +23,8 @@ RUN apk add --update --no-cache \ | |||
| php7-fileinfo \ | |||
| php7-simplexml \ | |||
| php7-session \ | |||
| php7-dom \ | |||
| php7-xmlwriter \ | |||
There was a problem hiding this comment.
I just confirmed; the current Alpine Dockerfile does break during composer install because of the lack of these two packages.
Dockerfile.alpine
Outdated
| @@ -45,7 +47,8 @@ WORKDIR /var/www/html | |||
|
|
|||
| COPY docker/docker.env /var/www/html/.env | |||
|
|
|||
| RUN chown -R apache:apache /var/www/html | |||
| RUN mkdir -p "/var/www/html/vendor" \ | |||
There was a problem hiding this comment.
The existing Alpine Dockerfile doesn't do this, and Composer seems to still work OK. Is your thinking here that we want to pre-create the directory so the following chown will make the ./vendor directory owned by Apache? Or is there some other reason?
There was a problem hiding this comment.
Sorry, while I was making the changes the composer presented a permission error when trying to change this folder, but apparently it was a storage problem in the Docker VM. I did a rebuild without this change and it worked fine. I will make a commit correcting this.
| @@ -49,5 +49,7 @@ php artisan migrate --force | |||
| php artisan config:clear | |||
| php artisan config:cache | |||
|
|
|||
| chown -R apache:root /var/www/html/storage/logs/laravel.log | |||
There was a problem hiding this comment.
I assume this must just be another permissions bug we're fixing - but this wasn't here before so I just want to make sure...
There was a problem hiding this comment.
I added this line because after a forced reboot of the container, the ownership of this file was transferred to the root user, preventing a new initialization. In a normal startup this line is not necessary
…t into fix/alpine-image-build
|
Congrats on merging your first pull request! 🎉🎉🎉 |
Description
The Alpine docker image build was broken. This PR aims to reestablish this build so that it is possible to use a reduced size Docker image and based on a distribution focused on security
Fixes #8884
Type of change
How Has This Been Tested?
The image was built and deployed in a complete production environment
Test Configuration:
Checklist: