Re: Review Request 65777: RANGER-1967 - Kafka 1.0 support
--- 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
--- 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
--- 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
> 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
> 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
--- 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
--- 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
--- 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
--- 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