> On Feb. 10, 2016, 3:04 a.m., Hao Hao wrote: > > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java, > > line 122 > > <https://reviews.apache.org/r/42924/diff/2/?file=1239150#file1239150line122> > > > > Why not make the Kafka action to be "*" instead of "ALL"? And here you > > probably want to use KafkaAuthorizable.ALL, so that no need extra checking > > for host?
'ALL' is defined as an action in Kafka, it is better to be inline with Kafka terminologies. I have refactored this logic a bit. Should make it easier to comprehend now. > On Feb. 10, 2016, 3:04 a.m., Hao Hao wrote: > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java, > > line 44 > > <https://reviews.apache.org/r/42924/diff/2/?file=1239146#file1239146line44> > > > > Can you add a comment on what "*" stands for? This is some left over code. Removed. - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42924/#review118530 ----------------------------------------------------------- On Feb. 10, 2016, 12:49 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42924/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2016, 12:49 a.m.) > > > Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff. > > > Bugs: SENTRY-1029 > https://issues.apache.org/jira/browse/SENTRY-1029 > > > Repository: sentry > > > Description > ------- > > SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative > tests for KafkaAction. > > > Diffs > ----- > > > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java > ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2 > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java > b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java > 9525aaff4f0b9b92212aa59919c2cb6078376b95 > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java > e0f4160269a652f2b4d0c1d7376b34ee80c0e10e > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java > 13421f9c0be31b372c910d304a081403bdea0998 > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java > 2577406fb3d3a4bbaab0f5733e7054c61498a22c > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java > 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce > > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java > 9e288b0c92b91964d2d8ab4e727d4f5257e47502 > > sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java > f22ebc03a27b5f08e501b40990c20fbf4372cddc > > sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java > 1abb116023e7a652684f981a6e8261abf8c4d8c5 > > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java > e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23 > > Diff: https://reviews.apache.org/r/42924/diff/ > > > Testing > ------- > > Added unit tests. > > > Thanks, > > Ashish Singh > >
