You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "szilard-nemeth (via GitHub)" <gi...@apache.org> on 2023/05/11 23:47:07 UTC
[GitHub] [hadoop] szilard-nemeth commented on a diff in pull request #5638: HADOOP-18709. Add curator based ZooKeeper communication support over…
szilard-nemeth commented on code in PR #5638:
URL: https://github.com/apache/hadoop/pull/5638#discussion_r1191770472
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java:
##########
@@ -157,12 +175,44 @@ public void start(List<AuthInfo> authInfos) throws IOException {
authInfos.add(new AuthInfo(zkAuth.getScheme(), zkAuth.getAuth()));
}
+ /* Pre-check on SSL/TLS client connection requirements to emit the name of the
+ configuration missing. It improves supportability. */
+ if(sslEnabled) {
+ if (StringUtils.isEmpty(conf.get(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION))) {
+ throw new ConfigurationException(
Review Comment:
Nit: Can you extract this to a helper method?
Only the config name should be passed, the rest of the method body (even the exception message) can be the same.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java:
##########
@@ -478,10 +558,53 @@ 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());
+ }
+ public void setSslConfiguration(ZKClientConfig zkClientConfig, ClientX509Util x509Util )
Review Comment:
Nit: Line break between methods.
Also it seems your formatter seems to be off, as method parentheses has an additional space before it after the parameters.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java:
##########
@@ -478,10 +558,53 @@ 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());
+ }
+ public void setSslConfiguration(ZKClientConfig zkClientConfig, ClientX509Util x509Util )
+ throws ConfigurationException {
+ LOG.info("Configuring the ZooKeeper client to use SSL/TLS encryption for connecting to the ZooKeeper server.");
+ if (StringUtils.isEmpty(this.keystoreLocation)) {
Review Comment:
Nit: Here you could also extract the isempty check + throwing the exception to a method.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
Review Comment:
can be static final
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ZKCuratorManager.java:
##########
@@ -478,10 +558,53 @@ 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());
+ }
+ public void setSslConfiguration(ZKClientConfig zkClientConfig, ClientX509Util x509Util )
+ throws ConfigurationException {
+ LOG.info("Configuring the ZooKeeper client to use SSL/TLS encryption for connecting to the ZooKeeper server.");
+ if (StringUtils.isEmpty(this.keystoreLocation)) {
+ throw new ConfigurationException(
+ "The keystore location parameter is empty for the ZooKeeper client connection.");
+ }
+ if (StringUtils.isEmpty(this.keystorePassword)) {
+ throw new ConfigurationException(
+ "The keystore password parameter is empty for the ZooKeeper client connection.");
+ }
+ if (StringUtils.isEmpty(this.truststoreLocation)) {
+ throw new ConfigurationException(
+ "The truststore location parameter is empty for the ZooKeeper client connection.");
+ }
+ if (StringUtils.isEmpty(this.truststorePassword)) {
+ throw new ConfigurationException(
+ "The truststore password parameter is empty for the ZooKeeper client connection.");
+ }
+ LOG.debug("Configuring the ZooKeeper client with {} {} location.",
+ this.keystoreLocation, CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION);
Review Comment:
maybe it's better / more straightforward as:
```suggestion
LOG.debug("Configuring the ZooKeeper client with {} location: {}",
this.keystoreLocation, CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION);
```
##########
hadoop-common-project/hadoop-common/pom.xml:
##########
@@ -342,6 +342,14 @@
</exclusion>
</exclusions>
</dependency>
+ <dependency>
Review Comment:
What's the reason of adding the netty dependencies here?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
Review Comment:
can be static final
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
Review Comment:
For better readability, these could be extracted to static final fields or local variables.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
+ -1,
+ true,
+ 1,
+ 100,
+ 10,
+ customConfiguration);
+ this.server = new TestingServer(spec, true);
+ hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString());
+ this.curator = new ZKCuratorManager(hadoopConf);
+ this.curator.start(new ArrayList<>(), true);
+ }
+
+ public Configuration setUpSecure() throws Exception {
+ Configuration hadoopConf = new Configuration();
+ String testDataPath = "src/test/java/org/apache/hadoop/util/curator/resources/data";
+ System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+ //System.setProperty("zookeeper.client.secure", "true");
Review Comment:
Can this be removed?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
+ -1,
+ true,
+ 1,
+ 100,
+ 10,
+ customConfiguration);
+ this.server = new TestingServer(spec, true);
+ hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString());
+ this.curator = new ZKCuratorManager(hadoopConf);
+ this.curator.start(new ArrayList<>(), true);
+ }
+
+ public Configuration setUpSecure() throws Exception {
+ Configuration hadoopConf = new Configuration();
+ String testDataPath = "src/test/java/org/apache/hadoop/util/curator/resources/data";
+ System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+ //System.setProperty("zookeeper.client.secure", "true");
+
+
+ System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "/ssl/keystore.jks");
+ System.setProperty("zookeeper.ssl.keyStore.password", "password");
+ System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "/ssl/truststore.jks");
+ System.setProperty("zookeeper.ssl.trustStore.password", "password");
+ System.setProperty("zookeeper.request.timeout", "12345");
+
+ System.setProperty("jute.maxbuffer", "469296129");
Review Comment:
Is there a meaning of this number?
Can it be calculated and saved to a static final int constant?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
+ -1,
+ true,
+ 1,
+ 100,
+ 10,
+ customConfiguration);
+ this.server = new TestingServer(spec, true);
+ hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString());
+ this.curator = new ZKCuratorManager(hadoopConf);
+ this.curator.start(new ArrayList<>(), true);
+ }
+
+ public Configuration setUpSecure() throws Exception {
+ Configuration hadoopConf = new Configuration();
+ String testDataPath = "src/test/java/org/apache/hadoop/util/curator/resources/data";
+ System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+ //System.setProperty("zookeeper.client.secure", "true");
+
+
+ System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "/ssl/keystore.jks");
+ System.setProperty("zookeeper.ssl.keyStore.password", "password");
+ System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "/ssl/truststore.jks");
+ System.setProperty("zookeeper.ssl.trustStore.password", "password");
+ System.setProperty("zookeeper.request.timeout", "12345");
+
+ System.setProperty("jute.maxbuffer", "469296129");
+
+ System.setProperty("javax.net.debug", "ssl");
+ System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
+
+
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + "/ssl/keystore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + "/ssl/truststore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password");
+ return hadoopConf;
+ }
+
+ @After
+ public void teardown() throws Exception {
+ this.curator.close();
+ if (this.server != null) {
+ this.server.close();
+ this.server = null;
Review Comment:
what's the reason of setting it to null?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
+ -1,
+ true,
+ 1,
+ 100,
+ 10,
+ customConfiguration);
+ this.server = new TestingServer(spec, true);
+ hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString());
+ this.curator = new ZKCuratorManager(hadoopConf);
+ this.curator.start(new ArrayList<>(), true);
+ }
+
+ public Configuration setUpSecure() throws Exception {
+ Configuration hadoopConf = new Configuration();
+ String testDataPath = "src/test/java/org/apache/hadoop/util/curator/resources/data";
+ System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+ //System.setProperty("zookeeper.client.secure", "true");
+
+
+ System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "/ssl/keystore.jks");
+ System.setProperty("zookeeper.ssl.keyStore.password", "password");
+ System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "/ssl/truststore.jks");
+ System.setProperty("zookeeper.ssl.trustStore.password", "password");
+ System.setProperty("zookeeper.request.timeout", "12345");
+
+ System.setProperty("jute.maxbuffer", "469296129");
+
+ System.setProperty("javax.net.debug", "ssl");
+ System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
+
+
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + "/ssl/keystore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + "/ssl/truststore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password");
+ return hadoopConf;
+ }
+
+ @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 factory1 =
+ new ZKCuratorManager.HadoopZookeeperFactory(
+ null,
+ null,
+ null,
+ true,
+ 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));
+ ZooKeeper zk1 = factory1.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),
+ zk1);
+ }
+
+ private void validateSSLConfiguration(
+ String keystoreLocation,
+ String keystorePassword,
+ String truststoreLocation,
+ String truststorePassword,
+ ZooKeeper zk){
+ ClientX509Util x509Util = new ClientX509Util();
Review Comment:
IntelliJ gives me a warning: 'ClientX509Util' used without 'try'-with-resources statement
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestSecureZKCuratorManager.java:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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;
+ private Integer secureClientPort = 2281;
+ private File zkDataDir = new File("testZkSSLClientConnectionDataDir");
+
+ @Before
+ public void setup() throws Exception {
+ //set zkServer
+ this.hadoopConf = setUpSecure();
+ Map<String, Object> customConfiguration = new HashMap<>();
+ customConfiguration.put("secureClientPort",this.secureClientPort.toString());
+ customConfiguration.put("audit.enable",true);
+
+ InstanceSpec spec = new InstanceSpec(
+ this.zkDataDir,
+ this.secureClientPort,
+ -1,
+ -1,
+ true,
+ 1,
+ 100,
+ 10,
+ customConfiguration);
+ this.server = new TestingServer(spec, true);
+ hadoopConf.set(CommonConfigurationKeys.ZK_ADDRESS, this.server.getConnectString());
+ this.curator = new ZKCuratorManager(hadoopConf);
+ this.curator.start(new ArrayList<>(), true);
+ }
+
+ public Configuration setUpSecure() throws Exception {
+ Configuration hadoopConf = new Configuration();
+ String testDataPath = "src/test/java/org/apache/hadoop/util/curator/resources/data";
+ System.setProperty("zookeeper.serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+ //System.setProperty("zookeeper.client.secure", "true");
+
+
+ System.setProperty("zookeeper.ssl.keyStore.location", testDataPath + "/ssl/keystore.jks");
+ System.setProperty("zookeeper.ssl.keyStore.password", "password");
+ System.setProperty("zookeeper.ssl.trustStore.location", testDataPath + "/ssl/truststore.jks");
+ System.setProperty("zookeeper.ssl.trustStore.password", "password");
+ System.setProperty("zookeeper.request.timeout", "12345");
+
+ System.setProperty("jute.maxbuffer", "469296129");
+
+ System.setProperty("javax.net.debug", "ssl");
+ System.setProperty("zookeeper.authProvider.x509", "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
+
+
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_LOCATION, testDataPath + "/ssl/keystore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_KEYSTORE_PASSWORD, "password");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_LOCATION, testDataPath + "/ssl/truststore.jks");
+ hadoopConf.set(CommonConfigurationKeys.ZK_SSL_TRUSTSTORE_PASSWORD, "password");
+ return hadoopConf;
+ }
+
+ @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 factory1 =
Review Comment:
nit: Can you call the variables "factory" and "zk"?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org