----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34492/#review86541 -----------------------------------------------------------
Hi Parth, thank you for your patch of authorization, and could you use "add comments" for replying Jun's comments? it will be easy to let the other person track the comments. core/src/main/scala/kafka/common/ErrorMapping.scala <https://reviews.apache.org/r/34492/#comment139453> Why not use 23 here? core/src/main/scala/kafka/security/auth/Authorizer.scala <https://reviews.apache.org/r/34492/#comment139464> 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. core/src/main/scala/kafka/security/auth/Authorizer.scala <https://reviews.apache.org/r/34492/#comment139458> 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? core/src/main/scala/kafka/security/auth/KafkaPrincipal.scala <https://reviews.apache.org/r/34492/#comment139452> I think it is better to use constants or enum to stand for UserType Thanks Dapeng - Dapeng Sun On 六月 5, 2015, 7:07 a.m., Parth Brahmbhatt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34492/ > ----------------------------------------------------------- > > (Updated 六月 5, 2015, 7:07 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. > > > Diffs > ----- > > core/src/main/scala/kafka/api/OffsetRequest.scala > 3d483bc7518ad76f9548772522751afb4d046b78 > 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.java 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.java PRE-CREATION > core/src/main/scala/kafka/server/KafkaApis.scala > 387e387998fc3a6c9cb585dab02b5f77b0381fbf > core/src/main/scala/kafka/server/KafkaConfig.scala > 6f25afd0e5df98258640252661dee271b1795111 > core/src/main/scala/kafka/server/KafkaServer.scala > e66710d2368334ece66f70d55f57b3f888262620 > core/src/test/resources/acl.json PRE-CREATION > 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 > 71f48c07723e334e6489efab500a43fa93a52d0c > > Diff: https://reviews.apache.org/r/34492/diff/ > > > Testing > ------- > > > Thanks, > > Parth Brahmbhatt > >
