----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43892/#review121203 -----------------------------------------------------------
Thank Ashish, I left some comments. sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java (line 23) <https://reviews.apache.org/r/43892/#comment182869> Make it public? Uppercase the variable name? sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java (line 48) <https://reviews.apache.org/r/43892/#comment182870> Should we throw an exception if we the cluster name(may ignore case sensitive) was not expected. sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java (line 63) <https://reviews.apache.org/r/43892/#comment182871> It's better to add a unit test for if cluster name isn't "kafka-cluster" - Dapeng Sun On 二月 24, 2016, 3:21 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43892/ > ----------------------------------------------------------- > > (Updated 二月 24, 2016, 3:21 a.m.) > > > Review request for sentry, Dapeng Sun and Hao Hao. > > > Bugs: SENTRY-1030 > https://issues.apache.org/jira/browse/SENTRY-1030 > > > Repository: sentry > > > Description > ------- > > SENTRY-1030: Restrict Kafka Cluster authorizable to only have kafka-cluster > as authorizable's name. > > NOTE that this also includes changes from SENTRY-1056 and SENTRY-1098. I will > rebase once those two are committed. > > > Diffs > ----- > > sentry-binding/sentry-binding-kafka/pom.xml > bd24c20edcbffb5caee69aedddb93cb5a6c8c112 > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java > 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java > 9e72d7890e0db3710ab0971ccbb71a32d3d36e87 > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java > 92e50e6452a89e676eb559a3657b17ba563bfddc > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java > e75ec7eddaa1a48b731d551736859a94c08af593 > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java > bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1 > > sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java > 20d5e8e518cb91b32fad930e9d39400764f40b55 > > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java > f1ed0001883e5d4b362389c258f4f72697a1e5a4 > > sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java > 513c2719424f994e8eb773e72073401b465b7d58 > > sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java > bcc119860860d928d092ed3ddc0a5a28e841c9aa > > Diff: https://reviews.apache.org/r/43892/diff/ > > > Testing > ------- > > Ran unit tests. > > > Thanks, > > Ashish Singh > >
