You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Rakesh R <ra...@intel.com> on 2016/06/05 07:56:33 UTC
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated June 5, 2016, 7:56 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
New patch fixes Michael's pending review comments.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml ab254b2
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review136362
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Han
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 230
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406659#file1406659line230>
> >
> > why is this debug? throwable seems pretty heavy handed?
>
> Rakesh R wrote:
> Agreed, will change to error.
Done
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java, line 172
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406658#file1406658line172>
> >
> > should we say "client" here or "learner"? just wondering re usability/understandability (vs a "real" zk client)
>
> Rakesh R wrote:
> like mentioned in jira, do we need to modify 'QuorumClient' to 'QuorumLearner' along with this changes?
>
> Patrick Hunt wrote:
> I think that would make it easier to understand. If we can be consistent and also not confuse folks btw "real" clients and servers communicating with other servers.
Done. Modified all the occurances of 'quorum client' to 'quorum learner'
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 249
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406659#file1406659line249>
> >
> > again, client vs learner? or some other term (peer perhaps? - as long as it's consistent)
Done. Modified all the occurances of 'quorum client' to 'quorum learner'. Please review it again. Thanks!
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1435
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406660#file1406660line1435>
> >
> > It seems to me that this should be info and not warning? Why is it a warning given you're configuring it this way?
>
> Rakesh R wrote:
> Agreed, will change to info
Done
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1461
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406660#file1406660line1461>
> >
> > is 20 the right number? regardless it seems we have 20 embedded in a few places, can we make this a constant?
>
> Rakesh R wrote:
> I will make a constant and use the same everywhere now. Its not an ideal value, needs to find an equation for calculating default value.
Done. I created a default constant variable and used. "QUORUM_CNXN_THREADS_SIZE_DEFAULT_VALUE".
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java, line 125
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406669#file1406669line125>
> >
> > print the actual value for debugging purposes.
>
> Rakesh R wrote:
> Agreed.
Done
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line 71
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406665#file1406665line71>
> >
> > print the actual status for debugging purposes.
>
> Rakesh R wrote:
> Agreed.
Done
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/util/SecurityUtils.java, line 93
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406671#file1406671line93>
> >
> > it may not be wise to expose all of this. a different class but see ZOOKEEPER-2405 for some background. pull out something useful but not sensitive - e.g. a pricipal name or somesuch.
>
> Rakesh R wrote:
> Good point, I will take care this.
Done.
LOG.debug("Added private credential to client principal name: '{}',",
clientPrincipal);
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/util/SecurityUtils.java, line 218
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406671#file1406671line218>
> >
> > ditto re creds in the log
>
> Rakesh R wrote:
> Good point, I will take care this.
Done.
LOG.debug("Added private credential to service principal name: '{}',"
+ " GSSCredential name:{}", servicePrincipalName, cred.getName());
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137886
-----------------------------------------------------------
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 230
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406659#file1406659line230>
> >
> > why is this debug? throwable seems pretty heavy handed?
Agreed, will change to error.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1435
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406660#file1406660line1435>
> >
> > It seems to me that this should be info and not warning? Why is it a warning given you're configuring it this way?
Agreed, will change to info
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1461
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406660#file1406660line1461>
> >
> > is 20 the right number? regardless it seems we have 20 embedded in a few places, can we make this a constant?
I will make a constant and use the same everywhere now. Its not an ideal value, needs to find an equation for calculating default value.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line 71
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406665#file1406665line71>
> >
> > print the actual status for debugging purposes.
Agreed.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java, line 125
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406669#file1406669line125>
> >
> > print the actual value for debugging purposes.
Agreed.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/util/SecurityUtils.java, line 93
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406671#file1406671line93>
> >
> > it may not be wise to expose all of this. a different class but see ZOOKEEPER-2405 for some background. pull out something useful but not sensitive - e.g. a pricipal name or somesuch.
Good point, I will take care this.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/util/SecurityUtils.java, line 218
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406671#file1406671line218>
> >
> > ditto re creds in the log
Good point, I will take care this.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java, line 172
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406658#file1406658line172>
> >
> > should we say "client" here or "learner"? just wondering re usability/understandability (vs a "real" zk client)
like mentioned in jira, do we need to modify 'QuorumClient' to 'QuorumLearner' along with this changes?
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137886
-----------------------------------------------------------
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java, line 172
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406658#file1406658line172>
> >
> > should we say "client" here or "learner"? just wondering re usability/understandability (vs a "real" zk client)
>
> Rakesh R wrote:
> like mentioned in jira, do we need to modify 'QuorumClient' to 'QuorumLearner' along with this changes?
I think that would make it easier to understand. If we can be consistent and also not confuse folks btw "real" clients and servers communicating with other servers.
- Patrick
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137886
-----------------------------------------------------------
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137886
-----------------------------------------------------------
I reviewed all of the LOG messages, here are some related comments:
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java (line 172)
<https://reviews.apache.org/r/47354/#comment203099>
should we say "client" here or "learner"? just wondering re usability/understandability (vs a "real" zk client)
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 223)
<https://reviews.apache.org/r/47354/#comment203101>
why is this debug? throwable seems pretty heavy handed?
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 242)
<https://reviews.apache.org/r/47354/#comment203103>
again, client vs learner? or some other term (peer perhaps? - as long as it's consistent)
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 1433)
<https://reviews.apache.org/r/47354/#comment203107>
It seems to me that this should be info and not warning? Why is it a warning given you're configuring it this way?
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 1459)
<https://reviews.apache.org/r/47354/#comment203108>
is 20 the right number? regardless it seems we have 20 embedded in a few places, can we make this a constant?
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java (line 71)
<https://reviews.apache.org/r/47354/#comment203109>
print the actual status for debugging purposes.
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java (line 125)
<https://reviews.apache.org/r/47354/#comment203110>
print the actual value for debugging purposes.
src/java/main/org/apache/zookeeper/util/SecurityUtils.java (line 93)
<https://reviews.apache.org/r/47354/#comment203112>
it may not be wise to expose all of this. a different class but see ZOOKEEPER-2405 for some background. pull out something useful but not sensitive - e.g. a pricipal name or somesuch.
src/java/main/org/apache/zookeeper/util/SecurityUtils.java (line 218)
<https://reviews.apache.org/r/47354/#comment203113>
ditto re creds in the log
- Patrick Hunt
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 28, 2016, 8:49 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java, line 34
> > <https://reviews.apache.org/r/47354/diff/7/?file=1430358#file1430358line34>
> >
> > The return value in the function signature is void, and the comment is saying it should be boolean. So this should be fixed.
Removed return value from the javadoc
> On June 28, 2016, 8:49 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java, line 37
> > <https://reviews.apache.org/r/47354/diff/7/?file=1430359#file1430359line37>
> >
> > The return value in the function signature is void, and the comment is saying it should be boolean. So this should be fixed.
Removed return value from the javadoc
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review139854
-----------------------------------------------------------
On June 26, 2016, 9:13 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 26, 2016, 9:13 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review139854
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java (line 34)
<https://reviews.apache.org/r/47354/#comment205201>
The return value in the function signature is void, and the comment is saying it should be boolean. So this should be fixed.
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java (line 37)
<https://reviews.apache.org/r/47354/#comment205203>
The return value in the function signature is void, and the comment is saying it should be boolean. So this should be fixed.
- Michael Han
On June 26, 2016, 9:13 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 26, 2016, 9:13 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, line 77
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498971#file1498971line77>
> >
> > How about test if 'quorumRequireSasl' flag is set or not, and if it's not set, immediately return here? By doing that we can remove a couple of 'if (quorumRequireSasl) {' conditions in the subsequent code.
>
> Rakesh R wrote:
> Please refer QuorumAuthUpgradeTest.java, there is a step while upgrade. To support this, we need to keep the 'if (quorumRequireSasl)' inside another 'if (!QuorumAuth.nextPacketIsAuth(din))'. We have couple of test cases verifying this behavior. Please refer, QuorumAuthUpgradeTest#testRollingUpgrade, QuorumCnxManagerTest.java test cases. Please let me know if I miss any specific, I'm happy to include.
>
> QuorumAuthUpgradeTest.java
> * step-2) Stop the server one by one and change the flags and restart the server.
> * quorum.auth.enableSasl=true, quorum.auth.learnerRequireSasl=true and quorum.auth.serverRequireSasl=false
> * Ensure that all the servers should complete this step. Now, move to next step.
Make sense, thanks.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149705
-----------------------------------------------------------
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, line 174
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498970#file1498970line174>
> >
> > The implementation of this is an almost identical copy of ZooKeeperSaslClient.createSaslToken (minors one logging changes). I suggest we add a TODO here about consolidating the createSaslToken implementation between ZooKeeperSaslClient and here , so we can refactor the code after the patch landed.
>
> Rakesh R wrote:
> Agreed, I will add a TODO.
Done
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, line 131
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498970#file1498970line131>
> >
> > This 'if' condition can be removed, because when we can't reach here unless the auth process finished successfully, which means we don't hit any exceptions plus the sc.isComplete would evaluate to true (otherwise we would not get out of the while loop.).
>
> Rakesh R wrote:
> Good catch. Please refer https://docs.oracle.com/javase/7/docs/api/javax/security/sasl/SaslClient.html javadoc. In example, they have an additional if check after the looping. I think I need to include additional check for ERROR after the loop, right? like below,
>
> if (sc.isComplete()) {
> if(qpStatus == QuorumAuth.Status.SUCCESS)
> LOG.info("Successfully completed the authentication using SASL. server addr: {}, status: {}",
> sock.getRemoteSocketAddress(), qpStatus);
> else {
> throw new SaslException(
> "Authentication failed against server addr: "
> + sock.getRemoteSocketAddress() + ", qpStatus: " + qpStatus);
> }
> }
Done. Removed unused 'sc.isComplete()' check and checked the status code after the auth exchange.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149705
-----------------------------------------------------------
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, line 77
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498971#file1498971line77>
> >
> > How about test if 'quorumRequireSasl' flag is set or not, and if it's not set, immediately return here? By doing that we can remove a couple of 'if (quorumRequireSasl) {' conditions in the subsequent code.
Please refer QuorumAuthUpgradeTest.java, there is a step while upgrade. To support this, we need to keep the 'if (quorumRequireSasl)' inside another 'if (!QuorumAuth.nextPacketIsAuth(din))'. We have couple of test cases verifying this behavior. Please refer, QuorumAuthUpgradeTest#testRollingUpgrade, QuorumCnxManagerTest.java test cases. Please let me know if I miss any specific, I'm happy to include.
QuorumAuthUpgradeTest.java
* step-2) Stop the server one by one and change the flags and restart the server.
* quorum.auth.enableSasl=true, quorum.auth.learnerRequireSasl=true and quorum.auth.serverRequireSasl=false
* Ensure that all the servers should complete this step. Now, move to next step.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, line 174
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498970#file1498970line174>
> >
> > The implementation of this is an almost identical copy of ZooKeeperSaslClient.createSaslToken (minors one logging changes). I suggest we add a TODO here about consolidating the createSaslToken implementation between ZooKeeperSaslClient and here , so we can refactor the code after the patch landed.
Agreed, I will add a TODO.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, line 131
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498970#file1498970line131>
> >
> > This 'if' condition can be removed, because when we can't reach here unless the auth process finished successfully, which means we don't hit any exceptions plus the sc.isComplete would evaluate to true (otherwise we would not get out of the while loop.).
Good catch. Please refer https://docs.oracle.com/javase/7/docs/api/javax/security/sasl/SaslClient.html javadoc. In example, they have an additional if check after the looping. I think I need to include additional check for ERROR after the loop, right? like below,
if (sc.isComplete()) {
if(qpStatus == QuorumAuth.Status.SUCCESS)
LOG.info("Successfully completed the authentication using SASL. server addr: {}, status: {}",
sock.getRemoteSocketAddress(), qpStatus);
else {
throw new SaslException(
"Authentication failed against server addr: "
+ sock.getRemoteSocketAddress() + ", qpStatus: " + qpStatus);
}
}
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 106
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498961#file1498961line106>
> >
> > Should we move both ThreadPoolExecutor and inprogressConnections into QuorumCnxManager? Both are implementation details around managing connections, and QuorumPeer does not need to know about these details. Abstract such details seems exactly what QuorumCnxManager was designed for.
Agreed, I will change it.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149705
-----------------------------------------------------------
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 106
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498961#file1498961line106>
> >
> > Should we move both ThreadPoolExecutor and inprogressConnections into QuorumCnxManager? Both are implementation details around managing connections, and QuorumPeer does not need to know about these details. Abstract such details seems exactly what QuorumCnxManager was designed for.
>
> Rakesh R wrote:
> Agreed, I will change it.
Done
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149705
-----------------------------------------------------------
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149705
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 106)
<https://reviews.apache.org/r/47354/#comment217393>
Should we move both ThreadPoolExecutor and inprogressConnections into QuorumCnxManager? Both are implementation details around managing connections, and QuorumPeer does not need to know about these details. Abstract such details seems exactly what QuorumCnxManager was designed for.
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java (line 131)
<https://reviews.apache.org/r/47354/#comment217413>
This 'if' condition can be removed, because when we can't reach here unless the auth process finished successfully, which means we don't hit any exceptions plus the sc.isComplete would evaluate to true (otherwise we would not get out of the while loop.).
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java (line 174)
<https://reviews.apache.org/r/47354/#comment217411>
The implementation of this is an almost identical copy of ZooKeeperSaslClient.createSaslToken (minors one logging changes). I suggest we add a TODO here about consolidating the createSaslToken implementation between ZooKeeperSaslClient and here , so we can refactor the code after the patch landed.
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java (line 77)
<https://reviews.apache.org/r/47354/#comment217428>
How about test if 'quorumRequireSasl' flag is set or not, and if it's not set, immediately return here? By doing that we can remove a couple of 'if (quorumRequireSasl) {' conditions in the subsequent code.
- Michael Han
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
> On Sept. 19, 2016, 7:21 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 248
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498958#file1498958line248>
> >
> > This check seems redundant to me - when we are here we either already have a leader elected or we are asking a new leader election. Now during leader election the leader and the learners that participated should authenticate each other mutually (in QuorumCnxManager authServer.authenticate(sock, din), and authLearner.authenticate(sock)).
> >
> > Note that it is possible that the leader and this specific learner that asking to connect to leader has not been mutually verified during LE - because we don't have 1 to 1 connections between every pair of servers. This should be fine as the authentication / authz will be checked when the actual connection is being made, again in QuorumCnxManager.
Chat with Rakesh more about this - the check here is required because not all connections between peer are done through QuorumCnxManager - in particular in reconfig case a new peer joining quorum will first query who the leader is and once he gots the leader adddress he will attemp direct connect to leader (which by pass QuorumCnxManager). So this is not an issue, and existing code in patch is doing fine.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149501
-----------------------------------------------------------
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 19, 2016, 7:21 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 248
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498958#file1498958line248>
> >
> > This check seems redundant to me - when we are here we either already have a leader elected or we are asking a new leader election. Now during leader election the leader and the learners that participated should authenticate each other mutually (in QuorumCnxManager authServer.authenticate(sock, din), and authLearner.authenticate(sock)).
> >
> > Note that it is possible that the leader and this specific learner that asking to connect to leader has not been mutually verified during LE - because we don't have 1 to 1 connections between every pair of servers. This should be fine as the authentication / authz will be checked when the actual connection is being made, again in QuorumCnxManager.
>
> Michael Han wrote:
> Chat with Rakesh more about this - the check here is required because not all connections between peer are done through QuorumCnxManager - in particular in reconfig case a new peer joining quorum will first query who the leader is and once he gots the leader adddress he will attemp direct connect to leader (which by pass QuorumCnxManager). So this is not an issue, and existing code in patch is doing fine.
Done, I've provided separate paths for auth and non-auth(existing code) to give more confidence by not touching the existing code flow.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149501
-----------------------------------------------------------
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149501
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/Learner.java (line 246)
<https://reviews.apache.org/r/47354/#comment217150>
This check seems redundant to me - when we are here we either already have a leader elected or we are asking a new leader election. Now during leader election the leader and the learners that participated should authenticate each other mutually (in QuorumCnxManager authServer.authenticate(sock, din), and authLearner.authenticate(sock)).
Note that it is possible that the leader and this specific learner that asking to connect to leader has not been mutually verified during LE - because we don't have 1 to 1 connections between every pair of servers. This should be fine as the authentication / authz will be checked when the actual connection is being made, again in QuorumCnxManager.
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java (line 167)
<https://reviews.apache.org/r/47354/#comment217180>
Similarly here, should we remove the auth check and delegate the auth check in QuorumCnxManager instead?
- Michael Han
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 219
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line219>
> >
> > Should we put the check of in progress sid here, or up on the caller site (connectOne) who calls initateConnection? I am asking because I spotted there could be a potential socket leak if we put check here. Consider this case:
> >
> > * In connectOne(long sid), we first open a new socket and then call initiateConnection(sock, sid).
> > * In side initiateConnection(sock, sid), we are doing this sid check and will return if we find out there is an ongoing outgoing connections to the same sid.
> > * Now, the opened socket in connectOne is a strayed socket, and will not get used, or closed.
> >
> > If we put same check in connectOne, this problem can be fixed. Does this make sense for the case I am describing above?
>
> Rakesh R wrote:
> Thanks Michael for pointing out this. Yes, its a connection leak, I will do the necessary changes.
Done
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149566
-----------------------------------------------------------
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 166
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line166>
> >
> > I have a mixed feeling about this change - it makes QuorumCnxManager's constructor more complicated comparing to the previous constructor QuorumCnxManager(QuorumPeer), where we just pass a QuorumPeer object. That said though, with the new QuorumPeer.createCnxManager method, this is not a big problem, as a QuorumCnxManager can be conveniently created given a QuorumPeer object.
> >
> > Alternatively, we can keep QuorumCnxManager's constructor as it is, so it still accepts a QuorumPeer object and inside QuorumCnxManager's constructor, we initialize various fields required here (such as authServer/Learner, and so on.). This requires QuorumCnxManager be able to access QuorumPeer's protected / private fields, so it requires adding some getter methods on QuorumPeer to expose those properties.
> >
> > Overall I don't have a strong opinion one way or the other, wondering what other reviewers think about this.
>
> Rakesh R wrote:
> Thanks Michael for the imprv point. Initially I've implemented the same way you proposed. Later based on the discussions, one point has come up to reduce the dependency with QuorumPeer, so that we could unit test esaily the QuorumCnxManager class. Please refer QuorumCnxManagerTest.java test class. How about will keep this point open and see others response about this change and based on that will take this ahead. Does this makes sense?
Agreed.
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 328
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line328>
> >
> > Here we create new threads to handle both incoming connection request handling and out going connection initiation and set up. It makes sense to me as SASL auth evaluation might take a while - I am curious, Rakesh, do you have any rough idea / data on how long the auth would take? Would it take at the magnitude of million seconds, or seconds, or even minutes? My impression is that SASL evaluation between client and server should be done at the scale of seconds at most, as it purely involves two servers which sending small data between each other, and does not involve KDC (as Kerberos login should have already been finished when we reach here.).
> >
> > I am trying to justify the usage of threads here because we did not create threads to handle such connection requests, so this change is pretty big in terms of impact to existing codebase. Because we don't create threads, the old code base was executed purely sequentially. With threads, the semantic is slightly changed as the code after receiveConnection might be executing without receiveConnection actually finished. So far I have not spotted anywhere in code base with such sequential dependency, but want to raise the awareness of this and see what others think about this.
> >
> > Also, note that this thread code path is by default (and the only) code path for QuorumCnxManager connection handling, regardless if users enable server to server auth or not. I am wondering should we have another code path so users who don't enable server to server auth will get exactly the old behavior. In other words, when server to server auth is not enabled, we will be very confident that our change will not introduce any regressions.
>
> Rakesh R wrote:
> Yes, the SASL evaluation path may take at the magnitude of seconds. Again, this depends on the I/O resources(network speed etc).
> Its good point to maintain the existing code as it is and add the new code separately. I also have a feeling that folks are having concerns about the complexity of the code changes, may be this this approach will help to gain more confidence from all. Let me think way to seggregate this out.
Chat with Rakesh more about this, and we think use threading here is justified and neccessary. The key reason we need use threading is because the QuormCnxManager handing connections for a specific peer in a sequential way - so if a server is trying to connect to multiple peers, the connection will block until previous request is fininshed, unless we use threading to handle each connection in parallel.
We also agreed that it would be good to keep the previous single threaded connection handling code path, and users who don't want enable server to server auth will get exact same old behavior with the previous single thread connection code path. This is good for stability and compatibility, and keep a different code path is not too complicated to do. In future releases we can decide if we need to deprecated the single thread code path or not, depending on feedback from production and more testing involved in the new code path.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149566
-----------------------------------------------------------
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 117
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line117>
> >
> > This inprogressConnections is used to make sure we don't have duplicated auth session for outgoing connections, so it make sense to me.
> >
> > However, this set only applies to outgoing connections. For incoming connections, there is no corresponding structure to deduplicate. Do we need to also do similar book keeping on the receiver side (i.e. in handleConnections)?
I've added the 'inprogressConnections' datastructure to control the number of connection init attempts. There are two places it will initiate the connection, QuorumCnxManager#connectAll() and QuorumCnxManager#handleConnection().
But for the outgoing connections, they are channeled via Listener#run, I think there won't be any duplication, any thoughts?
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 166
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line166>
> >
> > I have a mixed feeling about this change - it makes QuorumCnxManager's constructor more complicated comparing to the previous constructor QuorumCnxManager(QuorumPeer), where we just pass a QuorumPeer object. That said though, with the new QuorumPeer.createCnxManager method, this is not a big problem, as a QuorumCnxManager can be conveniently created given a QuorumPeer object.
> >
> > Alternatively, we can keep QuorumCnxManager's constructor as it is, so it still accepts a QuorumPeer object and inside QuorumCnxManager's constructor, we initialize various fields required here (such as authServer/Learner, and so on.). This requires QuorumCnxManager be able to access QuorumPeer's protected / private fields, so it requires adding some getter methods on QuorumPeer to expose those properties.
> >
> > Overall I don't have a strong opinion one way or the other, wondering what other reviewers think about this.
Thanks Michael for the imprv point. Initially I've implemented the same way you proposed. Later based on the discussions, one point has come up to reduce the dependency with QuorumPeer, so that we could unit test esaily the QuorumCnxManager class. Please refer QuorumCnxManagerTest.java test class. How about will keep this point open and see others response about this change and based on that will take this ahead. Does this makes sense?
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 219
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line219>
> >
> > Should we put the check of in progress sid here, or up on the caller site (connectOne) who calls initateConnection? I am asking because I spotted there could be a potential socket leak if we put check here. Consider this case:
> >
> > * In connectOne(long sid), we first open a new socket and then call initiateConnection(sock, sid).
> > * In side initiateConnection(sock, sid), we are doing this sid check and will return if we find out there is an ongoing outgoing connections to the same sid.
> > * Now, the opened socket in connectOne is a strayed socket, and will not get used, or closed.
> >
> > If we put same check in connectOne, this problem can be fixed. Does this make sense for the case I am describing above?
Thanks Michael for pointing out this. Yes, its a connection leak, I will do the necessary changes.
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 328
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498960#file1498960line328>
> >
> > Here we create new threads to handle both incoming connection request handling and out going connection initiation and set up. It makes sense to me as SASL auth evaluation might take a while - I am curious, Rakesh, do you have any rough idea / data on how long the auth would take? Would it take at the magnitude of million seconds, or seconds, or even minutes? My impression is that SASL evaluation between client and server should be done at the scale of seconds at most, as it purely involves two servers which sending small data between each other, and does not involve KDC (as Kerberos login should have already been finished when we reach here.).
> >
> > I am trying to justify the usage of threads here because we did not create threads to handle such connection requests, so this change is pretty big in terms of impact to existing codebase. Because we don't create threads, the old code base was executed purely sequentially. With threads, the semantic is slightly changed as the code after receiveConnection might be executing without receiveConnection actually finished. So far I have not spotted anywhere in code base with such sequential dependency, but want to raise the awareness of this and see what others think about this.
> >
> > Also, note that this thread code path is by default (and the only) code path for QuorumCnxManager connection handling, regardless if users enable server to server auth or not. I am wondering should we have another code path so users who don't enable server to server auth will get exactly the old behavior. In other words, when server to server auth is not enabled, we will be very confident that our change will not introduce any regressions.
Yes, the SASL evaluation path may take at the magnitude of seconds. Again, this depends on the I/O resources(network speed etc).
Its good point to maintain the existing code as it is and add the new code separately. I also have a feeling that folks are having concerns about the complexity of the code changes, may be this this approach will help to gain more confidence from all. Let me think way to seggregate this out.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149566
-----------------------------------------------------------
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review149566
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 112)
<https://reviews.apache.org/r/47354/#comment217205>
This inprogressConnections is used to make sure we don't have duplicated auth session for outgoing connections, so it make sense to me.
However, this set only applies to outgoing connections. For incoming connections, there is no corresponding structure to deduplicate. Do we need to also do similar book keeping on the receiver side (i.e. in handleConnections)?
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 161)
<https://reviews.apache.org/r/47354/#comment217204>
I have a mixed feeling about this change - it makes QuorumCnxManager's constructor more complicated comparing to the previous constructor QuorumCnxManager(QuorumPeer), where we just pass a QuorumPeer object. That said though, with the new QuorumPeer.createCnxManager method, this is not a big problem, as a QuorumCnxManager can be conveniently created given a QuorumPeer object.
Alternatively, we can keep QuorumCnxManager's constructor as it is, so it still accepts a QuorumPeer object and inside QuorumCnxManager's constructor, we initialize various fields required here (such as authServer/Learner, and so on.). This requires QuorumCnxManager be able to access QuorumPeer's protected / private fields, so it requires adding some getter methods on QuorumPeer to expose those properties.
Overall I don't have a strong opinion one way or the other, wondering what other reviewers think about this.
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 212)
<https://reviews.apache.org/r/47354/#comment217206>
Should we put the check of in progress sid here, or up on the caller site (connectOne) who calls initateConnection? I am asking because I spotted there could be a potential socket leak if we put check here. Consider this case:
* In connectOne(long sid), we first open a new socket and then call initiateConnection(sock, sid).
* In side initiateConnection(sock, sid), we are doing this sid check and will return if we find out there is an ongoing outgoing connections to the same sid.
* Now, the opened socket in connectOne is a strayed socket, and will not get used, or closed.
If we put same check in connectOne, this problem can be fixed. Does this make sense for the case I am describing above?
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 319)
<https://reviews.apache.org/r/47354/#comment217213>
Here we create new threads to handle both incoming connection request handling and out going connection initiation and set up. It makes sense to me as SASL auth evaluation might take a while - I am curious, Rakesh, do you have any rough idea / data on how long the auth would take? Would it take at the magnitude of million seconds, or seconds, or even minutes? My impression is that SASL evaluation between client and server should be done at the scale of seconds at most, as it purely involves two servers which sending small data between each other, and does not involve KDC (as Kerberos login should have already been finished when we reach here.).
I am trying to justify the usage of threads here because we did not create threads to handle such connection requests, so this change is pretty big in terms of impact to existing codebase. Because we don't create threads, the old code base was executed purely sequentially. With threads, the semantic is slightly changed as the code after receiveConnection might be executing without receiveConnection actually finished. So far I have not spotted anywhere in code base with such sequential dependency, but want to raise the awareness of this and see what others think about this.
Also, note that this thread code path is by default (and the only) code path for QuorumCnxManager connection handling, regardless if users enable server to server auth or not. I am wondering should we have another code path so users who don't enable server to server auth will get exactly the old behavior. In other words, when server to server auth is not enabled, we will be very confident that our change will not introduce any regressions.
- Michael Han
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Sept. 27, 2016, 9:27 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, line 131
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498970#file1498970line131>
> >
> > Right, I think the key is sc.isComplete == true does not imply the result is a success. Need an explicit check. Something like what they did in the example should work:
> >
> > if (sc.isComplete() && res.status == SUCCESS) {
> > LOG.info("Successfully completed the authentication using SASL.");
> > .......
> > }
Done
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review150630
-----------------------------------------------------------
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review150630
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java (line 131)
<https://reviews.apache.org/r/47354/#comment218655>
Right, I think the key is sc.isComplete == true does not imply the result is a success. Need an explicit check. Something like what they did in the example should work:
if (sc.isComplete() && res.status == SUCCESS) {
LOG.info("Successfully completed the authentication using SASL.");
.......
}
- Michael Han
On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2016, 4:48 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review151536
-----------------------------------------------------------
Ship it!
Thanks Rakesh, latest patch looks great to me.
- Michael Han
On Oct. 5, 2016, 4:14 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2016, 4:14 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ee6834c
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review158075
-----------------------------------------------------------
Ship it!
Latest patch looks good to me - the recent change addresses my remaining issues. I'm +1.
- Patrick Hunt
On Nov. 29, 2016, 6:26 p.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2016, 6:26 p.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml 505597a
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
> src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
> src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Nov. 29, 2016, 6:26 p.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
Attached new patch addressing Pat Hunt's comment.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml 505597a
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On Nov. 29, 2016, 5:16 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 449
> > <https://reviews.apache.org/r/47354/diff/12-13/?file=1560426#file1560426line449>
> >
> > Doesn't this change change the semantics of SID?
oops...I'm really sorry, I tested with 'observerCounter.getAndDecrement()' but prepared patch wrongly. I will change this to 'observerCounter.getAndDecrement()' soon. In that way, it will be same as existing behavior.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review157277
-----------------------------------------------------------
On Nov. 25, 2016, 7:28 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2016, 7:28 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml 505597a
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
> src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
> src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review157277
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 449)
<https://reviews.apache.org/r/47354/#comment227801>
Doesn't this change change the semantics of SID?
- Patrick Hunt
On Nov. 25, 2016, 7:28 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2016, 7:28 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml 505597a
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
> src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
> src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Nov. 25, 2016, 7:28 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
New patch addressing Pat Hunt's review comments. Following changes:
- Used AtomicInteger for observerCounter.
- Added counter to count connection processing threads. Now, I've added log messages in each LE iteration. Later if requires can expose bean attribute.
Also, I hope this will be helpful to tune the configuration property "quorum.cnxn.threads.size"
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml 505597a
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Nov. 10, 2016, 3:38 p.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
Attached new patch fixing Eugene Koontz's comments.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml 505597a
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review154205
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 188)
<https://reviews.apache.org/r/47354/#comment223706>
typo: should be initializeAuth(..)
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 189)
<https://reviews.apache.org/r/47354/#comment223707>
Seems a little strange to call initalizeAuth() if (quorumSaslAuthEnabled == false), but not a big deal.
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java (line 693)
<https://reviews.apache.org/r/47354/#comment223708>
I wonder if it would be clearer to change the variable name from clientLoginContext to learnerLoginContext since the we are always calling createAndStartManager() with the 3rd argument being the string "QuorumLearner"
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java (line 695)
<https://reviews.apache.org/r/47354/#comment223709>
As with last comment, maybe this should be called learnerRequireSasl?
- Eugene Koontz
On Oct. 25, 2016, 5:05 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2016, 5:05 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml 505597a
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
> src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Oct. 25, 2016, 5:05 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
New patch contains the following changes:
Renamed quorumcnxn.threads.size to quorum.cnxn.threads.size
Removed README.md from the patch as I attached feature doc pdf doc separately in jira
Replaced IOUtils.cleanup(LOG, sock) with simple sock.close(); as java-1.6 shows compilation errors.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml 505597a
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 7fdffde
src/java/main/org/apache/zookeeper/server/quorum/Follower.java 043a522
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/Observer.java e53f6f2
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Oct. 5, 2016, 4:14 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
Uploading new patch to address Michael's review comments. Major change - provided separate code path to execute the auth based connection requests asynchronously and for non-auth requests will be processed in old fashion sequentially.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml ee6834c
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Oct. 5, 2016, 4:10 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs
-----
build.xml ee6834c
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Oct. 5, 2016, 4:08 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
Uploading new patch to address Michael's review comments.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs
-----
build.xml ee6834c
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
File Attachments (updated)
----------------
New patch addressing Michael's review comments
https://reviews.apache.org/media/uploaded/files/2016/10/05/4dd9ed9a-817b-420b-bc6e-b638f6eaa7d9__ZOOKEEPER-1045-br-3-4-authz.patch
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated Sept. 15, 2016, 4:48 p.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Changes
-------
Attached new patch which includes the following:
1) Supported quorum peer server authorization logic to authz connecting learner.
2) Fixed flaky QuorumAuthUpgradeTest#testRollingUpgrade testcase.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml ee6834c
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352
src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 0924ef6
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 8db7fa8
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 85817b2
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated July 12, 2016, 7:28 p.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml ab254b2
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 78f3aa6
src/java/main/org/apache/zookeeper/server/quorum/Follower.java 2aeb0c3
src/java/main/org/apache/zookeeper/server/quorum/FollowerBean.java fd31fa2
src/java/main/org/apache/zookeeper/server/quorum/FollowerMXBean.java ded0e1c
src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
src/java/main/org/apache/zookeeper/server/quorum/LeaderBean.java b5a3a10
src/java/main/org/apache/zookeeper/server/quorum/LeaderMXBean.java bf08104
src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 28, 2016, 11:56 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, line 121
> > <https://reviews.apache.org/r/47354/diff/7/?file=1430362#file1430362line121>
> >
> > I recommend logging the content of the exception here which would make trouble shooting easier. Learned this today from an end user who tried to figure out why Kerberos auth not working (still not work, but close) and the existing log does not provide much info if the exception is thrown from here (in my case, the exception is 'GSS initiate failed').
Added error logs like,
LOG.error("Failed to authenticate using SASL", e);
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review139902
-----------------------------------------------------------
On June 26, 2016, 9:13 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 26, 2016, 9:13 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Michael Han <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review139902
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java (line 121)
<https://reviews.apache.org/r/47354/#comment205243>
I recommend logging the content of the exception here which would make trouble shooting easier. Learned this today from an end user who tried to figure out why Kerberos auth not working (still not work, but close) and the existing log does not provide much info if the exception is thrown from here (in my case, the exception is 'GSS initiate failed').
- Michael Han
On June 26, 2016, 9:13 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 26, 2016, 9:13 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/
-----------------------------------------------------------
(Updated June 26, 2016, 9:13 a.m.)
Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
Bugs: ZOOKEEPER-1045
https://issues.apache.org/jira/browse/ZOOKEEPER-1045
Repository: zookeeper-git
Description
-------
Quorum mutual authentication using SASL mechanism - Digest/Kerberos
Diffs (updated)
-----
build.xml ab254b2
ivy.xml 95b0e5a
src/java/main/org/apache/zookeeper/Login.java aaa220c
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
src/zookeeper.jute 6521e54
Diff: https://reviews.apache.org/r/47354/diff/
Testing
-------
Added unit test cases to verify the changes.
Thanks,
Rakesh R
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review136981
-----------------------------------------------------------
Ship it!
Ship It!
- Raul Gutierrez Segales
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 15, 2016, 11:12 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java, line 79
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406669#file1406669line79>
> >
> > why not make this info? it's output infrequently, correct?
Done. changed to info
> On June 15, 2016, 11:12 p.m., Patrick Hunt wrote:
> > src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java, line 341
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406680#file1406680line341>
> >
> > this is very likely a flakey test waiting to happen. Can we do something like a latch rather than a sleep?
Done, replaced with latch. Please review it. Thanks!
> On June 15, 2016, 11:12 p.m., Patrick Hunt wrote:
> > src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java, line 384
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406680#file1406680line384>
> >
> > Looks like there are a number of sleep operations (14 or so?) - can we review these and eliminate as many as possible (hopefully all).
Done. replaced with latch.
Thread.sleep exists in three more places. ALl are using sleep in retry fashion, so I feel its not required to change this logic.
QuorumCnxManagerTest#waitForElectionAddrBinding
QuorumCnxManagerTest#assertEventuallyConnected
QuorumCnxManagerTest#assertEventuallyNotConnected
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137875
-----------------------------------------------------------
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137875
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/auth/README.md (line 85)
<https://reviews.apache.org/r/47354/#comment203079>
Why? What does this effect? (perhaps also add a link if it's already documented somewhere?)
src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java (line 79)
<https://reviews.apache.org/r/47354/#comment203084>
why not make this info? it's output infrequently, correct?
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java (line 341)
<https://reviews.apache.org/r/47354/#comment203085>
this is very likely a flakey test waiting to happen. Can we do something like a latch rather than a sleep?
src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java (line 384)
<https://reviews.apache.org/r/47354/#comment203086>
Looks like there are a number of sleep operations (14 or so?) - can we review these and eliminate as many as possible (hopefully all).
- Patrick Hunt
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Rakesh R <ra...@intel.com>.
> On June 15, 2016, 10:49 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/Leader.java, line 328
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406656#file1406656line328>
> >
> > See my comment in the constructor for LearnerHandler
Done. Removed authSuccess flag by throwing SaslException.
> On June 15, 2016, 10:49 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java, line 175
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406658#file1406658line175>
> >
> > It seems very odd that we are setting some variable, adding an accessor method, then returning here. Why don't we just throw a security related exception? (perhaps a sub of ioexception?)
Done. Removed authSuccess flag by throwing SaslException.
> On June 15, 2016, 10:49 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, line 322
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406659#file1406659line322>
> >
> > why are we using a runnable here when we used a zookeeperthread in QuorumConnectionReqThread? I'm not saying to use one over the other, but isn't a thread, because it is named, result in easier to debug?
Done. Created QuorumConnectionReceiverThread to receive connections.
> On June 15, 2016, 10:49 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 402
> > <https://reviews.apache.org/r/47354/diff/6/?file=1406660#file1406660line402>
> >
> > the comment here is not super helpful - perhaps we could indicate why 20 vs some other number?
I created a default constant variable and used. "QUORUM_CNXN_THREADS_SIZE_DEFAULT_VALUE" and marked as // TODO item. I'm not sure about the way to evaluate this. Since its exposed as config, admin will be able to change it.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137866
-----------------------------------------------------------
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>
Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual
authentication using SASL mechanism
Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47354/#review137866
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/Leader.java (line 327)
<https://reviews.apache.org/r/47354/#comment203056>
See my comment in the constructor for LearnerHandler
src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java (line 175)
<https://reviews.apache.org/r/47354/#comment203057>
It seems very odd that we are setting some variable, adding an accessor method, then returning here. Why don't we just throw a security related exception? (perhaps a sub of ioexception?)
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java (line 313)
<https://reviews.apache.org/r/47354/#comment203059>
why are we using a runnable here when we used a zookeeperthread in QuorumConnectionReqThread? I'm not saying to use one over the other, but isn't a thread, because it is named, result in easier to debug?
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 402)
<https://reviews.apache.org/r/47354/#comment203068>
the comment here is not super helpful - perhaps we could indicate why 20 vs some other number?
- Patrick Hunt
On June 5, 2016, 7:56 a.m., Rakesh R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
>
> (Updated June 5, 2016, 7:56 a.m.)
>
>
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1045
> https://issues.apache.org/jira/browse/ZOOKEEPER-1045
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
>
>
> Diffs
> -----
>
> build.xml ab254b2
> ivy.xml 95b0e5a
> src/java/main/org/apache/zookeeper/Login.java aaa220c
> src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa
> src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 2fbd6ed
> src/java/main/org/apache/zookeeper/server/quorum/Leader.java 40c6748
> src/java/main/org/apache/zookeeper/server/quorum/Learner.java c73a8ee
> src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 8a748c7
> src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 20e5f16
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 8ae820d
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java e9c8007
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/README.md PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java PRE-CREATION
> src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION
> src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION
> src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 831d3ed
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java c1259d1
> src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 72e4fc9
> src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java a4c0cb0
> src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 39a53ca
> src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce
> src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java ef552db
> src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505
> src/zookeeper.jute 6521e54
>
> Diff: https://reviews.apache.org/r/47354/diff/
>
>
> Testing
> -------
>
> Added unit test cases to verify the changes.
>
>
> Thanks,
>
> Rakesh R
>
>