Skip to content

Conversation

@hazratmilad
Copy link

This PR adds optional SSL support to MySqlSchemaState when dumping schema via mysqldump.
The following improvements are included:

  • SSL CERT, KEY support

Values defined in PDO options are now passed through to the dump command.

  • MariaDB compatibility

MariaDB clients do not universally support --ssl-cert, --ssl-key, or --ssl-verify-server-cert.
These options are now applied only when the connection is not MariaDB.

@rodrigopedra
Copy link
Contributor

Shouldn't this be added to the Illuminate\Database\Schema\MariaDbSchemaState class instead?

You can override that method on that class and add the MariaDB-specific code to it.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

@hazratmilad
Copy link
Author

Shouldn't this be added to the Illuminate\Database\Schema\MariaDbSchemaState class instead?

You can override that method on that class and add the MariaDB-specific code to it.

Hi, no, we can't do that. because MariaDB clients do not universally support --ssl-cert, --ssl-key, or --ssl-verify-server-cert.

@rodrigopedra
Copy link
Contributor

My bad. I missed the ! not operator.

@hazratmilad
Copy link
Author

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

https://packagist.org/packages/hazratmilad/db-secure-schema-state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants