[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194233639 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java --- @@ -292,8 +298,20 @@ protected void connectToLeader(InetSocketAddress addr

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234222 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234308 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234667 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/UnifiedServerSocket.java --- @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194233492 --- Diff: src/java/main/org/apache/zookeeper/common/ZKTrustManager.java --- @@ -0,0 +1,144 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194233692 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java --- @@ -632,37 +639,46 @@ synchronized private boolean connectOne(long

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r148900246 --- Diff: src/java/main/org/apache/zookeeper/common/ClientX509Util.java --- @@ -0,0 +1,38 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234634 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/PrependableSocket.java --- @@ -0,0 +1,49 @@ +/** + * Licensed to the Apache Software

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r148901017 --- Diff: src/java/main/org/apache/zookeeper/common/ZKTrustManager.java --- @@ -0,0 +1,144 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234529 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -160,43 +214,120 @@ public static X509KeyManager createKeyManager(String

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r148901325 --- Diff: src/java/main/org/apache/zookeeper/common/ZKTrustManager.java --- @@ -0,0 +1,144 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194234574 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Leader.java --- @@ -227,19 +229,36 @@ public boolean isQuorumSynced(QuorumVerifier qv

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-09 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r148901276 --- Diff: src/java/main/org/apache/zookeeper/common/ZKTrustManager.java --- @@ -0,0 +1,144 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-15 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195891451 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Leader.java --- @@ -227,19 +229,36 @@ public boolean isQuorumSynced(QuorumVerifier qv

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-15 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195891467 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Leader.java --- @@ -227,19 +229,36 @@ public boolean isQuorumSynced(QuorumVerifier qv

[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-06-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/184 @anmolnar Most of the ssl-related config options start with `ssl.quorum`, but this isn't true of `sslQuorum` and `portUnification`. What do you think about renaming them. I'm open to suggestions

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195514675 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -339,4 +351,20 @@ private void configureSSLServerSocket(SSLServerSocket

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195517105 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -79,7 +91,7 @@ public X509Util() { String

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195517786 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195517644 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195517918 --- Diff: src/java/main/org/apache/zookeeper/common/ZKConfig.java --- @@ -107,14 +99,33 @@ private void init() { * this configuration

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195516340 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -79,7 +91,7 @@ public X509Util() { String

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195586921 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -79,7 +91,7 @@ public X509Util() { String

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195587480 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/UnifiedServerSocket.java --- @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-14 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r195587293 --- Diff: src/java/main/org/apache/zookeeper/common/ZKConfig.java --- @@ -107,14 +99,33 @@ private void init() { * this configuration

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194539816 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194437578 --- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java --- @@ -292,8 +298,20 @@ protected void connectToLeader(InetSocketAddress addr

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194438427 --- Diff: src/java/main/org/apache/zookeeper/common/ZKTrustManager.java --- @@ -0,0 +1,144 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194442041 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194460371 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194460585 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -18,64 +18,119 @@ package org.apache.zookeeper.common

[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-06-11 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/184#discussion_r194461322 --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java --- @@ -160,43 +214,120 @@ public static X509KeyManager createKeyManager(String

[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-06-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/184 @anmolnar thanks for the hard work! Our plan is to run this on a real cluster for about a month, if everything is working well, that will be a pretty good argument for "this code is

[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-06-21 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/184 @anmolnar actually I think the UnifiedSocket approach is fine for now, it works. It can be cleaned up to use the java 8 socket API in a separate pull request later. ---

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 @dain take a look at the changes I made to airlift's test ByteBuf allocator. With these changes, we (sometimes) get leak details printed to stderr if a ByteBuf leaks, before the test crashes

[GitHub] zookeeper issue #668: ZOOKEEPER-3165. Java 9: X509UtilTest.testCreateSSLCont...

2018-10-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/668 Your fix looks fine to me. It's fixed in my stack of FB internal diffs too (#627 for the time being), where I am explicit about JKS or PEM format. Didn't realize that Java 9 supports PKCS12 key

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-15 Thread ivmaykov
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/669 ZOOKEEPER-3152: Port ZK netty stack to netty4 Summary: Ported the client connection netty stack from netty3 to netty4. This includes both the server side (NettyServerCnxn and friends

[GitHub] zookeeper issue #627: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-10-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/627 @anmolnar it doesn't look like #184 is in master branch yet. Do you have any ETA on that? ---

[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-10-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/184 @anmolnar any ETA on merging this to master? I'd like to break up #627 into several parts with associated JIRAs and start the process on getting that code upstream, but need this code in master

[GitHub] zookeeper issue #627: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-10-15 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/627 I'm creating separate JIRAs for the different issues fixed in this PR. I will break it up into multiple PRs, one for each JIRA. Fix port unification: https://jira.apache.org/jira/browse

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-22 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 @eolivelli use `voidPromise()` to avoid allocations when writing to channel ---

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-22 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 Cleaner Epoll/Nio selection code ---

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-22 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 @eolivelli changes you requested: - use Epoll if available - if (LOG.isDebugEnabled()) around complex LOG.debug() statements - use netty-all artifact Haven't looked

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-20 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226845944 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java --- @@ -116,170 +115,94 @@ public void

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226757190 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java --- @@ -116,170 +115,94 @@ public void

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 @eolivelli thanks so much for the review! See my responses inline. ---

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226757240 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java --- @@ -335,29 +260,34 @@ public void

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226757057 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java --- @@ -116,170 +115,94 @@ public void

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226756739 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java --- @@ -200,24 +186,13 @@ public void setSessionId(long

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226755668 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -184,7 +213,9 @@ void cleanup

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226755419 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -103,71 +108,95 @@ boolean isConnected

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226755285 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -68,18 +70,21 @@ public class

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226756565 --- Diff: zookeeper-common/src/test/java/org/apache/zookeeper/common/TestByteBufAllocator.java --- @@ -0,0 +1,151 @@ +/* + * Licensed

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226756052 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -267,7 +298,7 @@ private void sendPkt(Packet p

[GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-19 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r226756353 --- Diff: zookeeper-common/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -439,13 +466,34 @@ public void messageReceived

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-10-24 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 Fixed various issues in netty code ---

[GitHub] zookeeper issue #627: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...

2018-10-24 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/627 I separated this into 4 PRs: #678, #679, #680, and #681. Closing this PR since it's obsolete now. ---

[GitHub] zookeeper pull request #627: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

2018-10-24 Thread ivmaykov
Github user ivmaykov closed the pull request at: https://github.com/apache/zookeeper/pull/627 ---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-24 Thread ivmaykov
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/678 ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores Add support for loading key and trust stores from PEM files. Unfortunately, this PR includes 2 JIRAs, because it was quite

[GitHub] zookeeper pull request #679: ZOOKEEPER-3172: Quorum TLS - fix port unificati...

2018-10-24 Thread ivmaykov
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/679 ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades Fix numerous problems with UnifiedServerSocket, such as hanging the accept() thread when the client doesn't send any

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-10-24 Thread ivmaykov
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/680 ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store Allow reloading SSL trust stores and key stores from disk when the files on disk change. Note that this is stacked on top

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

2018-10-24 Thread ivmaykov
GitHub user ivmaykov opened a pull request: https://github.com/apache/zookeeper/pull/681 ZOOKEEPER-3176: Quorum TLS - add SSL config options Add SSL config options for enabled protocols and client auth mode. Improve handling of SSL config options for protocols and cipher suites

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-28 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r228764349 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-28 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r228762467 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-29 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r229102998 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -167,47 +222,50 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-29 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar added KeyStoreLoader classes @eolivelli updated NOTICE file and PemReader.java ---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-28 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r228806955 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper issue #669: ZOOKEEPER-3152: Port ZK netty stack to netty4

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/669 @normanmaurer a review from you would be very much appreciated! I bought your book (Netty in Action) which helped me quite a bit :) This is the version of the code we've been testing on a real

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli what does a "(non binding)" +1 mean? Can we merge this to upstream/master at this point? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 Revert `FileNameUtils` change as it seems to be breaking contbuild ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar it's certainly easier to keep track of the copied code if we don't modify it much. Moving the logic into PEMFileLoader and making it non-static would make it harder to trace

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli capitalized "Airlift" in NOTICE.txt @anmolnar use `FileNameUtils.getExtension()` for file type detection everyone: fixed some copy-paste bugs in PEMFile

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r229524910 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java --- @@ -0,0 +1,224 @@ +/* + * Licensed under the Apache

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-30 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 Refactored common code shared by a bunch of tests that use `X509TestContext` into a new base class, `BaseX509ParameterizedTestCase`. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-25 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 The test failures don't look to be related to my changes. @anmolnar @hanm is there a way to trigger a Jenkins build re-run? ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-26 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @eolivelli should the copied code be put into the zookeeper-contrib subproject? Or can it still live in zookeeper-server? I could also use help with wording the message in the NOTICE file

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r228719526 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -546,4 +557,109 @@ public static StoreFileType

[GitHub] zookeeper issue #679: ZOOKEEPER-3172: Quorum TLS - fix port unification to a...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/679 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/680 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/681 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

2018-10-27 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/680 I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks. ---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230523204 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,180 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230522968 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,180 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230523105 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,180 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230560912 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230560515 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230560444 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230560416 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230574766 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

2018-11-03 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/680#discussion_r230574754 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java --- @@ -0,0 +1,191 @@ +/** + * Licensed

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/678 @anmolnar on my machine it worked, but on Jenkins it could not resolve the import - not sure why. It's not a big deal I think, we don't need to use it. ---

[GitHub] zookeeper pull request #:

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the pull request: https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31143262 In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java: In zookeeper-server/src/main/java/org/apache

[GitHub] zookeeper pull request #:

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the pull request: https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31143267 In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java: In zookeeper-server/src/main/java/org/apache

[GitHub] zookeeper pull request #:

2018-11-01 Thread ivmaykov
Github user ivmaykov commented on the pull request: https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31143275 In zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java: In zookeeper-server/src/test

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r230455088 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java --- @@ -0,0 +1,98 @@ +/** + * Licensed

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r230454818 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r230421726 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java --- @@ -0,0 +1,98 @@ +/** + * Licensed

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/678#discussion_r230421926 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java --- @@ -221,27 +229,47 @@ public SSLContext createSSLContext(ZKConfig

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

2018-11-02 Thread ivmaykov
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/680 @eolivelli switched to lambdas, kept the finalizer in for now but added a TODO to remove it. ---

  1   2   3   >