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 3 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
6 changes: 6 additions & 0 deletions packages/env/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

- `run` command now has a `--env-cwd` option to set the working directory in the container for the command to execute from.

### Bug fix

- Docker containers now run as the host user. This should resolve problems with permissions arising from different owners
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
between the host, web container, and cli container.


## 5.16.0 (2023-04-12)

## 5.15.0 (2023-03-29)
Expand Down
54 changes: 43 additions & 11 deletions packages/env/lib/build-docker-compose-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
const fs = require( 'fs' );
const path = require( 'path' );
const os = require( 'os' );

/**
* Internal dependencies
Expand Down Expand Up @@ -70,6 +71,24 @@ function getMounts(
];
}

/**
* Gets information about the host user so we can implement ownership parity.
*
* @return {Object} The host user's name, uid, and gid.
*/
function getHostUser() {
const hostUser = os.userInfo();
const uid = ( hostUser.uid === -1 ? 0 : hostUser.uid ).toString();
const gid = ( hostUser.gid === -1 ? 0 : hostUser.gid ).toString();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per Node's documentation, on Windows the uid and gid is -1. I think this only happens with a process.platform of win32. If you're using WSL you will realistically be running commands inside the container, so, this is more of an edge-case where they have Node installed on Windows.

The question I have is whether or not this breaks anything. I'm curious about what kind of permission magic takes place when going mounting from Windows -> Docker instead of WSL -> Docker. If it works anything like macOS, I would wager that having everything owned by root doesn't matter.


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

/**
* Creates a docker-compose config object which, when serialized into a
* docker-compose.yml file, tells docker-compose how to run the environment.
Expand All @@ -78,7 +97,13 @@ function getMounts(
*
* @return {Object} A docker-compose config object, ready to serialize into YAML.
*/
module.exports = function buildDockerComposeConfig( config ) {
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
Expand Down Expand Up @@ -190,12 +215,6 @@ module.exports = function buildDockerComposeConfig( config ) {
}
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 +246,20 @@ module.exports = function buildDockerComposeConfig( config ) {
volumes: [ 'mysql-test:/var/lib/mysql' ],
},
wordpress: {
build: '.',
build: {
context: '.',
args: {
HOST_USERNAME: hostUser.name,
HOST_UID: hostUser.uid,
HOST_GID: hostUser.gid,
},
},
depends_on: [ 'mysql' ],
image: developmentWpImage,
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 @@ -243,6 +271,8 @@ module.exports = function buildDockerComposeConfig( config ) {
image: testsWpImage,
ports: [ testsPorts ],
environment: {
APACHE_RUN_USER: '#' + hostUser.uid,
APACHE_RUN_GROUP: '#' + hostUser.gid,
...dbEnv.credentials,
...dbEnv.tests,
WP_TESTS_DIR: '/wordpress-phpunit',
Expand All @@ -253,7 +283,7 @@ module.exports = function buildDockerComposeConfig( config ) {
depends_on: [ 'wordpress' ],
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved
image: developmentWpCliImage,
volumes: developmentMounts,
user: cliUser,
user: hostUser.fullUser,
environment: {
...dbEnv.credentials,
...dbEnv.development,
Expand All @@ -264,7 +294,7 @@ module.exports = function buildDockerComposeConfig( config ) {
depends_on: [ 'tests-wordpress' ],
image: testsWpCliImage,
volumes: testsMounts,
user: cliUser,
user: hostUser.fullUser,
environment: {
...dbEnv.credentials,
...dbEnv.tests,
Expand Down Expand Up @@ -300,4 +330,6 @@ module.exports = function buildDockerComposeConfig( config ) {
'phpunit-uploads': {},
},
};
};
}

module.exports = { getHostUser, buildDockerComposeConfig };
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( '../build-docker-compose-config' );
ObliviousHarmony marked this conversation as resolved.
Show resolved Hide resolved

/**
* @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
14 changes: 13 additions & 1 deletion packages/env/lib/init-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const os = require( 'os' );
* Internal dependencies
*/
const { readConfig } = require( './config' );
const buildDockerComposeConfig = require( './build-docker-compose-config' );
const { buildDockerComposeConfig } = require( './build-docker-compose-config' );

/**
* @typedef {import('./config').WPConfig} WPConfig
Expand Down Expand Up @@ -157,6 +157,18 @@ function dockerFileContents( image, config ) {

RUN apt-get -qy install $PHPIZE_DEPS && touch /usr/local/etc/php/php.ini
${ shouldInstallXdebug ? installXdebug( config.xdebug ) : '' }

# Create the host's user so that we can match ownership in the container.
ARG HOST_USERNAME
ARG HOST_UID
ARG HOST_GID
RUN groupadd -o -g $HOST_GID $HOST_USERNAME
RUN useradd -M -u $HOST_UID -g $HOST_GID $HOST_USERNAME

# Set up sudo so they can have root access when using 'run' commands.
RUN apt-get update
RUN apt-get -qy install sudo
RUN echo "$HOST_USERNAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers
`;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/env/test/build-docker-compose-config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/**
* Internal dependencies
*/
const buildDockerComposeConfig = require( '../lib/build-docker-compose-config' );
const {
buildDockerComposeConfig,
} = require( '../lib/build-docker-compose-config' );

// The basic config keys which build docker compose config requires.
const CONFIG = {
Expand Down