----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43064/#review117341 -----------------------------------------------------------
Thank you for your patch, I left some comments below. sentry-binding/sentry-binding-kafka/pom.xml (line 74) <https://reviews.apache.org/r/43064/#comment178495> The version should be added to dependencyManagement in {project dir}/pom.xml sentry-binding/sentry-binding-kafka/pom.xml (line 79) <https://reviews.apache.org/r/43064/#comment178496> The version should be added to dependencyManagement in {project dir}/pom.xml sentry-binding/sentry-binding-kafka/pom.xml (line 74) <https://reviews.apache.org/r/43064/#comment178745> The version should be added to dependencyManagement in {project dir}/pom.xml sentry-binding/sentry-binding-kafka/pom.xml (line 79) <https://reviews.apache.org/r/43064/#comment178746> The version should be added to dependencyManagement in {project dir}/pom.xml sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/ConvertUtil.java (line 40) <https://reviews.apache.org/r/43064/#comment178747> I think we have change the type name of Group to "CONSUMERGROUP", could we return resourceTypeName? sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java (line 210) <https://reviews.apache.org/r/43064/#comment178754> The KafkaAuthorizer would be run by Kafka service user, if the getLoginUser() will return the Kafka service user, it will not have any authorization for acl operations. I think we can remove the acl operation, if the acl operations is needed at Kafka side, we may need to add shell commands likes SENTRY-399,SENTRY-995 - Dapeng Sun On 二月 3, 2016, 3:12 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43064/ > ----------------------------------------------------------- > > (Updated 二月 3, 2016, 3:12 a.m.) > > > Review request for sentry and Dapeng Sun. > > > Bugs: SENTRY-1011 > https://issues.apache.org/jira/browse/SENTRY-1011 > > > Repository: sentry > > > Description > ------- > > SENTRY-1011: Add Kafka binding > > > Diffs > ----- > > pom.xml aa99e313d8ae98bf048ce7d78574a9398f22732e > sentry-binding/pom.xml 0f2a987668cb3ea13d921636dfbc063bd17a3790 > sentry-binding/sentry-binding-kafka/pom.xml PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/ConvertUtil.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/MockGroupMappingServiceProvider.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/ConvertUtilTest.java > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java > PRE-CREATION > sentry-binding/sentry-binding-kafka/src/test/resources/core-site.xml > PRE-CREATION > sentry-binding/sentry-binding-kafka/src/test/resources/log4j.properties > PRE-CREATION > sentry-binding/sentry-binding-kafka/src/test/resources/sentry-site.xml > PRE-CREATION > > sentry-binding/sentry-binding-kafka/src/test/resources/test-authz-provider.ini > PRE-CREATION > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java > 6409015a92c94d37f21e0dbc8f7d7de74e3be5ec > > Diff: https://reviews.apache.org/r/43064/diff/ > > > Testing > ------- > > Tested with unit and e2e tests. > > > Thanks, > > Ashish Singh > >
