Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15074 )

Change subject: [java] KUDU-2972: add Kudu Ranger plugin
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/build.gradle
File java/kudu-ranger/build.gradle:

http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/build.gradle@21
PS4, Line 21:   // Note: We don't use the shaded version to avoid protobuf 
message type
            :   // got shaded and causes incompatible bounds error for type 
casting.
> OK, so given that it produces a compilation error, we don't need to go into
Done


http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java
File 
java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java:

http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java@42
PS4, Line 42:
> Please doc the mocking requirement.
Done


http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java
File 
java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java:

http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@45
PS4, Line 45:   private static final String APP_ID = "kudu";
> Can you document that?
Done


http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@52
PS4, Line 52:   RangerBasePlugin plugin;
            :
            :   public RangerKuduAuthorizer() {
            :
> Could you document that requirement in the init() Javadoc?
Done


http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@61
PS4, Line 61:     plugin.init();
            :   }
            :
            :   /**
            :    *  Authorizes a given <code>RangerRequestListPB</code> in Ran
> FYI, Guava's @Nullable annotations are "self-documenting" and can also be u
Done


http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@126
PS4, Line 126:     final RangerAccessRequestImpl request = new 
RangerAccessRequestImpl();
             :     request.setResource(resource);
             :     request.setAccessType(action);
> Do we know which group mapping mechanism will be used here?

I think it is up to the setting of the cluster, it can be any of the available 
mechanisms. So it doesn't have to be JniBasedUnixGroupsMappingWithFallback. As 
far as I know a lot of users may use ldap. And this is not a requirement for 
Kudu master, it is only used in Ranger side. Similarly, for Sentry integration, 
we relies on Sentry to resolve the user group mapping which under the hood uses 
the hadoop user group mapping. However, it is not an option for Ranger as of 
now (we have to pass the group info).



--
To view, visit http://gerrit.cloudera.org:8080/15074
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c995ac1a48ebf57667231cd3a82d3794f6ddf8d
Gerrit-Change-Number: 15074
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 19 Feb 2020 21:05:18 +0000
Gerrit-HasComments: Yes

Reply via email to