HBASE-28340. Use all Zk client properties that is found in HBase conf (#5669)

Signed-off-by: Balazs Meszaros <meszibali@apache.org>

Author: anmolnar

hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java

@@ -21,15 +21,13 @@
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Properties;
-import java.util.Set;
 import org.apache.commons.validator.routines.InetAddressValidator;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.yetus.audience.InterfaceAudience;
 
 import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
-import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
 
 /**
  * Utility methods for reading, and building the ZooKeeper configuration. The order and priority for
@@ -42,12 +40,6 @@ public final class ZKConfig {
   private static final String VARIABLE_START = "${";
   private static final String ZOOKEEPER_JAVA_PROPERTY_PREFIX = "zookeeper.";
 
-  /** Supported ZooKeeper client TLS properties */
-  static final Set<String> ZOOKEEPER_CLIENT_TLS_PROPERTIES =
-    ImmutableSet.of("client.secure", "clientCnxnSocket", "ssl.keyStore.location",
-      "ssl.keyStore.password", "ssl.keyStore.passwordPath", "ssl.trustStore.location",
-      "ssl.trustStore.password", "ssl.trustStore.passwordPath");
-
   private ZKConfig() {
   }
 
@@ -62,16 +54,12 @@ public static Properties makeZKProps(Configuration conf) {
   }
 
   /**
-   * Make a Properties object holding ZooKeeper config. Parses the corresponding config options from
-   * the HBase XML configs and generates the appropriate ZooKeeper properties.
-   * @param conf Configuration to read from.
-   * @return Properties holding mappings representing ZooKeeper config file.
+   * Directly map all the hbase.zookeeper.property.KEY properties. Synchronize on conf so no loading
+   * of configs while we iterate
    */
-  private static Properties makeZKPropsFromHbaseConfig(Configuration conf) {
+  private static Properties extractZKPropsFromHBaseConfig(final Configuration conf) {
     Properties zkProperties = new Properties();
 
-    // Directly map all of the hbase.zookeeper.property.KEY properties.
-    // Synchronize on conf so no loading of configs while we iterate
     synchronized (conf) {
       for (Entry<String, String> entry : conf) {
         String key = entry.getKey();
@@ -87,6 +75,18 @@ private static Properties makeZKPropsFromHbaseConfig(Configuration conf) {
       }
     }
 
+    return zkProperties;
+  }
+
+  /**
+   * Make a Properties object holding ZooKeeper config. Parses the corresponding config options from
+   * the HBase XML configs and generates the appropriate ZooKeeper properties.
+   * @param conf Configuration to read from.
+   * @return Properties holding mappings representing ZooKeeper config file.
+   */
+  private static Properties makeZKPropsFromHbaseConfig(Configuration conf) {
+    Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
+
     // If clientPort is not set, assign the default.
     if (zkProperties.getProperty(HConstants.CLIENT_PORT_STR) == null) {
       zkProperties.put(HConstants.CLIENT_PORT_STR, HConstants.DEFAULT_ZOOKEEPER_CLIENT_PORT);
@@ -343,24 +343,12 @@ public static String getClientZKQuorumServersString(Configuration conf) {
   }
 
   private static void setZooKeeperClientSystemProperties(String prefix, Configuration conf) {
-    synchronized (conf) {
-      for (Entry<String, String> entry : conf) {
-        String key = entry.getKey();
-        if (!key.startsWith(prefix)) {
-          continue;
-        }
-        String zkKey = key.substring(prefix.length());
-        if (!ZOOKEEPER_CLIENT_TLS_PROPERTIES.contains(zkKey)) {
-          continue;
-        }
-        String value = entry.getValue();
-        // If the value has variables substitutions, need to do a get.
-        if (value.contains(VARIABLE_START)) {
-          value = conf.get(key);
-        }
-        if (System.getProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + zkKey) == null) {
-          System.setProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + zkKey, value);
-        }
+    Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
+    for (Entry<Object, Object> entry : zkProperties.entrySet()) {
+      String key = entry.getKey().toString().trim();
+      String value = entry.getValue().toString().trim();
+      if (System.getProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + key) == null) {
+        System.setProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + key, value);
       }
     }
   }

hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java

@@ -17,12 +17,12 @@
  */
 package org.apache.hadoop.hbase.zookeeper;
 
-import static org.apache.hadoop.hbase.zookeeper.ZKConfig.ZOOKEEPER_CLIENT_TLS_PROPERTIES;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.util.Properties;
+import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseConfiguration;
@@ -33,13 +33,21 @@
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
+
 @Category({ MiscTests.class, SmallTests.class })
 public class TestZKConfig {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
     HBaseClassTestRule.forClass(TestZKConfig.class);
 
+  /** Supported ZooKeeper client TLS properties */
+  private static final Set<String> ZOOKEEPER_CLIENT_TLS_PROPERTIES = ImmutableSet.of(
+    "client.secure", "clientCnxnSocket", "ssl.keyStore.location", "ssl.keyStore.password",
+    "ssl.keyStore.passwordPath", "ssl.keyStore.type", "ssl.trustStore.location",
+    "ssl.trustStore.password", "ssl.trustStore.passwordPath", "ssl.trustStore.type");
+
   @Test
   public void testZKConfigLoading() throws Exception {
     Configuration conf = HBaseConfiguration.create();
@@ -133,6 +141,21 @@ public void testZooKeeperTlsPropertiesServer() {
     }
   }
 
+  @Test
+  public void testZooKeeperPropertiesDoesntOverwriteSystem() {
+    // Arrange
+    System.setProperty("zookeeper.a.b.c", "foo");
+    Configuration conf = HBaseConfiguration.create();
+    conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + "a.b.c", "bar");
+
+    // Act
+    ZKConfig.getZKQuorumServersString(conf);
+
+    // Assert
+    assertEquals("foo", System.getProperty("zookeeper.a.b.c"));
+    System.clearProperty("zookeeper.a.b.c");
+  }
+
   private void testKey(String ensemble, int port, String znode) throws IOException {
     testKey(ensemble, port, znode, false); // not support multiple client ports
   }