Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-03-01 Thread Colm O hEigeartaigh

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


Ship it!




Ship It!

- Colm O hEigeartaigh


On Feb. 27, 2018, 11:17 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated Feb. 27, 2018, 11:17 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml 294c422f 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/3/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-03-01 Thread Colm O hEigeartaigh

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



Looks good, I tested Kerberos + Kafka 1.0.0 + Ranger authorization.

- Colm O hEigeartaigh


On Feb. 27, 2018, 11:17 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated Feb. 27, 2018, 11:17 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml 294c422f 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/3/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-27 Thread Zsombor Gegesy

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

(Updated Feb. 27, 2018, 11:17 a.m.)


Review request for ranger.


Changes
---

Implementing suggested changes


Bugs: RANGER-1967
https://issues.apache.org/jira/browse/RANGER-1967


Repository: ranger


Description
---

The API changes are not too big, however running Kafka in embedded mode turned 
out to be a bit trickier beast. I had to add a couple of new flags, otherwise 
it would wait for 2 other broker to join.
 The commit also contains logging changes, even a test log4j configuration, 
just to make the debugging easier.


Diffs (updated)
-

  agents-audit/pom.xml 4fa44c5f 
  plugin-kafka/pom.xml f644646b 
  
plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
 b3d5a74d 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
 4ea39ed7 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
 fb541cd3 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
 fb0a2c0f 
  plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
  pom.xml 294c422f 
  ranger-kafka-plugin-shim/pom.xml f1aeee6f 


Diff: https://reviews.apache.org/r/65777/diff/3/

Changes: https://reviews.apache.org/r/65777/diff/2-3/


Testing
---

Unit & local tests


Thanks,

Zsombor Gegesy



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-27 Thread Zsombor Gegesy


> On Feb. 26, 2018, 4:30 p.m., Colm O hEigeartaigh wrote:
> > > JaasContext context = JaasContext.load(Type.SERVER, new 
> > > ListenerName("KafkaServer"), configs);
> > 
> > Should "KafkaServer" be configurable here?
> > 
> > Apart from that looks good to me - have you tested the plugin with a Kafka 
> > 1.0.0 deployment?
> 
> Qiang Zhang wrote:
> We configure listeners in server.properties as following:
> listeners=SASL_PLAINTEXT://HDC90:9092
> 
> In order to above function, I write following code and execute succefully:
> String listeners = (String)configs.get("listeners");
> ...
> JaasContext jaasContext = JaasContext.load(JaasContext.Type.SERVER, new 
> ListenerName(listeners.split(":")[0]), configs);
> 
> ListenerName("KafkaServer")
> 
> "KafkaServer" should equal to SASL_PLAINTEXT.

Adding a configurable option for specifying ListenerName is a good idea. 
This hardcoded "KafkaServer" will force using the 'KafkaServer' JAAS 
configuration - which makes our tests happy. However, If the user could specify 
this JAAS configuration name, they could pick any, which suits them, if needed, 
however the SASL_PLAINTEXT seems a better default configuration.
As I understood, there could be multiple listeners in kafka, so blindly parsing 
the 'listeners' could cause some trouble.


- Zsombor


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


On Feb. 24, 2018, 10:08 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated Feb. 24, 2018, 10:08 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml fa1d1c7b 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/2/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-26 Thread Qiang Zhang


> On 二月 26, 2018, 4:30 p.m., Colm O hEigeartaigh wrote:
> > > JaasContext context = JaasContext.load(Type.SERVER, new 
> > > ListenerName("KafkaServer"), configs);
> > 
> > Should "KafkaServer" be configurable here?
> > 
> > Apart from that looks good to me - have you tested the plugin with a Kafka 
> > 1.0.0 deployment?

We configure listeners in server.properties as following:
listeners=SASL_PLAINTEXT://HDC90:9092

In order to above function, I write following code and execute succefully:
String listeners = (String)configs.get("listeners");
...
JaasContext jaasContext = JaasContext.load(JaasContext.Type.SERVER, new 
ListenerName(listeners.split(":")[0]), configs);

ListenerName("KafkaServer")

"KafkaServer" should equal to SASL_PLAINTEXT.


- Qiang


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


On 二月 24, 2018, 10:08 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated 二月 24, 2018, 10:08 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml fa1d1c7b 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/2/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-26 Thread Colm O hEigeartaigh

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



> JaasContext context = JaasContext.load(Type.SERVER, new 
> ListenerName("KafkaServer"), configs);

Should "KafkaServer" be configurable here?

Apart from that looks good to me - have you tested the plugin with a Kafka 
1.0.0 deployment?

- Colm O hEigeartaigh


On Feb. 24, 2018, 10:08 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated Feb. 24, 2018, 10:08 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml fa1d1c7b 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/2/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-24 Thread Ramesh Mani

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


Ship it!




Ship It!

- Ramesh Mani


On Feb. 24, 2018, 10:08 a.m., Zsombor Gegesy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65777/
> ---
> 
> (Updated Feb. 24, 2018, 10:08 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1967
> https://issues.apache.org/jira/browse/RANGER-1967
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> The API changes are not too big, however running Kafka in embedded mode 
> turned out to be a bit trickier beast. I had to add a couple of new flags, 
> otherwise it would wait for 2 other broker to join.
>  The commit also contains logging changes, even a test log4j configuration, 
> just to make the debugging easier.
> 
> 
> Diffs
> -
> 
>   agents-audit/pom.xml 4fa44c5f 
>   plugin-kafka/pom.xml f644646b 
>   
> plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
>  b3d5a74d 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
>  4ea39ed7 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
>  fb541cd3 
>   
> plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
>  fb0a2c0f 
>   plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
>   pom.xml fa1d1c7b 
>   ranger-kafka-plugin-shim/pom.xml f1aeee6f 
> 
> 
> Diff: https://reviews.apache.org/r/65777/diff/2/
> 
> 
> Testing
> ---
> 
> Unit & local tests
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>



Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-24 Thread Zsombor Gegesy

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

(Updated Feb. 24, 2018, 10:08 a.m.)


Review request for ranger.


Changes
---

Apache header for log4j.properties


Bugs: RANGER-1967
https://issues.apache.org/jira/browse/RANGER-1967


Repository: ranger


Description
---

The API changes are not too big, however running Kafka in embedded mode turned 
out to be a bit trickier beast. I had to add a couple of new flags, otherwise 
it would wait for 2 other broker to join.
 The commit also contains logging changes, even a test log4j configuration, 
just to make the debugging easier.


Diffs (updated)
-

  agents-audit/pom.xml 4fa44c5f 
  plugin-kafka/pom.xml f644646b 
  
plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
 b3d5a74d 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
 4ea39ed7 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
 fb541cd3 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
 fb0a2c0f 
  plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
  pom.xml fa1d1c7b 
  ranger-kafka-plugin-shim/pom.xml f1aeee6f 


Diff: https://reviews.apache.org/r/65777/diff/2/

Changes: https://reviews.apache.org/r/65777/diff/1-2/


Testing
---

Unit & local tests


Thanks,

Zsombor Gegesy



Review Request 65777: RANGER-1967 - Kafka 1.0 support

2018-02-23 Thread Zsombor Gegesy

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

Review request for ranger.


Bugs: RANGER-1967
https://issues.apache.org/jira/browse/RANGER-1967


Repository: ranger


Description
---

The API changes are not too big, however running Kafka in embedded mode turned 
out to be a bit trickier beast. I had to add a couple of new flags, otherwise 
it would wait for 2 other broker to join.
 The commit also contains logging changes, even a test log4j configuration, 
just to make the debugging easier.


Diffs
-

  agents-audit/pom.xml 4fa44c5f 
  plugin-kafka/pom.xml f644646b 
  
plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java
 b3d5a74d 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java
 4ea39ed7 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerSASLSSLTest.java
 fb541cd3 
  
plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerTest.java
 fb0a2c0f 
  plugin-kafka/src/test/resources/log4j.properties PRE-CREATION 
  pom.xml fa1d1c7b 
  ranger-kafka-plugin-shim/pom.xml f1aeee6f 


Diff: https://reviews.apache.org/r/65777/diff/1/


Testing
---

Unit & local tests


Thanks,

Zsombor Gegesy