Skip to content
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

Make already existing files owned by current user #474

Conversation

michaelperrin
Copy link

@michaelperrin michaelperrin commented Feb 28, 2020

This PR makes all already existing files in the /var/www/html directory owned by the www-data user instead of root.

Issue description

It is a problem that has been mentioned several times here on GitHub (see #436 and #358) and on StackOverflow.

The problem happens for example when sharing a folder into the wp-content WordPress folder, like in this Docker Compose example:

services:
  wordpress:
    image: wordpress:5.3-php7.4-fpm-alpine
    volumes:
      - wordpress_data:/var/www/html
      - ./themes/my-theme:/var/www/html/wp-content/themes/my-theme

volumes:
  wordpress_data: {}

Running these commands:

docker-compose up -d
docker-compose exec wordpress ls -al /var/www/html/

Will result in a list like:

-rw-r--r--    1 www-data www-data      2898 Jan  8  2019 wp-config-sample.php
drwxr-xr-x    4 root     root          4096 Feb 28 20:33 wp-content
-rw-r--r--    1 www-data www-data      3955 Oct 10 22:52 wp-cron.php

The wp-content (which has been created by Docker when sharing the volumes) folder belongs to the root user instead of the www-data user.

Why the issue is happening

Docker creates the shared folder before WordPress is installed into the /var/www/html folder.

The docker-entrypoint.sh script is executed after and checks whether the /var/www/html folder already exists, and if so, fixes user ownership of it:

if [ "$(id -u)" = '0' ] && [ "$(stat -c '%u:%g' .)" = '0:0' ]; then
  chown "$user:$group" .
fi

The test is not relevant anymore. Since PHP 7.x, the Docker images create a /var/www/html folder that already belongs to www-data, meaning that the condition will never be met. You can check this by running this:

docker run --rm php:7.4-fpm-alpine ls -al /var/www

Result:

drwxrwxrwx    2 www-data www-data      4096 Oct 21 20:38 html

Same command for PHP 5.6:

docker run --rm php:5.6-fpm-alpine ls -al /var/www

Result:

drwxr-xr-x    2 root     root          4096 Jan 31  2019 html

Changes made

The following changes were made:

  • Do not check for /var/www/html root ownership.
  • Change ownership of all existing files.

@tianon
Copy link
Member

tianon commented Feb 28, 2020

We used to do this, and it was reverted intentionally (#74) because it causes massive issues for folks who are doing things like local development of WordPress themes / plugins.

Now that we can run as an arbitrary user, perhaps that can be reconsidered (and we'd then instead recommend that development users run as their own UID instead, thus avoiding the problem).

@michaelperrin
Copy link
Author

michaelperrin commented Feb 29, 2020

Thank you @tianon for looking at my PR.

From what I see, the PR you mentioned (#74) actually ensures that files in the /usr/src/wordpress folder have the www-data owner. These files are the ones that are then copied to /var/www/html in the docker-entrypoint.sh script.

The goal of this PR is to make sure that existing files in /var/www/html are owned by this user too. In most cases it will be www-data but not necessarily this one as there is an existing logic to determine which user to use on https://github.com/docker-library/wordpress/blob/master/docker-entrypoint.sh#L27-L46 . I made use of these variables to not restrict to www-data.

Having the wp-content owned by root has a lot of issues, including for local development of themes / plugins and uploads as well (files can't be uploaded from the WordPress admin).

Plus I think that the rule of checking whether the /var/www/html belongs to root to apply the rule seems outdated, as this was something that was happening on PHP Docker images before PHP 7.x .

If needed I can expose more the issues that I encountered when I have the wp-content folder belonging to root and would be happy to discuss all this!

@michaelperrin
Copy link
Author

Just as a side note, I would add that changing the file permissions inside the container changes them also on the host, which can make file manipulation on the host harder to manage.

@freechelmi
Copy link

Thanks @michaelperrin does this fix the "installing plugin failed : cannot write to directory " issue that I face when running 5-fpm ?

@michaelperrin
Copy link
Author

@freechelmi Just to be sure, do you install plugins via WP-CLI or the web interface? And which folders do you share between the host and the container? In either case, that should fix the issue. I can make the test for you if you wish.

@freechelmi
Copy link

@michaelperrin : Yes Web interface , and we mount wp-content , see our dc
https://lab.libreho.st/libre.sh/compose/wordpress/blob/develop/docker-compose.yml
I juts tested without custom volume and it works great, so yes it will be fixed by your PR for sure.

@thomasplevy
Copy link

hey @michaelperrin I've been struggling with this particular issue for a really really long time. I develop a WordPress plugin and I really need a solid docker solution so that I can start reliably adding e2e tests into my testing workflows.

I'd like to try this solution out but I can't figure out how to run your image locally to see if this works. Any recommendations on how I can try this PR out to see if it helps my situation?

@matzeeable
Copy link

Hi @thomasplevy ! Just stumpled over your comment as I have subscribed this thread - waiting for a solution, too. I have workaround'd this - I think it is not a workaround, more a possible solution:

Put an additional script after you run your WordPress container, as you can see here:
https://github.com/devowlio/wp-react-starter/blob/600b0fcdca469394fc51b6d0f5b457d9737aae0b/devops/scripts/container-wordpress-command.sh#L62-L66

It allows you to run E2E tests without any problem in your CI: https://gitlab.com/devowlio/wp-reactjs-starter/-/jobs/492094866

Perhaps you are interested in our open source boilerplate: WP React Starter

@thomasplevy
Copy link

@matzeeable oh man thank you so much. This is a really simple and elegant solution!

@shahariaazam
Copy link

Is there any update of this PR? Is it going to be accepted or not. This is causing few questions on SO.

@lucastercas
Copy link

To any one going through the same problem, create a new image for production, but dont COPY the plugin to /var/www/html/ like is being told on the README from the image, COPY the plugins to /usr/src/wordpress, like so:

FROM wordpress:5.4.1-apache
COPY --chown=www-data:www-data ./src /usr/src/wordpress/wp-content/themes/foo_theme
EXPOSE 80 443

This way, when the docker-entrypoint.sh script runs, it copies the /usr/src/wordpress to /var/www/html with the www-data:www-data permission.

However, if doing things this way, you can't mount the wordpress volume on the compose file:

volumes:
   - wordress:/var/www/html

If you do this, your plugin wont update, because it will use the version that is on the volume.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants