You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Prasad Mujumdar <pr...@cloudera.com> on 2014/06/13 10:50:12 UTC
Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/
-----------------------------------------------------------
Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
Bugs: SENTRY-162
https://issues.apache.org/jira/browse/SENTRY-162
Repository: sentry
Description
-------
Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
The new API is exposed over thrift and implemented in sentry service and client
Metastore post even hook call this API to remove the privileges related to object being dropped
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 15a2e43
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
Diff: https://reviews.apache.org/r/22550/diff/
Testing
-------
Added unit tests and E2E tests.
Thanks,
Prasad Mujumdar
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/
-----------------------------------------------------------
(Updated June 23, 2014, 9:16 a.m.)
Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
Changes
-------
Fixed broken tests. The metastore hook is not enabled for Hive e2e tests by default. This will require a major refactoring of tests to align object create/drop properly.
Bugs: SENTRY-162
https://issues.apache.org/jira/browse/SENTRY-162
Repository: sentry
Description
-------
Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
The new API is exposed over thrift and implemented in sentry service and client
Metastore post even hook call this API to remove the privileges related to object being dropped
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java eb9ef00
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cf381e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 2db73c6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 724dfa9
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 144e20e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
Diff: https://reviews.apache.org/r/22550/diff/
Testing
-------
Added unit tests and E2E tests.
Thanks,
Prasad Mujumdar
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46374
-----------------------------------------------------------
Ship it!
Ship It!
- Sravya Tirukkovalur
On June 21, 2014, 9:20 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 21, 2014, 9:20 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cf381e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 2db73c6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 724dfa9
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 144e20e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/
-----------------------------------------------------------
(Updated June 21, 2014, 9:20 a.m.)
Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
Changes
-------
- It turns out that we need to set the action in TPrivilege before after 281 and 303 patches. Updated that
- Fixed an issue in 281 patches related to determining DB privilege. That was causing an overlap privilege test case to fail
- Updated new test case to add missing role definitions
TestSentryStore test passes with that updated patch.
Bugs: SENTRY-162
https://issues.apache.org/jira/browse/SENTRY-162
Repository: sentry
Description
-------
Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
The new API is exposed over thrift and implemented in sentry service and client
Metastore post even hook call this API to remove the privileges related to object being dropped
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cf381e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 2db73c6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 724dfa9
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 144e20e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
Diff: https://reviews.apache.org/r/22550/diff/
Testing
-------
Added unit tests and E2E tests.
Thanks,
Prasad Mujumdar
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On June 20, 2014, 11:57 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1329
> > <https://reviews.apache.org/r/22550/diff/4/?file=613065#file613065line1329>
> >
> > Do we want to also drop the privilege objects from jdo along with removing the mapping?
Currently we don't do this even for explicit revoke. The privilege.roles for some reason is not populated by data nucleus. We need some ref counting in order to identify orphaned privilege.
There's a separate ticket to track that already.
> On June 20, 2014, 11:57 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1337
> > <https://reviews.apache.org/r/22550/diff/4/?file=613065#file613065line1337>
> >
> > How are we handling action here? Authorizable doesnt have the action context where as action is required in TSentryPrivilege.
It gets handled in constructPrivilegeName() we set action to ALL if it's not set.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46346
-----------------------------------------------------------
On June 19, 2014, 6:10 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 6:10 p.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java fb8cfc2
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 27f617f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46346
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81676>
Do we want to also drop the privilege objects from jdo along with removing the mapping?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81674>
How are we handling action here? Authorizable doesnt have the action context where as action is required in TSentryPrivilege.
- Sravya Tirukkovalur
On June 19, 2014, 6:10 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 6:10 p.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java fb8cfc2
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 27f617f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46214
-----------------------------------------------------------
Ship it!
Looks good Prasad !!
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81513>
Nit : this might not be required...
Since in the sentry_policy_service.thrift, we mark action as a required field.. maybe we should put the default value of '*' in the thrift file itself ?
Not really important.. so feel free to drop
- Arun Suresh
On June 19, 2014, 6:10 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 6:10 p.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java fb8cfc2
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 27f617f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/
-----------------------------------------------------------
(Updated June 19, 2014, 6:10 p.m.)
Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
Changes
-------
- Addressed review feedback
- action is stored as lowercase in the privilege name
- Added test cases for SELECT on DB privilege
Bugs: SENTRY-162
https://issues.apache.org/jira/browse/SENTRY-162
Repository: sentry
Description
-------
Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
The new API is exposed over thrift and implemented in sentry service and client
Metastore post even hook call this API to remove the privileges related to object being dropped
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java fb8cfc2
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 27f617f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
Diff: https://reviews.apache.org/r/22550/diff/
Testing
-------
Added unit tests and E2E tests.
Thanks,
Prasad Mujumdar
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On June 19, 2014, 1:36 a.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1145
> > <https://reviews.apache.org/r/22550/diff/3/?file=610511#file610511line1145>
> >
> > Same as above
Updated
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46165
-----------------------------------------------------------
On June 16, 2014, 1:34 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 1:34 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46165
-----------------------------------------------------------
It looks good Prasad !!
You might want to rebase with head though..
SENTRY-281 handles the case where if there already exists a ALL privilege for a Table, a SELECT/INSERT will be ignored..
Interested to see how your testcase functions (especially the Overlapped privilege one) once you rebase...
Thanks !!
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81412>
nit : I think we should use the safeLowerTrim() static method
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81414>
Aah.. looks like you are doing a toUpper(). I seem to remember a case where this was needed..
But I think we shouldn't be doing this.. since we are doing a toLower before storing it.
Maybe unrelated to this patch.. but we should clean this up and make it consistent
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81417>
You might have to do this for Db also.. once SENTRY-303 is committed
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81415>
Same as above
- Arun Suresh
On June 16, 2014, 1:34 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 1:34 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review46194
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment81482>
Agree. will log a follow up ticket to clean this.
- Prasad Mujumdar
On June 16, 2014, 1:34 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 1:34 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/
-----------------------------------------------------------
(Updated June 16, 2014, 1:34 a.m.)
Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
Changes
-------
Updated patch -
- Handle rename table by recreating same privileges for new table
- Merge the drop/recreate privileges in a single transaction
- Added/fixed tests
Bugs: SENTRY-162
https://issues.apache.org/jira/browse/SENTRY-162
Repository: sentry
Description
-------
Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
The new API is exposed over thrift and implemented in sentry service and client
Metastore post even hook call this API to remove the privileges related to object being dropped
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java c126743
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java b6c985e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
Diff: https://reviews.apache.org/r/22550/diff/
Testing
-------
Added unit tests and E2E tests.
Thanks,
Prasad Mujumdar
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On June 13, 2014, 5:44 p.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1089
> > <https://reviews.apache.org/r/22550/diff/2/?file=608520#file608520line1089>
> >
> > Does this really delete the actual Privilege row from the table ?
The current patch doesn't. We do have another ticket for tracking that in general.
> On June 13, 2014, 5:44 p.m., Arun Suresh wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1101
> > <https://reviews.apache.org/r/22550/diff/2/?file=608520#file608520line1101>
> >
> > Is it possible to iterate of the roleset here and do a privilege.removeRole() ?
> > In which case, I guess it would be included in the same transaction..
> >
Going to update the patch will handles the drop/rename in a single transaction.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review45632
-----------------------------------------------------------
On June 13, 2014, 8:50 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 8:50 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 15a2e43
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22550: SENTRY-162: Cleanup DB store privilege metadata on
Hive DDL statements
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22550/#review45632
-----------------------------------------------------------
It look good !!
Just a few minor comments from my side..
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment80526>
Does this really delete the actual Privilege row from the table ?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/22550/#comment80527>
Is it possible to iterate of the roleset here and do a privilege.removeRole() ?
In which case, I guess it would be included in the same transaction..
- Arun Suresh
On June 13, 2014, 8:50 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22550/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 8:50 a.m.)
>
>
> Review request for sentry, Arun Suresh and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-162
> https://issues.apache.org/jira/browse/SENTRY-162
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Sentry store supports a new method to drop the privileges and role mapping for given Authorizable object.
> The new API is exposed over thrift and implemented in sentry service and client
> Metastore post even hook call this API to remove the privileges related to object being dropped
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 6679193
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 15a2e43
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 097056b
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 86ff221
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 35ba83a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 0165806
>
> Diff: https://reviews.apache.org/r/22550/diff/
>
>
> Testing
> -------
>
> Added unit tests and E2E tests.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>