You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/09/04 19:02:14 UTC
Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/
-----------------------------------------------------------
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2315
https://issues.apache.org/jira/browse/sentry-2315
Repository: sentry
Description
-------
Fixes the issue where GRANT ALL does not include the ALTER privilege.
It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
Diff: https://reviews.apache.org/r/68624/diff/1/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Sept. 4, 2018, 9:49 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 818 (original), 818 (patched)
> > <https://reviews.apache.org/r/68624/diff/1/?file=2082219#file2082219line818>
> >
> > Do you know what AccessConstants.SOME is about? It seems to be the same as AccessConstants.ALL
I don't know about a use case or a grant syntax that uses the SOME keyword. Seems is like the all, but I don't know how it is used.
This line is here and it has not failed, so I should keep it this way.
> On Sept. 4, 2018, 9:49 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 866 (patched)
> > <https://reviews.apache.org/r/68624/diff/1/?file=2082219#file2082219line880>
> >
> > if action is ALL or ACTION_ALL, SHOULD you continue? Before calling this function, if the action to grant is all, and all is already granted, we can do nothing
No. This part of the code drops the privileges before granting the new one. If I leave ALL, then we might have two entries with ALL. OWNER is the special privilege that cannot be combined with ALL because means the same.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/#review208323
-----------------------------------------------------------
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Sept. 4, 2018, 9:49 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 866 (patched)
> > <https://reviews.apache.org/r/68624/diff/1/?file=2082219#file2082219line880>
> >
> > if action is ALL or ACTION_ALL, SHOULD you continue? Before calling this function, if the action to grant is all, and all is already granted, we can do nothing
>
> Sergio Pena wrote:
> No. This part of the code drops the privileges before granting the new one. If I leave ALL, then we might have two entries with ALL. OWNER is the special privilege that cannot be combined with ALL because means the same.
1) If the principle has "ALL with grant option", and to be granted with "ALL". If you remove existing one, then grant option is lost. Is this the correct behavior?
2) If the principle has "ALL", and to be granted with "ALL with grant option". If you remove existing one, then ALL with grant option is granted. I think this is the correct behavior.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/#review208323
-----------------------------------------------------------
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Sept. 4, 2018, 9:49 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 866 (patched)
> > <https://reviews.apache.org/r/68624/diff/1/?file=2082219#file2082219line880>
> >
> > if action is ALL or ACTION_ALL, SHOULD you continue? Before calling this function, if the action to grant is all, and all is already granted, we can do nothing
>
> Sergio Pena wrote:
> No. This part of the code drops the privileges before granting the new one. If I leave ALL, then we might have two entries with ALL. OWNER is the special privilege that cannot be combined with ALL because means the same.
>
> Na Li wrote:
> 1) If the principle has "ALL with grant option", and to be granted with "ALL". If you remove existing one, then grant option is lost. Is this the correct behavior?
> 2) If the principle has "ALL", and to be granted with "ALL with grant option". If you remove existing one, then ALL with grant option is granted. I think this is the correct behavior.
I don't think so. The code that is executed after all privileges are dropped is this:
mPrivilege = getMSentryPrivilege(privilege, pm);
if (mPrivilege == null) {
mPrivilege = convertToMSentryPrivilege(privilege);
}
mPrivilege.appendPrincipal(mPrincipal);
pm.makePersistent(mPrivilege);
The above code will get the privilege to be granted, if it does not exist (meaning if the all was dropped), then it will create a new ALL privilege with the grant option.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/#review208323
-----------------------------------------------------------
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208323
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 818 (original), 818 (patched)
<https://reviews.apache.org/r/68624/#comment292162>
Do you know what AccessConstants.SOME is about? It seems to be the same as AccessConstants.ALL
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 866 (patched)
<https://reviews.apache.org/r/68624/#comment292165>
if action is ALL or ACTION_ALL, SHOULD you continue? Before calling this function, if the action to grant is all, and all is already granted, we can do nothing
- Na Li
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208361
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Lines 927 (patched)
<https://reviews.apache.org/r/68624/#comment292246>
typo. "Invididual" should be "Individual".
- Na Li
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208375
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208378
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Lines 990 (patched)
<https://reviews.apache.org/r/68624/#comment292268>
change the comment to match actual behavior
- Na Li
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208379
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Lines 987 (patched)
<https://reviews.apache.org/r/68624/#comment292269>
this test only checks allWithGrant is present. It does not check "allPrivilege" is gone.
- Na Li
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208377
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Lines 984 (patched)
<https://reviews.apache.org/r/68624/#comment292267>
comment should be changed to match the actual behavior:
both "all" and "all with grant" privileges exist for that role
- Na Li
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Sept. 5, 2018, 8:50 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 871 (patched)
> > <https://reviews.apache.org/r/68624/diff/2/?file=2082565#file2082565line885>
> >
> > if tNotAll.grantOption is true, should you set tNotAll.grantOption to be true?
It is set automatically in "TSentryPrivilege tNotAll = new TSentryPrivilege(privilege);"
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/#review208374
-----------------------------------------------------------
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208374
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 871 (patched)
<https://reviews.apache.org/r/68624/#comment292265>
if tNotAll.grantOption is true, should you set tNotAll.grantOption to be true?
- Na Li
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/
-----------------------------------------------------------
(Updated Sept. 5, 2018, 4:57 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Changes
-------
Added test cases to verify that grant all replaces grant all with grant and viceversa.
Bugs: sentry-2315
https://issues.apache.org/jira/browse/sentry-2315
Repository: sentry
Description
-------
Fixes the issue where GRANT ALL does not include the ALTER privilege.
It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
Diffs (updated)
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
Diff: https://reviews.apache.org/r/68624/diff/2/
Changes: https://reviews.apache.org/r/68624/diff/1-2/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Sept. 5, 2018, 4:28 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
> > Lines 927 (patched)
> > <https://reviews.apache.org/r/68624/diff/1/?file=2082220#file2082220line927>
> >
> > Can you add the test cases that
> > 1) user already has "ALL with grant option", and then grant with "ALL", what happens?
> > 2) user already has "ALL", and then grant with "ALL with grant option", what happens?
Done, I added the new tests. They were no errors.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68624/#review208362
-----------------------------------------------------------
On Sept. 5, 2018, 4:57 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2018, 4:57 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68624: SENTRY-2315: The grant all operation is not
dropping the create/alter/drop/index/lock privileges.
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/68624/#review208362
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Lines 927 (patched)
<https://reviews.apache.org/r/68624/#comment292247>
Can you add the test cases that
1) user already has "ALL with grant option", and then grant with "ALL", what happens?
2) user already has "ALL", and then grant with "ALL with grant option", what happens?
- Na Li
On Sept. 4, 2018, 7:02 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68624/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2018, 7:02 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2315
> https://issues.apache.org/jira/browse/sentry-2315
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes the issue where GRANT ALL does not include the ALTER privilege.
>
> It does not fix the REVOKE command where revoking a single privilege will end up revoking ALL and granting all invididual supported privileges. For now, such behavior only works for SELECT and INSERT.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java db8c08ba0839400c65658e90ebd18906fed9919f
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 6b732ea5a89dafd94a6a3ae9c423747118352d84
>
>
> Diff: https://reviews.apache.org/r/68624/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>