----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34492/#review97554 -----------------------------------------------------------
Hmm, did you attach the right patch? It doesn't seem to apply to trunk and my last round of minor comments didn't seem to get addressed. Also, one more suggestion below. core/src/main/scala/kafka/server/KafkaApis.scala (line 675) <https://reviews.apache.org/r/34492/#comment153549> Could we change the test to the following to make it more consistent with the rest of places where we test authorization? if (! authorizer.map(az => az.authorize(request.session, Read, new Resource(ConsumerGroup, consumerMetadataRequest.group))).getOrElse(true)) core/src/main/scala/kafka/server/KafkaApis.scala (line 739) <https://reviews.apache.org/r/34492/#comment153554> Ditto as the above. - Jun Rao On Sept. 2, 2015, 9:50 p.m., Parth Brahmbhatt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34492/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2015, 9:50 p.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 > > > Moving PermissionType to trait instead of enum. Following the convention for > defining constants. > > > Adding authorizer.config.path back. > > > Addressing more comments from Jun. > > > Addressing more comments. > > > Now addressing Ismael's comments. Case sensitive checks. > > > Addressing Jun's comments. > > > Merge remote-tracking branch 'origin/trunk' into az > > Conflicts: > core/src/main/scala/kafka/server/KafkaApis.scala > core/src/main/scala/kafka/server/KafkaServer.scala > > Deleting KafkaConfigDefTest > > > Addressing comments from Ismael. > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az > > > Consolidating KafkaPrincipal. > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az > > Conflicts: > > clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java > > clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java > core/src/main/scala/kafka/server/KafkaApis.scala > > Making Acl structure take only one principal, operation and host. > > > Merge remote-tracking branch 'origin/trunk' into az > > > Reverting uninteded new line change. > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java > 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 > clients/src/main/java/org/apache/kafka/common/protocol/Errors.java > e17e390c507eca0eba28a2763c0e35d66077d1f2 > > clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java > b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 > > clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java > PRE-CREATION > 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/Operation.scala PRE-CREATION > core/src/main/scala/kafka/security/auth/PermissionType.scala 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 > a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 > core/src/main/scala/kafka/server/KafkaConfig.scala > d547a01cf7098f216a3775e1e1901c5794e1b24c > core/src/main/scala/kafka/server/KafkaServer.scala > 039c7eb919edeedbf8f1342c6e2fdf4736e224cd > core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION > core/src/test/scala/unit/kafka/security/auth/OperationTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 3da666f73227fc7ef7093e3790546344065f6825 > > Diff: https://reviews.apache.org/r/34492/diff/ > > > Testing > ------- > > > Thanks, > > Parth Brahmbhatt > >
