You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Ramesh Mani <rm...@hortonworks.com> on 2015/09/11 02:43:44 UTC
Review Request 38281: RANGER-586:Ranger plugins should not add
dependent libraries to component's CLASSPATH
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38281/
-----------------------------------------------------------
Review request for ranger, Don Bosco Durai, Madhan Neethiraj, and Selvamohan Neethiraj.
Repository: ranger
Description
-------
RANGER-586:Ranger plugins should not add dependent libraries to component's CLASSPATH
Diffs
-----
agents-common/scripts/enable-agent.sh 55130a5
hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java fa2155c
hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizerImpl.java PRE-CREATION
pom.xml 2ae8d3d
ranger-hdfs-plugin-shim/dependency-reduced-pom.xml PRE-CREATION
ranger-hdfs-plugin-shim/pom.xml PRE-CREATION
ranger-hdfs-plugin-shim/resources/META-INF/MANIFEST.MF PRE-CREATION
ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java PRE-CREATION
ranger-hdfs-plugin-shim/src/main/resources/META-INF/MANIFEST.MF PRE-CREATION
ranger-plugin-classloader/pom.xml PRE-CREATION
ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoader.java PRE-CREATION
ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoaderUtil.java PRE-CREATION
ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestChildFistClassLoader.java PRE-CREATION
ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPluginImpl.java PRE-CREATION
ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPrint.java PRE-CREATION
ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPlugin.java PRE-CREATION
ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPrintParent.java PRE-CREATION
src/main/assembly/hdfs-agent.xml 2c18001
Diff: https://reviews.apache.org/r/38281/diff/
Testing
-------
Testing done running hdfs system test
Thanks,
Ramesh Mani
Re: Review Request 38281: RANGER-586:Ranger plugins should not add
dependent libraries to component's CLASSPATH
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38281/#review98576
-----------------------------------------------------------
agents-common/scripts/enable-agent.sh (line 42)
<https://reviews.apache.org/r/38281/#comment155294>
This method returns "ext" and ignores the given arguments. Instead of using a method, directly assign PROJ_LIB_PLUGIN_DIR_NAME=ext
agents-common/scripts/enable-agent.sh (line 134)
<https://reviews.apache.org/r/38281/#comment155295>
PROJ_LIB_PLUGIN_DIR_NAME ==> PLUGIN_DEPENDENT_LIB_DIR
agents-common/scripts/enable-agent.sh (line 164)
<https://reviews.apache.org/r/38281/#comment155296>
"HCOMPONENT_LIB_PLUGIN_DIR" ==> "HCOMPONENT_PLUGIN_DEPENDENT_LIB_DIR"
Instead of using method getPluginDir(), using variables will be easier to read: "${HCOMPONENT_LIB_DIR}/${PROJ_NAME}-${COMPONENT_NAME}"
ranger-hdfs-plugin-shim/dependency-reduced-pom.xml (line 6)
<https://reviews.apache.org/r/38281/#comment155290>
I think dependency-reduced-pom.xml is not to be committed. Please review other projects.
ranger-hdfs-plugin-shim/pom.xml (line 36)
<https://reviews.apache.org/r/38281/#comment155291>
Please review all dependencies listed here; keep only those that are necessary.
ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 57)
<https://reviews.apache.org/r/38281/#comment155096>
It might be useful to look for plugin libraries in multiple directories - especially in deployments that use custom conditions and enrichers whose implementation is placed in a location different from hdfs plugin library.
ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoader.java (line 154)
<https://reviews.apache.org/r/38281/#comment155292>
It might be important to return the matching resources found in the plugin-libs AND component-libs (i.e. default classloader). This might require a simple wrapper to both iterators returned.
class MergeEnumeration<T> {
public MergeEnumeration(Enumeration<T> e1, Enumeration<T> e2) {
}
@Override
boolean hasMoreElement() {
return e1.hasMoreElement() || e2.hasMoreElement();
}
@Override
T next() {
// ...
}
}
src/main/assembly/hdfs-agent.xml (line 85)
<https://reviews.apache.org/r/38281/#comment155293>
It will be cleaner to remove the commented out lines.
- Madhan Neethiraj
On Sept. 11, 2015, 12:43 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38281/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 12:43 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Madhan Neethiraj, and Selvamohan Neethiraj.
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-586:Ranger plugins should not add dependent libraries to component's CLASSPATH
>
>
> Diffs
> -----
>
> agents-common/scripts/enable-agent.sh 55130a5
> hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java fa2155c
> hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizerImpl.java PRE-CREATION
> pom.xml 2ae8d3d
> ranger-hdfs-plugin-shim/dependency-reduced-pom.xml PRE-CREATION
> ranger-hdfs-plugin-shim/pom.xml PRE-CREATION
> ranger-hdfs-plugin-shim/resources/META-INF/MANIFEST.MF PRE-CREATION
> ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java PRE-CREATION
> ranger-hdfs-plugin-shim/src/main/resources/META-INF/MANIFEST.MF PRE-CREATION
> ranger-plugin-classloader/pom.xml PRE-CREATION
> ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoader.java PRE-CREATION
> ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoaderUtil.java PRE-CREATION
> ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestChildFistClassLoader.java PRE-CREATION
> ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPluginImpl.java PRE-CREATION
> ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPrint.java PRE-CREATION
> ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPlugin.java PRE-CREATION
> ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPrintParent.java PRE-CREATION
> src/main/assembly/hdfs-agent.xml 2c18001
>
> Diff: https://reviews.apache.org/r/38281/diff/
>
>
> Testing
> -------
>
> Testing done running hdfs system test
>
>
> Thanks,
>
> Ramesh Mani
>
>