Aggarwal-Raghav commented on code in PR #532:
URL: https://github.com/apache/tez/pull/532#discussion_r3769302978
##########
tez-api/src/main/java/org/apache/tez/client/registry/zookeeper/ZkConfig.java:
##########
@@ -110,17 +129,82 @@ 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 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 Optional<Boolean> isSslEnabled() {
+ if (this.sslEnabled == null || this.sslEnabled.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(Boolean.parseBoolean(sslEnabled));
+ }
+
public RetryPolicy getRetryPolicy() {
return new ExponentialBackoffRetry(getCuratorBackoffSleepMs(),
getCuratorMaxRetries());
}
public CuratorFramework createCuratorFramework() {
- return CuratorFrameworkFactory.newClient(
- getZkQuorum(),
- getSessionTimeoutMs(),
- getConnectionTimeoutMs(),
- getRetryPolicy()
- );
+ if (!isSslEnabled().isPresent()) {
+ return CuratorFrameworkFactory.newClient(
+ getZkQuorum(),
+ getSessionTimeoutMs(),
+ getConnectionTimeoutMs(),
+ getRetryPolicy()
+ );
+ }
+
+ 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());
Review Comment:
Your above solution, just small refactor
```patch
From e6ca2489dc5179e0b0cc9e3bc90385e78c31af62 Mon Sep 17 00:00:00 2001
From: Raghav Aggarwal <[email protected]>
Date: Wed, 12 Aug 2026 23:56:47 +0530
Subject: [PATCH] TEZ-4749: Refactor
---
.../client/registry/zookeeper/ZkConfig.java | 31 ++++++++++++-------
tez-tests/pom.xml | 1 -
2 files changed, 20 insertions(+), 12 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 7c3807c7c..cb8380e51 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 class ZkConfig {
}
public CuratorFramework createCuratorFramework() {
- if (!isSslEnabled().isPresent()) {
+ if (isSslEnabled().isEmpty()) {
return CuratorFrameworkFactory.newClient(
getZkQuorum(),
getSessionTimeoutMs(),
@@ -179,17 +179,13 @@ public class ZkConfig {
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");
+ try (ClientX509Util x509Util = new ClientX509Util()) {
+ setStoreConfig(zkClientConfig,
x509Util.getSslKeystoreLocationProperty(), getZookeeperKeyStoreLocation(),
+ x509Util.getSslKeystorePasswdProperty(),
getZookeeperKeyStorePassword(), "keystore");
+
+ setStoreConfig(zkClientConfig,
x509Util.getSslTruststoreLocationProperty(), getZookeeperTrustStoreLocation(),
+ x509Util.getSslTruststorePasswdProperty(),
getZookeeperTrustStorePassword(), "truststore");
}
- 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()
@@ -201,6 +197,19 @@ public class ZkConfig {
.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")
diff --git a/tez-tests/pom.xml b/tez-tests/pom.xml
index 3b76ed090..8920c17e6 100644
--- a/tez-tests/pom.xml
+++ b/tez-tests/pom.xml
@@ -136,7 +136,6 @@
<dependency>
<groupId>org.apache.curator</groupId>
<artifactId>curator-test</artifactId>
- <version>${curator.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
--
2.55.0
```
--
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]
