Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 14, 2015, 2:38 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 12, 2015, 9:47 p.m., fpj wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 486 https://reviews.apache.org/r/31277/diff/16/?file=890432#file890432line486 I don't understand why we have this for loop here iterating over cnxns, why can't we simply call closeSession on the sessionId? Hongchao Deng wrote: I'm trying to keep the original logic here. What if all ServerCnxnFactories are null (the comments said we are just playing with diff)? Then it's possible that we won't return here and go the the switch block. This is a left problem. Might be great if you can review it again. fpj wrote: I think you can pass the request to closeSession instead of just the session id and avoid the for loop by checking the same predicate and closing the session accordingly, like in the original code. My concern with this loop is that if the number of connections is large, it might take some time. Hi Flavio. I've addressed you concern with the loop. The reason behind this code is that I need to make sure the request session ID is in a ServerCnxnFactory that != null. Otherwise it's like the previous logic that playing diffs with leader and should return. Please take a look. Thanks! - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 --- On March 14, 2015, 2:38 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 14, 2015, 2:38 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 14, 2015, 2:59 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 12, 2015, 9:47 p.m., fpj wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 486 https://reviews.apache.org/r/31277/diff/16/?file=890432#file890432line486 I don't understand why we have this for loop here iterating over cnxns, why can't we simply call closeSession on the sessionId? Hongchao Deng wrote: I'm trying to keep the original logic here. What if all ServerCnxnFactories are null (the comments said we are just playing with diff)? Then it's possible that we won't return here and go the the switch block. This is a left problem. Might be great if you can review it again. I think you can pass the request to closeSession instead of just the session id and avoid the for loop by checking the same predicate and closing the session accordingly, like in the original code. My concern with this loop is that if the number of connections is large, it might take some time. - fpj --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 --- On March 13, 2015, 12:12 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 13, 2015, 12:12 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75917 --- Ship it! Ship It! - Raul Gutierrez Segales On March 10, 2015, 6:04 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 10, 2015, 6:04 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 13, 2015, 12:12 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 12, 2015, 9:47 p.m., fpj wrote: src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java, line 279 https://reviews.apache.org/r/31277/diff/16/?file=890434#file890434line279 why is this method synchronized? I should have added a comment here. Will do it. // The synchronized is to prevent the race on shared variable sslEngine. // Basically we only need to create it once. On March 12, 2015, 9:47 p.m., fpj wrote: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 621 https://reviews.apache.org/r/31277/diff/16/?file=890427#file890427line621 What if we change the text here to simply: the port to listen on for secure client connections using SSL. Fixed. On March 12, 2015, 9:47 p.m., fpj wrote: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1057 https://reviews.apache.org/r/31277/diff/16/?file=890427#file890427line1057 Don't you need to change this bit as well? Fixed. On March 12, 2015, 9:47 p.m., fpj wrote: src/java/test/org/apache/zookeeper/test/SSLTest.java, line 97 https://reviews.apache.org/r/31277/diff/16/?file=890452#file890452line97 It might be a good idea to do a CLIENT_COUNT for clarity. This is the server. It's trying connect to every server to test it. On March 12, 2015, 9:47 p.m., fpj wrote: src/java/test/org/apache/zookeeper/test/SSLTest.java, line 115 https://reviews.apache.org/r/31277/diff/16/?file=890452#file890452line115 Should we run an operation just to make sure the wiring on the server side is fine? Fixed. On March 12, 2015, 9:47 p.m., fpj wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 486 https://reviews.apache.org/r/31277/diff/16/?file=890432#file890432line486 I don't understand why we have this for loop here iterating over cnxns, why can't we simply call closeSession on the sessionId? I'm trying to keep the original logic here. What if all ServerCnxnFactories are null (the comments said we are just playing with diff)? Then it's possible that we won't return here and go the the switch block. This is a left problem. Might be great if you can review it again. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 --- On March 13, 2015, 12:12 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 13, 2015, 12:12 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 --- It looks pretty good, I just have a few small comments and clarifications below. src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml https://reviews.apache.org/r/31277/#comment123466 What if we change the text here to simply: the port to listen on for secure client connections using SSL. src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml https://reviews.apache.org/r/31277/#comment123470 Don't you need to change this bit as well? src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java https://reviews.apache.org/r/31277/#comment123807 I don't understand why we have this for loop here iterating over cnxns, why can't we simply call closeSession on the sessionId? src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment123808 why is this method synchronized? src/java/test/org/apache/zookeeper/test/SSLTest.java https://reviews.apache.org/r/31277/#comment123811 It might be a good idea to do a CLIENT_COUNT for clarity. src/java/test/org/apache/zookeeper/test/SSLTest.java https://reviews.apache.org/r/31277/#comment123812 Should we run an operation just to make sure the wiring on the server side is fine? - fpj On March 10, 2015, 6:04 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 10, 2015, 6:04 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 10, 2015, 6:04 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75737 --- Ship it! Ship It! - Rakesh R On March 7, 2015, 4:02 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 4:02 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 7, 2015, 5:24 a.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 493 https://reviews.apache.org/r/31277/diff/14/?file=888101#file888101line493 Can we extract this to a method to avoid duplication Rakesh R wrote: adding few more to the above comment, : both cf anf scf logic looks same, so we can extract to a closeSession method and pass cf argument. on return will check success then return else do scf. Yep. I wasn't paying much attention to refactoring when fixing the problem. Thanks for catching it up. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75616 --- On March 7, 2015, 4:02 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 4:02 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 4:02 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 10:48 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 10:41 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 356 https://reviews.apache.org/r/31277/diff/11/?file=886448#file886448line356 do we need synchronization here? It's not obvious here. I am going to add some comments. The reason is avoid race of shared sslEngine. On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 343 https://reviews.apache.org/r/31277/diff/11/?file=886448#file886448line343 please move sslEngine,sslContext inside the method initSSL(pipeline) as local variables? They only need to be initiated once. Later on, connections will reuse the sslEngine. On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 357 https://reviews.apache.org/r/31277/diff/11/?file=886448#file886448line357 I could see new ZKClientPipelineFactory() is always created and not required to do null checks here, isn't it? Isn't ZKClientPipelineFactory() created only once? Netty 3.x docs said that channelpipeline is per connection, and cpFactory creates for each. On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 363 https://reviews.apache.org/r/31277/diff/11/?file=886448#file886448line363 this log wouldn't help much, probably you can try include info like pipeline.getChannel() details. Or can make this DEBUG level priority. Right. Adding channel details will be useful when things go wrong. On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/common/X509Error.java, line 21 https://reviews.apache.org/r/31277/diff/11/?file=886450#file886450line21 I prefer to use X509Exception instead of X509Error, can you rename this to X509Exception? Sure - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75525 --- On March 6, 2015, 12:17 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 12:17 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 7, 2015, 1:08 a.m., Raul Gutierrez Segales wrote: src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java, line 849 https://reviews.apache.org/r/31277/diff/12-14/?file=887835#file887835line849 can we get rid of these red tabs pls? Yes I did a few other pushes to get rid of unnecessary changes :) See the latest version. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75593 --- On March 7, 2015, 1:04 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75593 --- src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java https://reviews.apache.org/r/31277/#comment122791 can we get rid of these red tabs pls? - Raul Gutierrez Segales On March 7, 2015, 1:04 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/common/X509Error.java, line 21 https://reviews.apache.org/r/31277/diff/11/?file=886450#file886450line21 I prefer to use X509Exception instead of X509Error, can you rename this to X509Exception? Hongchao Deng wrote: Sure Hi Rakesh, findbugs complains that X509Exception didn't extend Exception. I think changing back to X509Error will work. Any opinions? - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75525 --- On March 7, 2015, 1:04 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 6, 2015, 8:44 p.m., Rakesh R wrote: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 618 https://reviews.apache.org/r/31277/diff/11/?file=886447#file886447line618 Netty usage is pluggable. SSL feature will be enabled when user user plugged-in zookeeper.serverCnxnFactory, zookeeper.clientCnxnSocket as Netty. isn't it? Its good to capture in the document, whats your opinion? Sounds perfect! - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75542 --- On March 6, 2015, 12:17 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 12:17 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review75525
---
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
https://reviews.apache.org/r/31277/#comment122666
please move sslEngine,sslContext inside the method initSSL(pipeline) as
local variables?
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
https://reviews.apache.org/r/31277/#comment122662
do we need synchronization here?
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
https://reviews.apache.org/r/31277/#comment122667
I could see new ZKClientPipelineFactory() is always created and not
required to do null checks here, isn't it?
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
https://reviews.apache.org/r/31277/#comment122657
this log wouldn't help much, probably you can try include info like
pipeline.getChannel() details. Or can make this DEBUG level priority.
src/java/main/org/apache/zookeeper/common/X509Error.java
https://reviews.apache.org/r/31277/#comment122659
I prefer to use X509Exception instead of X509Error, can you rename this to
X509Exception?
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment122665
same as above, do we need synchronization here?
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment122664
this log wouldn't help much, probably you can try include info like
pipeline.getChannel() details. Or can make this DEBUG level priority.
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
https://reviews.apache.org/r/31277/#comment122714
FinalRequestProcessor also has the logic of #closeSession(). I couldn't see
the closure of session from secureCnxnFactory in FinalRequestProcessor.
Please add the same logic of #closeSession() in FinalRequestProcessor as
well.
Refer: FinalRequestProcessor#processRequest()
if (request.type == OpCode.closeSession) {
- Rakesh R
On March 6, 2015, 12:17 a.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated March 6, 2015, 12:17 a.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
202051f1f7f517b1e1a3c561c0008449ab3c48a6
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/ServerConfig.java
f2b8463e871739319bdf40be1f014d5ad0af5602
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
30a0ed390bb7473ddb36757da97bc7d5f4281887
src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
0eb5c7f979199f2f7dcb9e5cfa011a9b20113713
src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
b756d349abeb1fc69534100c3633db4c1c18e031
src/java/main/org/apache/zookeeper/server/quorum/Leader.java
20589045752a7ba4ae9c9090055a4fcbe86a8eda
src/java/main/org/apache/zookeeper/server/quorum/Learner.java
4dd1e947357080f3e055f3e7e2a78c979daa6ea7
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
badc8df1f05dea4be337bc8312d7ac22f6c77dc3
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
d17c58d59e0131a78adde1becb5c23ce8c7a16a7
src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java
2aab6d09f9bd980ed76f886fb8168aae2ac8f99f
src/java/test/data/ssl/README.md PRE-CREATION
src/java/test/data/ssl/testKeyStore.jks PRE-CREATION
src/java/test/data/ssl/testTrustStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
6ab19b1eb137c8b13b8ad031d474e213267da1ea
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75542 --- src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml https://reviews.apache.org/r/31277/#comment122717 Netty usage is pluggable. SSL feature will be enabled when user user plugged-in zookeeper.serverCnxnFactory, zookeeper.clientCnxnSocket as Netty. isn't it? Its good to capture in the document, whats your opinion? - Rakesh R On March 6, 2015, 12:17 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 12:17 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 6, 2015, 8:39 p.m., Rakesh R wrote:
src/java/main/org/apache/zookeeper/common/X509Error.java, line 21
https://reviews.apache.org/r/31277/diff/11/?file=886450#file886450line21
I prefer to use X509Exception instead of X509Error, can you rename this
to X509Exception?
Hongchao Deng wrote:
Sure
Hongchao Deng wrote:
Hi Rakesh, findbugs complains that X509Exception didn't extend Exception.
I think changing back to X509Error will work. Any opinions?
Can we do like,
@SuppressWarnings(serial)
public class X509Exception extends Exception {
public X509Exception(String message) {
super(message);
}
public X509Exception(Throwable cause) {
super(cause);
}
public X509Exception(String message, Throwable cause) {
super(message, cause);
}
public static class KeyManagerException extends X509Exception {
// ... add necessary calls
}
public static class TrustManagerException extends X509Exception {
// ... add necessary calls
}
public static class SSLContextException extends X509Exception {
// ... add necessary calls
}
- Rakesh
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review75525
---
On March 7, 2015, 1:04 a.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated March 7, 2015, 1:04 a.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
202051f1f7f517b1e1a3c561c0008449ab3c48a6
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
a97be4a5452006fbd85d355c0dcb16276cbf1c59
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/ServerConfig.java
f2b8463e871739319bdf40be1f014d5ad0af5602
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
30a0ed390bb7473ddb36757da97bc7d5f4281887
src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
0eb5c7f979199f2f7dcb9e5cfa011a9b20113713
src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
b756d349abeb1fc69534100c3633db4c1c18e031
src/java/main/org/apache/zookeeper/server/quorum/Leader.java
20589045752a7ba4ae9c9090055a4fcbe86a8eda
src/java/main/org/apache/zookeeper/server/quorum/Learner.java
4dd1e947357080f3e055f3e7e2a78c979daa6ea7
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
badc8df1f05dea4be337bc8312d7ac22f6c77dc3
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
d17c58d59e0131a78adde1becb5c23ce8c7a16a7
src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java
2aab6d09f9bd980ed76f886fb8168aae2ac8f99f
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
6ab19b1eb137c8b13b8ad031d474e213267da1ea
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
6ce058e48d17410d89d8348ee659dd7752bfd578
src/java/test/org/apache/zookeeper/test/ReconfigTest.java
8b238ee7463508122010208ebc3e786caa2cf1b1
src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31277/diff/
Testing
---
Thanks,
Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75616 --- src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java https://reviews.apache.org/r/31277/#comment122819 Can we extract this to a method to avoid duplication - Rakesh R On March 7, 2015, 1:04 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 7, 2015, 5:24 a.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 493 https://reviews.apache.org/r/31277/diff/14/?file=888101#file888101line493 Can we extract this to a method to avoid duplication adding few more to the above comment, : both cf anf scf logic looks same, so we can extract to a closeSession method and pass cf argument. on return will check success then return else do scf. - Rakesh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75616 --- On March 7, 2015, 1:04 a.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Exception.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 5, 2015, 10:37 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 12:17 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/data/ssl/README.md PRE-CREATION src/java/test/data/ssl/testKeyStore.jks PRE-CREATION src/java/test/data/ssl/testTrustStore.jks PRE-CREATION src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75084 --- Ship it! Ship It! - Raul Gutierrez Segales On March 2, 2015, 11:22 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 11:22 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74846 --- src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment121635 nit: boolean secure src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment121636 nit: why the line wrap? src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment121637 nit: secure src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java https://reviews.apache.org/r/31277/#comment121638 will support or can support? can both work at the same time or are they exclusive? src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java https://reviews.apache.org/r/31277/#comment121641 this makes me think, should we expose via the mntr and conf commands if secure is enabled? src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java https://reviews.apache.org/r/31277/#comment121642 i think you are changing the behavior here.. before we'd throw IllegalArgumentException if clientPortAddress != null clientPort == 0.. not anymore src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java https://reviews.apache.org/r/31277/#comment121643 typo: secure src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java https://reviews.apache.org/r/31277/#comment121644 nit: boolean secure src/java/test/org/apache/zookeeper/test/ReconfigTest.java https://reviews.apache.org/r/31277/#comment121645 no need for line-wrap src/java/test/org/apache/zookeeper/test/SSLTest.java https://reviews.apache.org/r/31277/#comment121648 coding style: imports from java's stdlib should go first, then the apache/junit ones - Raul Gutierrez Segales On March 2, 2015, 6:44 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 6:44 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 11:22 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On March 2, 2015, 11:04 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, line 311 https://reviews.apache.org/r/31277/diff/8/?file=882265#file882265line311 i think you are changing the behavior here.. before we'd throw IllegalArgumentException if clientPortAddress != null clientPort == 0.. not anymore Good catch. On March 2, 2015, 11:04 p.m., Raul Gutierrez Segales wrote: src/java/test/org/apache/zookeeper/test/SSLTest.java, line 38 https://reviews.apache.org/r/31277/diff/8/?file=882271#file882271line38 coding style: imports from java's stdlib should go first, then the apache/junit ones My IDE does the opposite. I corrected it manually. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74846 --- On March 2, 2015, 6:44 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 6:44 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1463
https://reviews.apache.org/r/31277/diff/5/?file=875863#file875863line1463
when should this be done? why is this needed? i fear this is even more
coupling between QuorumPeer, ZooKeeperServer(s) and the ServerCnxnFactory...
Hongchao Deng wrote:
That's a really good question. The reason for setZooKeeperServer,
closeAllConnections is that Leader, Learner, ROZKServer all do the duplicate
work.
{code}
if (cnxnFactory != null) {
...
}
if (secureCnxnFactory != null) {
...
}
{code}
Regarding coupling, those three classes are already calling each others.
I tried to refactor it but then I realized it's too many unrelated changes...
What's your suggestion here?
Raul Gutierrez Segales wrote:
No immediate suggestions, I need to check QuorumPeer.java again. Lets
cleanup the other more obvious parts and revisit this issue in a next pass.
Hi Raul, any idea on this refactoring issue?
- Hongchao
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review74038
---
On Feb. 26, 2015, 7:34 p.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated Feb. 26, 2015, 7:34 p.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/ServerConfig.java
f2b8463e871739319bdf40be1f014d5ad0af5602
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
30a0ed390bb7473ddb36757da97bc7d5f4281887
src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
0eb5c7f979199f2f7dcb9e5cfa011a9b20113713
src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
b756d349abeb1fc69534100c3633db4c1c18e031
src/java/main/org/apache/zookeeper/server/quorum/Leader.java
20589045752a7ba4ae9c9090055a4fcbe86a8eda
src/java/main/org/apache/zookeeper/server/quorum/Learner.java
4dd1e947357080f3e055f3e7e2a78c979daa6ea7
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
badc8df1f05dea4be337bc8312d7ac22f6c77dc3
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
d17c58d59e0131a78adde1becb5c23ce8c7a16a7
src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java
2aab6d09f9bd980ed76f886fb8168aae2ac8f99f
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
6ab19b1eb137c8b13b8ad031d474e213267da1ea
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
6ce058e48d17410d89d8348ee659dd7752bfd578
src/java/test/org/apache/zookeeper/test/ReconfigTest.java
8b238ee7463508122010208ebc3e786caa2cf1b1
src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31277/diff/
Testing
---
Thanks,
Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review74766
---
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
https://reviews.apache.org/r/31277/#comment121496
synchronized?
src/java/main/org/apache/zookeeper/ZooKeeper.java
https://reviews.apache.org/r/31277/#comment121494
i think we can just call this:
```
SECURE_CLIENT
```
i know some static fields in this class have the ZOOKEEPER prefix, but we
can probably drop it. this reads clearly enough:
```
ZooKeeper.SECURE_CLIENT
```
src/java/main/org/apache/zookeeper/common/X509Error.java
https://reviews.apache.org/r/31277/#comment121492
i like these exceptions! elegant and well encapsulated!
src/java/main/org/apache/zookeeper/common/X509Util.java
https://reviews.apache.org/r/31277/#comment121498
i'd drop ZOOKEEPER from these constants (not from the properties, but from
the class constants). i.e.;
```
public static final String SSL_KEYSTORE_LOCATION =
zookeeper.ssl.keyStore.location;
...
```
is as clear, but also much less verbose and repetitive.
src/java/main/org/apache/zookeeper/common/X509Util.java
https://reviews.apache.org/r/31277/#comment121505
But if a user specifies one, location and password are required.
src/java/main/org/apache/zookeeper/common/X509Util.java
https://reviews.apache.org/r/31277/#comment121506
do we care? i think this can just be:
```java
} catch (IOException e) {}
```
src/java/main/org/apache/zookeeper/common/X509Util.java
https://reviews.apache.org/r/31277/#comment121507
ditto
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121508
nit: simpler:
```java
public void configure(InetSocketAddress addr, maxcc, boolean secure) {
}
```
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121509
nit: no need for line wrap
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121510
nit: simpler:
```java
public void startup(ZooKeeperServer zks, boolean startServer) {
}
```
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121511
why Exception? can't we throw something a SSL specific exception?
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121513
nit: SSL handler added
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121514
boolean secure
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121515
this.secure
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121516
boolean startServer
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121517
startServer
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121518
boolean secure
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121519
int maxcc
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
https://reviews.apache.org/r/31277/#comment121520
int maxcc, boolean secure
- Raul Gutierrez Segales
On Feb. 26, 2015, 7:34 p.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated Feb. 26, 2015, 7:34 p.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 6:44 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report that SSL setup failed and go on without SSL? Hongchao Deng wrote: I have similar concerns too when SSL parameters are given wrong. I have in this JIRA intentionally kept this simple and not included failure cases in testing. Let's discuss more and fix it in another JIRA. Raul Gutierrez Segales wrote: I think that catching the exceptions from sslEngine and logging them (and then keep going on) is heavily desired as opposed to failing. I don't want this change to break things for people by accident (even if they need to explicitly turn this feature on, still we should cope with failures). Hongchao Deng wrote: OK. Will fix it. Hongchao Deng wrote: Hi Raul, 1. I haven't figured out a way to do failover. The SslHandler is added to the pipeline on both side. Not doing ssl on either side wouldn't work. 2. Is this needed? If the client failed on a secure connection, it can unset zookeeper.client.secure and connect to server's clientPort (which isn't secure). For the second point, I think some sort of notification might be needed. I haven't figured out either.. Please help out with any suggestion here. Made agreement with Raul that throwing the exception and logging are good. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- On Feb. 26, 2015, 7:34 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 26, 2015, 7:34 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report that SSL setup failed and go on without SSL? Hongchao Deng wrote: I have similar concerns too when SSL parameters are given wrong. I have in this JIRA intentionally kept this simple and not included failure cases in testing. Let's discuss more and fix it in another JIRA. Raul Gutierrez Segales wrote: I think that catching the exceptions from sslEngine and logging them (and then keep going on) is heavily desired as opposed to failing. I don't want this change to break things for people by accident (even if they need to explicitly turn this feature on, still we should cope with failures). Hongchao Deng wrote: OK. Will fix it. Hi Raul, 1. I haven't figured out a way to do failover. The SslHandler is added to the pipeline on both side. Not doing ssl on either side wouldn't work. 2. Is this needed? If the client failed on a secure connection, it can unset zookeeper.client.secure and connect to server's clientPort (which isn't secure). For the second point, I think some sort of notification might be needed. I haven't figured out either.. Please help out with any suggestion here. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- On Feb. 26, 2015, 7:34 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 26, 2015, 7:34 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 26, 2015, 7:34 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 5:35 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SslTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ZooKeeper.java, line 134 https://reviews.apache.org/r/31277/diff/5/?file=875852#file875852line134 nit: if you grep around for other properties, the convention seems to be: zookeeper.component.property. so maybe: zookeeper.client.enableSecure ? changed to zookeeper.client.secure - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- On Feb. 25, 2015, 5:35 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 5:35 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SslTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 7:18 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/common/X509Util.java, line 57
https://reviews.apache.org/r/31277/diff/5/?file=875853#file875853line57
why throw all of these? i think we should handle them and just throw
one (probably custom) exception (i.e.: FailedKeyManager or such). No need
to leak all these details.
Sounds good to me. Will fix it soon. Can you give more details what it should
look like?
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java, line
279
https://reviews.apache.org/r/31277/diff/5/?file=875855#file875855line279
nit: initSSL()
also, throws Exception is to broad. we should catch the specific ssl
exceptions and just throw a custom one (i.e.: InitFailedSSL) (to make this
code future-proof when we change the underlying ssl libs, etc).
Sure. Will fix it.
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353
https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353
so if any of this fails, we just give up and throw? why not catch it,
report that SSL setup failed and go on without SSL?
I have similar concerns too when SSL parameters are given wrong. I have in this
JIRA intentionally kept this simple and not included failure cases in testing.
Let's discuss more and fix it in another JIRA.
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1463
https://reviews.apache.org/r/31277/diff/5/?file=875863#file875863line1463
when should this be done? why is this needed? i fear this is even more
coupling between QuorumPeer, ZooKeeperServer(s) and the ServerCnxnFactory...
That's a really good question. The reason for setZooKeeperServer,
closeAllConnections is that Leader, Learner, ROZKServer all do the duplicate
work.
{code}
if (cnxnFactory != null) {
...
}
if (secureCnxnFactory != null) {
...
}
{code}
Regarding coupling, those three classes are already calling each others. I
tried to refactor it but then I realized it's too many unrelated changes...
What's your suggestion here?
- Hongchao
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review74038
---
On Feb. 25, 2015, 7:18 p.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated Feb. 25, 2015, 7:18 p.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/ServerConfig.java
f2b8463e871739319bdf40be1f014d5ad0af5602
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
30a0ed390bb7473ddb36757da97bc7d5f4281887
src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
0eb5c7f979199f2f7dcb9e5cfa011a9b20113713
src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
b756d349abeb1fc69534100c3633db4c1c18e031
src/java/main/org/apache/zookeeper/server/quorum/Leader.java
20589045752a7ba4ae9c9090055a4fcbe86a8eda
src/java/main/org/apache/zookeeper/server/quorum/Learner.java
4dd1e947357080f3e055f3e7e2a78c979daa6ea7
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
badc8df1f05dea4be337bc8312d7ac22f6c77dc3
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
d17c58d59e0131a78adde1becb5c23ce8c7a16a7
src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java
2aab6d09f9bd980ed76f886fb8168aae2ac8f99f
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
6ab19b1eb137c8b13b8ad031d474e213267da1ea
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java https://reviews.apache.org/r/31277/#comment120503 just use Boolean.getBoolean(), no need for System.getProperty. src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java https://reviews.apache.org/r/31277/#comment120505 so if any of this fails, we just give up and throw? why not catch it, report that SSL setup failed and go on without SSL? src/java/main/org/apache/zookeeper/ZooKeeper.java https://reviews.apache.org/r/31277/#comment120507 nit: if you grep around for other properties, the convention seems to be: zookeeper.component.property. so maybe: zookeeper.client.enableSecure ? src/java/main/org/apache/zookeeper/common/X509Util.java https://reviews.apache.org/r/31277/#comment120508 nit: no need to say it's complicated :-) maybe just: /* * Utility code for X509 handling */ src/java/main/org/apache/zookeeper/common/X509Util.java https://reviews.apache.org/r/31277/#comment120511 why the line wrap, this is 80 cols. src/java/main/org/apache/zookeeper/common/X509Util.java https://reviews.apache.org/r/31277/#comment120512 why throw all of these? i think we should handle them and just throw one (probably custom) exception (i.e.: FailedKeyManager or such). No need to leak all these details. src/java/main/org/apache/zookeeper/common/X509Util.java https://reviews.apache.org/r/31277/#comment120514 ditto about bubbling up specific exceptions src/java/main/org/apache/zookeeper/common/X509Util.java https://reviews.apache.org/r/31277/#comment120515 ditto src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment120517 nit: initSSL() also, throws Exception is to broad. we should catch the specific ssl exceptions and just throw a custom one (i.e.: InitFailedSSL) (to make this code future-proof when we change the underlying ssl libs, etc). src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java https://reviews.apache.org/r/31277/#comment120519 lets be explicit about public/private for readability src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java https://reviews.apache.org/r/31277/#comment120520 why the line wrap? it's a short line src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java https://reviews.apache.org/r/31277/#comment120522 should it not be the sum of secure + non-secure? src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java https://reviews.apache.org/r/31277/#comment120538 when should this be done? why is this needed? i fear this is even more coupling between QuorumPeer, ZooKeeperServer(s) and the ServerCnxnFactory... src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java https://reviews.apache.org/r/31277/#comment120540 (a bit of nit): simplify this by assigning the intermediate strings, otherwise it's a bit confusing with the two ternaries. src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java https://reviews.apache.org/r/31277/#comment120542 tis could be LOG.info src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java https://reviews.apache.org/r/31277/#comment120543 just TIMEOUT? src/java/test/org/apache/zookeeper/test/SslTest.java https://reviews.apache.org/r/31277/#comment120544 nit: SSLTest src/java/test/org/apache/zookeeper/test/SslTest.java https://reviews.apache.org/r/31277/#comment120545 why the line wrap? the following lines are even longer src/java/test/org/apache/zookeeper/test/SslTest.java https://reviews.apache.org/r/31277/#comment120546 nit: use String.format(), concatenation is harder to read/maintain - Raul Gutierrez Segales On Feb. 25, 2015, 5:35 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 5:35 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353
https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353
so if any of this fails, we just give up and throw? why not catch it,
report that SSL setup failed and go on without SSL?
Hongchao Deng wrote:
I have similar concerns too when SSL parameters are given wrong. I have
in this JIRA intentionally kept this simple and not included failure cases in
testing. Let's discuss more and fix it in another JIRA.
I think that catching the exceptions from sslEngine and logging them (and then
keep going on) is heavily desired as opposed to failing. I don't want this
change to break things for people by accident (even if they need to explicitly
turn this feature on, still we should cope with failures).
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/common/X509Util.java, line 57
https://reviews.apache.org/r/31277/diff/5/?file=875853#file875853line57
why throw all of these? i think we should handle them and just throw
one (probably custom) exception (i.e.: FailedKeyManager or such). No need
to leak all these details.
Hongchao Deng wrote:
Sounds good to me. Will fix it soon. Can you give more details what it
should look like?
how about something like:
```java
class KeyManagerError extends Exception {}
public static X509KeyManager createKeyManager(String keyStoreLocation, String
keyStorePassword)
throws KeyManagerError {
// if something goes wrong
throw new KeyManagerError(desc);
}
```
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote:
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1463
https://reviews.apache.org/r/31277/diff/5/?file=875863#file875863line1463
when should this be done? why is this needed? i fear this is even more
coupling between QuorumPeer, ZooKeeperServer(s) and the ServerCnxnFactory...
Hongchao Deng wrote:
That's a really good question. The reason for setZooKeeperServer,
closeAllConnections is that Leader, Learner, ROZKServer all do the duplicate
work.
{code}
if (cnxnFactory != null) {
...
}
if (secureCnxnFactory != null) {
...
}
{code}
Regarding coupling, those three classes are already calling each others.
I tried to refactor it but then I realized it's too many unrelated changes...
What's your suggestion here?
No immediate suggestions, I need to check QuorumPeer.java again. Lets cleanup
the other more obvious parts and revisit this issue in a next pass.
- Raul
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/#review74038
---
On Feb. 25, 2015, 7:18 p.m., Hongchao Deng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31277/
---
(Updated Feb. 25, 2015, 7:18 p.m.)
Review request for zookeeper.
Repository: zookeeper-git
Description
---
ZOOKEEPER-2125: SSL on Netty client-server communication
Diffs
-
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION
src/java/main/org/apache/zookeeper/ZooKeeper.java
dd13cc9ba5096312b06999a03ae0057cd3677623
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
acabb33f6c7a000706763ccba94cbaf5aaaca08e
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
41268805fe16244aeea4db3f35f13a6987b30187
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/ServerConfig.java
f2b8463e871739319bdf40be1f014d5ad0af5602
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
30a0ed390bb7473ddb36757da97bc7d5f4281887
src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
0eb5c7f979199f2f7dcb9e5cfa011a9b20113713
src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
b756d349abeb1fc69534100c3633db4c1c18e031
src/java/main/org/apache/zookeeper/server/quorum/Leader.java
20589045752a7ba4ae9c9090055a4fcbe86a8eda
src/java/main/org/apache/zookeeper/server/quorum/Learner.java
4dd1e947357080f3e055f3e7e2a78c979daa6ea7
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
badc8df1f05dea4be337bc8312d7ac22f6c77dc3
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report that SSL setup failed and go on without SSL? Hongchao Deng wrote: I have similar concerns too when SSL parameters are given wrong. I have in this JIRA intentionally kept this simple and not included failure cases in testing. Let's discuss more and fix it in another JIRA. Raul Gutierrez Segales wrote: I think that catching the exceptions from sslEngine and logging them (and then keep going on) is heavily desired as opposed to failing. I don't want this change to break things for people by accident (even if they need to explicitly turn this feature on, still we should cope with failures). OK. Will fix it. - Hongchao --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- On Feb. 25, 2015, 7:18 p.m., Hongchao Deng wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 7:18 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 6:21 a.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocket.java 5ca0ba77bccfcaa799e42e98f5a5f04d3f0cf0d3 src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/TestableZooKeeper.java 4d46fdf738a7cdd07335546307599d4d6ff9c76b src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 6ab19b1eb137c8b13b8ad031d474e213267da1ea src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 src/java/test/org/apache/zookeeper/test/SslTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 22, 2015, 7:24 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java badc8df1f05dea4be337bc8312d7ac22f6c77dc3 src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java d17c58d59e0131a78adde1becb5c23ce8c7a16a7 src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 6ce058e48d17410d89d8348ee659dd7752bfd578 src/java/test/org/apache/zookeeper/test/ReconfigTest.java 8b238ee7463508122010208ebc3e786caa2cf1b1 Diff: https://reviews.apache.org/r/31277/diff/ Testing --- Thanks, Hongchao Deng
Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 22, 2015, 7:21 p.m.) Review request for zookeeper. Repository: zookeeper-git Description --- ZOOKEEPER-2125: SSL on Netty client-server communication Diffs (updated) - CHANGES.txt bc587349866843af3fe43a70cd3c674338495926 build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c src/docs/src/documentation/content/xdocs/recipes.xml f977a543be9b8a7bb9529ebc102b8a04980acac7 src/docs/src/documentation/content/xdocs/releasenotes.xml 41f079450f3f1067c12344daf9a869a3607ce8a6 src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 202051f1f7f517b1e1a3c561c0008449ab3c48a6 src/docs/src/documentation/content/xdocs/zookeeperInternals.xml 4954123387ae69c19d2592208b663388120e445a src/docs/src/documentation/content/xdocs/zookeeperObservers.xml 99f80250438699c9568d651f16afa4a7fee31b13 src/docs/src/documentation/content/xdocs/zookeeperOver.xml a2961d334d38e9752be5d7ade6eedb8c9f5fa669 src/docs/src/documentation/content/xdocs/zookeeperTutorial.xml 10b3606c842e1bd9cbcf626bfdf1ccb8c4d1ad49 src/java/main/org/apache/zookeeper/ClientCnxn.java b4ece07735d13f1114dc79add6d5f306f6916988 src/java/main/org/apache/zookeeper/ClientCnxnSocket.java 5ca0ba77bccfcaa799e42e98f5a5f04d3f0cf0d3 src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java adb27ee843cd7ba310807dd777376a3c244ca30f src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION src/java/main/org/apache/zookeeper/Login.java 6d248ab37a0a6b11358f5f3adc9dc363b1a9c73b src/java/main/org/apache/zookeeper/Shell.java 62169d797a7a103d921634c4676fffea878def51 src/java/main/org/apache/zookeeper/ZKUtil.java 4713a08a934175c2b297f69740e204c7288c078c src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9ba5096312b06999a03ae0057cd3677623 src/java/main/org/apache/zookeeper/ZooKeeperMain.java 496e88748cf6aa291c8b71583a28fdb55fdf7761 src/java/main/org/apache/zookeeper/ZooKeeperTestable.java 775d1a26d881ec77eb403fc3268f14f6be26719f src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java dbc108019493ce13cc76a0965c4a3b2cf08353d5 src/java/main/org/apache/zookeeper/common/Time.java PRE-CREATION src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION src/java/main/org/apache/zookeeper/server/ConnectionBean.java 917aacfdcdcd50576029faab65ca98b89cfb2df9 src/java/main/org/apache/zookeeper/server/ExpiryQueue.java a037bf49235e386cc20ee68633ec162b1db013d1 src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java a97be4a5452006fbd85d355c0dcb16276cbf1c59 src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java acabb33f6c7a000706763ccba94cbaf5aaaca08e src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 41268805fe16244aeea4db3f35f13a6987b30187 src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 7a03b4b4f4f7b60fd16eca60423202006ad5833d src/java/main/org/apache/zookeeper/server/RateLogger.java fc951cf5147bedbf1786ff1047a1e1a5fd7f5121 src/java/main/org/apache/zookeeper/server/Request.java ee01dcfa63784a9dd380f91d768e1b3f28b9cce9 src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c src/java/main/org/apache/zookeeper/server/ServerConfig.java f2b8463e871739319bdf40be1f014d5ad0af5602 src/java/main/org/apache/zookeeper/server/ServerStats.java c3246293e409d863412144ed76b2a91ca1ac98f2 src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 0c2c042e276c557a86f47d7ab5333e6860e12bd9 src/java/main/org/apache/zookeeper/server/WorkerService.java c55ff48f92e5e3ae7783ad5be0262a5d9899c521 src/java/main/org/apache/zookeeper/server/ZKDatabase.java f336049f0afb7b539460223b4903d323e2558aed src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 30a0ed390bb7473ddb36757da97bc7d5f4281887 src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java b756d349abeb1fc69534100c3633db4c1c18e031 src/java/main/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java 6cd0af88292d9cb89652f1c6d2a80ec2726b5b6a src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java dfe692f4889a11b8a8eb3a4cbbd150ed5cac6a9f src/java/main/org/apache/zookeeper/server/quorum/Follower.java 6dbb0b22a4e0658a6b04629e6efdf1ac722375e5 src/java/main/org/apache/zookeeper/server/quorum/Leader.java 20589045752a7ba4ae9c9090055a4fcbe86a8eda src/java/main/org/apache/zookeeper/server/quorum/Learner.java 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 src/java/main/org/apache/zookeeper/server/quorum/LearnerSnapshotThrottler.java 97b48915321aab6ea31bd7db8fe1197165507feb
