dajac commented on a change in pull request #9938:
URL: https://github.com/apache/kafka/pull/9938#discussion_r561680234



##########
File path: 
core/src/test/scala/integration/kafka/api/DescribeAuthorizedOperationsTest.scala
##########
@@ -32,17 +32,55 @@ import org.junit.jupiter.api.{AfterEach, BeforeEach, Test}
 
 import scala.jdk.CollectionConverters._
 
+object DescribeAuthorizedOperationsTest {
+  val Group1 = "group1"
+  val Group2 = "group2"
+  val Group3 = "group3"
+  val Topic1 = "topic1"
+  val Topic2 = "topic2"
+
+  val Group1Acl = new AclBinding(
+    new ResourcePattern(ResourceType.GROUP, Group1, PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
ALL, ALLOW))
+
+  val Group2Acl = new AclBinding(
+    new ResourcePattern(ResourceType.GROUP, Group2, PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
DESCRIBE, ALLOW))
+
+  val Group3Acl = new AclBinding(
+    new ResourcePattern(ResourceType.GROUP, Group3, PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
DELETE, ALLOW))
+
+  val ClusterAllAcl = new AclBinding(
+    new ResourcePattern(ResourceType.CLUSTER, Resource.CLUSTER_NAME, 
PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
ALL, ALLOW))
+
+  val Topic1Acl = new AclBinding(
+    new ResourcePattern(ResourceType.TOPIC, Topic1, PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
ALL, ALLOW))
+
+  val Topic2All = new AclBinding(
+    new ResourcePattern(ResourceType.TOPIC, Topic2, PatternType.LITERAL),
+    accessControlEntry(JaasTestUtils.KafkaClientPrincipalUnqualifiedName2, 
DELETE, ALLOW))
+
+  private def accessControlEntry(
+    userName: String,
+    operation: AclOperation,
+    permissionType: AclPermissionType

Review comment:
       Indeed, let me remove that field.

##########
File path: 
core/src/test/scala/integration/kafka/api/DescribeAuthorizedOperationsTest.scala
##########
@@ -32,17 +32,55 @@ import org.junit.jupiter.api.{AfterEach, BeforeEach, Test}
 
 import scala.jdk.CollectionConverters._
 
+object DescribeAuthorizedOperationsTest {
+  val Group1 = "group1"

Review comment:
       In Scala, we tend to only capitalize the first letter of constants. At 
least, this how I have been doing it so far. I prefer to keep it as is.

##########
File path: 
core/src/test/scala/integration/kafka/api/DescribeAuthorizedOperationsTest.scala
##########
@@ -73,11 +114,6 @@ class DescribeAuthorizedOperationsTest extends 
IntegrationTestHarness with SaslS
     TestUtils.waitUntilBrokerMetadataIsPropagated(servers)
   }
 
-  private def accessControlEntry(userName: String, permissionType: 
AclPermissionType, operation: AclOperation): AccessControlEntry = {
-    new AccessControlEntry(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, 
userName).toString,
-      AclEntry.WildcardHost, operation, permissionType)
-  }
-
   @AfterEach
   override def tearDown(): Unit = {
     if (client != null)

Review comment:
       I think that it is better to keep it as `client` could not be 
initialized. For instance, let's say that there is an exception raised when the 
client is constructed. In this case, `client` would remain `null`.

##########
File path: 
core/src/test/scala/integration/kafka/api/DescribeAuthorizedOperationsTest.scala
##########
@@ -73,11 +114,6 @@ class DescribeAuthorizedOperationsTest extends 
IntegrationTestHarness with SaslS
     TestUtils.waitUntilBrokerMetadataIsPropagated(servers)
   }
 
-  private def accessControlEntry(userName: String, permissionType: 
AclPermissionType, operation: AclOperation): AccessControlEntry = {
-    new AccessControlEntry(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, 
userName).toString,
-      AclEntry.WildcardHost, operation, permissionType)
-  }
-
   @AfterEach
   override def tearDown(): Unit = {
     if (client != null)

Review comment:
       Got it, thanks for the clarification. I have removed the null check.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to