You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sun Dapeng <da...@intel.com> on 2014/08/22 11:32:12 UTC
Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/
-----------------------------------------------------------
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
The patch include:
* SENTRY Thrift API changed :
1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
* Change {{SentryStore}} after Thrift API changed
* Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
* Change {{Auditlog}} after Thrift API changed
Diffs
-----
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java 62b6b31
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java bbd9536
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderRequest.java 10ab56b
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderResponse.java 4c571c2
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java d34205a
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java 13f22ff
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryActiveRoleSet.java 573dc26
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryRole.java f43a6d5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 4b1d7de
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 33600e9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java d4c5806
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java f227a02
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift b14616b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 5244094
Diff: https://reviews.apache.org/r/24967/diff/
Testing
-------
Unit Tests in local
Thanks,
Sun Dapeng
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/#review61833
-----------------------------------------------------------
I am still a bit skepticle about using the combinitions of fields to determin version mismatch. It's hard to distinguish the old client vs new client setting wrong fields. Note that Sentry service has non java clients like Hue that use the raw thrift calls and no the sentry client methods.
IMO we should increment the rpc version, and use the version to detect the invalid field settings.
Any case, I am fine with doing this in a followup patch. The current code should work fine with new client and new server combinition.
- Prasad Mujumdar
On Nov. 11, 2014, 3:13 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24967/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2014, 3:13 p.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch include:
> * SENTRY Thrift API changed :
> 1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
>
> 2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
>
> * Change {{SentryStore}} after Thrift API changed
>
> * Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
>
> * Change {{Auditlog}} after Thrift API changed
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 9f16b73
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f6699d2
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 39371b7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b20e71e
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 7e6ade5
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 2e829d6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 50ee559
>
> Diff: https://reviews.apache.org/r/24967/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/
-----------------------------------------------------------
(Updated 十一月 11, 2014, 11:13 p.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
Update **SentryPolicyStoreProcessor.java** to make it forward compatible.
Repository: sentry
Description
-------
The patch include:
* SENTRY Thrift API changed :
1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
* Change {{SentryStore}} after Thrift API changed
* Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
* Change {{Auditlog}} after Thrift API changed
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 9f16b73
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f6699d2
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 39371b7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b20e71e
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 7e6ade5
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 2e829d6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 50ee559
Diff: https://reviews.apache.org/r/24967/diff/
Testing
-------
Unit Tests in local
Thanks,
Dapeng Sun
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Sun Dapeng <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/
-----------------------------------------------------------
(Updated 十月 29, 2014, 3:41 p.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
Rebased with master
Repository: sentry
Description
-------
The patch include:
* SENTRY Thrift API changed :
1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
* Change {{SentryStore}} after Thrift API changed
* Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
* Change {{Auditlog}} after Thrift API changed
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java 62b6b31
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java 5246889
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java bbd9536
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesByAuthRequest.java 2bd860e
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesByAuthResponse.java d0cfe0b
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderRequest.java 10ab56b
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderResponse.java 4c571c2
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java d34205a
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java 13f22ff
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryActiveRoleSet.java 573dc26
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilegeMap.java 50b4979
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryRole.java f43a6d5
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/service/thrift/sentry_common_serviceConstants.java 4fdeaeb
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 9f16b73
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 017cf08
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 39371b7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java d64d019
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 52eaeed
sentry-provider/sentry-provider-db/src/main/resources/sentry_common_service.thrift 9456274
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 7e6ade5
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java befecf4
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 50ee559
Diff: https://reviews.apache.org/r/24967/diff/
Testing
-------
Unit Tests in local
Thanks,
Sun Dapeng
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Dapeng Sun <da...@intel.com>.
> On 十月 5, 2014, 3:01 p.m., Prasad Mujumdar wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift, line 109
> > <https://reviews.apache.org/r/24967/diff/2/?file=683821#file683821line109>
> >
> > This will make the older clients incompatible with new service.
> > We should change the protocol version to V2 so that the older client can get a more meaningful error.
Hi Prasad, I updated **SentryPolicyStoreProcessor** and **sentry_policy_service.thrift** to make the API forward compatible, because if the old version client try to connect the new version server, it will be blocked by **privileges is unset** before we compared the version of request in **SentryPolicyStoreProcessor**. In my new patch, I keep the fields **TSentryPrivilege privilege** and ** set< TSentryPrivilege> privileges** and made them **optional**, so it could goto the method in **SentryPolicyStoreProcessor**.
Thank you for your review, if you have any comment, feel free to let me know.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/#review55467
-----------------------------------------------------------
On 十一月 11, 2014, 11:13 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24967/
> -----------------------------------------------------------
>
> (Updated 十一月 11, 2014, 11:13 p.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch include:
> * SENTRY Thrift API changed :
> 1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
>
> 2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
>
> * Change {{SentryStore}} after Thrift API changed
>
> * Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
>
> * Change {{Auditlog}} after Thrift API changed
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 9f16b73
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f6699d2
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 39371b7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b20e71e
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 7e6ade5
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 2e829d6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 50ee559
>
> Diff: https://reviews.apache.org/r/24967/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/#review55467
-----------------------------------------------------------
Looks fine. Just one comment below.
Also you'll need to rebase the patch since the thrift interface has other change. Please regenerate the thirft java code after rebase.
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
<https://reviews.apache.org/r/24967/#comment95843>
This will make the older clients incompatible with new service.
We should change the protocol version to V2 so that the older client can get a more meaningful error.
- Prasad Mujumdar
On Sept. 9, 2014, 10:15 a.m., Sun Dapeng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24967/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 10:15 a.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch include:
> * SENTRY Thrift API changed :
> 1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
>
> 2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
>
> * Change {{SentryStore}} after Thrift API changed
>
> * Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
>
> * Change {{Auditlog}} after Thrift API changed
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java 62b6b31
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java bbd9536
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderRequest.java 10ab56b
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderResponse.java 4c571c2
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java d34205a
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java 13f22ff
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryActiveRoleSet.java 573dc26
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryRole.java f43a6d5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 4b1d7de
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 718306d
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 6895927
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 070c494
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift b14616b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 985a73d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 5244094
>
> Diff: https://reviews.apache.org/r/24967/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Sun Dapeng
>
>
Re: Review Request 24967: SENTRY-390 Extend Thrift API to support
column-level privilege
Posted by Sun Dapeng <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24967/
-----------------------------------------------------------
(Updated 九月 9, 2014, 6:15 p.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
rebase on SENTRY-407
Repository: sentry
Description
-------
The patch include:
* SENTRY Thrift API changed :
1. We change the field {{TSentryPrivilege privilege}} to {{set<TSentryPrivilege> privileges}} in {{TAlterSentryRoleGrantPrivilegeRequest}} and {{TAlterSentryRoleRevokePrivilegeRequest}}, The reason is the HIVE GRANT may like {{Grant SELECT (tb1.col1, tb2.col2) on TABLE table1 to role roleName}}, it contains two privileges ({{col1}} and {{col2}}) for SENTRY, to reduce the request API calls, we make it change.
2. Another way to Implement it, maybe add a {{column list}} to {{TSentryPrivilege}}, but it will bring more problems, we know SentryStore has many convert methods between {{TSentryPrivilege}} and {{MSentryPrivilege}}, and query an unique {{MSentryPrivilege}} use {{TSentryPrivilege}} as query condition, so we should make them one-to-one correspondence.
* Change {{SentryStore}} after Thrift API changed
* Change {{SentryPolicyStoreProcessor}} and {{SentryPolicyServiceClient}} after Thrift API changed, include the grant/revoke methods about column privilege
* Change {{Auditlog}} after Thrift API changed
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java 62b6b31
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java bbd9536
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderRequest.java 10ab56b
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderResponse.java 4c571c2
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java d34205a
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java 13f22ff
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryActiveRoleSet.java 573dc26
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryRole.java f43a6d5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java e1d8a9e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 2cc8194
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 841eeb3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 4b1d7de
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 718306d
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 6895927
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 070c494
sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift b14616b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java cd0a435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java fc9c716
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java 5da8591
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 985a73d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 5244094
Diff: https://reviews.apache.org/r/24967/diff/
Testing
-------
Unit Tests in local
Thanks,
Sun Dapeng