aminghadersohi commented on PR #44723:
URL: https://github.com/apache/superset/pull/44723#issuecomment-5902694783
Review summary at 0afef7e82e (reviewed 485c4272bc plus the merge 9a0e549926).
**Driver paths:** mysqlclient (REQUIRED on recognized Oracle clients,
VERIFY_CA on MariaDB Connector/C and unknown clients), Connector/Python and
PyMySQL (`ssl_verify_cert=True`, PyMySQL >= 1.2), the Aurora Data API (HTTPS,
`ssl` consumed), and all other drivers (rejected) fail closed. Connections
without the toggle or `ssl=1` are returned unchanged. I also confirmed that
Connector/C 3.3.19 reports `3.3.19` from `mysql_get_client_info()`, so the
version check routes it to VERIFY_CA. The IAM path keeps the enforced args
because `ssl_args={}` merges into them.
**Findings and fixes (0afef7e82e):**
1. Merge resolution: the MySQL heading ended up above master's `/register/`
bullet, so that bullet read as part of the MySQL section. I moved the MySQL
section after the bullet, so the diff against master is a pure addition again.
Apart from this, the merge kept both sides and only the three PR files differ
from master.
2. Tests: three fail-closed branches had no coverage. These were the nested
PyMySQL `ssl` dict with the toggle, an invalid `ssl` value type, and
unsupported drivers (cymysql, mariadbconnector, aiomysql). I added a test for
each.
3. rebenitez1802's low item about MariaDB: UPDATING and the metadata notes
now say that `mariadb://` (MariaDB engine) and other MySQL-compatible engines
keep their existing SSL handling.
**Reviewer asks at head:** everything is closed. That covers all five of
EnxDev's inline notes (server_cert/CA docs, the 6.1.0 snapshot being untouched
with the text in `metadata.notes`, the PyMySQL < 1.2 test, the
client-library-based mode, and SSH tunnel guidance). It also covers both of
rebenitez1802's items (upgrade impact/recovery and the Aurora RDS CA), plus the
low REQUIRED and PyMySQL-gate items.
**Not changed (pre-existing, admin-controlled):** a `connect_args` key in
encrypted extra replaces the engine `connect_args` after
`adjust_engine_params`. That bypasses enforcement for such configs, the same as
before this PR.
**Checks:** MySQL, AWS IAM, Aurora and MariaDB spec tests: 190 passed.
Changed-file pre-commit passed, including mypy, pylint and metadata validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
