----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25983/#review60764 -----------------------------------------------------------
A couple of comments below, related to the JAAS code. Also with Sentry HA enabled, the client side sentry-site much contain _HOST in the server principal. You might want to add a Precondition check to verify the configuration. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java <https://reviews.apache.org/r/25983/#comment102172> Nit: Is it possible to avoid hardcoding these properties ? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java <https://reviews.apache.org/r/25983/#comment102173> In secure mode, Sentry service is already running with JAAS context. https://github.com/apache/incubator-sentry/blob/master/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java#L151 Do we still need to setup all the security parameters for ZK ? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/JaasConfiguration.java <https://reviews.apache.org/r/25983/#comment102175> Same as above. SentryService is already obtaining Kerberos ticket. Do we need to redo all that in ZK access code path ? - Prasad Mujumdar On Oct. 27, 2014, 6:35 a.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25983/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2014, 6:35 a.m.) > > > Review request for sentry, Arun Suresh, Lenni Kuff, Prasad Mujumdar, and > Sravya Tirukkovalur. > > > Bugs: SENTRY-459 > https://issues.apache.org/jira/browse/SENTRY-459 > > > Repository: sentry > > > Description > ------- > > Support Kerberos for SENTRY high availability. In security mode, Zookeeper > will use Kerberos for authentication, SENTRY should use the **principal** and > **keytab** in sentry configuration for authentication > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/JaasConfiguration.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 52eaeed > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForHAWithKerberos.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > cc12099 > > Diff: https://reviews.apache.org/r/25983/diff/ > > > Testing > ------- > > All Unit tests passed in local > > > Thanks, > > Dapeng Sun > >
