Thanks a lot Patrick Hunt for the review comments. Please take another look at the wiki page when you get a chance.
I've updated the wiki page addressing these,. 1) ===> DONE. Added JCE encryption part. 2) ===> DONE. Corrected case. 3) ===> DONE. Included version. 4) ===> DONE. Corrected numbering format. 5) ===> DONE. Added an example case to understand the tuning mechanism. 6) ===> DONE. I've removed this part because it can be discussed separately and added if someone has a use case. 7) ===> DONE. Rephrased upgrade feature section Thanks, Rakesh On Wed, Dec 14, 2016 at 9:03 AM, Patrick Hunt <ph...@apache.org> wrote: > Nice job Rakesh, some comments: > > 1) the appendix is a great idea, should be useful for many people. One > thing I noticed > "There is no additional dependencies needed to use SASL with Java since it > is part of the the Java Standard Edition." - you might want to mention/link > the JCE? The JVM doesn't come with very modern encryption - some of the > distros use more strong encryption out of the box with kerberos. I've run > into this a number of times (need to also install JCE). > > 2) consistently use "ZooKeeper" rather than "Zookeeper". Only noticed this > in a few places... > > 3) on client-server it would be good to mention when it was added (3.4.0+), > similar to what you did with 1045. > > 4) on "ZooKeeper SASL configurations" the numbering of the bullets starts > at 2.1. and finishes at 2.4. I suspect the formatting didn't copy over > quite right? > > 5) similar formatting issue for "# Defaulting to > 20quorum.cnxn.threads.size=20" > > Can we give any insight into how this value should be set? i.e. why is 20 > the default and when should it be raised/lowered? > > 6) can the doc shed any light on why we are recommending > "javax.security.auth.useSubjectCredsOnly=false" ? I'm not familiar with > this myself. > > 7) "This feature is supported in 3.4 branch" is ambiguous - perhaps > rephrase. What "feature" are you referring to, 1045 or to rolling upgrade? > Also the ref to 3.4 itself is ambiguous - perhaps change to 3.4.10+? > > These are some minor nits, overall impressive effort -- thanks again > Rakesh! > > Patrick > > > > On Tue, Dec 13, 2016 at 6:56 PM, Rakesh Radhakrishnan <rake...@apache.org> > wrote: > > > Hi All, > > > > I've incorporated ZK-1045 feature details into the Apache ZooKeeper > project > > cwiki. Since "ZooKeeper and SASL" section is quite large I've splitted > > ZooKeeper client-server and server-server sections into sub-pages. Please > > read the following page, > > > > https://cwiki.apache.org/confluence/display/ZOOKEEPER/ > ZooKeeper+and+SASL+ > > authentication > > > > *ZooKeeper and SASL authentication* > > > > - Client-Server mutual authentication > > - Server-Server mutual authentication > > - Appendix: Kerberos, GSSAPI, SASL, and JAAS > > > > I have reused the content from the "Client-Server" and "Appendix" > sections > > from the existing page > > https://cwiki.apache.org/confluence/display/ZOOKEEPER/Zookeeper+and+SASL > > Presently I've maintained this original page as a history, probably we > need > > to delete this page after everyone agrees on the changes. > > > > Appreciate your feedback, thanks! > > > > Regards, > > Rakesh > > >
