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

Console Commands: Possible bug - Using MasterSlaveConnection does not respect driverOptions #1230

Closed
petewalker opened this issue Oct 12, 2020 · 6 comments · Fixed by #1240
Closed

Comments

@petewalker
Copy link
Contributor

We're using MasterSlaveConnection to allow us to make use of read replicas in production.

I'm trying to set up an SSL certificate with the connections as we'd like to enforce TLS on our connections. I'm using the below config:

doctrine:
  dbal:
    driver: pdo_mysql
    host: "%env(DATABASE_WRITER_HOST)%"
    port: "%env(DATABASE_WRITER_PORT)%"
    dbname: "%env(DATABASE_NAME)%"
    user: "%env(DATABASE_USER)%"
    password: "%env(DATABASE_PASSWORD)%"
    slaves:
      slave1:
        host: "%env(DATABASE_READER_HOST)%"
        port: "%env(DATABASE_READER_PORT)%"
        dbname: "%env(DATABASE_NAME)%"
        user: "%env(DATABASE_USER)%"
        password: "%env(DATABASE_PASSWORD)%"
    options:
      !php/const:PDO::MYSQL_ATTR_SSL_CA: "%kernel.project_dir%/certs/ca.crt"

Symfony responds to this config fine, and a TLS connection seems to be established with the MySQL server. However, when I try to run a CLI command:

bin/console doctrine:database:drop --force --env test

In AbstractMySQLDriver.php line 106:
                                                                                                                       
  An exception occurred in driver: SQLSTATE[HY000] [3159] Connections using insecure transport are prohibited while -  
  -require_secure_transport=ON.  

When I inspect what's going on in the command:

$params = $connection->getParams();
if (isset($params['master'])) {
    $params = $params['master'];
}
// Yields
/*
array(6) {
  ["host"]=>
  string(9) "op_api_db"
  ["port"]=>
  string(4) "3306"
  ["dbname"]=>
  string(14) "local"
  ["user"]=>
  string(4) "root"
  ["password"]=>
  string(12) "insecurepass"
  ["driver"]=>
  string(9) "pdo_mysql"
}
*/

The driverOptions key is present in the initial $params array, but is then lost by this condition. If I replace with:

$params = $connection->getParams();
$driverOptions = $params['driverOptions'] ?? [];
if (isset($params['master'])) {
    $params = $params['master'];
}
$params['driverOptions'] = $driverOptions;

It works. Is this a bug, or have I misconfigured?

@ostrolucky
Copy link
Member

ostrolucky commented Oct 12, 2020

It's a bc break in dbal 2.11. This is most likely duplicate issue of #1214. Please always check what you updated before regressions started to happen.

@petewalker
Copy link
Contributor Author

This isn't the same issue. We're currently pinned at dbal 2.10 because of that issue.

The change I made was to try and configure TLS. It works without the TLS config.

@dmaicher
Copy link
Contributor

To me this indeed looks like a bug in the command. We will then also have the same issue in other commands like CreateDatabaseDoctrineCommand

@kralos
Copy link

kralos commented Oct 22, 2020

@dmaicher technically not a bug in doctrine-bundle, it's a BC break in dbal. See doctrine/dbal#4295 (comment)

that said, it's pretty dirty to be touching (the now @internal) $params from a dbal connection... especially considering how much the params are manipulated inside MasterSlaveConnection / PrimaryReadReplicaConnection.

it would probably be more future proof if we could get create/drop database methods added to dbal allowing properties to remain dbal's problem.

@petewalker
Copy link
Contributor Author

@kralos As mentioned above, this is not caused by doctrine/dbal#4295 (comment) - it's a separate issue. In the above code, the master key is present in the $params array. I originally encountered this issue whilst using dbal 2.10.

However, I definitely agree with your sentiment that doctrine-bundle shouldn't be messing with the $params array in this way.

ostrolucky added a commit that referenced this issue Oct 25, 2020
@ostrolucky
Copy link
Member

Check #1240

@ostrolucky ostrolucky added the Bug label Oct 25, 2020
ostrolucky added a commit that referenced this issue Oct 28, 2020
Fix #1230 - Console commands do not respect driverOptions when master/slave configuration is used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants