You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Szehon Ho <sz...@cloudera.com> on 2014/05/29 10:14:16 UTC
Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/
-----------------------------------------------------------
Review request for hive.
Bugs: HIVE-7119
https://issues.apache.org/jira/browse/HIVE-7119
Repository: hive-git
Description
-------
This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
Diffs
-----
common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 83fadeb
shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java e6493eb
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
Diff: https://reviews.apache.org/r/22016/diff/
Testing
-------
For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
Thanks,
Szehon Ho
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Ashish Singh <as...@cloudera.com>.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java, line 666
> > <https://reviews.apache.org/r/22016/diff/1/?file=598635#file598635line666>
> >
> > Shouldn't we be trying to change permission even if we fail to set group? Also, if I remember correctly, trying to set group was leading to exceptions.
> >
>
> Szehon Ho wrote:
> FsShell serves that purpose. If error it will just return -1 as exitCode, which I log in the helper method.
Right, I missed that.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java, line 430
> > <https://reviews.apache.org/r/22016/diff/1/?file=598636#file598636line430>
> >
> > This is similar to Hadoop20FileStatus. Could we push them together into a base class?
>
> Szehon Ho wrote:
> I was heavily-considering, but unfortunately the tree is: Hadoop20Shims (isolated subclass), HadoopShimSecure -> Hadoop20Shims+Hadoop20Shim. So unfortunately the ones that are common in this case (Hadoop20+Hadoop20S) dont form a common tree, so hard to inherit unless I create a proper non-inner class.
>
> That's definitely an option if necessary, but I didnt want to invest in it, as Hadoop20Shims should be retired soon.
>
> Szehon Ho wrote:
> Typo, tree is : Hadoop20Shims (isolated subclass), HadoopShimsSecure -> Hadoop23Shims and Hadoop20SShims.
Makes sense.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44272
-----------------------------------------------------------
On May 29, 2014, 10:49 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:49 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Szehon Ho <sz...@cloudera.com>.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java, line 666
> > <https://reviews.apache.org/r/22016/diff/1/?file=598635#file598635line666>
> >
> > Shouldn't we be trying to change permission even if we fail to set group? Also, if I remember correctly, trying to set group was leading to exceptions.
> >
FsShell serves that purpose. If error it will just return -1 as exitCode, which I log in the helper method.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java, line 416
> > <https://reviews.apache.org/r/22016/diff/1/?file=598636#file598636line416>
> >
> > Same as previous comment regarding settign group.
Same
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java, line 430
> > <https://reviews.apache.org/r/22016/diff/1/?file=598636#file598636line430>
> >
> > This is similar to Hadoop20FileStatus. Could we push them together into a base class?
I was heavily-considering, but unfortunately the tree is: Hadoop20Shims (isolated subclass), HadoopShimSecure -> Hadoop20Shims+Hadoop20Shim. So unfortunately the ones that are common in this case (Hadoop20+Hadoop20S) dont form a common tree, so hard to inherit unless I create a proper non-inner class.
That's definitely an option if necessary, but I didnt want to invest in it, as Hadoop20Shims should be retired soon.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java, line 538
> > <https://reviews.apache.org/r/22016/diff/1/?file=598637#file598637line538>
> >
> > Same as previous comment on setting group.
Same response.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java, line 21
> > <https://reviews.apache.org/r/22016/diff/1/?file=598634#file598634line21>
> >
> > Is this here by mistake?
Somehow it came , as I didnt rebase the patch with latest trunk.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java, line 524
> > <https://reviews.apache.org/r/22016/diff/1/?file=598637#file598637line524>
> >
> > Objects.equal(conf.get("dfs.namenode.acls.enabled"), "true") is used at more than one place. Should we extract it out to a static method?
Done
- Szehon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44272
-----------------------------------------------------------
On May 29, 2014, 10:49 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:49 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Szehon Ho <sz...@cloudera.com>.
> On May 29, 2014, 8:28 p.m., Ashish Singh wrote:
> > shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java, line 430
> > <https://reviews.apache.org/r/22016/diff/1/?file=598636#file598636line430>
> >
> > This is similar to Hadoop20FileStatus. Could we push them together into a base class?
>
> Szehon Ho wrote:
> I was heavily-considering, but unfortunately the tree is: Hadoop20Shims (isolated subclass), HadoopShimSecure -> Hadoop20Shims+Hadoop20Shim. So unfortunately the ones that are common in this case (Hadoop20+Hadoop20S) dont form a common tree, so hard to inherit unless I create a proper non-inner class.
>
> That's definitely an option if necessary, but I didnt want to invest in it, as Hadoop20Shims should be retired soon.
Typo, tree is : Hadoop20Shims (isolated subclass), HadoopShimsSecure -> Hadoop23Shims and Hadoop20SShims.
- Szehon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44272
-----------------------------------------------------------
On May 29, 2014, 10:49 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:49 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44272
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java
<https://reviews.apache.org/r/22016/#comment78625>
Is this here by mistake?
shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java
<https://reviews.apache.org/r/22016/#comment78630>
Shouldn't we be trying to change permission even if we fail to set group? Also, if I remember correctly, trying to set group was leading to exceptions.
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
<https://reviews.apache.org/r/22016/#comment78633>
Same as previous comment regarding settign group.
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
<https://reviews.apache.org/r/22016/#comment78635>
This is similar to Hadoop20FileStatus. Could we push them together into a base class?
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
<https://reviews.apache.org/r/22016/#comment78637>
Objects.equal(conf.get("dfs.namenode.acls.enabled"), "true") is used at more than one place. Should we extract it out to a static method?
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
<https://reviews.apache.org/r/22016/#comment78636>
Same as previous comment on setting group.
- Ashish Singh
On May 29, 2014, 8:14 a.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 8:14 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 83fadeb
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java e6493eb
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44329
-----------------------------------------------------------
Ship it!
LGTM!
- Ashish Singh
On May 29, 2014, 10:49 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:49 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/
-----------------------------------------------------------
(Updated June 1, 2014, 1:07 a.m.)
Review request for hive.
Changes
-------
Rebase the patch.
Bugs: HIVE-7119
https://issues.apache.org/jira/browse/HIVE-7119
Repository: hive-git
Description
-------
This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 5c19ee5
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 4a0e72d
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java d6336e2
shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java 39dbc3b
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java b8fdd85
Diff: https://reviews.apache.org/r/22016/diff/
Testing
-------
For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
Thanks,
Szehon Ho
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/
-----------------------------------------------------------
(Updated May 30, 2014, 7:17 p.m.)
Review request for hive.
Changes
-------
Missed license headers, thanks Xuefu for the catch
Bugs: HIVE-7119
https://issues.apache.org/jira/browse/HIVE-7119
Repository: hive-git
Description
-------
This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
Diff: https://reviews.apache.org/r/22016/diff/
Testing
-------
For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
Thanks,
Szehon Ho
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/#review44404
-----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java
<https://reviews.apache.org/r/22016/#comment78770>
License header is expected.
- Xuefu Zhang
On May 29, 2014, 10:49 p.m., Szehon Ho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 10:49 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7119
> https://issues.apache.org/jira/browse/HIVE-7119
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
>
> This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
>
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
> ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
> shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
> shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
> shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
> shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
> shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
>
> Diff: https://reviews.apache.org/r/22016/diff/
>
>
> Testing
> -------
>
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
>
>
> Thanks,
>
> Szehon Ho
>
>
Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if
warehouse perm inheritance enabled
Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22016/
-----------------------------------------------------------
(Updated May 29, 2014, 10:49 p.m.)
Review request for hive.
Changes
-------
Thanks Ashish for ! Updated the patch and also left some response
Bugs: HIVE-7119
https://issues.apache.org/jira/browse/HIVE-7119
Repository: hive-git
Description
-------
This completes the permission inheritance story, by also inheriting the new concept of extended ACL's in HDFS from parent.
This is a bit tricky because only Hadoop 2.4 has extended ACL's. My strategy is to use the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then only if the flag "dfs.namenode.acls.enabled" is true.
It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus and aclEntry). So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses both normal file status , and Aclstatus if acl's are enabled.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java PRE-CREATION
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java 4f566d2
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474
shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689
shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java bcb2660
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae
shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java c27df64
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce
Diff: https://reviews.apache.org/r/22016/diff/
Testing
-------
For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission to test traditional permission without acl's, and TestExtendedAcl's to test acls).
Thanks,
Szehon Ho