wangzhigang1999 commented on code in PR #7431:
URL: https://github.com/apache/kyuubi/pull/7431#discussion_r3317577525
##########
externals/kyuubi-data-agent-engine/src/main/java/org/apache/kyuubi/engine/dataagent/provider/chatcompletion/ChatCompletionProvider.java:
##########
@@ -118,15 +118,29 @@ private static DataSource attachJdbcDataSource(
if (jdbcUrl == null) {
return null;
}
- LOG.info("Data Agent JDBC URL configured ({})",
jdbcUrl.replaceAll("//.*@", "//<redacted>@"));
-
- String sessionUser =
- ConfUtils.optionalString(conf,
KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY());
-
- DataSource ds = DataSourceFactory.create(jdbcUrl, sessionUser);
+ JdbcDialect dialect = JdbcDialect.fromUrl(jdbcUrl);
+ if (dialect == null) {
+ throw new IllegalArgumentException(
+ "Invalid " + KyuubiConf.ENGINE_DATA_AGENT_JDBC_URL().key());
+ }
+ LOG.info("Data Agent JDBC datasource configured ({})",
dialect.datasourceName());
+
+ // The kyuubi session user is only meaningful when we connect back to
Kyuubi/HiveServer2,
+ // where it drives proxy-user impersonation for the downstream engine. For
external JDBC
+ // backends (mysql, postgresql, trino, ...) it would shadow URL-supplied
credentials and
+ // cause auth failures (e.g. MySQL Connector/J overriding ?user=root with
anonymous), so
+ // leave the username unset and let the driver read it from the URL.
+ String subprotocol = JdbcDialect.extractSubprotocol(jdbcUrl);
Review Comment:
Good point. I checked the JDBC engine pattern: it uses `propagateCredential`
(default `false`) plus explicit `kyuubi.engine.jdbc.connection.user` /
`.password`, passed via JDBC `Properties`.
For this PR I kept the fix minimal: for Kyuubi/Hive2 URLs, the session user
is only passed when the URL has no explicit `;user=` session variable. So
`jdbc:kyuubi://host/db;user=alice` is left for the driver to handle as-is.
I agree the complete fix should be a follow-up: Data Agent JDBC
user/password configs plus a `propagateCredential` opt-in, with UI inputs so
passwords do not need to be embedded in URLs.
--
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]
