ranger-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bolke de Bruin <bdbr...@gmail.com>
Subject Re: Review Request 72272: Upgrade and improve Presto plugin
Date Fri, 10 Apr 2020 08:53:48 GMT


> On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote:
> > plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/72272/diff/2/?file=2215813#file2215813line76>
> >
> >     Since 'useUgi' is set only in the constructor,  consider marking this as a 'final'.
Same applies for 'rangerPlugin' as well. Please review.

that won't work for useUgi. Declaring it final thecompiler complains that it cannot set it.


> On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote:
> > plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java
> > Lines 157 (patched)
> > <https://reviews.apache.org/r/72272/diff/2/?file=2215813#file2215813line166>
> >
> >     result.isRowFilterEnabled() already checks if the filter-expr is empty or not;
so 'StringUtils.isNotEmpty(result.getFilterExpr());' is not needed here. Please  review.

came from the Hive Authorizer. Removed


> On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote:
> > ranger-presto-plugin-shim/pom.xml
> > Lines 93 (patched)
> > <https://reviews.apache.org/r/72272/diff/2/?file=2215820#file2215820line93>
> >
> >     The changes in this module don't seem to refer hadoop-hdfs library contents
directly. Is it necessary to explicitly add this dependency?

I'm not getting it into the assembly otherwise for auditing purposes? please advise.


> On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote:
> > ranger-presto-plugin-shim/pom.xml
> > Lines 134 (patched)
> > <https://reviews.apache.org/r/72272/diff/2/?file=2215820#file2215820line134>
> >
> >     The changes in this module don't seem to refer protobuf-java library contents
directly. Is it necessary to explicitly add this dependency?

Idem as above. How to get it in the assembly? It seems not to be included otherwise (struggling
with that overall btw)


- Bolke


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


On March 30, 2020, 5:20 p.m., Bolke de Bruin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72272/
> -----------------------------------------------------------
> 
> (Updated March 30, 2020, 5:20 p.m.)
> 
> 
> Review request for ranger, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, and Ramesh
Mani.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/RANGER-2754
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/RANGER-2754
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Upgrade and improve Presto plugin
> - Presto SQL 331 has changed its security API and has Row level / column masking functionality
> - Upgraded Hadoop dependency to 3.1.3 (from 3.1.1) due to improved security handling
> - New features like session properties and system properties
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/resources/service-defs/ranger-servicedef-presto.json 56a8f5ac0

>   distro/src/main/assembly/plugin-presto.xml d2075bfe7 
>   plugin-presto/pom.xml b63f7dede 
>   plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java
3ab63f590 
>   plugin-presto/src/test/java/org/apache/ranger/authorization/presto/authorizer/RangerAdminClientImpl.java
PRE-CREATION 
>   plugin-presto/src/test/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControlTest.java
PRE-CREATION 
>   plugin-presto/src/test/resources/log4j.properties PRE-CREATION 
>   plugin-presto/src/test/resources/presto-policies.json PRE-CREATION 
>   plugin-presto/src/test/resources/ranger-presto-security.xml PRE-CREATION 
>   pom.xml 22926fd7d 
>   ranger-presto-plugin-shim/pom.xml d8ff88d0f 
>   ranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerConfig.java
67b0d2434 
>   ranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java
e89f646e1 
> 
> 
> Diff: https://reviews.apache.org/r/72272/diff/2/
> 
> 
> Testing
> -------
> 
> - New Unit tests added
> - Tested locally in production
> 
> 
> Thanks,
> 
> Bolke de Bruin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message