ranger-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 73344: RANGER-3231 Ranger should use kafka Authorizer from KIP-504
Date Tue, 18 May 2021 00:12:20 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73344/#review223001
-----------------------------------------------------------




plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
Line 122 (original), 132 (patched)
<https://reviews.apache.org/r/73344/#comment312155>

    For consistency with rest of Ranger source code, and better readability, consider moving
private methods after all public and protected methods. Here are the coding guidelines for
Apache Ranger: https://cwiki.apache.org/confluence/display/RANGER/Coding+guidelines



plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
Lines 199 (patched)
<https://reviews.apache.org/r/73344/#comment312156>

    Since Scala Set is no more used, consider importing java.util.Set (similar to Map/List/..),
and drop the package name from here.



ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
Line 130 (original), 130 (patched)
<https://reviews.apache.org/r/73344/#comment312153>

    For consistency with rest of Ranger source code, and better readability, consider moving
private methods after all public and protected methods. Here are the coding guidelines for
Apache Ranger: https://cwiki.apache.org/confluence/display/RANGER/Coding+guidelines



ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
Line 149 (original), 144 (patched)
<https://reviews.apache.org/r/73344/#comment312154>

    Shim should simply forward all calls to the implementation instance. If this is not feasible
here, please add a comment. Else forward with:
    
      try {
        activatePluginClassLoader();
    
        return rangerKakfaAuthorizerImpl.start(authorizerServerInfo);
      } finally {
        deactivatePluginClassLoader();
      }


- Madhan Neethiraj


On May 10, 2021, 9:06 a.m., Chia-Ping Tsai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73344/
> -----------------------------------------------------------
> 
> (Updated May 10, 2021, 9:06 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> As described in the KIP, `org.apache.kafka.server.authorizer.Authorizer` is an improvement
over `kafka.security.auth.Authorizer` and it's a pure Java interface (instead of Scala).
> `kafka.security.auth.Authorizer` has been deprecated since December 2019 and it will
be removed in Apache Kafka 3.0 (roughly planned for July/August).
> See the KIP for more details:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-504+-+Add+new+Java+Authorizer+Interface
> 
> 
> Diffs
> -----
> 
>   plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
2a1b812e0 
>   ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
9d72ae0c8 
> 
> 
> Diff: https://reviews.apache.org/r/73344/diff/1/
> 
> 
> Testing
> -------
> 
> run `mvn clean test` and all pass on my local.
> 
> 
> Thanks,
> 
> Chia-Ping Tsai
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message