Support for MySQL SSL options (--mysql-ssl-cert, --mysql-ssl-key, --mysql-ssl-ca)
#77
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce new SSL configuration options for MySQL connections, enhancing security by allowing users to specify paths for SSL certificate, key, and CA certificate files. Additionally, a Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant CLI
participant Transporter
participant MySQL
User->>CLI: Provide SSL options (--mysql-ssl-cert, --mysql-ssl-key, --mysql-ssl-ca)
CLI->>Transporter: Pass SSL options
Transporter->>MySQL: Establish connection with SSL parameters
MySQL-->>Transporter: Confirm connection
Transporter-->>CLI: Notify successful connection
CLI-->>User: Display connection status
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- README.md (1 hunks)
- docs/README.rst (1 hunks)
- src/mysql_to_sqlite3/cli.py (3 hunks)
- src/mysql_to_sqlite3/transporter.py (2 hunks)
Additional context used
LanguageTool
README.md
[locale-violation] ~5-~5: license must be spelled with a “c” when used as a noun in British English. Use “licence”. (LICENCE_LICENSE_NOUN_SINGULAR)
Context: ...10.2+|+10.3+|+10.4+|+10.5&color=C0765A) [
Heading levels should only increment by one level at a time
31-31: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (6)
docs/README.rst (1)
46-48: The documentation for the new SSL options is clear and concise.src/mysql_to_sqlite3/cli.py (2)
103-105: The addition of SSL options in the CLI is implemented correctly.
Line range hint
148-182: The integration of SSL options into the main function is handled properly. Ensure that themysql_ssl_disabledflag is tested thoroughly.README.md (1)
71-73: The documentation for the new SSL options in the main README is clear and concise.src/mysql_to_sqlite3/transporter.py (2)
91-96: The addition of SSL options in theMySQLtoSQLiteclass constructor is implemented correctly.
132-134: The use of SSL options in the MySQL connection setup is handled properly.
|
Thanks for your contribution. 💪 Can you please fix the linter errors and add some extra tests for this feature (if possible)? |
|
Hi @techouse, ok I'll try in the next few days, be patient but I'm new to Python.. |
|
Hi @techouse, sorry for the delay, I fixed the linters errors and added new ssl tests.
[testenv]
allowlist_externals = pytest
commands = pytest -v --cov=src/mysql_to_sqlite3 --cov-report=xml --docker-mysql-image=mysql:8-oraclelinux8and run the tests this way: tox -c tox.ini -c tox.override.ini
Let me know if that works out! |
|
Hi @frugan-dev Looks like there are still some fundamental errors in your code. You don't have to use tox to test it locally, running something like pytest -v -xshould do the trick. As for the Tox setup, please do not commit / git-track these changes as they are specific to your setup and do not reflect a wider problem. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #77 +/- ##
==========================================
+ Coverage 85.85% 86.05% +0.19%
==========================================
Files 8 8
Lines 629 638 +9
==========================================
+ Hits 540 549 +9
Misses 89 89 ☔ View full report in Codecov by Sentry. |
Hmm, this was never an issue before. Maybe it's SSL-specific. You can try to commit the change to |
Thanks, I made some fixes.
Sure, I only committed the |
Commit whatever you think is necessary to make the CI tests pass. :) |
Actually on my PC it connects after about 20-25 attempts, but for now let's leave it like this, there's no problem.. I wouldn't want the increase to affect the execution time of the CI tests.
Ok I'll take a look ASAP! |
Pull Request: Add MySQL SSL Options
Description
This pull request adds support for MySQL SSL options (
--mysql-ssl-cert,--mysql-ssl-key,--mysql-ssl-ca), that allow users to specify the paths to SSL certificate, key, and CA certificate files when connecting to MySQL databases that require secure transport (--require_secure_transport=ON).This enhancement aims to address the issue where connections using insecure transport are prohibited due to MySQL server settings, e.g. when this error message appears:
Type of change
How Has This Been Tested?
These changes have been tested locally by connecting to MySQL databases with
--require_secure_transport=ONusing the newly introduced SSL options.Checklist: