Gutenberg: Handle folder permissions in wp-env

[talldan]

Is your feature request related to a problem? Please describe.
In some recent merged PRs, there's been a requirement to change the permissions of certain folders to be writable when using wp-env, in particular when using travis.

The way this is handled should be consistent, and there may also be a requirement to handle other folders (e.g. all of the upload folder).

It'd also be good to get an understanding of why this is required in certain environments.

Previous examples:

• https://github.com/WordPress/gutenberg/pull/20280/files#diff-03588f5a7a029851c0d4bde5d961748fR91-R96
• https://github.com/WordPress/gutenberg/pull/20090/files#diff-354f30a63fb0907d4ad57269548329e3R171

Describe the solution you'd like
Open to suggestions, but the code should be made consistent as a first improvement.

[noahtallen]

note: blocking some tests here: https://github.com/WordPress/gutenberg/pull/22454#issuecomment-633154131

[dd32]

It'd also be good to get an understanding of why this is required in certain environments.

This happens because wp-env assumes that the user id's between systems will be consistent, ie. that the www-data user is the same ID on the host AND container, but also kind of assumes that it's being RUN AS www-data on the host system (which it never should be).

An example of this is https://github.com/WordPress/gutenberg/blob/master/packages/env/lib/wordpress.js#L36-L48 where it chown's folders to www-data on the host system, but that user might not match the ID within the container. https://github.com/WordPress/gutenberg/pull/20403 is another example of that.

I was looking into this yesterday, and I have a few ideas that I hashed out, one of which worked, I just need to explore it further (I was testing in an interactive GitHub action, so limited patching ability)
The "simplest" option, since Docker doesn't support ID remapping in the needed way, is to alter the www-data user within the container to match the ID of the owner of the files being mounted. That can be done by a) a new image that usermod -u $UID www-data && groupmod -g $GID www-data or b) overriding/mounting a new docker entrypoint.

Another option was to alter the upstream wordpress docker image to allow executing as the needed user (making use of it's APACHE_RUN_AS var), but that's not as ideal/easy, as it may require creating a new user anyway, and that user may already exist using the specified ID.

The other option is that the WordPress mount and everything under it could just be chmod 777'd and deal with the mismatch between file ownerships on the parent system later. That's super not-ideal, and limits the ability to have tests that alter filesystems be realistic.

[noahtallen]

The other option is that the WordPress mount and everything under it could just be chmod 777'd

Unfortunately, we have many examples of this too. :( Even chmod 777 on the whole mount would be an improvement to scattering it around everywhere.

• To get phpunit tests modifying the filesystem to work: https://github.com/WordPress/gutenberg/blob/df5f28de92fa1eda4f60b7381c01b8d13c1f570b/.github/workflows/unit-test.yml#L83
• Same for e2e tests: https://github.com/WordPress/gutenberg/blob/df5f28de92fa1eda4f60b7381c01b8d13c1f570b/.github/workflows/end2end-test.yml#L46
• Also for phpunit upload directories: https://github.com/WordPress/gutenberg/blob/df5f28de92fa1eda4f60b7381c01b8d13c1f570b/packages/env/lib/wordpress.js#L199
• And it turns out that running wp-env start twice in a row in GitHub CI was broken as well without a chmod: https://github.com/WordPress/gutenberg/pull/23797/files?file-filters%5B%5D=.js&file-filters%5B%5D=.json#diff-8d4127612b24d9e7336db10981da03b3R133

The "simplest" option, since Docker doesn't support ID remapping in the needed way, is to alter the www-data user within the container

This sounds pretty great! Is it possible to modify this user without having to change images (which seems like a large undertaking, particularly since multiple images are used)? For example, the user property in docker-compose: https://docs.docker.com/compose/compose-file/#domainname-hostname-ipc-mac_address-privileged-read_only-shm_size-stdin_open-tty-user-working_dir. Though I don't know what exactly that does (I'm fairly inexperienced with Docker!)

[dd32]

This sounds pretty great! Is it possible to modify this user without having to change images (which seems like a large undertaking, particularly since multiple images are used)?

Thankfully with Docker you don't have to completely change the upstream images, creating a "new local image" from the upstream image is a few extra lines of code, effectively you can run a few extra CLI commands between the upstreams commands and the upstreams execution point. It does however add a few seconds (and an extra 2 temporary docker container builds) to the bootup process.

The other option is that you change the entrypoint, the "default command" that executes when you start a docket container, to be a wrapper script that's effectively custom-commands && run default upstream image entrypoint which is a little faster than a local image, but not as futureproof (ie. if upstream were to change the default entry point, we'd have to update to reference the new file location). It's also a little more fragile for when you use a docker run command since the default command will probably have been overridden.

I'm looking at the first option today :)

For example, the user property in docker-compose: https://docs.docker.com/compose/compose-file/#domainname-hostname-ipc-mac_address-privileged-read_only-shm_size-stdin_open-tty-user-working_dir. Though I don't know what exactly that does

Unfortunately the user property doesn't actually help a lot here, it simply defines what userNAME commands run as by default inside the container, and since there's no guaranteed HostUserID:ContainerUserID mapping, they sometimes work but othertimes don't.
ie. When the wordpress:cli image runs as user 33:33 (which is www-data:www-data on most debian systems I believe) it's actually running as the xfs:xfs user/group on the alpine subsystem - the X Font Server user - which isn't a huge issue, since we only really care about the filesystem permissions in that case, but for example $HOME would be /etc/X11/fs and it might have extra unexpected privileges or that user ID may not even exist in a future image (Why is there an X Font Server user in the first place on a headless image?!)

I'm fairly inexperienced with Docker!

I wouldn't call myself an expert, but I've run into these problems myself and researched such things a lot!

[noahtallen]

Thank you for investigating! I look forward to learning more about it

[dd32]

Not fully tested, but https://github.com/WordPress/gutenberg/compare/master...dd32:fix/wp-env-users is basically what I was thinking of.

It's working for me in my testing on linux/mac, I suspect it might not work for Windows systems - I'd need to spin up a Windows box to try.

[noahtallen]

Nice! Do you think similar changes would be needed for the phpunit or composer services?

[dd32]

Do you think similar changes would be needed for the phpunit or composer services?

If the proposed changes work over other environments, then it would make sense to also add it to the other containers.
I wasn't able to test the phpunit/composer containers properly, so I haven't included those here yet (Also why I haven't PR'd it yet)

The composer container runs as root within the container currently, and is alpine based, so the changes here should apply to that container without issue, it looks like running docker run --rm -ti -u www-data composer works as expected so I don't see why it wouldn't work.

I hadn't dug too deep into the phpunit container, but it's not as straight forward I don't think, due to some workarounds already in there: https://github.com/WordPress/wpdev-docker-images/pull/16

It may make more sense to standardise on what's done there, running as a wp_php user within the container rather than juggling as I've done here to change the www-data user, but effectively it's the same thing.
I think for phpunit could even just add {PHP_FPM_UID: localUser, PHP_FPM_GID: localGroup } to the environment param and have it work.
It's also pointed out the -o param for usermod/groupmod which removes the need to change the IDs of existing users, making the changes even simpler (ie Removing the 4 EXISTING_USER/EXISTING_GROUP lines)

[noahtallen]

I think for phpunit could even just add {PHP_FPM_UID: localUser, PHP_FPM_GID: localGroup } to the environment param and have it work.

Nice. I think theoretically change away from wordpressdevelop/phpunit if we need to. As far as I know, we only use that image to get a phpunit binary included. All test libs/wp sources come from elsewhere

[ajlende]

Found this issue when I was having permissions issues when using rootless docker on Linux. It may be an edge-case that only needs documentation, but it would be great if the solution for this issue also worked with rootless docker. (And I can help test that scenario when a PR becomes available)