----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6290/#review9727 -----------------------------------------------------------
Looks read to go to me except for the noted issue. /src/java/test/org/apache/zookeeper/test/SaslAuthDesignatedServerTest.java <https://reviews.apache.org/r/6290/#comment20709> This timeout is too low. Typically we make it the connection timeout, otw it has the tendency to fail intermittently (for example on slow/loaded test hardware) - Patrick Hunt On Aug. 1, 2012, 8:33 p.m., Matteo Bertozzi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6290/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2012, 8:33 p.m.) > > > Review request for zookeeper, Patrick Hunt and Eugene Koontz. > > > Description > ------- > > Currently the CnxnFactory checks for "java.security.auth.login.config" to > decide whether or not enable SASL. > - zookeeper/server/NIOServerCnxnFactory.java > - zookeeper/server/NettyServerCnxnFactory.java > - configure() checks for "java.security.auth.login.config" > - If present start the new Login("Server", > SaslServerCallbackHandler(conf)) > > But since the SaslServerCallbackHandler does the right thing just checking if > getAppConfigurationEntry() is empty, we can allow SASL with JAAS > configuration to be programmatically just checking weather or not a > configuration entry is present instead of "java.security.auth.login.config". > (Something quite similar was done for the SaslClient in ZOOKEEPER-1373) > > > This addresses bug ZOOKEEPER-1497. > https://issues.apache.org/jira/browse/ZOOKEEPER-1497 > > > Diffs > ----- > > /src/java/main/org/apache/zookeeper/Environment.java 1368201 > /src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 1368201 > /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > 1368201 > /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 1368201 > /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1368201 > /src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 1368201 > > /src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java > 1368201 > /src/java/test/org/apache/zookeeper/JaasConfiguration.java PRE-CREATION > /src/java/test/org/apache/zookeeper/test/ClientBase.java 1368201 > /src/java/test/org/apache/zookeeper/test/SaslAuthDesignatedServerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/6290/diff/ > > > Testing > ------- > > New testcase added SaslAuthDesignatedServerTest to check if > ZooKeeperSaslServer.LOGIN_CONTEXT_NAME_KEY is used. > (A new JaasConfiguration class was added to wrap the jaas.conf) > > +Manual testing for HBASE-4791 > > > Thanks, > > Matteo Bertozzi > >
