fix: dbConnect() can enable SSL based on client_flag again

[d-hansen]

Allows connection to proceed when SSL is required without having to rely on a default file or group with ssl-enforce=1.

[aviator-app]

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

---

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

[d-hansen]

Okay, apparently I need to wrap this code with a check based on whether or not it's MariaDB or MySQL as the MySQL library does not appear to have some of those options. I will re-work in the next few days to be compatible with the MySQL library.

[d-hansen]

Hmm, my systems with the MySQL library compiled fine too and definitely have those two options defined in /usr/include/mysql/mysql.h... I will wait for advice or comment. I guess I could always just wrap them with some #if logic...

[krlmlr]

If this compiles with MariaDB (and is necessary there), we could use preprocessor logic. Does it also depend on the version of the MySQL library?

[d-hansen]

If this compiles with MariaDB (and is necessary there), we could use preprocessor logic. Does it also depend on the version of the MySQL library?

Compiles fine with both MariaDB 10.5 and MySQL 5.7.44. I will test that it also compiles with MySQL 8.0 on Monday.
Can you give me any info on the environment used in the smoke test? Clearly the MYSQL_OPT_SSL_ENFORCE enum is missing in that environment. I will also craft up the appropriate preprocessor logic to drop that code out if the enum's are missing.

Thanks!

[d-hansen]

If this compiles with MariaDB (and is necessary there), we could use preprocessor logic. Does it also depend on the version of the MySQL library?

Compiles fine with both MariaDB 10.5 and MySQL 5.7.44. I will test that it also compiles with MySQL 8.0 on Monday. Can you give me any info on the environment used in the smoke test? Clearly the MYSQL_OPT_SSL_ENFORCE enum is missing in that environment. I will also craft up the appropriate preprocessor logic to drop that code out if the enum's are missing.

Thanks!

Nevermind on the environment question - I found it. It's using libmysqlclient-dev_8.0.36-0ubuntu0.22.04.1_amd64.deb
So, I will update on Monday with the appropriate logic to handle the scenario with MySQL 8.0

[d-hansen]

Well, I found the relevant issue and will work on an update to handle MySQL 8.0: Changes in MySQL 8.0.0 for MYSQL_OPT_SSL_ENFORCE

[d-hansen]

Note: though this now compiles with MySQL 8.0, I'm thinking maybe DbConnection should get updated to use a new/different argument ssl_mode which would then better align with MySQL 8.0. Note: MySQL 8.0 also marks CLIENT_SSL_VERIFY_SERVER_CERT as deprecated MySQL 8.0 CLIENT_SSL_VERIFY_SERVER_CERT option.

Interestingly, the MariaDB connector is still using the older method (relying on MYSQL_OPT_SSL_ENFORCE), so perhaps since this module is actually called RMariaDB it should stick with how MariaDB is enabling SSL.

[d-hansen]

Also, I am now looking at why the "test" fails in the smoke test.

[krlmlr]

Great, thanks!

CI/CD is failing on the main branch, I'll take a look.

[krlmlr]

Is the cast to void * necessary here? Omitting it feels slightly clearer, but no strong preference.

[d-hansen]

That would depend on the compiler warnings. I can certainly check whether it throws warnings in my environment. I was just following the same practice that was being done with mysql_options in other places in this file.

[krlmlr]

Thanks. Can you confirm that this fixes #256?

[aviator-app]

This pull request failed to merge: some CI status(es) failed. Once the issues are resolved, remove the blocked label and re-queue the pull request. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Failed CI(s): windows-latest (release)

[aviator-app]

This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. Once the issues are resolved, remove the blocked label and re-queue the pull request. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Additional debug info: Failed to rebase this PR onto the latest changes from the base branch. You will probably need to rebase this PR manually and resolve conflicts).

[aviator-app]

This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. Once the issues are resolved, remove the blocked label and re-queue the pull request. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Additional debug info: Failed to rebase this PR onto the latest changes from the base branch. You will probably need to rebase this PR manually and resolve conflicts).

[aviator-app]

This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. Once the issues are resolved, remove the blocked label and re-queue the pull request. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Additional debug info: Failed to rebase this PR onto the latest changes from the base branch. You will probably need to rebase this PR manually and resolve conflicts).

[d-hansen]

Thanks. Can you confirm that this fixes #256?

Yes, it should.
Note: I am now testing on three variants: MySQL 5.7, MariaDB 10.5, and MySQL 8.0 to make sure it works with or without SSL on each of those. So, standby for any potential updates.

Once I have confirmed it works on all three, I will also update the documentation to reflect the different ways it can be configured.