> On June 27, 2018, 5 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> > Lines 311 (patched)
> > <https://reviews.apache.org/r/67759/diff/2/?file=2046406#file2046406line311>
> >
> > Sergio, maybe this is the right opportunity to have a single method
> > instead of one for roles and one for users and return something like
> > Map<TSentryPrincipal, Set<TSentryPrivileges>> ?
I thought about that early, but the intention of this API is that you could get
either all roles and privileges or all users and privileges. If we do return
all users and roles, then the caller of this API would need to separate roles
and users which would take O(n) time where n is the number of roles + users.
For instance:
Map<TSentryPrincipal, Set<TSentryPrivileges>> privileges =
client.list_all_privileges();
Map<String, Set<TSentryPrivileges>> allRoles, allUsers;
for (TSentryPrincipal princ : privileges.keySet()) {
if (princ.getType() == SentryPrincipal.ROLE)
allRoles.put(princ.getName(), privileges.get(princ));
else if (princ.getType() == SentryPrincipal.USER)
allUsers.put(princ.getName(), privileges.get(princ));
else
;// principal not supported
}
With the above code, now the caller can check if a specific role or a
specific user has privileges, or display only user privileges or only role
privileges.
That's why I didn't go with the above solution. If I go to the current one,
then the caller would not need to split users vs roles, and it will be able to
check privileges in O(1).
Another idea is to add in the request to return only user privileges or only
role privileges, but how can the caller made sure the returned principals are
for user or for roles? They would need to check if the princ.geType() is equals
to USER or ROLE everytime just to make sure there were no errors.
So, I prefered to sacrify two API calls instead of 1 to let the caller have a
simple data type with either roles or users.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67759/#review205452
-----------------------------------------------------------
On June 27, 2018, 4:49 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67759/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 4:49 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2284
> https://issues.apache.org/jira/browse/sentry-2284
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add two API methods to the Sentry client:
> - listAllRolesPrivileges
> - listAllUsersPrivileges
>
> Both methods return a map object of the form:
> - [roleName, set<privileges>]
> - [userName, set<privileges>]
>
> Unit tests, thrift API code and Client methods are provided.
>
>
> Diffs
> -----
>
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java
> d456d7ff64a8146473ff14a65f06e7cb664939b7
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesRequest.java
> PRE-CREATION
>
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrivilegesResponse.java
> PRE-CREATION
>
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
> dc1d67ba16c9c7acf76eb3318864ba0606e2aa5a
>
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
> 5f76cb0aa3bd17d6aca4adc52ba59ded5ec0b900
>
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift
> 56aedcb5267495e610e3dace1f38e708d68ffe84
>
> sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java
> 1666e326462b771674930e52e3790ca92f467778
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> 39271500df179e0ddf8ce4f1589eaaa8137afa25
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> d8ab1fc90255c6ed42c00e3d3aa6103e47a40b29
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
> f61ae575c99474a76886d3c7a74765fdb067acd6
>
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java
> de4e0012d521cf402f4773a04b673f8056a5337c
>
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
> 52ce72cebc8b69f08988b05ba54e2bbedd201c1a
>
>
> Diff: https://reviews.apache.org/r/67759/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergio Pena
>
>
