Gabriel39 commented on code in PR #66890:
URL: https://github.com/apache/doris/pull/66890#discussion_r3804160104
##########
fe/fe-connector/fe-connector-hms/src/main/java/org/apache/doris/connector/hms/HmsConfHelper.java:
##########
@@ -58,21 +69,92 @@ public static HiveConf createHiveConf(Map<String, String>
properties) {
// (fixes SecurityUtil.<clinit>) but cannot fix this conf-cached CL.
Pinning here keeps the whole
// hive-metastore class graph in one loader.
hiveConf.setClassLoader(HmsConfHelper.class.getClassLoader());
+ addConfResources(hiveConf, confResources);
for (Map.Entry<String, String> entry : properties.entrySet()) {
+ // A blank username was ignored by the legacy copy-if-present
path; preserving a resource value (or
+ // the "hadoop" default) avoids createRemoteUser("") failing
before the first HMS RPC.
+ if ("hadoop.username".equals(entry.getKey()) &&
isBlank(entry.getValue())) {
+ continue;
+ }
hiveConf.set(entry.getKey(), entry.getValue());
}
// A kerberized HMS requires SASL transport on the metastore Thrift
connection. The legacy fe-core
// HMSBaseProperties.initHadoopAuthenticator auto-enabled
hive.metastore.sasl.enabled whenever the
// metastore/hadoop auth was kerberos; preserve that here so a catalog
that only declares kerberos auth
// (without an explicit hive.metastore.sasl.enabled) still negotiates
SASL, instead of opening a plain
// TSocket that a kerberized metastore drops with TTransportException.
- if
("kerberos".equalsIgnoreCase(properties.get("hadoop.security.authentication"))
- ||
"kerberos".equalsIgnoreCase(properties.get("hive.metastore.authentication.type")))
{
+ String hmsAuthType =
properties.get("hive.metastore.authentication.type");
+ boolean explicitSimple = "simple".equalsIgnoreCase(hmsAuthType);
Review Comment:
Fixed. Explicit SIMPLE now writes hive.metastore.sasl.enabled=false after
resource and raw-property layering in both HMS configuration builders. Added
resource-backed and raw-property regression tests.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java:
##########
@@ -489,14 +502,40 @@ private Catalog createCatalogFromContext(CatalogContext
catalogContext, String f
ClassLoader previous = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
- return context.executeAuthenticated(() ->
CatalogFactory.createCatalog(catalogContext));
+ return context.executeAuthenticated(() -> {
+ // Paimon's CachedClientPool eagerly opens a SIMPLE client in
the HiveCatalog constructor, so
+ // construction must cross the HMS boundary too. FileIO only
stores its configuration here and
+ // still creates filesystems lazily under the connector's
separate storage authenticator.
+ Catalog catalog = hmsAuth == null
+ ? CatalogFactory.createCatalog(catalogContext)
+ : hmsAuth.doAs(() ->
CatalogFactory.createCatalog(catalogContext));
Review Comment:
Fixed. Paimon now performs FileIO discovery and warehouse validation under
the storage identity, then enters the HMS identity only for HiveCatalog and
client-pool construction. Added a real mixed-identity catalog regression.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonHmsClientPool.java:
##########
@@ -0,0 +1,119 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.connector.paimon;
+
+import org.apache.doris.kerberos.HadoopAuthenticator;
+
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.DelegateCatalog;
+import org.apache.paimon.client.ClientPool;
+import org.apache.paimon.hive.HiveCatalog;
+import org.apache.thrift.TException;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.security.PrivilegedAction;
+
+/** Applies the HMS identity at Paimon's metastore client acquisition and RPC
boundary. */
+final class PaimonHmsClientPool implements ClientPool<IMetaStoreClient,
TException> {
+
+ private final ClientPool<IMetaStoreClient, TException> delegate;
+ private final HadoopAuthenticator authenticator;
+
+ private PaimonHmsClientPool(ClientPool<IMetaStoreClient, TException>
delegate,
+ HadoopAuthenticator authenticator) {
+ this.delegate = delegate;
+ this.authenticator = authenticator;
+ }
+
+ static ClientPool<IMetaStoreClient, TException> wrap(
+ ClientPool<IMetaStoreClient, TException> delegate,
HadoopAuthenticator authenticator) {
+ return new PaimonHmsClientPool(delegate, authenticator);
+ }
+
+ static Catalog install(Catalog catalog, HadoopAuthenticator authenticator)
{
+ if (authenticator == null) {
+ return catalog;
+ }
+ Catalog root = DelegateCatalog.rootCatalog(catalog);
+ if (!(root instanceof HiveCatalog)) {
+ throw new IllegalStateException("Expected a Paimon HiveCatalog for
HMS authentication");
+ }
+ try {
+ Field clients = HiveCatalog.class.getDeclaredField("clients");
+ clients.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ ClientPool<IMetaStoreClient, TException> delegate =
+ (ClientPool<IMetaStoreClient, TException>)
clients.get(root);
+ // Paimon exposes no client-pool injection seam; replacing only
this field prevents the HMS user
+ // from leaking into FileIO while covering both cached-pool client
creation and every metastore RPC.
+ clients.set(root, wrap(delegate, authenticator));
Review Comment:
Fixed. Added a serializable catalog-loader wrapper that rebuilds the HMS
authenticator, loads fresh HiveCatalog instances under it, and reinstalls the
client-pool boundary. The regression verifies a loader-created real HiveCatalog.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -969,8 +974,32 @@ private Catalog createCatalog() {
LOG.info("Creating Iceberg catalog '{}' flavor='{}' impl='{}'",
catalogName, flavor,
catalogOptions.get(CatalogProperties.CATALOG_IMPL));
- return buildCatalogAuthenticated(flavor,
- () -> CatalogUtil.buildIcebergCatalog(catalogName,
catalogOptions, conf));
+ return buildCatalogAuthenticated(flavor, () -> {
+ if (!IcebergCatalogProperties.TYPE_HMS.equals(flavor)) {
+ return CatalogUtil.buildIcebergCatalog(catalogName,
catalogOptions, conf);
+ }
+ HadoopAuthenticator hmsAuth = buildHmsAuthenticator(properties,
storageHadoopConfig);
+ Catalog catalog = CatalogUtil.buildIcebergCatalog(catalogName,
catalogOptions, conf);
+ return IcebergHmsClientPool.install(catalog, hmsAuth);
+ });
+ }
+
+ static String appendHmsCacheKeys(String existing) {
+ String keys = appendCacheKey(existing, "conf:hadoop.username");
+ keys = appendCacheKey(keys, "conf:hive.metastore.client.principal");
+ return appendCacheKey(keys, "conf:hadoop.kerberos.principal");
+ }
+
+ static String appendCacheKey(String existing, String required) {
+ if (StringUtils.isBlank(existing)) {
+ return required;
+ }
+ for (String element : existing.split(",")) {
+ if (required.equalsIgnoreCase(element.trim())) {
Review Comment:
Fixed. The conf: marker is matched case-insensitively while the
configuration-key suffix is matched with exact case in both connectors. Added
mis-cased-key regressions.
--
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]
