> On May 16, 2017, 6:32 p.m., Vamsee Yarlagadda wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java > > Lines 77-79 (original) > > <https://reviews.apache.org/r/59317/diff/1/?file=1721744#file1721744line77> > > > > I think it is not a good idea to remove this section. This patch > > handles it well for Generic clients but for HDFS and Policy client the > > behavior is unexpected. > > > > The reason why we are getting the right UGI for HDFS, Hive is that > > their codebases are doing some UGI related stuff before starting the Sentry > > client connection and that is why we are getting that context for free. And > > Hive, HDFS logic of setting UGI is not aware that a sentry client is > > depenedent on it. In future, if they restructure code it is very likely > > that they will break us and a more diffucult problem would be to debug this > > and fix it. So i would rather suggest the client do all the things (setting > > UGI) it needs so that these long chain of dependencies could be avoided. > > kalyan kumar kalvagadda wrote: > From what I understand, In hadoop if one component wants to talk to other > component securely using kerberos, source component should call > UserGroupInformation.setConfiguration(conf); before calling the thift client > of destination component. setConfiguration would update the static data in > class UserGroupInformation so that appropriate information is avaialble when > UserGroupInformation is instantiated. I have checked Hive, sentry code to > confirm the behavior. > > For generic clients, things are done differently as the configuraion > provided to sentry doesn't have HADOOP_SECURITY_AUTHENTICATION propery. > Sentry generic policy client updates the configuration and performs > setConfiguration explicitly. > > > If sentry client calls setConfiguration on UserGroupInformation for every > client, this update would have impact on other secured connections that > source component initiates. I'm sure if we should be doing it. If you are > sure of it, i will update the patch.
I understand your argument but IMO from the debugging perspective, it is good that the client does all the things it needs for proper execution rather than assuming some other precursor process would take care of it. bq. this update would have impact on other secured connections that source component initiates. I doubt that it does given that it is the same conf being used for all invocations. I am also ok with the proposed solution provided we give a good set of comments before we call the loginUser() that the sentry client assumes UGI has been set properly before the line gets executed. - Vamsee ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59317/#review175135 ----------------------------------------------------------- On May 16, 2017, 6:21 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59317/ > ----------------------------------------------------------- > > (Updated May 16, 2017, 6:21 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1736 > https://issues.apache.org/jira/browse/SENTRY-1736 > > > Repository: sentry > > > Description > ------- > > As part of SENTRY-1593, code in SentryGenericServiceClientDefaultImpl which > set HADOOP_SECURITY_AUTHENTICATION property and update UserGroupInformation > class is missed. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java > 9b9f9e8 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > b7ac640 > > > Diff: https://reviews.apache.org/r/59317/diff/1/ > > > Testing > ------- > > Made sure that Solr client is able to talk to sentry with kerberos enabled. > > > Thanks, > > kalyan kumar kalvagadda > >
