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

Match Container's User and Group to Host #49962

Merged
merged 20 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/env/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
- Use test environment's `WP_SITEURL` instead of `WP_TESTS_DOMAIN` as the WordPress URL.
- Automatically add the environment's port to `WP_TESTS_DOMAIN`.
- `run` command now has a `--env-cwd` option to set the working directory in the container for the command to execute from.
- 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. You _may_ need to run `npx wp-env destroy` to get everything working as
expected.
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved

### Bug fix

- Ensure `wordpress`, `tests-wordpress`, `cli`, and `tests-cli` always build the correct Docker image.

## 5.16.0 (2023-04-12)

Expand Down
6 changes: 3 additions & 3 deletions packages/env/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,17 @@ $ wp-env destroy
$ wp-env start
```

### 7. Debug mode and inspecting the generated dockerfile.
### 7. Debug mode and inspecting the generated docker files.

`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
Start `wp-env` in debug mode:

```sh
wp-env start --debug
```

`wp-env` will output its config which includes `dockerComposeConfigPath`.
`wp-env` will output its config which includes `dockerComposeConfigPath`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc section is actually out of date now that we have npx install-path! We could probably change the entire thing to say that you can just cd $(npx wp-env install-path) and then see all the generated config files


```sh
ℹ Config:
Expand Down
117 changes: 66 additions & 51 deletions packages/env/lib/build-docker-compose-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
*
Expand All @@ -29,6 +31,7 @@ const { dbEnv } = require( './config' );
function getMounts(
workDirectoryPath,
config,
hostUsername,
wordpressDefault = 'wordpress'
) {
// Top-level WordPress directory mounts (like wp-content/themes)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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' )
Expand Down Expand Up @@ -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,
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
...dbEnv.credentials,
...dbEnv.development,
WP_TESTS_DIR: '/wordpress-phpunit',
Expand All @@ -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',
Expand All @@ -251,9 +256,13 @@ module.exports = function buildDockerComposeConfig( config ) {
},
cli: {
depends_on: [ 'wordpress' ],
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
image: developmentWpCliImage,
build: {
context: '.',
dockerfile: 'CLI.Dockerfile',
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
args: imageBuildArgs,
},
volumes: developmentMounts,
user: cliUser,
user: hostUser.fullUser,
environment: {
...dbEnv.credentials,
...dbEnv.development,
Expand All @@ -262,9 +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,
user: hostUser.fullUser,
environment: {
...dbEnv.credentials,
...dbEnv.tests,
Expand Down Expand Up @@ -298,6 +311,8 @@ module.exports = function buildDockerComposeConfig( config ) {
mysql: {},
'mysql-test': {},
'phpunit-uploads': {},
'user-home': {},
'tests-user-home': {},
},
};
};
18 changes: 11 additions & 7 deletions packages/env/lib/commands/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( [
Expand Down Expand Up @@ -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.';

Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions packages/env/lib/commands/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 );

Expand All @@ -61,6 +68,8 @@ function spawnCommandDirectly( config, container, command, envCwd, spinner ) {
'run',
'-w',
envCwd,
'--user',
hostUser.fullUser,
'--rm',
container,
...command.split( ' ' ), // The command will fail if passed as a complete string.
Expand Down
5 changes: 5 additions & 0 deletions packages/env/lib/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,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.';
Expand Down
30 changes: 30 additions & 0 deletions packages/env/lib/get-host-user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* External dependencies
*/
const os = require( 'os' );

/**
* Gets information about the host user.
*
* @return {Object} The host user's name, uid, and gid.
*/
module.exports = function getHostUser() {
const hostUser = os.userInfo();

// On Windows the uid and gid will be -1. Since there isn't a great way to handle this,
// we're just going to say that the host user is root. On Windows you'll likely be
// using WSL to run commands inside the container, which has a uid and gid. If
// you aren't, you'll be mounting directories from Windows, to a Linux
// VM (Docker Desktop uses one), to the guest OS. I assume that
// when dealing with this configuration that file ownership
// has the same kind of magic handling that macOS uses.
const uid = ( hostUser.uid === -1 ? 0 : hostUser.uid ).toString();
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
const gid = ( hostUser.gid === -1 ? 0 : hostUser.gid ).toString();

return {
name: hostUser.username,
uid,
gid,
fullUser: uid + ':' + gid,
};
};
Loading