chia7712 commented on a change in pull request #10206:
URL: https://github.com/apache/kafka/pull/10206#discussion_r585784412



##########
File path: 
core/src/test/scala/integration/kafka/api/AuthorizerIntegrationTest.scala
##########
@@ -1813,17 +1813,25 @@ class AuthorizerIntegrationTest extends BaseRequestTest 
{
     producer.beginTransaction()
     producer.send(new ProducerRecord(tp.topic, tp.partition, "1".getBytes, 
"1".getBytes)).get
 
+    def assertListTransactionResult(
+      expectedTransactionalIds: Set[String]
+    ): Unit = {
+      val listTransactionsRequest = new ListTransactionsRequest.Builder(new 
ListTransactionsRequestData()).build()
+      val listTransactionsResponse = 
connectAndReceive[ListTransactionsResponse](listTransactionsRequest)
+      assertEquals(Errors.NONE, 
Errors.forCode(listTransactionsResponse.data.errorCode))
+      assertEquals(expectedTransactionalIds, 
listTransactionsResponse.data.transactionStates.asScala.map(_.transactionalId).toSet)
+    }
+
     // First verify that we can list the transaction
-    val listTransactionsRequest = new ListTransactionsRequest.Builder(new 
ListTransactionsRequestData()).build()
-    val authorizedResponse = 
connectAndReceive[ListTransactionsResponse](listTransactionsRequest)
-    assertEquals(Errors.NONE, 
Errors.forCode(authorizedResponse.data.errorCode))
-    assertEquals(Set(transactionalId), 
authorizedResponse.data.transactionStates.asScala.map(_.transactionalId).toSet)
+    assertListTransactionResult(expectedTransactionalIds = 
Set(transactionalId))
 
     // Now revoke authorization and verify that the transaction is no longer 
listable
     removeAndVerifyAcls(Set(new AccessControlEntry(clientPrincipalString, 
WildcardHost, WRITE, ALLOW)), transactionalIdResource)
-    val unauthorizedResponse = 
connectAndReceive[ListTransactionsResponse](listTransactionsRequest)
-    assertEquals(Errors.NONE, 
Errors.forCode(unauthorizedResponse.data.errorCode))
-    assertEquals(Set(), 
unauthorizedResponse.data.transactionStates.asScala.map(_.transactionalId).toSet)
+    assertListTransactionResult(expectedTransactionalIds = Set())
+
+    // The minimum permission needed is `Describe`
+    addAndVerifyAcls(Set(new AccessControlEntry(clientPrincipalString, 
WildcardHost, WRITE, ALLOW)), transactionalIdResource)

Review comment:
       Should permission be describe rather than write?




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to