[ https://issues.apache.org/jira/browse/HADOOP-18709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17724473#comment-17724473 ]
ASF GitHub Bot commented on HADOOP-18709: ----------------------------------------- szilard-nemeth commented on code in PR #5638: URL: https://github.com/apache/hadoop/pull/5638#discussion_r1199527654 ########## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java: ########## @@ -503,4 +644,50 @@ private void setJaasConfiguration(ZKClientConfig zkClientConfig) throws IOExcept zkClientConfig.setProperty(ZKClientConfig.LOGIN_CONTEXT_NAME_KEY, JAAS_CLIENT_ENTRY); } } -} \ No newline at end of file + + /** + * Helper class to contain the Truststore/Keystore paths for the ZK client connection over + * SSL/TLS. + */ + public static class TruststoreKeystore{ + private static String keystoreLocation; + private static String keystorePassword; + private static String truststoreLocation; + private static String truststorePassword; + /** Configuration for the ZooKeeper connection when SSL/TLS is enabled. + * When a value is not configured, ensure that empty string is set instead of null. + * @param conf ZooKeeper Client configuration + */ + public TruststoreKeystore(Configuration conf){ + + keystoreLocation = + StringUtils.defaultString(conf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, Review Comment: Why the StringUtils.defaultString is needed? I mean, conf.get() will return an empty string if the config is not found, given that you passed empty strings for all conf.get calls already. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, Review Comment: Extracting these (at least 1, 100, and 10) to static finals would make this more readable and straightforward. ########## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java: ########## @@ -452,21 +502,50 @@ public static class HadoopZookeeperFactory implements ZookeeperFactory { private final String zkPrincipal; private final String kerberosPrincipal; private final String kerberosKeytab; + private final Boolean sslEnabled; + /** + * Constructor for the helper class to configure the ZooKeeper client connection. + * @param zkPrincipal Optional. + */ public HadoopZookeeperFactory(String zkPrincipal) { this(zkPrincipal, null, null); } - + /** + * Constructor for the helper class to configure the ZooKeeper client connection. + * @param zkPrincipal Optional. + * @param kerberosPrincipal Optional. Use along with kerberosKeytab. + * @param kerberosKeytab Optional. Use along with kerberosPrincipal. + */ public HadoopZookeeperFactory(String zkPrincipal, String kerberosPrincipal, String kerberosKeytab) { + this(zkPrincipal, kerberosPrincipal, kerberosKeytab, false, + new TruststoreKeystore(new Configuration())); } + + /** + * Constructor for the helper class to configure the ZooKeeper client connection. + * @param zkPrincipal Optional. + * @param kerberosPrincipal Optional. Use along with kerberosKeytab. + * @param kerberosKeytab Optional. Use along with kerberosPrincipal. + * @param sslEnabled Flag to enable SSL/TLS ZK client connection for each component + * independently. + * @param truststoreKeystore TruststoreKeystore object containing the keystoreLocation, + * keystorePassword, truststoreLocation, truststorePassword for + * SSL/TLS connection when sslEnabled is set to true. + */ + public HadoopZookeeperFactory(String zkPrincipal, + String kerberosPrincipal, + String kerberosKeytab, + boolean sslEnabled, + TruststoreKeystore truststoreKeystore) { Review Comment: the parameter called 'truststoreKeystore' is unused. Please remove it ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { + Configuration conf = new Configuration(); + System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server" + + ".NettyServerCnxnFactory"); + + System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "keystore.jks"); + System.setProperty("zookeeper.ssl.keyStore.password", "password"); + System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "truststore.jks"); + System.setProperty("zookeeper.ssl.trustStore.password", "password"); + System.setProperty("zookeeper.request.timeout", "12345"); + + System.setProperty("jute.maxbuffer", JUTE_MAXBUFFER.toString()); + + System.setProperty("javax.net.debug", "ssl"); + System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth" + + ".X509AuthenticationProvider"); + + + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + + "keystore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + + "truststore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password"); + return conf; + } + + @After + public void teardown() throws Exception { + this.curator.close(); + if (this.server != null) { + this.server.close(); + this.server = null; + } + } + + @Test + public void testSecureZKConfiguration() throws Exception { + LOG.info("Entered to the testSecureZKConfiguration test case."); + // Validate that HadoopZooKeeperFactory will set ZKConfig with given principals + ZKCuratorManager.HadoopZookeeperFactory factory = + new ZKCuratorManager.HadoopZookeeperFactory( + null, + null, + null, + true, + new ZKCuratorManager.TruststoreKeystore(hadoopConf)); + ZooKeeper zk = factory.newZooKeeper(this.server.getConnectString(), 1000, null, false); + validateSSLConfiguration( + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD), + zk); + } + + private void validateSSLConfiguration( + String keystoreLocation, + String keystorePassword, + String truststoreLocation, + String truststorePassword, + ZooKeeper zk){ + try (ClientX509Util x509Util = new ClientX509Util()) { + //testing if custom values are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", keystoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslKeystoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", keystorePassword, + zk.getClientConfig().getProperty(x509Util.getSslKeystorePasswdProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslTruststoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststorePassword, + zk.getClientConfig().getProperty(x509Util.getSslTruststorePasswdProperty())); + } + //testing if constant values hardcoded into the code are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", "true", + zk.getClientConfig().getProperty(ZKClientConfig.SECURE_CLIENT)); + assertEquals("Validate that expected clientConfig is set in ZK config", + "org.apache.zookeeper.ClientCnxnSocketNetty", + zk.getClientConfig().getProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET)); + } + + @Test + public void testTruststoreKeystoreConfiguration(){ + LOG.info("Entered to the testTruststoreKeystoreConfiguration test case."); + /** Review Comment: nit: Should be a block comment, now it's a dangling javadoc like this. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { + Configuration conf = new Configuration(); + System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server" + + ".NettyServerCnxnFactory"); + + System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "keystore.jks"); + System.setProperty("zookeeper.ssl.keyStore.password", "password"); + System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "truststore.jks"); + System.setProperty("zookeeper.ssl.trustStore.password", "password"); + System.setProperty("zookeeper.request.timeout", "12345"); + + System.setProperty("jute.maxbuffer", JUTE_MAXBUFFER.toString()); + + System.setProperty("javax.net.debug", "ssl"); + System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth" + + ".X509AuthenticationProvider"); + + + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + + "keystore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + + "truststore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password"); + return conf; + } + + @After + public void teardown() throws Exception { + this.curator.close(); + if (this.server != null) { + this.server.close(); + this.server = null; + } + } + + @Test + public void testSecureZKConfiguration() throws Exception { + LOG.info("Entered to the testSecureZKConfiguration test case."); + // Validate that HadoopZooKeeperFactory will set ZKConfig with given principals + ZKCuratorManager.HadoopZookeeperFactory factory = + new ZKCuratorManager.HadoopZookeeperFactory( + null, + null, + null, + true, + new ZKCuratorManager.TruststoreKeystore(hadoopConf)); + ZooKeeper zk = factory.newZooKeeper(this.server.getConnectString(), 1000, null, false); + validateSSLConfiguration( + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD), + zk); + } + + private void validateSSLConfiguration( + String keystoreLocation, + String keystorePassword, + String truststoreLocation, + String truststorePassword, + ZooKeeper zk){ + try (ClientX509Util x509Util = new ClientX509Util()) { + //testing if custom values are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", keystoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslKeystoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", keystorePassword, + zk.getClientConfig().getProperty(x509Util.getSslKeystorePasswdProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslTruststoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststorePassword, + zk.getClientConfig().getProperty(x509Util.getSslTruststorePasswdProperty())); + } + //testing if constant values hardcoded into the code are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", "true", + zk.getClientConfig().getProperty(ZKClientConfig.SECURE_CLIENT)); + assertEquals("Validate that expected clientConfig is set in ZK config", + "org.apache.zookeeper.ClientCnxnSocketNetty", + zk.getClientConfig().getProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET)); + } + + @Test + public void testTruststoreKeystoreConfiguration(){ + LOG.info("Entered to the testTruststoreKeystoreConfiguration test case."); + /** + * By default the truststore/keystore configurations are not set, hence the values are null. + * Validate that the null values are converted into empty strings by the class. + */ + Configuration conf = new Configuration(); + new ZKCuratorManager.TruststoreKeystore(conf); + + assertEquals("Validate that null value is converted to empty string.", "", + ZKCuratorManager.TruststoreKeystore.getKeystoreLocation()); + assertEquals("Validate that null value is converted to empty string.", "", + ZKCuratorManager.TruststoreKeystore.getKeystorePassword()); + assertEquals("Validate that null value is converted to empty string.", "", + ZKCuratorManager.TruststoreKeystore.getTruststoreLocation()); + assertEquals("Validate that null value is converted to empty string.", "", + ZKCuratorManager.TruststoreKeystore.getTruststorePassword()); + + //Validate that non-null values will remain intact + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, "/keystore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "keystorePassword"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, "/truststore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "truststorePassword"); + new ZKCuratorManager.TruststoreKeystore(conf); Review Comment: It doesn't make too much sense to invoke a constructor of TruststoreKeystore and store all values from the config as static fields. Static is not really for this. I would just convert all the 4 static fields to instance fields in TruststoreKeystore. Funnily I only noticed this weirdness from the usage of this class in here in the testcase. So basically, what will be changed here is to create a new instance of TruststoreKeystore with the conf and use the same getters, but on that object rather than in a static fashion like what you have currently. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { + Configuration conf = new Configuration(); + System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server" + + ".NettyServerCnxnFactory"); + + System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "keystore.jks"); + System.setProperty("zookeeper.ssl.keyStore.password", "password"); + System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "truststore.jks"); + System.setProperty("zookeeper.ssl.trustStore.password", "password"); + System.setProperty("zookeeper.request.timeout", "12345"); + + System.setProperty("jute.maxbuffer", JUTE_MAXBUFFER.toString()); + + System.setProperty("javax.net.debug", "ssl"); + System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth" + + ".X509AuthenticationProvider"); + + + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + + "keystore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + + "truststore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password"); + return conf; + } + + @After + public void teardown() throws Exception { + this.curator.close(); + if (this.server != null) { + this.server.close(); + this.server = null; + } + } + + @Test + public void testSecureZKConfiguration() throws Exception { + LOG.info("Entered to the testSecureZKConfiguration test case."); + // Validate that HadoopZooKeeperFactory will set ZKConfig with given principals + ZKCuratorManager.HadoopZookeeperFactory factory = + new ZKCuratorManager.HadoopZookeeperFactory( + null, + null, + null, + true, + new ZKCuratorManager.TruststoreKeystore(hadoopConf)); + ZooKeeper zk = factory.newZooKeeper(this.server.getConnectString(), 1000, null, false); + validateSSLConfiguration( + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD), + zk); + } + + private void validateSSLConfiguration( + String keystoreLocation, + String keystorePassword, + String truststoreLocation, + String truststorePassword, + ZooKeeper zk){ + try (ClientX509Util x509Util = new ClientX509Util()) { + //testing if custom values are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", keystoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslKeystoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", keystorePassword, + zk.getClientConfig().getProperty(x509Util.getSslKeystorePasswdProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslTruststoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststorePassword, + zk.getClientConfig().getProperty(x509Util.getSslTruststorePasswdProperty())); + } + //testing if constant values hardcoded into the code are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", "true", Review Comment: Nit: "true" --> Boolean.TRUE.toString() ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; Review Comment: could be primitive int. Moreover, the name 'defaultValue' does not tell me anything. Please add 2 static final ints and name them that resembles quorum port and election port. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { Review Comment: Maybe setUpSecureConfig is a better name. ########## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java: ########## @@ -478,10 +557,72 @@ public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, if (zkClientConfig.isSaslClientEnabled() && !isJaasConfigurationSet(zkClientConfig)) { setJaasConfiguration(zkClientConfig); } + if (sslEnabled) { + setSslConfiguration(zkClientConfig); + } return new ZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly, zkClientConfig); } + /** + * Configure ZooKeeper Client with SSL/TLS connection. + * @param zkClientConfig ZooKeeper Client configuration + * */ + private void setSslConfiguration(ZKClientConfig zkClientConfig) throws ConfigurationException { + this.setSslConfiguration(zkClientConfig, new ClientX509Util()); + } + + private void setSslConfiguration(ZKClientConfig zkClientConfig, ClientX509Util x509Util) + throws ConfigurationException { + validateSslConfiguration( + TruststoreKeystore.keystoreLocation, + TruststoreKeystore.keystorePassword, + TruststoreKeystore.truststoreLocation, + TruststoreKeystore.truststorePassword); + LOG.info("Configuring the ZooKeeper client to use SSL/TLS encryption for connecting to the " + + "ZooKeeper server."); + LOG.debug("Configuring the ZooKeeper client with {} location: {}.", + TruststoreKeystore.keystoreLocation, + CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION); + LOG.debug("Configuring the ZooKeeper client with {} location: {}.", + TruststoreKeystore.truststoreLocation, + CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION); + + zkClientConfig.setProperty(ZKClientConfig.SECURE_CLIENT, "true"); + zkClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, + "org.apache.zookeeper.ClientCnxnSocketNetty"); + zkClientConfig.setProperty(x509Util.getSslKeystoreLocationProperty(), + TruststoreKeystore.keystoreLocation); + zkClientConfig.setProperty(x509Util.getSslKeystorePasswdProperty(), + TruststoreKeystore.keystorePassword); + zkClientConfig.setProperty(x509Util.getSslTruststoreLocationProperty(), + TruststoreKeystore.truststoreLocation); + zkClientConfig.setProperty(x509Util.getSslTruststorePasswdProperty(), + TruststoreKeystore.truststorePassword); + } + + private void validateSslConfiguration(String keystoreLocation, String keystorePassword, Review Comment: All parameters are unused in this method. Please remove them. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { + Configuration conf = new Configuration(); + System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server" + Review Comment: Nit: Getting the name of the class from the Class object of NettyServerCnxnFactory would be cleaner. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { + Configuration conf = new Configuration(); + System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server" + + ".NettyServerCnxnFactory"); + + System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "keystore.jks"); + System.setProperty("zookeeper.ssl.keyStore.password", "password"); + System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "truststore.jks"); + System.setProperty("zookeeper.ssl.trustStore.password", "password"); + System.setProperty("zookeeper.request.timeout", "12345"); + + System.setProperty("jute.maxbuffer", JUTE_MAXBUFFER.toString()); + + System.setProperty("javax.net.debug", "ssl"); + System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth" + + ".X509AuthenticationProvider"); + + + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + + "keystore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + + "truststore.jks"); + conf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password"); + return conf; + } + + @After + public void teardown() throws Exception { + this.curator.close(); + if (this.server != null) { + this.server.close(); + this.server = null; + } + } + + @Test + public void testSecureZKConfiguration() throws Exception { + LOG.info("Entered to the testSecureZKConfiguration test case."); + // Validate that HadoopZooKeeperFactory will set ZKConfig with given principals + ZKCuratorManager.HadoopZookeeperFactory factory = + new ZKCuratorManager.HadoopZookeeperFactory( + null, + null, + null, + true, + new ZKCuratorManager.TruststoreKeystore(hadoopConf)); + ZooKeeper zk = factory.newZooKeeper(this.server.getConnectString(), 1000, null, false); + validateSSLConfiguration( + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION), + this.hadoopConf.get(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD), + zk); + } + + private void validateSSLConfiguration( + String keystoreLocation, + String keystorePassword, + String truststoreLocation, + String truststorePassword, + ZooKeeper zk){ + try (ClientX509Util x509Util = new ClientX509Util()) { + //testing if custom values are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", keystoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslKeystoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", keystorePassword, + zk.getClientConfig().getProperty(x509Util.getSslKeystorePasswdProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststoreLocation, + zk.getClientConfig().getProperty(x509Util.getSslTruststoreLocationProperty())); + assertEquals("Validate that expected clientConfig is set in ZK config", truststorePassword, + zk.getClientConfig().getProperty(x509Util.getSslTruststorePasswdProperty())); + } + //testing if constant values hardcoded into the code are set properly + assertEquals("Validate that expected clientConfig is set in ZK config", "true", + zk.getClientConfig().getProperty(ZKClientConfig.SECURE_CLIENT)); + assertEquals("Validate that expected clientConfig is set in ZK config", + "org.apache.zookeeper.ClientCnxnSocketNetty", Review Comment: Nit: Getting the name from the Class object will be cleaner. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java: ########## @@ -0,0 +1,192 @@ +/** + * 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.hadoop.util.curator; + +import org.apache.curator.test.InstanceSpec; +import org.apache.curator.test.TestingServer; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.client.ZKClientConfig; +import org.apache.zookeeper.common.ClientX509Util; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +import static org.apache.hadoop.fs.FileContext.LOG; +import static org.junit.Assert.assertEquals; + + +/** + * Test the manager for ZooKeeper Curator when SSL/TLS is enabled for the ZK server-client + * connection. + */ +public class TestSecureZKCuratorManager { + + private TestingServer server; + private ZKCuratorManager curator; + private Configuration hadoopConf; + static final Integer SECURE_CLIENT_PORT = 2281; + static final Integer JUTE_MAXBUFFER = 400000000; + static final File ZK_DATA_DIR = new File("testZkSSLClientConnectionDataDir"); + + @Before + public void setup() throws Exception { + Integer defaultValue = -1; + Map<String, Object> customConfiguration = new HashMap<>(); + customConfiguration.put("secureClientPort", SECURE_CLIENT_PORT.toString()); + customConfiguration.put("audit.enable", true); + this.hadoopConf = setUpSecure(); + InstanceSpec spec = new InstanceSpec(ZK_DATA_DIR, SECURE_CLIENT_PORT, + defaultValue, + defaultValue, + true, + 1, + 100, + 10, + customConfiguration); + this.server = new TestingServer(spec, true); + this.hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString()); + this.curator = new ZKCuratorManager(this.hadoopConf); + this.curator.start(new ArrayList<>(), true); + } + + public static Configuration setUpSecure() throws Exception { + return setUpSecure("src/test/java/org/apache/hadoop/util/curator/resources/data/ssl/"); + } + public static Configuration setUpSecure(String testDataPath) throws Exception { Review Comment: This method does not trow any Exception according to IntelliJ > Add curator based ZooKeeper communication support over SSL/TLS into the > common library > -------------------------------------------------------------------------------------- > > Key: HADOOP-18709 > URL: https://issues.apache.org/jira/browse/HADOOP-18709 > Project: Hadoop Common > Issue Type: Improvement > Reporter: Ferenc Erdelyi > Assignee: Ferenc Erdelyi > Priority: Major > Labels: pull-request-available > > With HADOOP-16579 the ZooKeeper client is capable of securing communication > with SSL. > To follow the convention introduced in HADOOP-14741, proposing to add to the > core-default.xml the following configurations, as the groundwork for the > components to enable encrypted communication between the individual > components and ZooKeeper: > * hadoop.zk.ssl.keystore.location > * hadoop.zk.ssl.keystore.password > * hadoop.zk.ssl.truststore.location > * hadoop.zk.ssl.truststore.password > These parameters along with the component-specific ssl.client.enable option > (e.g. yarn.zookeeper.ssl.client.enable) should be passed to the > ZKCuratorManager to build the CuratorFramework. The ZKCuratorManager needs a > new overloaded start() method to build the encrypted communication. > * The secured ZK Client uses Netty, hence the dependency is included in the > pom.xml. Added netty-handler and netty-transport-native-epoll dependency to > the pom.xml based on ZOOKEEPER-3494 - "No need to depend on netty-all (SSL)". > * The change was exclusively tested with the unit test, which is a kind of > integration test, as a ZK Server was brought up and the communication tested > between the client and the server. > * This code change is in the common code base and there is no component > calling it yet. Once YARN-11468 - "Zookeeper SSL/TLS support" is implemented, > we can test it in a real cluster environment. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org