[GitHub] [zookeeper] symat commented on a change in pull request #1170: ZOOKEEPER-3643: Testing and documenting secure and unsecure ZK client connections

[GitBox <gi...@apache.org>]

symat commented on a change in pull request #1170: ZOOKEEPER-3643: Testing and documenting secure and unsecure ZK client connections
URL: https://github.com/apache/zookeeper/pull/1170#discussion_r356567078
 
 

 ##########
 File path: zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
 ##########
 @@ -138,6 +118,55 @@ public void testClientServerSSL(boolean useSecurePort) throws Exception {
         }
     }
 
+    /**
+     * This test covers the case when from the same JVM we connect to both secure and unsecure
+     * clusters. In this case we can't use the Java System Properties, but we need to specify client
+     * configuration.
+     *
+     * In this test the servers has two client ports open, one used only for secure connection and one
+     * used only for unsecure connections. (the client port unification is disabled)
+     */
+    @Test
+    public void testClientCanConnectBothSecureAndUnsecure() throws Exception {
+
+      // to make sure the test is testing the case we want, we disable client port unification in the
+      // server, and also disable the property which would instruct the client to connect using SSL
+      System.clearProperty(NettyServerCnxnFactory.PORT_UNIFICATION_KEY);
+      System.clearProperty(ZKClientConfig.SECURE_CLIENT);
+
+      final int SERVER_COUNT = 3;
+      final int[] clientPorts = new int[SERVER_COUNT];
+      int[] secureClientPorts = new int[SERVER_COUNT];
+
+      MainThread[] mt = startThreeNodeSSLCluster(clientPorts, secureClientPorts);
+
+      // Servers have been set up. Now go test if both secure and unsecure connection is successful.
+      for (int i = 0; i < SERVER_COUNT; i++) {
+
+        // testing the secure connection, also do some simple operation to verify that it works
+        ZKClientConfig secureClientConfig = new ZKClientConfig();
+        secureClientConfig.setProperty(ZKClientConfig.SECURE_CLIENT, "true");
+        ZooKeeper zkSecure = ClientBase.createZKClient("127.0.0.1:" + secureClientPorts[i], TIMEOUT, secureClientConfig);
+        zkSecure.create("/test", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
 
 Review comment:
   Thanks, it's a great question!
   
   To initiate SSL connection (instead of a non-secure one) is a decision taken in the actual Client Socket implementation. (When we are creating a new ZooKeeper client session, it is not enough to request to have SSL connection in the client config, but we also have to use Netty instead of NIO, otherwise we will not have an SSL connection)
   
   For the tests it would be possible extract this information by extending the `ZooKeeperTestable` class, but I think it is actually an information that can be important even for the users of the Java Client API. So I propose a new method on the `ZooKeeper` class. See my implementation in the next commit.
   
   What do you think?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services