> On March 13, 2015, 6:33 a.m., Rakesh R wrote: > > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java, > > line 233 > > <https://reviews.apache.org/r/31963/diff/1/?file=891814#file891814line233> > > > > I couldn't see anyone is calling this. Do we need this method?
This should be used in the Netty factory. I missed an integration point--thanks for catching. > On March 13, 2015, 6:33 a.m., Rakesh R wrote: > > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java, > > line 80 > > <https://reviews.apache.org/r/31963/diff/1/?file=891814#file891814line80> > > > > One general doubt about the exception handling in the constructor: > > > > Should we continue with the server startup if there is an exception > > while instantiating the X509AuthenticationProvider ? > > > > Down the code I could see #handleAuthentication() is checking > > trustManager == null and failing all the clients > > KeeperException.Code.AUTHFAILED. Since no clients are allowed to establish > > a connection, it would good we fail the server startup rather than > > rejecting the client connections. What do you say? Yes, because X509AuthenticationProvider is created by ProviderRegistry whether we want SSL or not. Instead, I can make it so that NettyServerCnxnFactory.initSSL() fails when there is an issue getting the trust and key managers. - Ian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31963/#review76341 ----------------------------------------------------------- On March 12, 2015, 12:01 a.m., Ian Dimayuga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31963/ > ----------------------------------------------------------- > > (Updated March 12, 2015, 12:01 a.m.) > > > Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R. > > > Repository: zookeeper-git > > > Description > ------- > > Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java > e02753f4fb926a8cc6c7a7c10af42f949c1e210c > src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java > b4bdc82f8b52f736c6ee3d67bb793a3616c1b436 > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 09a794844978456fc3580adc22b6064e2a12cf77 > src/java/main/org/apache/zookeeper/server/ServerCnxn.java > a47d85662970cc0c219a46b226737a8689f8fe96 > src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java > 406015f84a51e6afcfe704b881f8494bdd687a25 > > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/TestServerCnxn.java PRE-CREATION > src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/31963/diff/ > > > Testing > ------- > > > Thanks, > > Ian Dimayuga > >
