github-actions[bot] commented on code in PR #66890:
URL: https://github.com/apache/doris/pull/66890#discussion_r3802821471
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java:
##########
@@ -681,6 +681,8 @@ private HadoopAuthenticator pluginAuthenticator() {
* {@link KerberosAuthenticationConfig}, so the {@code doAs} logs in
the same client identity fe-core
* used. The HMS <em>service</em> principal / SASL settings ride the
catalog's own HiveConf, not the
* login.</li>
+ * <li><b>Simple HMS</b> — the configured {@code
hive.metastore.username}/{@code hadoop.username}, or the
Review Comment:
**[P1] Honor explicit SIMPLE HMS before the storage Kerberos fallback**
For a supported mixed catalog with
`hive.metastore.authentication.type=simple` and Kerberized HDFS, the earlier
raw-storage branch returns the HDFS Kerberos authenticator before this SIMPLE
arm is reached; `HmsConfHelper` also enables HMS SASL from that storage key.
Legacy HMS initialization and the shared metastore-property contract give
explicit HMS SIMPLE priority, so this configuration should contact the simple
HMS under the resolved simple UGI, not use the storage principal/SASL. Please
resolve the explicit HMS mode first (using storage Kerberos only as the
fallback) and add a SIMPLE-HMS/Kerberos-storage test.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java:
##########
@@ -596,9 +598,9 @@ private HmsClient createClient() {
context.getCatalogName(), config.getMetastoreUri(),
config.getMetastoreType(), poolSize);
- // For a Kerberos catalog run the metastore RPC under the PLUGIN's UGI
doAs (buildPluginAuthenticator),
- // NOT the FE-injected context: after the catalog flip that context
resolves to NOOP (SIMPLE) auth, which
- // would silently downgrade a Kerberos HMS. AuthAction.execute is a
generic method (<T> T execute(...)),
+ // Run the metastore RPC under the PLUGIN's UGI doAs
(buildPluginAuthenticator), NOT the FE-injected
+ // context: after the catalog flip that context resolves to NOOP auth
and loses both the configured simple
Review Comment:
**[P1] Apply the simple-HMS identity fix to the other plugin HMS paths**
The same post-flip NOOP context remains in the routed Hudi/Iceberg
connectors and standalone Paimon HMS. They all return `null` for SIMPLE auth:
Hudi creates the same `ThriftHmsClient`, while the Iceberg and Paimon pools
create `HiveMetaStoreClient`, whose `set_ugi` reads the current UGI rather than
the `hadoop.username` HiveConf key. Thus this patch restores
`hadoop.username=hive` only for primary Hive RPCs; those other HMS paths still
use the FE process user. Please apply the resolved HMS SIMPLE UGI at each
path's client-creation/RPC boundary and test it, while preserving the
intentional split between `hive.metastore.username` and the storage identity.
Paimon's static cached pool must also include/isolate the HMS identity, or
same-URI catalogs with different users can reuse the first pool.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java:
##########
@@ -710,7 +712,17 @@ static HadoopAuthenticator
buildPluginAuthenticator(Map<String, String> properti
return HadoopAuthenticator.getHadoopAuthenticator(
new
KerberosAuthenticationConfig(spec.get().getPrincipal(), spec.get().getKeytab(),
conf));
}
- return null;
+ if (hms.getAuthType() == AuthType.KERBEROS) {
+ return null;
+ }
+ String hadoopUser =
hms.toHiveConfOverrides("").get(AuthenticationConfig.HADOOP_USER_NAME);
+ Configuration conf = buildHadoopConf(properties);
Review Comment:
**[P2] Preserve the default for an explicitly blank username**
A catalog can retain `hadoop.username=""`: typed alias binding supplies no
nonblank replacement, while raw/storage passthrough and
`buildHadoopConf(properties)` preserve the empty key.
`getSimpleAuthenticationConfig` then reads `""` instead of the legacy `hadoop`
default, and `HadoopSimpleAuthenticator` calls
`UserGroupInformation.createRemoteUser("")`, which throws
`IllegalArgumentException("Null user")` before any HMS client is created. The
legacy path did not copy a blank bound username. Please remove/replace blank
raw usernames before constructing the simple config and cover empty/whitespace
values.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java:
##########
@@ -710,7 +712,17 @@ static HadoopAuthenticator
buildPluginAuthenticator(Map<String, String> properti
return HadoopAuthenticator.getHadoopAuthenticator(
new
KerberosAuthenticationConfig(spec.get().getPrincipal(), spec.get().getKeytab(),
conf));
}
- return null;
+ if (hms.getAuthType() == AuthType.KERBEROS) {
+ return null;
+ }
+ String hadoopUser =
hms.toHiveConfOverrides("").get(AuthenticationConfig.HADOOP_USER_NAME);
+ Configuration conf = buildHadoopConf(properties);
+ if (hadoopUser != null) {
+ conf.set(AuthenticationConfig.HADOOP_USER_NAME, hadoopUser);
+ }
+ // HMS set_ugi reads the current UGI, so its simple-auth identity
must match the DFS writer.
+ return HadoopAuthenticator.getHadoopAuthenticator(
+ AuthenticationConfig.getSimpleAuthenticationConfig(conf));
Review Comment:
**[P1] Load supported Hive config resources before resolving the simple UGI**
Legacy HMS initialization loaded the files named by `hive.conf.resources`
before calling `getSimpleAuthenticationConfig`, and the shared HMS property
contract still treats those files as the base configuration. Here
`buildHadoopConf` only stores the filename property in a plain `Configuration`;
it never loads the XML. A catalog whose resource supplies only
`hadoop.username=hive` therefore selects the `hadoop` default instead of
`hive`, reproducing the same ownership/permission mismatch. Please seed these
resources before applying raw/resolved overrides and add a resource-only
username test.
--
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]
