[ 
https://issues.apache.org/jira/browse/HADOOP-18709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17724469#comment-17724469
 ] 

ASF GitHub Bot commented on HADOOP-18709:
-----------------------------------------

szilard-nemeth commented on code in PR #5638:
URL: https://github.com/apache/hadoop/pull/5638#discussion_r1199521345


##########
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:
   There was also a missing one before method: validateSslConfiguration
   Can you fix the javadoc as it is not starting with /** (but with: /*) ? 
Something must be odd with your formatter.





> 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

Reply via email to