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

Fix Error: Could not process the 'wp-config.php' transformation #29800

Merged
merged 9 commits into from
Mar 12, 2021
34 changes: 30 additions & 4 deletions packages/env/lib/build-docker-compose-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const path = require( 'path' );
* Internal dependencies
*/
const { hasSameCoreSource } = require( './wordpress' );
const { dbEnv } = require( './config' );

/**
* @typedef {import('./config').WPConfig} WPConfig
Expand Down Expand Up @@ -176,26 +177,40 @@ module.exports = function buildDockerComposeConfig( config ) {
image: 'mariadb',
ports: [ '3306' ],
environment: {
MYSQL_ALLOW_EMPTY_PASSWORD: 'yes',
MYSQL_ROOT_PASSWORD:
dbEnv.credentials.WORDPRESS_DB_PASSWORD,
MYSQL_DATABASE: dbEnv.development.WORDPRESS_DB_NAME,
},
volumes: [ 'mysql:/var/lib/mysql' ],
},
'tests-mysql': {
Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of adding an extra MySQL container. It will mean there's a lot of extra memory usage and startup time.

Copy link
Member

Choose a reason for hiding this comment

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

How about we keep to a single container and create two databases ourselves using wp db create?

https://developer.wordpress.org/cli/commands/db/create/

Copy link
Member

Choose a reason for hiding this comment

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

Could that be a follow-up task? It seems higher effort than the current approach

Copy link
Member

Choose a reason for hiding this comment

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

Yep absolutely.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about using one database, and different table prefixes for dev and tests, respectively? The wordpress docker image supports a WORDPRESS_TABLE_PREFIX env variable for that purpose.

image: 'mariadb',
ports: [ '3306' ],
Copy link
Member

Choose a reason for hiding this comment

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

This port is already being used by the mysql container above.

Copy link
Member Author

@ribaricplusplus ribaricplusplus Mar 11, 2021

Choose a reason for hiding this comment

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

I think those ports just denote which ports are exposed within a container. Ports are isolated within individual containers, so if I'm not mistaken there should be no conflict.

EDIT: According to compose specification this short notation means that the port 3306 inside the container is going to be assigned an ephemeral port on the host machine. Still, that means there should be no trouble.

environment: {
MYSQL_ROOT_PASSWORD:
dbEnv.credentials.WORDPRESS_DB_PASSWORD,
MYSQL_DATABASE: dbEnv.tests.WORDPRESS_DB_NAME,
},
volumes: [ 'mysql-test:/var/lib/mysql' ],
},
wordpress: {
build: '.',
depends_on: [ 'mysql' ],
image: developmentWpImage,
ports: [ developmentPorts ],
environment: {
WORDPRESS_DB_NAME: 'wordpress',
...dbEnv.credentials,
...dbEnv.development,
},
volumes: developmentMounts,
},
'tests-wordpress': {
depends_on: [ 'mysql' ],
depends_on: [ 'tests-mysql' ],
image: testsWpImage,
ports: [ testsPorts ],
environment: {
WORDPRESS_DB_NAME: 'tests-wordpress',
...dbEnv.credentials,
...dbEnv.tests,
},
volumes: testsMounts,
},
Expand All @@ -204,12 +219,20 @@ module.exports = function buildDockerComposeConfig( config ) {
image: developmentWpCliImage,
volumes: developmentMounts,
user: cliUser,
environment: {
...dbEnv.credentials,
...dbEnv.development,
},
},
'tests-cli': {
depends_on: [ 'tests-wordpress' ],
image: testsWpCliImage,
volumes: testsMounts,
user: cliUser,
environment: {
...dbEnv.credentials,
...dbEnv.tests,
},
},
composer: {
image: 'composer',
noahtallen marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -228,13 +251,16 @@ module.exports = function buildDockerComposeConfig( config ) {
LOCAL_DIR: 'html',
WP_PHPUNIT__TESTS_CONFIG:
'/var/www/html/phpunit-wp-config.php',
...dbEnv.credentials,
...dbEnv.tests,
},
},
},
volumes: {
...( ! config.coreSource && { wordpress: {} } ),
...( ! config.coreSource && { 'tests-wordpress': {} } ),
mysql: {},
'mysql-test': {},
'phpunit-uploads': {},
},
};
Expand Down
6 changes: 6 additions & 0 deletions packages/env/lib/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const downloadSources = require( '../download-sources' );
const {
checkDatabaseConnection,
makeContentDirectoriesWritable,
makeConfigWritable,
configureWordPress,
setupWordPressDirectories,
} = require( '../wordpress' );
Expand Down Expand Up @@ -140,6 +141,11 @@ module.exports = async function start( { spinner, debug, update, xdebug } ) {
: [],
} );

await Promise.all( [
makeConfigWritable( 'development', config ),
makeConfigWritable( 'tests', config ),
] );

// Only run WordPress install/configuration when config has changed.
if ( shouldConfigureWp ) {
spinner.text = 'Configuring WordPress.';
Expand Down
23 changes: 23 additions & 0 deletions packages/env/lib/config/db-env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Username and password used in all databases
const credentials = {
WORDPRESS_DB_USER: 'root',
WORDPRESS_DB_PASSWORD: 'password',
};

// Environment for test database
const tests = {
WORDPRESS_DB_NAME: 'tests-wordpress',
WORDPRESS_DB_HOST: 'tests-mysql',
};

// Environment for development database. DB host gets default value which is set
// elsewhere
const development = {
WORDPRESS_DB_NAME: 'wordpress',
};

module.exports = {
credentials,
tests,
development,
};
2 changes: 2 additions & 0 deletions packages/env/lib/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
const readConfig = require( './config' );
const { ValidationError } = require( './validate-config' );
const dbEnv = require( './db-env' );

/**
* @typedef {import('./config').WPConfig} WPConfig
Expand All @@ -13,4 +14,5 @@ const { ValidationError } = require( './validate-config' );
module.exports = {
ValidationError,
readConfig,
dbEnv,
};
22 changes: 22 additions & 0 deletions packages/env/lib/wordpress.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,27 @@ async function makeContentDirectoriesWritable(
);
}

/**
* Makes wp-config.php owned by www-data so that WordPress can work with the
* file.
*
* @param {WPEnvironment} environment The environment to check. Either 'development' or 'tests'.
* @param {WPConfig} config The wp-env config object.
*/
async function makeConfigWritable(
noahtallen marked this conversation as resolved.
Show resolved Hide resolved
environment,
{ dockerComposeConfigPath, debug }
) {
await dockerCompose.exec(
environment === 'development' ? 'wordpress' : 'tests-wordpress',
'chown www-data:www-data wp-config.php',
{
config: dockerComposeConfigPath,
log: debug,
}
);
}

/**
* Checks a WordPress database connection. An error is thrown if the test is
* unsuccessful.
Expand Down Expand Up @@ -260,6 +281,7 @@ async function copyCoreFiles( fromPath, toPath ) {
module.exports = {
hasSameCoreSource,
makeContentDirectoriesWritable,
makeConfigWritable,
checkDatabaseConnection,
configureWordPress,
resetDatabase,
Expand Down