Attach connection error to CONNECTION_URL#1626
Attach connection error to CONNECTION_URL#1626Praveen Kumar G (pgajendran-confluent) wants to merge 2 commits intomasterfrom
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
Fixes sink connector validation so database connection failures are surfaced in the returned config validation results by attaching the error to a real config key.
Changes:
- Switch validation error attachment from
connection.host(not inJdbcSinkConfig.CONFIG_DEF) toconnection.url(defined). - Update static import to use
JdbcSinkConfig.CONNECTION_URL.
Comments suppressed due to low confidence (1)
src/main/java/io/confluent/connect/jdbc/validation/JdbcSinkConnectorValidation.java:112
validateConnection()catchesException, but the log line hard-codes "SQLException during validation". Either narrow the catch toSQLException(and handle other exceptions separately) or update the log message to reflect that any exception type may be thrown here.
configValue(validationResult, CONNECTION_URL)
.ifPresent(hostName ->
hostName.addErrorMessage(
String.format("Could not connect to database. %s", e.getMessage())));
log.error("SQLException during validation", e);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hostName.addErrorMessage( | ||
| String.format("Could not connect to database. %s", e.getMessage()))); |
There was a problem hiding this comment.
Including the raw exception message (e.getMessage()) in the connector validation error can leak sensitive connection details (some JDBC drivers include the full URL and credentials in exception messages). Consider using a generic message (and optionally include only non-sensitive fields like exception class and/or SQLState when e is a SQLException) while keeping the full exception details in logs.
Arihant Jain (arihant-confluent)
left a comment
There was a problem hiding this comment.
LGTM.
Are there any existing UTs to test it?
Thanks. No UTs currently to test it though. |
|
Can you raise the PR against the earliest feature branch in which the issue is present. |
Problem
JdbcSinkConnectorValidation.validateConnection() catches database connection errors (e.g., SSL failures) but silently drops them because it tries to attach the error to connection.host, which is not defined in JdbcSinkConfig.CONFIG_DEF.
connection.host is a connect-templates variable, not a native kafka-connect-jdbc config. The configValue() helper returns Optional.empty() and the ifPresent callback never fires — the error is logged but never added to the validation result.
Solution
Attach the connection error to CONNECTION_URL (connection.url) instead of CONNECTION_HOST (connection.host), since connection.url IS defined in CONFIG_DEF.
Does this solution apply anywhere else?
If yes, where?
Test Strategy
Testing done:
Release Plan