Add support for MySQL socket connection

[wojtekn]

Related https://github.com/wp-cli/wp-cli/issues/5859

In this diff, I propose to add support for MySQL socket connection to the wp config create command.

I used the approach proposed by @aonsyed under https://github.com/wp-cli/config-command/issues/169

Testing steps:

1. Start MySQL/MariaDB, download WordPress and unpack it
2. Test MySQL socket connection e.g.

   mysql -S /tmp/mysql.sock

3. Create a database user
4. Run command

   vendor/bin/wp config create --dbname=dbname --dbuser=dbuser --dbpass=somevalue --dbprefix=wp_ --url=site.com --dbhost=localhost:/tmp/mysql.sock --allow-root --skip-themes --skip-packages --skip-plugins --path=/Sites/wordpress-root

5. Confirm the success message:

   Success: Generated 'wp-config.php' file.

6. Check if the wp-config.php file was created and confirm that it has a socket configured in place of dbhost

[wojtekn]

I'm unsure how to test the --dbhost=localhost:/var/run/mysqld/mysqld.sock format as it looks specific to Linux. On MacOS, I need to provide the socket file name, e.g. --dbhost=/tmp/mysql.sock.

However, as soon as the format passes the file_exists() check and mysqli_real_connect() can use it as a valid socket, it will be written to the generated wp-config.php in that form.

[danielbachhuber]

Can we add a functional test for this?

[aonsyed]

Different OSes use different paths but file check should succeed for 'unix://path/mysql.sock' or 'localhost://path/mysql.sock' too or you can use realpath() function to get the actual path to the file which would be the same in this case, on mac localhost one should work since that's supposed to point to device itself

[wojtekn]

Can we add a functional test for this?

@danielbachhuber I've added one, but I couldn't make it work on GH environments, so I added --skip-check so now it only checks if the value is written to the wp-config.php file.

Do you think it's enough? I couldn't quickly figure out how we could use MySQL with a socket connection during GH action.

[schlessera]

@wojtekn At the very least, you can try to run the command without the --skip-check but with --debug, expect it to fail and assert that the debugging output shows the right MySQL command (the --debug output will dump the exact command being used, IIRC).

[danielbachhuber]

@wp-cli/committers 976b7c6754c9f2b6c9bc74ae09555d4fb3945695 passes for me locally, but then fails in the container.

What do you think we should do? I think I'd be fine adding a @broken flag for the test, and landing the PR.

[aonsyed]

@danielbachhuber why not try the proper sock path /var/run/mysqld/mysqld.sock instead of fetching it from socket which seems to be sending both sock and port

[danielbachhuber]

why not try the proper sock path /var/run/mysqld/mysqld.sock instead of fetching it from socket which seems to be sending both sock and port

@aonsyed I'm not sure I follow. Can you show me with a diff?

I'm using wp db query 'SELECT @@GLOBAL.SOCKET' --skip-column-names because the socket can vary between platforms.

[wojsmol]

@danielbachhuber Looking at tast failures --dbhost is pass twice - $ wp config create --dbname='wp_cli_test' --dbuser='wp_cli_test' --dbpass='password1' --dbhost='127.0.0.1:32768' --dbhost=/var/run/mysqld/mysqld.sock

[aonsyed]

@danielbachhuber definitely it can change but it seems that docker environement for testing is pretty standard debian based linux for all the MySQL version from 5.3 to 8.0 in the your test environment and the socket path is always /var/run/mysqld/mysqld.sock so you can use that and it will pass the tests

The best solution for a controlled testing would be to change this in parameters echo "WP_CLI_TEST_DBHOST=127.0.0.1:32768" >> $GITHUB_ENV use sock instead but that would mean changing the test environment for everything

Basically use the previous test method instead of getting socket path from variable and use /var/run/mysqld/mysqld.sock instead of /tmp/mysql.sock

[danielbachhuber]

I think it's finding the right socket.

Unfortunately, there's an authentication error for < PHP 7.4:

I'm seeing Error: Database connection error (1045) Access denied for user 'wp_cli_test'@'localhost' (using password: YES) for PHP 8+

[aonsyed]

@danielbachhuber the issue lies with {CORE_CONFIG_SETTINGS}, it already contains dbhost from the docker config here

  echo "MYSQL_HOST=127.0.0.1" >> $GITHUB_ENV
  echo "MYSQL_TCP_PORT=32768" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBROOTUSER=root" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBROOTPASS=root" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBNAME=wp_cli_test" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBUSER=wp_cli_test" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBPASS=password1" >> $GITHUB_ENV
  echo "WP_CLI_TEST_DBHOST=127.0.0.1:32768" >> $GITHUB_ENV

Alternatively, you can change the test like this

    When I run `wp config create  --dbname='wp_cli_test' --dbuser='wp_cli_test' --dbpass='password1' --dbhost={SOCKET}`

Alternatively, some other way to drop--dbhost='127.0.0.1:32768' from the {CORE_CONFIG_SETTINGS} would do the same thing.

[aonsyed]

@wojtekn This test won't work since {CORE_CONFIG_SETTINGS} contains the DB settings from the enviorment, you have to test it without that for it to work

[wojtekn]

@aonsyed, thanks, I will check those.

In the meantime, I found out that my original implementation was not correct - it was accepting a socket file path, e.g., /tmp/mysql.sock, validating it correctly, and using that value in the generated wp-config.php file, but it was not enough. The wpdb() class expects that if dbhost is a socket, it should include the host part so the valid accepted and written format should be localhost:/tmp/mysql.sock.

[sitamet]

When trying to configure db socket connection:

wp config create --dbname=wordpress --dbuser=root --dbpass=secret --dbhost=localhost:/var/run/mysql/mysqld.sock --dbprefix=wp_

Returns: Error: Database connection error (2002) Cannot assign requested address

Socket connections expressed as localhost:socketfile worked until v2.9.0

[swissspidy]

@sitamet Just so I understand, are you reporting the same issue as #169? If so, this PR here is trying to solve it.

[danielbachhuber]

The tests pass fine locally 😭

$ composer behat -- features/config-create.feature
> run-behat-tests 'features/config-create.feature'
...................................................................... 70
.........................

9 scenarios (9 passed)
95 steps (95 passed)
0m20.62s (10.89Mb)

My guess is that the tests are finding a socket in GitHub Actions, but the socket doesn't correlate to the MySQL test database. I don't have any great ideas to work around this.

[wojtekn]

My guess is that the tests are finding a socket in GitHub Actions, but the socket doesn't correlate to the MySQL test database. I don't have any great ideas to work around this.

@danielbachhuber they are finding a socket but failing on authorization:

Error: Database connection error (1045) Access denied for user 'wp_cli_test'@'localhost' (using password: YES)

Isn't it something related to how we add a user e.g. we add a GRANT for 'wp_cli_test'@'127.0.0.1' but not for 'wp_cli_test'@'localhost' or something similar?

Another thing is that some tests are falling on missing authentication protocol e.g. caching_sha2_password. We would need to either fix those on Docker image level, or use-flags to not run tests for affected PHP versions.

[schlessera]

@danielbachhuber @swissspidy FYI: I finally got this to work, but in order to do so, I had to rethink the way the database is handled in GHA. We previously had a GHA service with a MySQL image running. It turned out that this did not actually work as intended, as there was confusion between the MySQL server bundled with the Ubuntu OS and the one from the service. They were using the same name, and the "DB start" action we had was actually starting the Ubuntu-bundled one, as the other was already running.

The system I'm using now does not use a GHA service, but rather uses an action that downloads the MySQL server and then launches it. This has the following impact:

• it lets us control when the DB docker container is being assembled. This important for the socket testing here, as a socket within a container needs to be created within a bind mount, otherwise the host system has no access. With the previous service-based approach, there was no way of establishing that bind mount before the socket was created (note: the socket file itself cannot be mounted into the host).
• it is easier to configure the provided server, as we can adapt the my.cnf file through previous actions and other data generated during the job.
• it removes the need for a GHA service, which was one of the blockers for testing against Windows and Macs, as GHA services are only supported on Linux images.

[wojtekn]

Thanks for solving it @schlessera !

it removes the need for a GHA service, which was one of the blockers for testing against Windows and Macs, as GHA services are only supported on Linux images.

This is a great outcome 👍

[danielbachhuber]

Awesome! Thanks @schlessera 😊