visiblevc / wordpress-starter

A slightly less shitty wordpress development workflow
688 stars 167 forks source link

fix permissions issues once and for all with bindfs #136

Closed dsifford closed 6 years ago

dsifford commented 6 years ago

Intro

The purpose of this PR is to solve the longstanding permissions issues that have existed when working with files/folders needing to be shared between the admin user (admin) and the server user (www-data).

Currently, and in all iterations in the past, whenever a user opened a tty session in the wordpress container and then manipulated any files manually, or with wp-cli, the permissions of those files would modify from being owned by www-data to being owned by admin. This becomes an issue if those files then needed subsequent modification by the server.

A common example of this happening is when a user installs a new plugin with wp plugin instal XXX, the server could then not update or remove the plugin itself from the wordpress dashboard (due to improper permissions).

What this PR does

To combat these permissions issues once and for all, I opted to implement a bindfs mount to the webroot (/var/www/html) from the /app directory.

Bindfs is a FUSE based filesystem that allows a filesystem on a system to be "re-mounted" virtually in another location on the same system. This has useful implications for us because by using bindfs, we're able to "re-mount" the /app directory to the webroot and force all the files in the webroot directory to be owned by www-data.

What's nice about this also is that if the server creates any files in the webroot, those files are synced backwards to the /app directory and permissions are then changed back to "owner:admin group:admin".

TL;DR

This PR solves the permissions issues permanently.

BREAKING CHANGE NOTE:

The WordPress service must now be configured in one of the following ways:

(preferred)

services:
    wordpress:
        cap_add:
            - SYS_ADMIN
        devices:
            - /dev/fuse

OR

(try to avoid if possible)

services:
    wordpress:
        privileged: true

cc: @karellm

dsifford commented 6 years ago

Of note: I've been using this for the last 3 days without issue.

dsifford commented 6 years ago

Of note, of note: Still using this. It's been a dream since day 1. No probs at all and all headaches gone.

Let me know when we're okay to merge. @karellm

karellm commented 6 years ago

I will review by the end of the week. Thanks for the PR :)

dsifford commented 6 years ago

@karellm Sorry for the delay.

The reason I lay out the two options is that in situations where you are solely using the setup for quickly throwing together a development server to be used locally, the second option is way easier to remember and zero risk.

Figured it would be good to know for those cases.

dsifford commented 6 years ago

Let me know what you'd prefer to do Re: showing the configuration settings. If you'd prefer to just show the 1, that's fine.

Once I hear back I'll go ahead and merge / push out. :+1:

karellm commented 6 years ago

@dsifford I trust your judgement on this one. LGTM 👍

Vantablack commented 6 years ago

Hi I'm getting an issue when running docker-compose up

Creating volume "example_data" with default driver
Creating example_db_1         ... done
Creating example_wordpress_1  ... done
Creating example_phpmyadmin_1 ... done
Attaching to example_db_1, example_wordpress_1, example_phpmyadmin_1
db_1          | Initializing database
wordpress_1   | ERROR: Container running with improper privileges.
wordpress_1   | 
wordpress_1   | Be sure your service is configured with the following options:
wordpress_1   | ___
wordpress_1   | services:
wordpress_1   | wordpress:
wordpress_1   |     cap_add:
wordpress_1   |     - SYS_ADMIN
wordpress_1   |     devices:
wordpress_1   |     - /dev/fuse
wordpress_1   | ___
wordpress_1   | 
wordpress_1   | OR (use first option if possible)
wordpress_1   | ___
wordpress_1   | services:
wordpress_1   | wordpress:
wordpress_1   |     privileged: true
wordpress_1   | ___
wordpress_1   | 
example_wordpress_1 exited with code 1
phpmyadmin_1  | 2018-05-18 04:11:42,681 CRIT Supervisor running as root (no user in config file)
phpmyadmin_1  | 2018-05-18 04:11:42,681 WARN Included extra file "/etc/supervisor.d/nginx.ini" during parsing
phpmyadmin_1  | 2018-05-18 04:11:42,681 WARN Included extra file "/etc/supervisor.d/php.ini" during parsing
phpmyadmin_1  | 2018-05-18 04:11:42,689 INFO RPC interface 'supervisor' initialized
phpmyadmin_1  | 2018-05-18 04:11:42,689 CRIT Server 'unix_http_server' running without any HTTP authentication checking
phpmyadmin_1  | 2018-05-18 04:11:42,689 INFO supervisord started with pid 1
phpmyadmin_1  | 2018-05-18 04:11:43,693 INFO spawned: 'php-fpm' with pid 22
phpmyadmin_1  | 2018-05-18 04:11:43,695 INFO spawned: 'nginx' with pid 23
db_1          | 
db_1          | PLEASE REMEMBER TO SET A PASSWORD FOR THE MariaDB root USER !
db_1          | To do so, start the server, then issue the following commands:

I'm using the latest docker-compose.yml file.

Is there any options in the OS that I have to enable?

dsifford commented 6 years ago

You have the cap_add and devices options enabled and it's still giving you those issues?

Vantablack commented 6 years ago

Yeap, on Ubuntu 16.04.4

dsifford commented 6 years ago

Can you paste over your compose file?

Vantablack commented 6 years ago

I'm using the latest one which is https://github.com/visiblevc/wordpress-starter/blob/d909f9e1b488e6e5648ba084330aac6f1b62e83b/example/docker-compose.yml

Vantablack commented 6 years ago

Hey @dsifford it seems like adding --security-opt apparmor:unconfined worked (on Ubuntu)

...
services:
  wordpress:
    # required for mounting bindfs
    cap_add:
      - SYS_ADMIN
    devices:
      - /dev/fuse
    security_opt:
      - apparmor:unconfined
...

The solution was from https://github.com/moby/moby/issues/16429#issuecomment-217126586 and this issue was also mentioned in https://github.com/visiblevc/wordpress-starter/issues/132#issuecomment-390498838

karellm commented 6 years ago

@dsifford or @Vantablack could you make a PR documenting this in the README?