You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org> on 2017/10/30 00:07:12 UTC
Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/
-----------------------------------------------------------
Review request for sentry.
Bugs: SENTRY-2021
https://issues.apache.org/jira/browse/SENTRY-2021
Repository: sentry
Description
-------
Allow MR session ACLs to start with a space or end in a space.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
Diff: https://reviews.apache.org/r/63402/diff/1/
Testing
-------
local run junit test and e2e automated testing through build
Thanks,
Wilfred Spiegelenburg
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review189653
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java
Lines 391 (patched)
<https://reviews.apache.org/r/63402/#comment266853>
whan should be when
- Na Li
On Oct. 30, 2017, 12:07 a.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2017, 12:07 a.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review191743
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Nov. 15, 2017, 9:13 p.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2017, 9:13 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/15/aea2b310-88f8-4139-804a-250fbfd81aa9__SENTRY-2021.005.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/17/c550fdd7-949a-496a-85aa-b6d997f72249__SENTRY-2021.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org>.
> On Nov. 21, 2017, 3:21 p.m., Sergio Pena wrote:
> > For some reason I cannot see the most updated file nor download it.
> > Could you submit the file with a different version number?
The latest revision of the patch is SENTRY-2021.patch (revision 3) on this link https://reviews.apache.org/r/63402/file/1656/
Let me know if that still does not show the newest revision for you.
- Wilfred
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review191581
-----------------------------------------------------------
On Nov. 15, 2017, 9:13 p.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2017, 9:13 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/15/aea2b310-88f8-4139-804a-250fbfd81aa9__SENTRY-2021.005.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/17/c550fdd7-949a-496a-85aa-b6d997f72249__SENTRY-2021.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review191581
-----------------------------------------------------------
For some reason I cannot see the most updated file nor download it.
Could you submit the file with a different version number?
- Sergio Pena
On Nov. 15, 2017, 9:13 p.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2017, 9:13 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/15/aea2b310-88f8-4139-804a-250fbfd81aa9__SENTRY-2021.005.patch
> SENTRY-2021.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/17/c550fdd7-949a-496a-85aa-b6d997f72249__SENTRY-2021.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/
-----------------------------------------------------------
(Updated Nov. 15, 2017, 9:13 p.m.)
Review request for sentry.
Bugs: SENTRY-2021
https://issues.apache.org/jira/browse/SENTRY-2021
Repository: sentry
Description
-------
Allow MR session ACLs to start with a space or end in a space.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
Diff: https://reviews.apache.org/r/63402/diff/1/
Testing
-------
local run junit test and e2e automated testing through build
File Attachments (updated)
----------------
SENTRY-2021.003.patch
https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
SENTRY-2021.patch
https://reviews.apache.org/media/uploaded/files/2017/11/15/aea2b310-88f8-4139-804a-250fbfd81aa9__SENTRY-2021.005.patch
Thanks,
Wilfred Spiegelenburg
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/
-----------------------------------------------------------
(Updated Nov. 15, 2017, 9:02 p.m.)
Review request for sentry.
Changes
-------
Updated to implement comments from review
Bugs: SENTRY-2021
https://issues.apache.org/jira/browse/SENTRY-2021
Repository: sentry
Description
-------
Allow MR session ACLs to start with a space or end in a space.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
Diff: https://reviews.apache.org/r/63402/diff/1/
Testing
-------
local run junit test and e2e automated testing through build
File Attachments (updated)
----------------
SENTRY-2021.003.patch
https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
SENTRY-2021.005.patch
https://reviews.apache.org/media/uploaded/files/2017/11/15/45849e2b-e7a0-4d6a-93b5-c198a3095ed5__SENTRY-2021.005.patch
Thanks,
Wilfred Spiegelenburg
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org>.
> On Nov. 14, 2017, 8:19 p.m., Sergio Pena wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java
> > Lines 116-117 (original), 116-122 (patched)
> > <https://reviews.apache.org/r/63402/diff/1/?file=1872582#file1872582line116>
> >
> > I think this requires a special function instead ofu sing appendConfVar().
> >
> > I was reading more about this ACL configuration and I learned that it can also use the "*" value. In this patch, the new value would be "user1,*", right? Would that work? * already implies that ALL users and groups are allowed, so there is no reason to modify it, right?
> >
> > What about writing a new private function to append new user information only and handle those cases?
>
> Sergio Pena wrote:
> Weird, when you see the empty quotes, I meant to add an asterisk, but the review board removed it.
Implemented the comments:
- used a private function to update setting
- handle wildcard ACL
- trimmed the ACL when done which allowed the test change to be rolled back
- updated the HiveAuthzBindingSessionHookV2 also because it had the same problem
- Wilfred
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review190997
-----------------------------------------------------------
On Nov. 15, 2017, 9:02 p.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2017, 9:02 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
> SENTRY-2021.005.patch
> https://reviews.apache.org/media/uploaded/files/2017/11/15/45849e2b-e7a0-4d6a-93b5-c198a3095ed5__SENTRY-2021.005.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Nov. 14, 2017, 8:19 p.m., Sergio Pena wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java
> > Lines 116-117 (original), 116-122 (patched)
> > <https://reviews.apache.org/r/63402/diff/1/?file=1872582#file1872582line116>
> >
> > I think this requires a special function instead ofu sing appendConfVar().
> >
> > I was reading more about this ACL configuration and I learned that it can also use the "*" value. In this patch, the new value would be "user1,*", right? Would that work? * already implies that ALL users and groups are allowed, so there is no reason to modify it, right?
> >
> > What about writing a new private function to append new user information only and handle those cases?
Weird, when you see the empty quotes, I meant to add an asterisk, but the review board removed it.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review190997
-----------------------------------------------------------
On Oct. 31, 2017, 12:32 a.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2017, 12:32 a.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/#review190997
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java
Lines 116-117 (original), 116-122 (patched)
<https://reviews.apache.org/r/63402/#comment268585>
I think this requires a special function instead ofu sing appendConfVar().
I was reading more about this ACL configuration and I learned that it can also use the "*" value. In this patch, the new value would be "user1,*", right? Would that work? * already implies that ALL users and groups are allowed, so there is no reason to modify it, right?
What about writing a new private function to append new user information only and handle those cases?
- Sergio Pena
On Oct. 31, 2017, 12:32 a.m., Wilfred Spiegelenburg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63402/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2017, 12:32 a.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-2021
> https://issues.apache.org/jira/browse/SENTRY-2021
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Allow MR session ACLs to start with a space or end in a space.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
>
>
> Diff: https://reviews.apache.org/r/63402/diff/1/
>
>
> Testing
> -------
>
> local run junit test and e2e automated testing through build
>
>
> File Attachments
> ----------------
>
> SENTRY-2021.003.patch
> https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
>
>
> Thanks,
>
> Wilfred Spiegelenburg
>
>
Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding
does not handle all types of ACLs
Posted by Wilfred Spiegelenburg via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63402/
-----------------------------------------------------------
(Updated Oct. 31, 2017, 12:32 a.m.)
Review request for sentry.
Changes
-------
fixed whan typo and removed space at the end of the line
Bugs: SENTRY-2021
https://issues.apache.org/jira/browse/SENTRY-2021
Repository: sentry
Description
-------
Allow MR session ACLs to start with a space or end in a space.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 7250891
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 02ac514
Diff: https://reviews.apache.org/r/63402/diff/1/
Testing
-------
local run junit test and e2e automated testing through build
File Attachments (updated)
----------------
SENTRY-2021.003.patch
https://reviews.apache.org/media/uploaded/files/2017/10/31/1b8e6eec-9e95-47f6-bcb7-4b5d7a4ffd57__SENTRY-2021.003.patch
Thanks,
Wilfred Spiegelenburg