From 3ab0a9c2209154b1ac09d671cccecbb8b269b8ba Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Wed, 5 Mar 2025 22:24:32 +0100 Subject: [PATCH 1/8] TEZ-4749: Support SSL/TLS connections to ZooKeeper in ZkAMRegistry and ZkAMRegistryClient Add explicit per-connection SSL/TLS configuration for Tez's ZooKeeper connections, removing the need to rely on JVM-wide system properties. - New configuration properties: tez.am.zookeeper.ssl.enable, keystore/truststore location and password - New SSLZookeeperFactory using Netty-based secure ZK client connection - When ssl.enable is "true", CuratorFramework uses SSLZookeeperFactory; when "false", JVM-level secure properties are overridden to force insecure; when unset, existing JVM default behavior is preserved - Integration tests for secure/insecure ZK connections --- .../zookeeper/SSLZookeeperFactory.java | 76 +++++++ .../client/registry/zookeeper/ZkConfig.java | 69 +++++- .../apache/tez/dag/api/TezConfiguration.java | 57 +++++ .../registry/zookeeper/TestZkConfig.java | 68 ++++++ tez-tests/pom.xml | 6 + .../tez/test/TestZkAMRegistryClient.java | 207 ++++++++++++++++++ 6 files changed, 477 insertions(+), 6 deletions(-) create mode 100644 tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java create mode 100644 tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java new file mode 100644 index 0000000000..ea8a99728e --- /dev/null +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java @@ -0,0 +1,76 @@ +/* + * 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.tez.client.registry.zookeeper; + +import org.apache.commons.lang3.StringUtils; +import org.apache.curator.utils.ZookeeperFactory; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Factory to create Zookeeper clients with the zookeeper.client.secure enabled, + * allowing SSL communication with the Zookeeper server. + */ +public class SSLZookeeperFactory implements ZookeeperFactory { + + private static final Logger LOG = LoggerFactory.getLogger(SSLZookeeperFactory.class); + + private boolean sslEnabled; + private String keyStoreLocation; + private String keyStorePassword; + private String trustStoreLocation; + private String trustStorePassword; + + public SSLZookeeperFactory(boolean sslEnabled, String keyStoreLocation, String keyStorePassword, + String trustStoreLocation, String trustStorePassword) { + + this.sslEnabled = sslEnabled; + this.keyStoreLocation = keyStoreLocation; + this.keyStorePassword = keyStorePassword; + this.trustStoreLocation = trustStoreLocation; + this.trustStorePassword = trustStorePassword; + if (sslEnabled) { + if (StringUtils.isEmpty(keyStoreLocation)) { + LOG.warn("Missing keystoreLocation parameter"); + } + if (StringUtils.isEmpty(trustStoreLocation)) { + LOG.warn("Missing trustStoreLocation parameter"); + } + } + } + + @Override + public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, + boolean canBeReadOnly) throws Exception { + ZKClientConfig clientConfig = new ZKClientConfig(); + clientConfig.setProperty(ZKClientConfig.SECURE_CLIENT, Boolean.toString(sslEnabled)); + clientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty"); + ClientX509Util x509Util = new ClientX509Util(); + clientConfig.setProperty(x509Util.getSslKeystoreLocationProperty(), this.keyStoreLocation); + clientConfig.setProperty(x509Util.getSslKeystorePasswdProperty(), this.keyStorePassword); + clientConfig.setProperty(x509Util.getSslTruststoreLocationProperty(), this.trustStoreLocation); + clientConfig.setProperty(x509Util.getSslTruststorePasswdProperty(), this.trustStorePassword); + return new ZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly, clientConfig); + } +} diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java index c4e5104bdd..2b0cbe0ea7 100644 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java @@ -49,6 +49,11 @@ public class ZkConfig { private final int curatorMaxRetries; private final int sessionTimeoutMs; private final int connectionTimeoutMs; + private final String sslEnabled; + private final String sslKeystoreLocation; + private final String sslKeystorePassword; + private final String sslTruststoreLocation; + private final String sslTruststorePassword; public ZkConfig(Configuration conf) { zkQuorum = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM); @@ -84,6 +89,16 @@ public ZkConfig(Configuration conf) { TezConfiguration.TEZ_AM_CURATOR_SESSION_TIMEOUT_DEFAULT, TimeUnit.MILLISECONDS)); connectionTimeoutMs = Math.toIntExact(conf.getTimeDuration(TezConfiguration.TEZ_AM_CURATOR_CONNECTION_TIMEOUT, TezConfiguration.TEZ_AM_CURATOR_CONNECTION_TIMEOUT_DEFAULT, TimeUnit.MILLISECONDS)); + sslEnabled = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE); + Preconditions.checkArgument( + isValidSslEnabledValue(sslEnabled), + "If the optional %s setting is set, then the value should be a boolean value instead of '%s'", + TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, + sslEnabled); + sslKeystoreLocation = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_LOCATION); + sslKeystorePassword = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD); + sslTruststoreLocation = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_LOCATION); + sslTruststorePassword = conf.get(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD); } public String getZkQuorum() { @@ -110,17 +125,59 @@ public int getConnectionTimeoutMs() { return connectionTimeoutMs; } + public String getZookeeperTrustStorePassword() { return sslTruststorePassword; } + + public String getZookeeperTrustStoreLocation() { return sslTruststoreLocation; } + + public String getZookeeperKeyStorePassword() { return sslKeystorePassword; } + + public String getZookeeperKeyStoreLocation() { return sslKeystoreLocation; } + + /** + * Returns whether the zookeeper connection will be secure or insecure. + * @return An optional boolean value that indicates whether zookeeper client uses a secure + * zookeeper connection. A null value indicates that it is not specified, and in this case + * the default settings of zookeeper are used, which can be controlled by specific JVM + * properties. + * @see TezConfiguration#TEZ_AM_ZOOKEEPER_SSL_ENABLE + */ + public Boolean isSslEnabled() { + if (this.sslEnabled == null || this.sslEnabled.isEmpty()) { + return null; + } + return Boolean.parseBoolean(sslEnabled); + } + public RetryPolicy getRetryPolicy() { return new ExponentialBackoffRetry(getCuratorBackoffSleepMs(), getCuratorMaxRetries()); } public CuratorFramework createCuratorFramework() { - return CuratorFrameworkFactory.newClient( - getZkQuorum(), - getSessionTimeoutMs(), - getConnectionTimeoutMs(), - getRetryPolicy() - ); + if (isSslEnabled() == null) { + return CuratorFrameworkFactory.newClient( + getZkQuorum(), + getSessionTimeoutMs(), + getConnectionTimeoutMs(), + getRetryPolicy() + ); + } + + return CuratorFrameworkFactory.builder() + .connectString(getZkQuorum()) + .sessionTimeoutMs(getSessionTimeoutMs()) + .connectionTimeoutMs(getConnectionTimeoutMs()) + .retryPolicy(getRetryPolicy()) + .zookeeperFactory( + new SSLZookeeperFactory(isSslEnabled(), getZookeeperKeyStoreLocation(), + getZookeeperKeyStorePassword(), getZookeeperTrustStoreLocation(), + getZookeeperTrustStorePassword())) + .build(); + } + + private boolean isValidSslEnabledValue(String sslEnabled) { + return sslEnabled == null || sslEnabled.isEmpty() + || sslEnabled.trim().equalsIgnoreCase("true") + || sslEnabled.trim().equalsIgnoreCase("false"); } /** diff --git a/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java b/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java index 0f7d6d3754..14bff85cab 100644 --- a/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java +++ b/tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java @@ -2268,6 +2268,63 @@ static Set getPropertySet() { public static final String TEZ_SHARED_EXECUTOR_MAX_THREADS = "tez.shared-executor.max-threads"; public static final int TEZ_SHARED_EXECUTOR_MAX_THREADS_DEFAULT = -1; + /** + * Optional boolean value represented by string type. A value of "true" enables secure + * Zookeeper connection in ZkAMRegistry and ZkAMRegistryClient classes, while a value + * of "false" disables secure Zookeeper connection. + * If not specified or empty string, then zookeeper enables/disables the secure Zookeeper + * connection based on JVM properties. + * Default: Empty + */ + @ConfigurationScope(Scope.AM) + @ConfigurationProperty + public static final String TEZ_AM_ZOOKEEPER_SSL_ENABLE = TEZ_AM_PREFIX + + "zookeeper.ssl.client.enable"; + + /** + * String value + * An optional setting that specifies the path to the keystore used for the secure + * zookeeper connection. + * Default: Empty + */ + @ConfigurationScope(Scope.AM) + @ConfigurationProperty + public static final String TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_LOCATION = TEZ_AM_PREFIX + + "zookeeper.ssl.keystore.location"; + + /** + * String value + * An optional setting that specifies the password of the keystore used for the secure + * zookeeper connection. + * Default: Empty + */ + @ConfigurationScope(Scope.AM) + @ConfigurationProperty + public static final String TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD = TEZ_AM_PREFIX + + "zookeeper.ssl.keystore.password"; + + /** + * String value + * An optional setting that specifies the path to the truststore used for the secure + * zookeeper connection. + * Default: Empty + */ + @ConfigurationScope(Scope.AM) + @ConfigurationProperty + public static final String TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_LOCATION = TEZ_AM_PREFIX + + "zookeeper.ssl.truststore.location"; + + /** + * String value + * An optional setting that specifies the password of the truststore used for the secure + * zookeeper connection. + * Default: Empty + */ + @ConfigurationScope(Scope.AM) + @ConfigurationProperty + public static final String TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD = TEZ_AM_PREFIX + + "zookeeper.ssl.truststore.password"; + /** * Acquire all FileSystems info. e.g., all namenodes info of HDFS federation cluster. */ diff --git a/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java b/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java index 3927e67710..3f3485bb6f 100644 --- a/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java +++ b/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.concurrent.TimeUnit; @@ -231,4 +232,71 @@ public void testDefaultNamespace() { assertEquals("/tez-external-sessions" + TezConfiguration.TEZ_AM_REGISTRY_NAMESPACE_DEFAULT, zkConfig.getZkNamespace()); } + + @Test + public void testZkConfigTezAmZookeeperSslEnableNotSpecified() { + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); + ZkConfig zkConf = new ZkConfig(conf); + + assertNull(zkConf.isSslEnabled()); + assertNull(zkConf.getZookeeperKeyStoreLocation()); + assertNull(zkConf.getZookeeperKeyStorePassword()); + assertNull(zkConf.getZookeeperTrustStoreLocation()); + assertNull(zkConf.getZookeeperTrustStorePassword()); + } + + @Test + public void testZkConfigTezAmZookeeperSslEnableEmpty() { + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, ""); // empty means not set + ZkConfig zkConf = new ZkConfig(conf); + + assertNull(zkConf.isSslEnabled()); + assertNull(zkConf.getZookeeperKeyStoreLocation()); + assertNull(zkConf.getZookeeperKeyStorePassword()); + assertNull(zkConf.getZookeeperTrustStoreLocation()); + assertNull(zkConf.getZookeeperTrustStorePassword()); + } + + @Test + public void testZkConfigSslEnabled() { + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "true"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_LOCATION, "/keystore.jks"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD, "secret"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_LOCATION, "/truststore.jks"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD, "changeit"); + ZkConfig zkConf = new ZkConfig(conf); + + assertEquals(zkConf.isSslEnabled(), Boolean.TRUE); + assertEquals(zkConf.getZookeeperKeyStoreLocation(), "/keystore.jks"); + assertEquals(zkConf.getZookeeperKeyStorePassword(), "secret"); + assertEquals(zkConf.getZookeeperTrustStoreLocation(), "/truststore.jks"); + assertEquals(zkConf.getZookeeperTrustStorePassword(), "changeit"); + } + + @Test + public void testZkConfigSslDisabled() { + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "False"); + ZkConfig zkConf = new ZkConfig(conf); + + assertEquals(zkConf.isSslEnabled(), Boolean.FALSE); + assertNull(zkConf.getZookeeperKeyStoreLocation()); + assertNull(zkConf.getZookeeperKeyStorePassword()); + assertNull(zkConf.getZookeeperTrustStoreLocation()); + assertNull(zkConf.getZookeeperTrustStorePassword()); + } + + @Test + public void testZkConfigAmZookeeperSslEnableInvalid() { + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "invalidValue"); + assertThrows(IllegalArgumentException.class, () -> new ZkConfig(conf)); + } } diff --git a/tez-tests/pom.xml b/tez-tests/pom.xml index ba75496363..3b76ed0903 100644 --- a/tez-tests/pom.xml +++ b/tez-tests/pom.xml @@ -133,6 +133,12 @@ org.junit.jupiter junit-jupiter + + org.apache.curator + curator-test + ${curator.version} + test + diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java b/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java new file mode 100644 index 0000000000..a466771ad3 --- /dev/null +++ b/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java @@ -0,0 +1,207 @@ +/* + * 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.tez.test; + +import static org.apache.tez.test.TestSecureShuffle.generateCertificate; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.net.InetAddress; +import java.security.KeyPair; +import java.security.cert.X509Certificate; +import java.util.HashMap; +import java.util.Map; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.security.ssl.KeyStoreTestUtil; +import org.apache.hadoop.yarn.api.records.ApplicationId; +import org.apache.tez.client.registry.AMRecord; +import org.apache.tez.client.registry.zookeeper.ZkAMRegistryClient; +import org.apache.tez.dag.api.TezConfiguration; +import org.apache.tez.dag.api.client.registry.zookeeper.ZkAMRegistry; + +import com.google.common.collect.ImmutableMap; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +public class TestZkAMRegistryClient { + + private static final String KEYSTORE_PASSWORD = "secret"; + private static final String TRUSTSTORE_PASSWORD = "changeit"; + private static String TEST_ROOT_DIR = "target" + Path.SEPARATOR + + TestZkAMRegistryClient.class.getName() + "-tmpDir"; + private static File keysStoresDir = new File(TEST_ROOT_DIR, "keystores"); + private static String serverKS; + private static String trustKS; + + private static TestingServer zkServer; + private static Integer clientPort; + private static Integer secureClientPort; + + @BeforeAll + public static void setupZookeeperTestServer() throws Exception { + clientPort = InstanceSpec.getRandomPort(); + secureClientPort = InstanceSpec.getRandomPort(); + + setupKeyStores(); + + Map customProperties = ImmutableMap.of( + // NettyServerCnxnFactory required for SSL/TLS support + "serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory", + // secureClientPort opens a new port for secure connections + "secureClientPort", Integer.toString(secureClientPort), + "ssl.clientAuth", "none", + "ssl.keyStore.location", serverKS, + "ssl.keyStore.password", KEYSTORE_PASSWORD, + "ssl.trustStore.location", trustKS, + "ssl.trustStore.password", TRUSTSTORE_PASSWORD, + "ssl.keyStore.type", "JKS", + "ssl.trustStore.type", "JKS" + ); + + // the clientPort parameter causes an insecure port to be opened + InstanceSpec spec = new InstanceSpec(null, clientPort, -1, -1, true, 1, -1, -1, customProperties); + zkServer = new TestingServer(spec, true); + } + + @AfterAll + public static void shutdownZookeeperTestServer() throws IOException { + zkServer.stop(); + } + + public void enableZookeeperSecureClientWithJVMProperties() { + System.setProperty("zookeeper.client.secure", "true"); + System.setProperty("zookeeper.clientCnxnSocket", "org.apache.zookeeper.ClientCnxnSocketNetty"); + } + + @AfterEach + public void clearZookeeperSecureClientJVMProperties() { + System.clearProperty("zookeeper.client.secure"); + System.clearProperty("zookeeper.clientCnxnSocket"); + } + + @Test + @Timeout(30) + public void testZkAMRegistryClient() throws Exception { + // configure zookeeper connection to use the insecure client port + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_REGISTRY_NAMESPACE, "/test-am-registry"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "localhost:" + clientPort); + + runAmRecordTestWithConfiguration(conf); + } + + @Test + @Timeout(30) + public void testZkAMRegistryClientWithSecureClientJVMProperties() throws Exception { + // this affects all zookeeper clients in JVM + enableZookeeperSecureClientWithJVMProperties(); + + // configure zookeeper connection to use the secure client port + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_REGISTRY_NAMESPACE, "/test-am-registry-with-secure-client-jvm-properties"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "localhost:" + secureClientPort); + + runAmRecordTestWithConfiguration(conf); + } + + @Test + @Timeout(30) + public void testZkAMRegistryClientWithSecureZookeeperPort() throws Exception { + // configure zookeeper connection to use the secure client port without JVM properties + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_REGISTRY_NAMESPACE, "/test-am-registry-with-secure-connection"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "localhost:" + secureClientPort); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "true"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_LOCATION, trustKS); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD, TRUSTSTORE_PASSWORD); + + runAmRecordTestWithConfiguration(conf); + } + + @Test + @Timeout(30) + public void testZkAMRegistryClientWithInsecureZookeeperPort() throws Exception { + // this affects all zookeeper clients in JVM + enableZookeeperSecureClientWithJVMProperties(); + + // override the JVM properties above and configure zookeeper connection + // to use the insecure client port + Configuration conf = new Configuration(); + conf.set(TezConfiguration.TEZ_AM_REGISTRY_NAMESPACE, "/test-am-registry-with-insecure-connection"); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "localhost:" + clientPort); + conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "false"); + + runAmRecordTestWithConfiguration(conf); + } + + private void runAmRecordTestWithConfiguration(Configuration conf) throws Exception { + String zkAMRegistryId = "testRegistry" + System.currentTimeMillis(); + try (ZkAMRegistry registry = new ZkAMRegistry(zkAMRegistryId)) { + registry.init(conf); + registry.start(); + + ApplicationId appId = ApplicationId.newInstance(System.currentTimeMillis(), 1); + AMRecord amRecordRegistered = new AMRecord(appId, "hostName", "testHostIp", 1234, "testExternalId", "testComputeName"); + registry.add(amRecordRegistered); + + ZkAMRegistryClient registryClient = ZkAMRegistryClient.getClient(conf); + registryClient.start(); + + // information registered in registry eventually reaches the registry client + AMRecord amRecordFetched = registryClient.getRecord(appId); + while (amRecordFetched == null) { + Thread.sleep(1000); + amRecordFetched = registryClient.getRecord(appId); + } + assertEquals(amRecordFetched, amRecordRegistered); + + registryClient.close(); + } + } + + /** + * Create keystore and truststore for the tests + * + * @throws Exception + */ + private static void setupKeyStores() throws Exception { + keysStoresDir.mkdirs(); + Map certs = new HashMap(); + + String localhostName = InetAddress.getLocalHost().getHostName(); + KeyPair sKP = KeyStoreTestUtil.generateKeyPair("RSA"); + X509Certificate sCert = + generateCertificate("CN="+localhostName+", O=server", sKP, 30, "SHA256WITHRSA"); + serverKS = keysStoresDir.getAbsolutePath() + "/serverKS.jks"; + KeyStoreTestUtil.createKeyStore(serverKS, KEYSTORE_PASSWORD, "server", sKP.getPrivate(), sCert); + certs.put("server", sCert); + trustKS = keysStoresDir.getAbsolutePath() + "/trustKS.jks"; + KeyStoreTestUtil.createTrustStore(trustKS, TRUSTSTORE_PASSWORD, certs); + } + +} From aeca114bec4065bd39f7df76fad76ad03d426826 Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Mon, 10 Aug 2026 16:37:02 +0200 Subject: [PATCH 2/8] TEZ-4749: Fix checkstyle violations in ZkConfig - Expand single-line getter methods to multi-line (LeftCurly) - Rename parameter in isValidSslEnabledValue to avoid field shadowing (HiddenField) --- .../client/registry/zookeeper/ZkConfig.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java index 2b0cbe0ea7..581bec2e53 100644 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java @@ -125,13 +125,21 @@ public int getConnectionTimeoutMs() { return connectionTimeoutMs; } - public String getZookeeperTrustStorePassword() { return sslTruststorePassword; } + public String getZookeeperTrustStorePassword() { + return sslTruststorePassword; + } - public String getZookeeperTrustStoreLocation() { return sslTruststoreLocation; } + public String getZookeeperTrustStoreLocation() { + return sslTruststoreLocation; + } - public String getZookeeperKeyStorePassword() { return sslKeystorePassword; } + public String getZookeeperKeyStorePassword() { + return sslKeystorePassword; + } - public String getZookeeperKeyStoreLocation() { return sslKeystoreLocation; } + public String getZookeeperKeyStoreLocation() { + return sslKeystoreLocation; + } /** * Returns whether the zookeeper connection will be secure or insecure. @@ -174,10 +182,10 @@ public CuratorFramework createCuratorFramework() { .build(); } - private boolean isValidSslEnabledValue(String sslEnabled) { - return sslEnabled == null || sslEnabled.isEmpty() - || sslEnabled.trim().equalsIgnoreCase("true") - || sslEnabled.trim().equalsIgnoreCase("false"); + private boolean isValidSslEnabledValue(String value) { + return value == null || value.isEmpty() + || value.trim().equalsIgnoreCase("true") + || value.trim().equalsIgnoreCase("false"); } /** From 011bcbbf1f4f7f9ec4142283e225c60d8d2aebec Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Mon, 10 Aug 2026 16:38:40 +0200 Subject: [PATCH 3/8] TEZ-4749: Fix checkstyle violations in TestZkAMRegistryClient - Rename TEST_ROOT_DIR to testRootDir (StaticVariableName) - Fix indentation of ImmutableMap.of arguments (Indentation) - Break long AMRecord constructor line (LineLength > 120) - Add period to Javadoc first sentence (JavadocStyle) --- .../tez/test/TestZkAMRegistryClient.java | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java b/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java index a466771ad3..aec3e2ca08 100644 --- a/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java +++ b/tez-tests/src/test/java/org/apache/tez/test/TestZkAMRegistryClient.java @@ -52,9 +52,9 @@ public class TestZkAMRegistryClient { private static final String KEYSTORE_PASSWORD = "secret"; private static final String TRUSTSTORE_PASSWORD = "changeit"; - private static String TEST_ROOT_DIR = "target" + Path.SEPARATOR + private static String testRootDir = "target" + Path.SEPARATOR + TestZkAMRegistryClient.class.getName() + "-tmpDir"; - private static File keysStoresDir = new File(TEST_ROOT_DIR, "keystores"); + private static File keysStoresDir = new File(testRootDir, "keystores"); private static String serverKS; private static String trustKS; @@ -70,17 +70,17 @@ public static void setupZookeeperTestServer() throws Exception { setupKeyStores(); Map customProperties = ImmutableMap.of( - // NettyServerCnxnFactory required for SSL/TLS support - "serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory", - // secureClientPort opens a new port for secure connections - "secureClientPort", Integer.toString(secureClientPort), - "ssl.clientAuth", "none", - "ssl.keyStore.location", serverKS, - "ssl.keyStore.password", KEYSTORE_PASSWORD, - "ssl.trustStore.location", trustKS, - "ssl.trustStore.password", TRUSTSTORE_PASSWORD, - "ssl.keyStore.type", "JKS", - "ssl.trustStore.type", "JKS" + // NettyServerCnxnFactory required for SSL/TLS support + "serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory", + // secureClientPort opens a new port for secure connections + "secureClientPort", Integer.toString(secureClientPort), + "ssl.clientAuth", "none", + "ssl.keyStore.location", serverKS, + "ssl.keyStore.password", KEYSTORE_PASSWORD, + "ssl.trustStore.location", trustKS, + "ssl.trustStore.password", TRUSTSTORE_PASSWORD, + "ssl.keyStore.type", "JKS", + "ssl.trustStore.type", "JKS" ); // the clientPort parameter causes an insecure port to be opened @@ -166,7 +166,8 @@ private void runAmRecordTestWithConfiguration(Configuration conf) throws Excepti registry.start(); ApplicationId appId = ApplicationId.newInstance(System.currentTimeMillis(), 1); - AMRecord amRecordRegistered = new AMRecord(appId, "hostName", "testHostIp", 1234, "testExternalId", "testComputeName"); + AMRecord amRecordRegistered = + new AMRecord(appId, "hostName", "testHostIp", 1234, "testExternalId", "testComputeName"); registry.add(amRecordRegistered); ZkAMRegistryClient registryClient = ZkAMRegistryClient.getClient(conf); @@ -185,9 +186,7 @@ private void runAmRecordTestWithConfiguration(Configuration conf) throws Excepti } /** - * Create keystore and truststore for the tests - * - * @throws Exception + * Create keystore and truststore for the tests. */ private static void setupKeyStores() throws Exception { keysStoresDir.mkdirs(); From f14f7df5e010d813b6262618f186f49f4a577c5a Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Mon, 10 Aug 2026 16:54:29 +0200 Subject: [PATCH 4/8] TEZ-4749: Exclude ZkConfig.isSslEnabled from NP_BOOLEAN_RETURN_NULL findbugs check The method intentionally returns null to indicate "not configured", distinguishing it from explicit true/false. --- tez-api/findbugs-exclude.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tez-api/findbugs-exclude.xml b/tez-api/findbugs-exclude.xml index 464b735cbc..fd64ef0b6b 100644 --- a/tez-api/findbugs-exclude.xml +++ b/tez-api/findbugs-exclude.xml @@ -142,4 +142,11 @@ + + + + + + + From 0a9b41c03711e82e42c0205cad45df242e70ca7e Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Wed, 12 Aug 2026 11:24:51 +0200 Subject: [PATCH 5/8] TEZ-4749: Use Optional for ZkConfig.isSslEnabled() instead of nullable Boolean Replace nullable Boolean return with Optional to avoid the NP_BOOLEAN_RETURN_NULL spotbugs warning without needing a findbugs exclusion. Revert the findbugs-exclude.xml entry added previously. --- tez-api/findbugs-exclude.xml | 6 ------ .../client/registry/zookeeper/ZkConfig.java | 19 ++++++++++--------- .../registry/zookeeper/TestZkConfig.java | 13 +++++++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/tez-api/findbugs-exclude.xml b/tez-api/findbugs-exclude.xml index fd64ef0b6b..154e8d269a 100644 --- a/tez-api/findbugs-exclude.xml +++ b/tez-api/findbugs-exclude.xml @@ -143,10 +143,4 @@ - - - - - - diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java index 581bec2e53..1128d10800 100644 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java @@ -18,6 +18,7 @@ */ package org.apache.tez.client.registry.zookeeper; +import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.curator.RetryPolicy; @@ -143,17 +144,17 @@ public String getZookeeperKeyStoreLocation() { /** * Returns whether the zookeeper connection will be secure or insecure. - * @return An optional boolean value that indicates whether zookeeper client uses a secure - * zookeeper connection. A null value indicates that it is not specified, and in this case - * the default settings of zookeeper are used, which can be controlled by specific JVM - * properties. + * @return An Optional containing the boolean value that indicates whether zookeeper client + * uses a secure zookeeper connection. An empty Optional indicates that it is not specified, + * and in this case the default settings of zookeeper are used, which can be controlled by + * specific JVM properties. * @see TezConfiguration#TEZ_AM_ZOOKEEPER_SSL_ENABLE */ - public Boolean isSslEnabled() { + public Optional isSslEnabled() { if (this.sslEnabled == null || this.sslEnabled.isEmpty()) { - return null; + return Optional.empty(); } - return Boolean.parseBoolean(sslEnabled); + return Optional.of(Boolean.parseBoolean(sslEnabled)); } public RetryPolicy getRetryPolicy() { @@ -161,7 +162,7 @@ public RetryPolicy getRetryPolicy() { } public CuratorFramework createCuratorFramework() { - if (isSslEnabled() == null) { + if (!isSslEnabled().isPresent()) { return CuratorFrameworkFactory.newClient( getZkQuorum(), getSessionTimeoutMs(), @@ -176,7 +177,7 @@ public CuratorFramework createCuratorFramework() { .connectionTimeoutMs(getConnectionTimeoutMs()) .retryPolicy(getRetryPolicy()) .zookeeperFactory( - new SSLZookeeperFactory(isSslEnabled(), getZookeeperKeyStoreLocation(), + new SSLZookeeperFactory(isSslEnabled().get(), getZookeeperKeyStoreLocation(), getZookeeperKeyStorePassword(), getZookeeperTrustStoreLocation(), getZookeeperTrustStorePassword())) .build(); diff --git a/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java b/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java index 3f3485bb6f..a303d84d81 100644 --- a/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java +++ b/tez-api/src/test/java/org/apache/tez/client/registry/zookeeper/TestZkConfig.java @@ -19,10 +19,13 @@ package org.apache.tez.client.registry.zookeeper; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.curator.RetryPolicy; @@ -239,7 +242,7 @@ public void testZkConfigTezAmZookeeperSslEnableNotSpecified() { conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_QUORUM, "dummyZkQuorum"); ZkConfig zkConf = new ZkConfig(conf); - assertNull(zkConf.isSslEnabled()); + assertEquals(Optional.empty(), zkConf.isSslEnabled()); assertNull(zkConf.getZookeeperKeyStoreLocation()); assertNull(zkConf.getZookeeperKeyStorePassword()); assertNull(zkConf.getZookeeperTrustStoreLocation()); @@ -253,7 +256,7 @@ public void testZkConfigTezAmZookeeperSslEnableEmpty() { conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, ""); // empty means not set ZkConfig zkConf = new ZkConfig(conf); - assertNull(zkConf.isSslEnabled()); + assertEquals(Optional.empty(), zkConf.isSslEnabled()); assertNull(zkConf.getZookeeperKeyStoreLocation()); assertNull(zkConf.getZookeeperKeyStorePassword()); assertNull(zkConf.getZookeeperTrustStoreLocation()); @@ -271,7 +274,8 @@ public void testZkConfigSslEnabled() { conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD, "changeit"); ZkConfig zkConf = new ZkConfig(conf); - assertEquals(zkConf.isSslEnabled(), Boolean.TRUE); + assertTrue(zkConf.isSslEnabled().isPresent()); + assertTrue(zkConf.isSslEnabled().get()); assertEquals(zkConf.getZookeeperKeyStoreLocation(), "/keystore.jks"); assertEquals(zkConf.getZookeeperKeyStorePassword(), "secret"); assertEquals(zkConf.getZookeeperTrustStoreLocation(), "/truststore.jks"); @@ -285,7 +289,8 @@ public void testZkConfigSslDisabled() { conf.set(TezConfiguration.TEZ_AM_ZOOKEEPER_SSL_ENABLE, "False"); ZkConfig zkConf = new ZkConfig(conf); - assertEquals(zkConf.isSslEnabled(), Boolean.FALSE); + assertTrue(zkConf.isSslEnabled().isPresent()); + assertFalse(zkConf.isSslEnabled().get()); assertNull(zkConf.getZookeeperKeyStoreLocation()); assertNull(zkConf.getZookeeperKeyStorePassword()); assertNull(zkConf.getZookeeperTrustStoreLocation()); From 9a58d60894d5452600e6869ee745a83115218ddb Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Wed, 12 Aug 2026 11:50:11 +0200 Subject: [PATCH 6/8] TEZ-4749: Use Curator 5.x native zkClientConfig() instead of custom ZookeeperFactory Replace SSLZookeeperFactory with Curator's built-in .zkClientConfig() builder method. The ZKClientConfig with SSL properties is now configured inline in ZkConfig.createCuratorFramework(), eliminating the need for a separate factory class. --- tez-api/findbugs-exclude.xml | 1 - .../zookeeper/SSLZookeeperFactory.java | 76 ------------------- .../client/registry/zookeeper/ZkConfig.java | 26 ++++++- 3 files changed, 22 insertions(+), 81 deletions(-) delete mode 100644 tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java diff --git a/tez-api/findbugs-exclude.xml b/tez-api/findbugs-exclude.xml index 154e8d269a..464b735cbc 100644 --- a/tez-api/findbugs-exclude.xml +++ b/tez-api/findbugs-exclude.xml @@ -142,5 +142,4 @@ - diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java deleted file mode 100644 index ea8a99728e..0000000000 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/SSLZookeeperFactory.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * 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.tez.client.registry.zookeeper; - -import org.apache.commons.lang3.StringUtils; -import org.apache.curator.utils.ZookeeperFactory; -import org.apache.zookeeper.Watcher; -import org.apache.zookeeper.ZooKeeper; -import org.apache.zookeeper.client.ZKClientConfig; -import org.apache.zookeeper.common.ClientX509Util; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Factory to create Zookeeper clients with the zookeeper.client.secure enabled, - * allowing SSL communication with the Zookeeper server. - */ -public class SSLZookeeperFactory implements ZookeeperFactory { - - private static final Logger LOG = LoggerFactory.getLogger(SSLZookeeperFactory.class); - - private boolean sslEnabled; - private String keyStoreLocation; - private String keyStorePassword; - private String trustStoreLocation; - private String trustStorePassword; - - public SSLZookeeperFactory(boolean sslEnabled, String keyStoreLocation, String keyStorePassword, - String trustStoreLocation, String trustStorePassword) { - - this.sslEnabled = sslEnabled; - this.keyStoreLocation = keyStoreLocation; - this.keyStorePassword = keyStorePassword; - this.trustStoreLocation = trustStoreLocation; - this.trustStorePassword = trustStorePassword; - if (sslEnabled) { - if (StringUtils.isEmpty(keyStoreLocation)) { - LOG.warn("Missing keystoreLocation parameter"); - } - if (StringUtils.isEmpty(trustStoreLocation)) { - LOG.warn("Missing trustStoreLocation parameter"); - } - } - } - - @Override - public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, - boolean canBeReadOnly) throws Exception { - ZKClientConfig clientConfig = new ZKClientConfig(); - clientConfig.setProperty(ZKClientConfig.SECURE_CLIENT, Boolean.toString(sslEnabled)); - clientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty"); - ClientX509Util x509Util = new ClientX509Util(); - clientConfig.setProperty(x509Util.getSslKeystoreLocationProperty(), this.keyStoreLocation); - clientConfig.setProperty(x509Util.getSslKeystorePasswdProperty(), this.keyStorePassword); - clientConfig.setProperty(x509Util.getSslTruststoreLocationProperty(), this.trustStoreLocation); - clientConfig.setProperty(x509Util.getSslTruststorePasswdProperty(), this.trustStorePassword); - return new ZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly, clientConfig); - } -} diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java index 1128d10800..7c3807c7c2 100644 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java @@ -21,6 +21,7 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; +import org.apache.commons.lang3.StringUtils; import org.apache.curator.RetryPolicy; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; @@ -28,6 +29,8 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.tez.dag.api.TezConfiguration; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -171,15 +174,30 @@ public CuratorFramework createCuratorFramework() { ); } + ZKClientConfig zkClientConfig = new ZKClientConfig(); + zkClientConfig.setProperty(ZKClientConfig.SECURE_CLIENT, Boolean.toString(isSslEnabled().get())); + zkClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, + "org.apache.zookeeper.ClientCnxnSocketNetty"); + if (isSslEnabled().get()) { + ClientX509Util x509Util = new ClientX509Util(); + if (StringUtils.isEmpty(getZookeeperKeyStoreLocation())) { + LOG.warn("Missing keystoreLocation parameter"); + } + if (StringUtils.isEmpty(getZookeeperTrustStoreLocation())) { + LOG.warn("Missing trustStoreLocation parameter"); + } + zkClientConfig.setProperty(x509Util.getSslKeystoreLocationProperty(), getZookeeperKeyStoreLocation()); + zkClientConfig.setProperty(x509Util.getSslKeystorePasswdProperty(), getZookeeperKeyStorePassword()); + zkClientConfig.setProperty(x509Util.getSslTruststoreLocationProperty(), getZookeeperTrustStoreLocation()); + zkClientConfig.setProperty(x509Util.getSslTruststorePasswdProperty(), getZookeeperTrustStorePassword()); + } + return CuratorFrameworkFactory.builder() .connectString(getZkQuorum()) .sessionTimeoutMs(getSessionTimeoutMs()) .connectionTimeoutMs(getConnectionTimeoutMs()) .retryPolicy(getRetryPolicy()) - .zookeeperFactory( - new SSLZookeeperFactory(isSslEnabled().get(), getZookeeperKeyStoreLocation(), - getZookeeperKeyStorePassword(), getZookeeperTrustStoreLocation(), - getZookeeperTrustStorePassword())) + .zkClientConfig(zkClientConfig) .build(); } From 993662c1df88ccf042b72f9fec48c58f6530d593 Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Wed, 12 Aug 2026 19:42:33 +0200 Subject: [PATCH 7/8] TEZ-4749: Remove redundant version tag from curator-test dependency Version is already managed by dependencyManagement in the parent POM. --- tez-tests/pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/tez-tests/pom.xml b/tez-tests/pom.xml index 3b76ed0903..8920c17e65 100644 --- a/tez-tests/pom.xml +++ b/tez-tests/pom.xml @@ -136,7 +136,6 @@ org.apache.curator curator-test - ${curator.version} test From 53723a560f9a15c93e5c1e0da91981a6eceb5916 Mon Sep 17 00:00:00 2001 From: Gergely Farkas Date: Wed, 12 Aug 2026 19:49:26 +0200 Subject: [PATCH 8/8] TEZ-4749: Refactor SSL store config in createCuratorFramework() Extract setStoreConfig() helper to conditionally set keystore/truststore properties only when a location is configured. Use try-with-resources for ClientX509Util and Optional.isEmpty() for readability. --- .../client/registry/zookeeper/ZkConfig.java | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java index 7c3807c7c2..cb8380e519 100644 --- a/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java +++ b/tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java @@ -165,7 +165,7 @@ public RetryPolicy getRetryPolicy() { } public CuratorFramework createCuratorFramework() { - if (!isSslEnabled().isPresent()) { + if (isSslEnabled().isEmpty()) { return CuratorFrameworkFactory.newClient( getZkQuorum(), getSessionTimeoutMs(), @@ -179,17 +179,13 @@ public CuratorFramework createCuratorFramework() { zkClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty"); if (isSslEnabled().get()) { - ClientX509Util x509Util = new ClientX509Util(); - if (StringUtils.isEmpty(getZookeeperKeyStoreLocation())) { - LOG.warn("Missing keystoreLocation parameter"); - } - if (StringUtils.isEmpty(getZookeeperTrustStoreLocation())) { - LOG.warn("Missing trustStoreLocation parameter"); + try (ClientX509Util x509Util = new ClientX509Util()) { + setStoreConfig(zkClientConfig, x509Util.getSslKeystoreLocationProperty(), getZookeeperKeyStoreLocation(), + x509Util.getSslKeystorePasswdProperty(), getZookeeperKeyStorePassword(), "keystore"); + + setStoreConfig(zkClientConfig, x509Util.getSslTruststoreLocationProperty(), getZookeeperTrustStoreLocation(), + x509Util.getSslTruststorePasswdProperty(), getZookeeperTrustStorePassword(), "truststore"); } - zkClientConfig.setProperty(x509Util.getSslKeystoreLocationProperty(), getZookeeperKeyStoreLocation()); - zkClientConfig.setProperty(x509Util.getSslKeystorePasswdProperty(), getZookeeperKeyStorePassword()); - zkClientConfig.setProperty(x509Util.getSslTruststoreLocationProperty(), getZookeeperTrustStoreLocation()); - zkClientConfig.setProperty(x509Util.getSslTruststorePasswdProperty(), getZookeeperTrustStorePassword()); } return CuratorFrameworkFactory.builder() @@ -201,6 +197,19 @@ public CuratorFramework createCuratorFramework() { .build(); } + private void setStoreConfig(ZKClientConfig config, String locationProp, String locationVal, String passwordProp, + String passwordVal, String storeName) { + if (StringUtils.isEmpty(locationVal)) { + LOG.info("No {} location configured, using ZooKeeper client defaults", storeName); + return; + } + + config.setProperty(locationProp, locationVal); + if (StringUtils.isNotEmpty(passwordVal)) { + config.setProperty(passwordProp, passwordVal); + } + } + private boolean isValidSslEnabledValue(String value) { return value == null || value.isEmpty() || value.trim().equalsIgnoreCase("true")