-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32847/#review81279
-----------------------------------------------------------

Ship it!


- Prasad Mujumdar


On April 23, 2015, 1:51 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32847/
> -----------------------------------------------------------
> 
> (Updated April 23, 2015, 1:51 a.m.)
> 
> 
> Review request for sentry, shen guoquan, Prasad Mujumdar, and Vamsee 
> Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Adds a unit test for using different group mappings that fails without the 
> patch and passes with the patch.
> 
> The basic issue is that in solr we don't have hadoop Configuration files on 
> the classpath and the hadoop group inevitably get set to "new 
> Configuration()" (the jira has an example where calling FileSystem.get(conf) 
> sets up the group mappings to "new Configuration" -- not the conf you pass 
> in.  But this is just one example, I've seen it all over the place in hadoop 
> code.
> 
> So, this patch does two things:
> 1) sets the configuration earlier in the binding so we get the group mappings 
> before the provider backend constructor (which reads from hdfs, so can call 
> FileSystem.get(...)
> 2) changes the HadoopGroupResourceAuthorizationProvider from using 
> Groups.getUserToGroupsMappingService(...) to using new Groups(...).  That is, 
> the groups now use the actual configuration you passed in rather than 
> whatever Groups were originally set up as (possibly mistakenly, as in the 
> example above).  In the case where Groups.getUserToGroupMappingService(...) 
> hasn't been called yet, the behavior is the same.  This is definitely an 
> improvement in Solr, and I don't think it should affect hive because from my 
> understanding Hive has the configuration files on the classpath, so the 
> behavior should be identical.  It would be good if someone could confirm that 
> though.
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  373ee8c7fd0b3bc569de39dd664c8876d5597b30 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  1bc01a2d837f0afc78547c1667f701cdbd1f6193 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
>  14e2d05c919ec540acf845056ed8972239a27768 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupResourceAuthorizationProvider.java
>  626fd909cac9630dc11a2386886423bde4fba190 
> 
> Diff: https://reviews.apache.org/r/32847/diff/
> 
> 
> Testing
> -------
> 
> Ran the changed unit test.  We'll see what the QA bot says.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>

Reply via email to