----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20803/#review42102 -----------------------------------------------------------
Ship it! Looks fine to me. A couple of minor nits apart from the comments raised by Lenni. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java <https://reviews.apache.org/r/20803/#comment75845> Nit: Given that for now the the access will be restricted to admin only, we can remove this. The non-admin access can be tracked by a ticket sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift <https://reviews.apache.org/r/20803/#comment75846> I guess this is trusted impersonation of Sentry service. It trusts the connected users and allow them to submit a request on behalf of any other user. - Prasad Mujumdar On May 1, 2014, 1:04 a.m., Sravya Tirukkovalur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20803/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 1:04 a.m.) > > > Review request for sentry, Lenni Kuff and Prasad Mujumdar. > > > Bugs: SENTRY-184 > https://issues.apache.org/jira/browse/SENTRY-184 > > > Repository: sentry > > > Description > ------- > > This patch adds the following thrift API to query the list of privileges > granted for a given role: > * TListSentryPrivilegesResponse > list_sentry_privileges_by_role(1:TListSentryPrivilegesRequest request) > ** Allows to get granted privileges for a given role > > > It also fixes the issues with: > * TListSentryRolesResponse > list_sentry_roles_by_group(1:TListSentryRolesRequest request) > ** Looks up the roles for a given group. > ** Also can be used to list all roles in the system (when group_name = null) > > Getting rid of following API as we do not support mapping a role to multiple > roles. > * TListSentryRolesResponse > list_sentry_roles_by_role_name(1:TListSentryRolesRequest request) > > Outstanding issues: > > * We still need to decide on the semantics of "Show grant" and "show role > grant": > ** Right now, these meta data query operations can be performed only by the > admin user. But ideally we should allow even non admin users to query as long > as they are part of the group/role they are requesting info from. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 33c8d1af864282b2b56a54038573cfdcaa9a44f3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > 7e73b2c7195c1f4561a30dab018bb5ba0d68f3ec > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 6c52fa421eaa5d036b522f827a9d0f8433676275 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > 677047fe359e346c2c3cd9f7bcb6597288d83cc5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > dcaa246ea72e37c7aa7aed51f5c65b96bec61405 > > Diff: https://reviews.apache.org/r/20803/diff/ > > > Testing > ------- > > Added new tests. > > > Thanks, > > Sravya Tirukkovalur > >
