You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Ryan Pridgeon <rn...@cloudera.com> on 2015/08/04 02:26:10 UTC
Review Request 37058: Grant select on Server results in ALL for server
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/
-----------------------------------------------------------
Review request for sentry.
Repository: sentry
Description
-------
SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
Diff: https://reviews.apache.org/r/37058/diff/
Testing
-------
Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
ADMINGROUP:server_all
USERGROUP1:server_select
USERGROUP2:server_insert
I then checked each level to ensure that they did not reflect that of ALL:
server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
*****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
Thanks,
Ryan Pridgeon
Re: Review Request 37058: Grant select on Server results in ALL for
server
Posted by Ryan Pridgeon <rn...@cloudera.com>.
> On Aug. 4, 2015, 5:53 a.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java, line 2117
> > <https://reviews.apache.org/r/37058/diff/1/?file=1028166#file1028166line2117>
> >
> > Is it interesting to test multiple DBs/TBLs?
> >
> > Also, this should be:
> > for (String db: dbs) <- note the plural on the array name
The idea behind the multiple DBs/TBLs was to show that a select on server actually spanned the entire metastore. i.e. Grant select on Database1 will not grant a group select on a table under Database2. Like wise a Grant select on db1.tbl1 grant a group acess to db1.tbl but not db1.tbl2. However with Grant select on Server I have select accesss on all tables and databases that fall within that sever namespace. I suppose however by showing a select working on db1.tbl1 and db2.tbl1 I accomplish the same with less iterations.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/#review93999
-----------------------------------------------------------
On Aug. 4, 2015, 12:26 a.m., Ryan Pridgeon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37058/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2015, 12:26 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
>
> Diff: https://reviews.apache.org/r/37058/diff/
>
>
> Testing
> -------
>
> Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
>
> ADMINGROUP:server_all
> USERGROUP1:server_select
> USERGROUP2:server_insert
>
> I then checked each level to ensure that they did not reflect that of ALL:
>
> server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
> server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
> server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
>
> *****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
>
> Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
>
>
> Thanks,
>
> Ryan Pridgeon
>
>
Re: Review Request 37058: Grant select on Server results in ALL for
server
Posted by Ryan Pridgeon <rn...@cloudera.com>.
> On Aug. 4, 2015, 5:53 a.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java, line 2087
> > <https://reviews.apache.org/r/37058/diff/1/?file=1028166#file1028166line2087>
> >
> > Use javadoc style comments:
> > /**
> > * SENTY-827
> > */
This will comment everything below it out. Instead I will /* SENTRY-827 */, unless there is something I am missing
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/#review93999
-----------------------------------------------------------
On Aug. 4, 2015, 12:26 a.m., Ryan Pridgeon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37058/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2015, 12:26 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
>
> Diff: https://reviews.apache.org/r/37058/diff/
>
>
> Testing
> -------
>
> Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
>
> ADMINGROUP:server_all
> USERGROUP1:server_select
> USERGROUP2:server_insert
>
> I then checked each level to ensure that they did not reflect that of ALL:
>
> server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
> server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
> server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
>
> *****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
>
> Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
>
>
> Thanks,
>
> Ryan Pridgeon
>
>
Re: Review Request 37058: Grant select on Server results in ALL for
server
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/#review93999
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java (line 494)
<https://reviews.apache.org/r/37058/#comment148491>
nit: space after "," here and on line 487
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 89)
<https://reviews.apache.org/r/37058/#comment148492>
can you just delete this test while you are here? Doesn't look like it is useful.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 325)
<https://reviews.apache.org/r/37058/#comment148493>
extra ;
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 400)
<https://reviews.apache.org/r/37058/#comment148494>
delete
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 502)
<https://reviews.apache.org/r/37058/#comment148495>
delete
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 928)
<https://reviews.apache.org/r/37058/#comment148496>
You might want to check your IDE settings. Looks like lots of formatting fixes which add some noise to the review (not to discourage you from fixing up the code blocks you are actually working on).
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 2085)
<https://reviews.apache.org/r/37058/#comment148497>
Use javadoc style comments:
/**
* SENTY-827
*/
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 2115)
<https://reviews.apache.org/r/37058/#comment148498>
Is it interesting to test multiple DBs/TBLs?
Also, this should be:
for (String db: dbs) <- note the plural on the array name
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 2145)
<https://reviews.apache.org/r/37058/#comment148499>
verify that a SELECT on the table fails?
- Lenni Kuff
On Aug. 4, 2015, 12:26 a.m., Ryan Pridgeon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37058/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2015, 12:26 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
>
> Diff: https://reviews.apache.org/r/37058/diff/
>
>
> Testing
> -------
>
> Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
>
> ADMINGROUP:server_all
> USERGROUP1:server_select
> USERGROUP2:server_insert
>
> I then checked each level to ensure that they did not reflect that of ALL:
>
> server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
> server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
> server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
>
> *****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
>
> Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
>
>
> Thanks,
>
> Ryan Pridgeon
>
>
Re: Review Request 37058: Grant select on Server results in ALL for
server
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/#review94202
-----------------------------------------------------------
Ship it!
Ship It!
- Lenni Kuff
On Aug. 4, 2015, 4:04 p.m., Ryan Pridgeon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37058/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2015, 4:04 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
>
> Diff: https://reviews.apache.org/r/37058/diff/
>
>
> Testing
> -------
>
> Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
>
> ADMINGROUP:server_all
> USERGROUP1:server_select
> USERGROUP2:server_insert
>
> I then checked each level to ensure that they did not reflect that of ALL:
>
> server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
> server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
> server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
>
> *****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
>
> Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
>
>
> Thanks,
>
> Ryan Pridgeon
>
>
Re: Review Request 37058: Grant select on Server results in ALL for
server
Posted by Ryan Pridgeon <rn...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37058/
-----------------------------------------------------------
(Updated Aug. 4, 2015, 4:04 p.m.)
Review request for sentry.
Changes
-------
Added updated diff
Repository: sentry
Description
-------
SENTRY-827: Allow Select,Insert and ALL on Server scope priviliges.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 2a60a23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9c2d384
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 09b3d99
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
Diff: https://reviews.apache.org/r/37058/diff/
Testing
-------
Grant ALL , SELECT and INSERT to three different roles, mapped to three different roles:
ADMINGROUP:server_all
USERGROUP1:server_select
USERGROUP2:server_insert
I then checked each level to ensure that they did not reflect that of ALL:
server_select: Pass SELECT * ; Fail LOAD DATA IN PATH
server_insert: Fail SELECT * ; Pass LOAD DATA IN PATH
server_all: Pass SELECT * ; Pass LOAD DATA IN PATH
*****admiditly someone had already remedied this. You could still only revoke ALL form the server scope however***
Lastly I ensured that ADMINGROUP could revoke the individual privilige from the server scope.
Thanks,
Ryan Pridgeon