maks3201 commented on PR #66559:
URL: https://github.com/apache/doris/pull/66559#issuecomment-5228559213
Thanks for the triage. You were right that the fix direction was correct but
the PR had no regression test, so I have added one: `MySqlStartupSslITCase`.
Following your suggestion it starts a MySQL container, turns on
`require_secure_transport=ON`, and runs a CDC job with `offset=latest` and
`ssl_mode=require`, which is the mode that reaches
`initializeEffectiveOffset()`.
I verified it in both directions, because a regression test that passes on
patched and unpatched code alike would be worthless.
**On this PR's code the test passes:**
```
INFO: SSL enabled
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 56.82
s -- in org.apache.doris.cdcclient.itcase.MySqlStartupSslITCase
[INFO] BUILD SUCCESS
```
**With only `MySqlSourceReader.java` reverted to master, the test fails, and
for the right reason:**
```
[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0 <<< FAILURE! -- in
MySqlStartupSslITCase
org.apache.flink.util.FlinkRuntimeException: Cannot read the binlog filename
and position via 'SHOW MASTER STATUS'
at
org.apache.flink.cdc.connectors.mysql.debezium.DebeziumUtils.currentBinlogOffset(DebeziumUtils.java:143)
at
org.apache.flink.cdc.connectors.mysql.source.offset.BinlogOffsetUtils.initializeEffectiveOffset(BinlogOffsetUtils.java:58)
at
org.apache.doris.cdcclient.source.reader.mysql.MySqlSourceReader.initializeEffectiveOffset(MySqlSourceReader.java:1075)
at
org.apache.doris.cdcclient.source.reader.mysql.MySqlSourceReader.generateMySqlConfig(MySqlSourceReader.java:960)
Caused by: java.sql.SQLException: Connections using insecure transport are
prohibited while --require_secure_transport=ON.
```
That is exactly the path described in the issue: the early JDBC connection
inside `initializeEffectiveOffset()` is opened before
`configFactory.jdbcProperties(...)` is set, so it carries no SSL settings and
the server rejects it.
Note that only 1 of the 3 methods fails on unpatched code. That is intended.
The non-TLS control case runs against a second container without the
secure-transport requirement, and the plaintext sanity check only asserts the
server enforces TLS, so neither is affected by the ordering bug. A sibling
test, `MySqlStartupLatestITCase`, passed in both runs, which rules out an
environment problem.
Two notes on the test design, in case they come up in review:
1. `require_secure_transport=ON` is enabled after container startup via
`execInContainer`, because the Testcontainers readiness probe connects without
TLS and would otherwise never see the container as ready. For the same reason
all SQL the test issues against that container goes through the mysql CLI over
the Unix socket, which is exempt from the requirement, instead of JDBC.
2. The non-TLS control uses a separate container rather than toggling the
flag off and on. With a shared container, a failure between the toggle and the
restore would leave the requirement OFF and silently make the SSL assertions
pass for the wrong reason.
I only covered `offset=latest`. `earliest` and the timestamp mode go through
the same `initializeEffectiveOffset()` call, so they add no coverage of the
ordering bug, and they would need different assertions since they replay the
rows written during setup instead of skipping them. Happy to add them if you
would rather have all three.
One thing worth flagging for CI: when I ran this locally the
`spring-boot-maven-plugin` `repackage` goal moved the application classes under
`BOOT-INF/classes`, and failsafe then failed with `NoClassDefFoundError` on
`org.apache.doris.cdcclient.common.Env` before any test ran. I worked around it
with `-Dspring-boot.repackage.skip=true`. If the project's integration-test job
hits the same thing, that is the cause, and it is not specific to this test
since the existing `MySqlStartupLatestITCase` failed identically.
--
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]
