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

Mysql_db dump/import add missing SSL mode #570

Closed
wants to merge 2 commits into from

Conversation

FlorianPerrot
Copy link

SUMMARY

Add missing argument --ssl for db dump and import if check_hostname is true.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • plugins/modules/mysql_db.py
ADDITIONAL INFORMATION

Useful, if you want is ssl mode without certifcat.
https://mariadb.com/kb/en/securing-connections-for-client-and-server/#enabling-one-way-tls-for-mariadb-clients-without-server-certificate-verification

@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Patch coverage has no change and project coverage change: -2.75% ⚠️

Comparison is base (9439282) 76.96% compared to head (c07f865) 74.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
- Coverage   76.96%   74.22%   -2.75%     
==========================================
  Files          28       18      -10     
  Lines        2379     2258     -121     
  Branches      579      562      -17     
==========================================
- Hits         1831     1676     -155     
- Misses        389      409      +20     
- Partials      159      173      +14     
Flag Coverage Δ
integration 73.60% <0.00%> (-0.14%) ⬇️
sanity ?
units ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
plugins/modules/mysql_db.py 73.84% <0.00%> (-1.33%) ⬇️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Andersson007
Copy link
Collaborator

@FlorianPerrot hello, thanks for the patch!
could you please add a changelog fragment https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment ?

Copy link
Member

@betanummeric betanummeric left a comment

Choose a reason for hiding this comment

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

The --ssl option is implied by other --ssl-* options. As a workaround, you could set the ssl_ca variable to some file with trusted CA certs.

The --ssl argument does not exist in MySQL 8, it was removed in favor of --ssl-mode.

The name check_hostname is ambiguous: What is checked?
I would expect it to mean --ssl-verify-server-cert for MariaDB and --ssl-mode=VERIFY_IDENTITY for MySQL. But this should be decided and documented.

MariaDB docs: https://mariadb.com/kb/en/securing-connections-for-client-and-server/#enabling-one-way-tls-for-mariadb-clients
MySQL docs: https://dev.mysql.com/doc/refman/8.0/en/using-encrypted-connections.html#using-encrypted-connections-client-side-configuration

@FlorianPerrot
Copy link
Author

Hum good point. I hadn't seen "--ssl" argument different with mariadb and mysql...
But I have a use case where it is useful to require "--ssl" for One-Way TLS without server certificate verification.
I think my feature is not necessary, I can use "config_file" parameters and "check_hostname" is indeed ambiguous.

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