> On June 9, 2015, 7:13 a.m., Dapeng Sun wrote: > > core/src/main/scala/kafka/common/ErrorMapping.scala, line 57 > > <https://reviews.apache.org/r/34492/diff/4/?file=979556#file979556line57> > > > > Why not use 23 here?
I have changed it so it now uses 23. > On June 9, 2015, 7:13 a.m., Dapeng Sun wrote: > > core/src/main/scala/kafka/security/auth/Authorizer.scala, line 32 > > <https://reviews.apache.org/r/34492/diff/4/?file=979558#file979558line32> > > > > Resource related operations are authorized, but authorizer itself seems > > not be authorized. Could a normal user operated the ACL? we may need to > > add something (eg. Session) to addACL, removeACL and etc. Given add/edit/remove acls will be invoked from CLI and these are not server side APIs (how I wish I would have started with that) that authorization will be done in a different way. Its not really part of this CR but essentially we will have to rely on the acl storage layer's authorization for this (zookeeper in case of default authorizer implementation). > On June 9, 2015, 7:13 a.m., Dapeng Sun wrote: > > core/src/main/scala/kafka/security/auth/Authorizer.scala, line 60 > > <https://reviews.apache.org/r/34492/diff/4/?file=979558#file979558line60> > > > > I didn't found any code invoke "removeAcls", it seems Acl entities are > > not cleaned when resource(eg. topic) is deleting, is that really so? or I > > missed? It is part of the PR that implements CLI. > On June 9, 2015, 7:13 a.m., Dapeng Sun wrote: > > core/src/main/scala/kafka/security/auth/KafkaPrincipal.scala, line 23 > > <https://reviews.apache.org/r/34492/diff/4/?file=979559#file979559line23> > > > > I think it is better to use constants or enum to stand for UserType We are using constants by using val. Enum is avoided so a custom authorizer implementation could add any principalType that they wish without having to change code in kafka codebase. - Parth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34492/#review86541 ----------------------------------------------------------- On July 10, 2015, 1 a.m., Parth Brahmbhatt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34492/ > ----------------------------------------------------------- > > (Updated July 10, 2015, 1 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2210 > https://issues.apache.org/jira/browse/KAFKA-2210 > > > Repository: kafka > > > Description > ------- > > Addressing review comments from Jun. > > > Adding CREATE check for offset topic only if the topic does not exist already. > > > Addressing some more comments. > > > Removing acl.json file > > > Diffs > ----- > > core/src/main/scala/kafka/api/OffsetRequest.scala > f418868046f7c99aefdccd9956541a0cb72b1500 > core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION > core/src/main/scala/kafka/common/ErrorMapping.scala > c75c68589681b2c9d6eba2b440ce5e58cddf6370 > core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/KafkaPrincipal.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/PermissionType.java PRE-CREATION > core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION > core/src/main/scala/kafka/server/KafkaApis.scala > 18f5b5b895af1469876b2223841fd90a2dd255e0 > core/src/main/scala/kafka/server/KafkaConfig.scala > c1f0ccad4900d74e41936fae4c6c2235fe9314fe > core/src/main/scala/kafka/server/KafkaServer.scala > 18917bc4464b9403b16d85d20c3fd4c24893d1d3 > core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION > core/src/test/scala/unit/kafka/security/auth/KafkaPrincipalTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/security/auth/ResourceTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 98a5b042a710d3c1064b0379db1d152efc9eabee > > Diff: https://reviews.apache.org/r/34492/diff/ > > > Testing > ------- > > > Thanks, > > Parth Brahmbhatt > >
