Match Container's User and Group to Host (#49962)

* Changed Environment Container User Instead of running the web service as `root` and the CLI service as `33:33`, we should be running them as the host user. By doing this we ensure ownership parity in mounted folders on platforms where having different owners would result in permission issues. * Removed Upload Permission Mod With the changes in this pull request we no longer need to do this. * Added Shared Home Directory This will ensure things like `wp plugin install` have a directory to use as a cache. Anything else can now refer to the home directory for the same purpose. * Fixed Dangling Containers Some of our `docker-compose run` actions weren't removing the container after being executed. This was breaking `wp-env destroy` when volumes were in-use. * Isolated Docker Service Images By defining an `image` property in our `docker-compose.yml` we were replacing the corresponding image locally after building. This meant `tests-wordpress` would have the same image as `wordpress` as long as the PHP versions matched. This only worked on accident, and as a consequence, also replaced the local `wordpress` image. This commit isolates all of the `build` operations so they don't replace the images. This also adds support so that `destroy` will remove them. * Fixed Debian Stretch Repositories Debian stretch was just removed from deb.debian.org and moved to the archive repository. This commit changes the sources so that, on stretch, the apt-get update won't break. Co-authored-by: Noah Allen <[email protected]>
WordPress · Apr 28, 2023 · 602ba69 · 602ba69
1 parent 358a423
commit 602ba69
Show file tree

Hide file tree

Showing 11 changed files with 252 additions and 110 deletions.
diff --git a/packages/env/CHANGELOG.md b/packages/env/CHANGELOG.md
@@ -2,6 +2,15 @@
 
 ## Unreleased
 
+### Breaking Change
+
+-   Docker containers now run as the host user. This should resolve problems with permissions arising from different owners
+between the host, web container, and cli container. If you still encounter permissions issues, try running `npx wp-env destroy` so that the environment can be recreated with the correct permissions.
+
+### Bug fix
+
+-   Ensure `wordpress`, `tests-wordpress`, `cli`, and `tests-cli` always build the correct Docker image.
+
 ### Enhancement
 
 -   `wp-env run ...` now uses docker-compose exec instead of docker-compose run. As a result, it is much faster, since commands are executed against existing services, rather than creating them from scratch each time.

diff --git a/packages/env/README.md b/packages/env/README.md
@@ -175,25 +175,6 @@ $ wp-env destroy
 $ wp-env start
 ```
 
-### 7. Debug mode and inspecting the generated dockerfile.
-
-`wp-env` uses docker behind the scenes. Inspecting the generated docker-compose file can help to understand what's going on.
-
-Start `wp-env` in debug mode
-
-```sh
-wp-env start --debug
-```
-
-`wp-env` will output its config which includes `dockerComposeConfigPath`.
-
-```sh
-ℹ Config:
-	...
-	"dockerComposeConfigPath": "/Users/$USERNAME/.wp-env/5a619d332a92377cd89feb339c67b833/docker-compose.yml",
-	...
-```
-
 ## Using included WordPress PHPUnit test files
 
 Out of the box `wp-env` includes the [WordPress' PHPUnit test files](https://develop.svn.wordpress.org/trunk/tests/phpunit/) corresponding to the version of WordPress installed. There is an environment variable, `WP_TESTS_DIR`, which points to the location of these files within each container. By including these files in the environment, we remove the need for you to use a package or install and mount them yourself. If you do not want to use these files, you should ignore the `WP_TESTS_DIR` environment variable and load them from the location of your choosing.
@@ -409,13 +390,6 @@ Success: Installed 1 of 1 plugins.
 ✔ Ran `plugin install custom-post-type-ui` in 'cli'. (in 6s 483ms)
 ```
 
-**NOTE**: Depending on your host OS, you may experience errors when trying to install plugins or themes (e.g. `Warning: Could not create directory.`). This is typically because the user ID used within the container does not have write access to the mounted directories created by `wp-env`. To resolve this, run the `docker-compose` command directly from the directory created by `wp-env` and add `-u $(id -u)` and `-e HOME=/tmp` the `run` command as options:
-
-```sh
-$ cd ~/wp-env/500cd328b649d63e882d5c4695871d04
-$ docker-compose run --rm -u $(id -u) -e HOME=/tmp cli [plugin|theme] install <plugin|theme>
-```
-
 ### `wp-env destroy`
 
 ```sh
@@ -445,7 +419,7 @@ Options:
 
 ### `wp-env install-path`
 
-Outputs the absolute path to the WordPress environment files.
+Get the path where all of the environment files are stored. This includes the Docker files, WordPress, PHPUnit files, and any sources that were downloaded.
 
 Example:
 

diff --git a/packages/env/lib/build-docker-compose-config.js b/packages/env/lib/build-docker-compose-config.js
@@ -10,6 +10,7 @@ const path = require( 'path' );
  */
 const { hasSameCoreSource } = require( './wordpress' );
 const { dbEnv } = require( './config' );
+const getHostUser = require( './get-host-user' );
 
 /**
  * @typedef {import('./config').WPConfig} WPConfig
@@ -21,6 +22,7 @@ const { dbEnv } = require( './config' );
  *
  * @param {string}          workDirectoryPath The working directory for wp-env.
  * @param {WPServiceConfig} config            The service config to get the mounts from.
+ * @param {string}          hostUsername      The username of the host running wp-env.
  * @param {string}          wordpressDefault  The default internal path for the WordPress
  *                                            source code (such as tests-wordpress).
  *
@@ -29,6 +31,7 @@ const { dbEnv } = require( './config' );
 function getMounts(
 	workDirectoryPath,
 	config,
+	hostUsername,
 	wordpressDefault = 'wordpress'
 ) {
 	// Top-level WordPress directory mounts (like wp-content/themes)
@@ -46,9 +49,10 @@ function getMounts(
 			`${ source.path }:/var/www/html/wp-content/themes/${ source.basename }`
 	);
 
-	const coreMount = `${
-		config.coreSource ? config.coreSource.path : wordpressDefault
-	}:/var/www/html`;
+	const userHomeMount =
+		wordpressDefault === 'wordpress'
+			? `user-home:/home/${ hostUsername }`
+			: `tests-user-home:/home/${ hostUsername }`;
 
 	const corePHPUnitMount = `${ path.join(
 		workDirectoryPath,
@@ -59,10 +63,15 @@ function getMounts(
 		'phpunit'
 	) }:/wordpress-phpunit`;
 
+	const coreMount = `${
+		config.coreSource ? config.coreSource.path : wordpressDefault
+	}:/var/www/html`;
+
 	return [
 		...new Set( [
-			coreMount,
+			coreMount, // Must be first because of some operations later that expect it to be!
 			corePHPUnitMount,
+			userHomeMount,
 			...directoryMounts,
 			...pluginMounts,
 			...themeMounts,
@@ -79,16 +88,32 @@ function getMounts(
  * @return {Object} A docker-compose config object, ready to serialize into YAML.
  */
 module.exports = function buildDockerComposeConfig( config ) {
+	// Since we are mounting files from the host operating system
+	// we want to create the host user in some of our containers.
+	// This ensures ownership parity and lets us access files
+	// and folders between the containers and the host.
+	const hostUser = getHostUser();
+
 	const developmentMounts = getMounts(
 		config.workDirectoryPath,
-		config.env.development
+		config.env.development,
+		hostUser.name
 	);
 	const testsMounts = getMounts(
 		config.workDirectoryPath,
 		config.env.tests,
+		hostUser.name,
 		'tests-wordpress'
 	);
 
+	// We use a custom Dockerfile in order to make sure that
+	// the current host user exists inside the container.
+	const imageBuildArgs = {
+		HOST_USERNAME: hostUser.name,
+		HOST_UID: hostUser.uid,
+		HOST_GID: hostUser.gid,
+	};
+
 	// When both tests and development reference the same WP source, we need to
 	// ensure that tests pulls from a copy of the files so that it maintains
 	// a separate DB and config. Additionally, if the source type is local we
@@ -143,59 +168,28 @@ module.exports = function buildDockerComposeConfig( config ) {
 	const developmentPorts = `\${WP_ENV_PORT:-${ config.env.development.port }}:80`;
 	const testsPorts = `\${WP_ENV_TESTS_PORT:-${ config.env.tests.port }}:80`;
 
-	// Set the WordPress, WP-CLI, PHPUnit PHP version if defined.
-	const developmentPhpVersion = config.env.development.phpVersion
-		? config.env.development.phpVersion
-		: '';
-	const testsPhpVersion = config.env.tests.phpVersion
-		? config.env.tests.phpVersion
-		: '';
-
-	// Set the WordPress images with the PHP version tag.
-	const developmentWpImage = `wordpress${
-		developmentPhpVersion ? ':php' + developmentPhpVersion : ''
-	}`;
-	const testsWpImage = `wordpress${
-		testsPhpVersion ? ':php' + testsPhpVersion : ''
-	}`;
-	// Set the WordPress CLI images with the PHP version tag.
-	const developmentWpCliImage = `wordpress:cli${
-		! developmentPhpVersion || developmentPhpVersion.length === 0
-			? ''
-			: '-php' + developmentPhpVersion
-	}`;
-	const testsWpCliImage = `wordpress:cli${
-		! testsPhpVersion || testsPhpVersion.length === 0
-			? ''
-			: '-php' + testsPhpVersion
-	}`;
-
 	// Defaults are to use the most recent version of PHPUnit that provides
 	// support for the specified version of PHP.
 	// PHP Unit is assumed to be for Tests so use the testsPhpVersion.
 	let phpunitTag = 'latest';
-	const phpunitPhpVersion = '-php-' + testsPhpVersion + '-fpm';
-	if ( testsPhpVersion === '5.6' ) {
+	const phpunitPhpVersion = '-php-' + config.env.tests.phpVersion + '-fpm';
+	if ( config.env.tests.phpVersion === '5.6' ) {
 		phpunitTag = '5' + phpunitPhpVersion;
-	} else if ( testsPhpVersion === '7.0' ) {
+	} else if ( config.env.tests.phpVersion === '7.0' ) {
 		phpunitTag = '6' + phpunitPhpVersion;
-	} else if ( testsPhpVersion === '7.1' ) {
+	} else if ( config.env.tests.phpVersion === '7.1' ) {
 		phpunitTag = '7' + phpunitPhpVersion;
-	} else if ( testsPhpVersion === '7.2' ) {
+	} else if ( config.env.tests.phpVersion === '7.2' ) {
 		phpunitTag = '8' + phpunitPhpVersion;
 	} else if (
-		[ '7.3', '7.4', '8.0', '8.1', '8.2' ].indexOf( testsPhpVersion ) >= 0
+		[ '7.3', '7.4', '8.0', '8.1', '8.2' ].indexOf(
+			config.env.tests.phpVersion
+		) >= 0
 	) {
 		phpunitTag = '9' + phpunitPhpVersion;
 	}
 	const phpunitImage = `wordpressdevelop/phpunit:${ phpunitTag }`;
 
-	// The www-data user in wordpress:cli has a different UID (82) to the
-	// www-data user in wordpress (33). Ensure we use the wordpress www-data
-	// user for CLI commands.
-	// https://github.com/docker-library/wordpress/issues/256
-	const cliUser = '33:33';
-
 	// If the user mounted their own uploads folder, we should not override it in the phpunit service.
 	const isMappingTestUploads = testsMounts.some( ( mount ) =>
 		mount.endsWith( ':/var/www/html/wp-content/uploads' )
@@ -227,11 +221,16 @@ module.exports = function buildDockerComposeConfig( config ) {
 				volumes: [ 'mysql-test:/var/lib/mysql' ],
 			},
 			wordpress: {
-				build: '.',
 				depends_on: [ 'mysql' ],
-				image: developmentWpImage,
+				build: {
+					context: '.',
+					dockerfile: 'WordPress.Dockerfile',
+					args: imageBuildArgs,
+				},
 				ports: [ developmentPorts ],
 				environment: {
+					APACHE_RUN_USER: '#' + hostUser.uid,
+					APACHE_RUN_GROUP: '#' + hostUser.gid,
 					...dbEnv.credentials,
 					...dbEnv.development,
 					WP_TESTS_DIR: '/wordpress-phpunit',
@@ -240,9 +239,15 @@ module.exports = function buildDockerComposeConfig( config ) {
 			},
 			'tests-wordpress': {
 				depends_on: [ 'tests-mysql' ],
-				image: testsWpImage,
+				build: {
+					context: '.',
+					dockerfile: 'Tests-WordPress.Dockerfile',
+					args: imageBuildArgs,
+				},
 				ports: [ testsPorts ],
 				environment: {
+					APACHE_RUN_USER: '#' + hostUser.uid,
+					APACHE_RUN_GROUP: '#' + hostUser.gid,
 					...dbEnv.credentials,
 					...dbEnv.tests,
 					WP_TESTS_DIR: '/wordpress-phpunit',
@@ -251,10 +256,13 @@ module.exports = function buildDockerComposeConfig( config ) {
 			},
 			cli: {
 				depends_on: [ 'wordpress' ],
-				image: developmentWpCliImage,
+				build: {
+					context: '.',
+					dockerfile: 'CLI.Dockerfile',
+					args: imageBuildArgs,
+				},
 				volumes: developmentMounts,
-				user: cliUser,
-				command: 'sleep infinity', // Keeps the service alive.
+				user: hostUser.fullUser,
 				environment: {
 					...dbEnv.credentials,
 					...dbEnv.development,
@@ -263,10 +271,13 @@ module.exports = function buildDockerComposeConfig( config ) {
 			},
 			'tests-cli': {
 				depends_on: [ 'tests-wordpress' ],
-				image: testsWpCliImage,
+				build: {
+					context: '.',
+					dockerfile: 'Tests-CLI.Dockerfile',
+					args: imageBuildArgs,
+				},
 				volumes: testsMounts,
-				user: cliUser,
-				command: 'sleep infinity', // Keeps the service alive.
+				user: hostUser.fullUser,
 				environment: {
 					...dbEnv.credentials,
 					...dbEnv.tests,
@@ -300,6 +311,8 @@ module.exports = function buildDockerComposeConfig( config ) {
 			mysql: {},
 			'mysql-test': {},
 			'phpunit-uploads': {},
+			'user-home': {},
+			'tests-user-home': {},
 		},
 	};
 };
diff --git a/packages/env/lib/cli.js b/packages/env/lib/cli.js
@@ -215,7 +215,7 @@ module.exports = function cli() {
 	);
 	yargs.command(
 		'install-path',
-		'Get the path where environment files are located.',
+		'Get the path where all of the environment files are stored. This includes the Docker files, WordPress, PHPUnit files, and any sources that were downloaded.',
 		() => {},
 		withSpinner( env.installPath )
 	);

diff --git a/packages/env/lib/commands/destroy.js b/packages/env/lib/commands/destroy.js
@@ -39,7 +39,7 @@ module.exports = async function destroy( { spinner, debug } ) {
 	}
 
 	spinner.info(
-		'WARNING! This will remove Docker containers, volumes, and networks associated with the WordPress instance.'
+		'WARNING! This will remove Docker containers, volumes, networks, and images associated with the WordPress instance.'
 	);
 
 	const { yesDelete } = await inquirer.prompt( [
@@ -69,10 +69,13 @@ module.exports = async function destroy( { spinner, debug } ) {
 	const directoryHash = path.basename( workDirectoryPath );
 
 	spinner.text = 'Removing docker volumes.';
-	await removeDockerItems( 'volume', directoryHash );
+	await removeDockerItems( 'volume', 'name', directoryHash );
 
 	spinner.text = 'Removing docker networks.';
-	await removeDockerItems( 'network', directoryHash );
+	await removeDockerItems( 'network', 'name', directoryHash );
+
+	spinner.text = 'Removing docker images.';
+	await removeDockerItems( 'image', 'reference', directoryHash + '*' );
 
 	spinner.text = 'Removing local files.';
 
@@ -84,12 +87,13 @@ module.exports = async function destroy( { spinner, debug } ) {
 /**
  * Removes docker items, like networks or volumes, matching the given name.
  *
- * @param {string} itemType The item type, like "network" or "volume"
- * @param {string} name     Remove items whose name match this string.
+ * @param {string} itemType    The item type, like "volume", or "network".
+ * @param {string} filter      The filtering to search using.
+ * @param {string} filterValue The filtering value that we're looking for.
  */
-async function removeDockerItems( itemType, name ) {
+async function removeDockerItems( itemType, filter, filterValue ) {
 	const { stdout: items } = await exec(
-		`docker ${ itemType } ls -q --filter name=${ name }`
+		`docker ${ itemType } ls -q --filter ${ filter }='${ filterValue }'`
 	);
 	if ( items ) {
 		await exec(

diff --git a/packages/env/lib/commands/run.js b/packages/env/lib/commands/run.js
@@ -8,6 +8,7 @@ const path = require( 'path' );
  * Internal dependencies
  */
 const initConfig = require( '../init-config' );
+const getHostUser = require( '../get-host-user' );
 
 /**
  * @typedef {import('../config').WPConfig} WPConfig
@@ -52,6 +53,12 @@ module.exports = async function run( {
  * @param {Object}   spinner   A CLI spinner which indicates progress.
  */
 function spawnCommandDirectly( config, container, command, envCwd, spinner ) {
+	// Both the `wordpress` and `tests-wordpress` containers have the host's
+	// user so that they can maintain ownership parity with the host OS.
+	// We should run any commands as that user so that they are able
+	// to interact with the files mounted from the host.
+	const hostUser = getHostUser();
+
 	// We need to pass absolute paths to the container.
 	envCwd = path.resolve( '/var/www/html', envCwd );
 
@@ -63,6 +70,8 @@ function spawnCommandDirectly( config, container, command, envCwd, spinner ) {
 		! isTTY ? '--no-TTY' : '',
 		'-w',
 		envCwd,
+		'--user',
+		hostUser.fullUser,
 		container,
 		...command.split( ' ' ), // The command will fail if passed as a complete string.
 	];

diff --git a/packages/env/lib/commands/start.js b/packages/env/lib/commands/start.js
@@ -162,6 +162,11 @@ module.exports = async function start( { spinner, debug, update, xdebug } ) {
 		}
 	);
 
+	// Make sure we've consumed the custom CLI dockerfile.
+	if ( shouldConfigureWp ) {
+		await dockerCompose.buildOne( [ 'cli' ], { ...dockerComposeConfig } );
+	}
+
 	// Only run WordPress install/configuration when config has changed.
 	if ( shouldConfigureWp ) {
 		spinner.text = 'Configuring WordPress.';