> On Aug. 1, 2012, 6:38 p.m., Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/SaslAuthDesignatedServerTest.java, > > line 77 > > <https://reviews.apache.org/r/6290/diff/1/?file=132325#file132325line77> > > > > why is this sleep here? > > Eugene Koontz wrote: > These sleep()s are present in other SASL-related tests (for example > SaslAuthTest) as a work-around for ZOOKEEPER-1437: without the sleep()s, the > client may be denied permission because the SASL authentication process has > not completed yet. In the patch for ZOOKEEPER-1437, these sleeps are removed, > and thus could be removed also here, too. > > Patrick Hunt wrote: > Ah. Thanks Eugene. However given we know this is a SASL connection can't > we just look for the KeeperState.SaslAuthenticated to come to the watcher?
Sure, that should work. Perhaps I should file a new JIRA for removing the various sleep()s in the existing tests (and mentioning this JIRA too), and replace with watches? - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6290/#review9703 ----------------------------------------------------------- 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 > >
