Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Sergio Pena via Review Board]

> On Nov. 30, 2018, 5:46 p.m., kalyan kumar kalvagadda wrote:
> > I understand there is a gap here but we should be carefull in changing the 
> > current behavior.I assume, the idea here is to find a way to remove all the 
> > privileges granted to an authrozable(server/database/table)
> > 
> > Using "ALL" to soleve is not backward compatable. There could be customers 
> > who want to remove only "ALL" privilege and nothing else when "ALL" is 
> > revoked. Instead we should try to solve this with out using "ALL".
> 
> Arjun Mishra wrote:
> Thanks Kalyan. I'll put this in the backburner.

How do other databases work with the ALL privilege?


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/#review210983
---


On Nov. 30, 2018, 5:21 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69415/
> ---
> 
> (Updated Nov. 30, 2018, 5:21 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2463
> https://issues.apache.org/jira/browse/SENTRY-2463
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Right now if we initially grant ALL and revoke SELECT or INSERT, the 
> privilege gets "modified" to INSERT or SELECT. However, conversely if we 
> initially grant SELECT or INSERT and revoke ALL, no privileges are dropped
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2d6c85ac 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  6a7c1f392 
> 
> 
> Diff: https://reviews.apache.org/r/69415/diff/2/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
> mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Arjun Mishra via Review Board]

> On Nov. 30, 2018, 5:46 p.m., kalyan kumar kalvagadda wrote:
> > I understand there is a gap here but we should be carefull in changing the 
> > current behavior.I assume, the idea here is to find a way to remove all the 
> > privileges granted to an authrozable(server/database/table)
> > 
> > Using "ALL" to soleve is not backward compatable. There could be customers 
> > who want to remove only "ALL" privilege and nothing else when "ALL" is 
> > revoked. Instead we should try to solve this with out using "ALL".

Thanks Kalyan. I'll put this in the backburner.


- Arjun


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/#review210983
---


On Nov. 30, 2018, 5:21 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69415/
> ---
> 
> (Updated Nov. 30, 2018, 5:21 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2463
> https://issues.apache.org/jira/browse/SENTRY-2463
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Right now if we initially grant ALL and revoke SELECT or INSERT, the 
> privilege gets "modified" to INSERT or SELECT. However, conversely if we 
> initially grant SELECT or INSERT and revoke ALL, no privileges are dropped
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2d6c85ac 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  6a7c1f392 
> 
> 
> Diff: https://reviews.apache.org/r/69415/diff/2/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
> mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[kalyan kumar kalvagadda via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/#review210983
---



I understand there is a gap here but we should be carefull in changing the 
current behavior.I assume, the idea here is to find a way to remove all the 
privileges granted to an authrozable(server/database/table)

Using "ALL" to soleve is not backward compatable. There could be customers who 
want to remove only "ALL" privilege and nothing else when "ALL" is revoked. 
Instead we should try to solve this with out using "ALL".

- kalyan kumar kalvagadda


On Nov. 30, 2018, 5:21 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69415/
> ---
> 
> (Updated Nov. 30, 2018, 5:21 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2463
> https://issues.apache.org/jira/browse/SENTRY-2463
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Right now if we initially grant ALL and revoke SELECT or INSERT, the 
> privilege gets "modified" to INSERT or SELECT. However, conversely if we 
> initially grant SELECT or INSERT and revoke ALL, no privileges are dropped
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2d6c85ac 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  6a7c1f392 
> 
> 
> Diff: https://reviews.apache.org/r/69415/diff/2/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
> mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/
---

(Updated Nov. 30, 2018, 5:21 p.m.)


Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
Sergio Pena.


Changes
---

Addressed Lina's comments


Bugs: SENTRY-2463
https://issues.apache.org/jira/browse/SENTRY-2463


Repository: sentry


Description
---

Right now if we initially grant ALL and revoke SELECT or INSERT, the privilege 
gets "modified" to INSERT or SELECT. However, conversely if we initially grant 
SELECT or INSERT and revoke ALL, no privileges are dropped


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 e2d6c85ac 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
 6a7c1f392 


Diff: https://reviews.apache.org/r/69415/diff/2/

Changes: https://reviews.apache.org/r/69415/diff/1-2/


Testing
---

mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore


Thanks,

Arjun Mishra

Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Haley Reeve via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/#review210899
---


Ship it!




Ship It!

- Haley Reeve


On Nov. 26, 2018, 7:36 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69415/
> ---
> 
> (Updated Nov. 26, 2018, 7:36 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-2463
> https://issues.apache.org/jira/browse/SENTRY-2463
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Right now if we initially grant ALL and revoke SELECT or INSERT, the 
> privilege gets "modified" to INSERT or SELECT. However, conversely if we 
> initially grant SELECT or INSERT and revoke ALL, no privileges are dropped
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2d6c85ac 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  6a7c1f392 
> 
> 
> Diff: https://reviews.apache.org/r/69415/diff/1/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
> mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Na Li via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/#review210870
---




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
Lines 2303 (patched)


you should test "insert", "create" as well



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
Lines 2311 (patched)


You don't need to close statement and connection for each command, and then 
created for the same user. You can do it only at the end of the test.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
Lines 2350 (patched)


test "insert" and "create"


- Na Li


On Nov. 20, 2018, 7:47 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69415/
> ---
> 
> (Updated Nov. 20, 2018, 7:47 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2463
> https://issues.apache.org/jira/browse/SENTRY-2463
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Right now if we initially grant ALL and revoke SELECT or INSERT, the 
> privilege gets "modified" to INSERT or SELECT. However, conversely if we 
> initially grant SELECT or INSERT and revoke ALL, no privileges are dropped
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2d6c85ac 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
>  6a7c1f392 
> 
> 
> Diff: https://reviews.apache.org/r/69415/diff/1/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
> mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Review Request 69415: SENTRY-2463: Revoking ALL or * should revoke any other privilege on the entity

[Arjun Mishra via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69415/
---

Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Bugs: SENTRY-2463
https://issues.apache.org/jira/browse/SENTRY-2463


Repository: sentry


Description
---

Right now if we initially grant ALL and revoke SELECT or INSERT, the privilege 
gets "modified" to INSERT or SELECT. However, conversely if we initially grant 
SELECT or INSERT and revoke ALL, no privileges are dropped


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 e2d6c85ac 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
 6a7c1f392 


Diff: https://reviews.apache.org/r/69415/diff/1/


Testing
---

mvn -f sentry-tests/sentry-tests-hive/pom.xml test -Dtest=TestDatabaseProvider
mvn -f sentry-service/sentry-service-server/pom.xml test -Dtest=TestSentryStore


Thanks,

Arjun Mishra