search

magento / magento-cloud-docker

All Submissions you make to Magento Inc. (“Magento") through GitHub are subject to the following terms and conditions: (1) You grant Magento a perpetual, worldwide, non-exclusive, no charge, royalty free, irrevocable license under your applicable copyrights and patents to reproduce, prepare derivative works of, display, publically perform, sublicense and distribute any feedback, ideas, code, or other information (“Submission") you submit through GitHub. (2) Your Submission is an original work of authorship and you are the owner or are legally entitled to grant the license stated above. (3) You agree to the Contributor License Agreement found here: https://github.com/magento/magento2/blob/master/CONTRIBUTOR_LICENSE_AGREEMENT.html
Open Software License 3.0
257 stars 192 forks source link

Avoid creating files as root with php cli container #303

Closed lobre closed 3 years ago

lobre commented 3 years ago

Hello,

We are currently using this Docker stack alongside Magento Commerce Cloud for a large project (~30 developers).

When using a container created from the php cli image, the default user is root. That means all commands are run as root. This becomes a problem when the application code is mounted as a volume in the container.

Effectively, an operation such as composer install would create vendor files with UID 0, which corresponds to the root user on the local filesystem. This is really not convenient as you now need sudo permission with your local user if you want to edit/delete those files.

This has been recently painful and confusing in the team because we need to give developers full permissions on their machines, and because they don't really understand why their local filesystem end up with root files.

I can see in the git log that this problem was tackled using gosu in the past. The cmd was executed as www using exec gosu www "$@". I am not sure however why this was removed. Do you have information?

I would suggest to re-add that kind of behavior to re-establish this non-root process in the cli. Before moving forward, drafting that idea, and proposing a PR, I wanted to have your feedback.

PS: as of today, there are still traces of gosu in the current implementation: https://github.com/magento/magento-cloud-docker/blob/develop/images/php/fpm/Dockerfile#L3

Loric

shiftedreality commented 3 years ago

Hello @lobre, thank you for the request.

We were not able to make gosu work as expected. If I remember correctly, we had to add gosu to change permissions recursively into https://github.com/magento/magento-cloud-docker/blob/develop/images/php/7.2-fpm/docker-entrypoint.sh which let to drastic degradation

lobre commented 3 years ago

I am not sure to understand the relationship between gosu and the fact that you need to change permissions recursively.

Gosu has apparently being removed in https://github.com/magento/magento-cloud-docker/commit/08e8f09537a64030ad14fe075b858e1b603cc451.

The idea is not to alter permissions here, but simply to launch the main cmd as www. To me, it would be common sense to expect that www in the container has the same UID as the mounted filesystem. That way, www would not have any trouble interacting with the codebase.

And to ensure that, there is already the $UPDATE_UID_GID that can be used if that is not the case.

Another point that you may have misunderstood is that I am not talking about the fpm version of the image here. For this one, it makes sense that it should run as root. Then fpm will have its own internal user configurations for the spawned processes. I am only referring to the cli version of the image so that adhoc commands don't lead to files created as root.

shiftedreality commented 3 years ago

Yes, it definitely makes sense to run it from the user.

But it may (need to check) lead to issues when the FPM container creates files as root and then CLI containers cannot read them with user permissions. We were not able to make it work with gosu first time, but I think we can try it again (leaving FPM running as root)

lobre commented 3 years ago

Sounds like a good plan. I will give that a go, make a bunch of tests and try to submit a PR if conclusive.