FrankYang0529 commented on code in PR #17424:
URL: https://github.com/apache/kafka/pull/17424#discussion_r1802525992
##########
core/src/test/scala/unit/kafka/security/authorizer/AuthorizerTest.scala:
##########
@@ -392,161 +409,17 @@ class AuthorizerTest extends QuorumTestHarness with
BaseAuthorizerTest {
//test remove all acls for resource
removeAcls(authorizer1, Set.empty, resource)
TestUtils.waitAndVerifyAcls(Set.empty[AccessControlEntry], authorizer1,
resource)
- if (quorum.equals(ZK)) {
- assertFalse(zkClient.resourceExists(resource))
- }
//test removing last acl also deletes ZooKeeper path
acls = changeAclAndVerify(Set.empty, Set(acl1), Set.empty)
changeAclAndVerify(acls, Set.empty, acls)
- if (quorum.equals(ZK)) {
- assertFalse(zkClient.resourceExists(resource))
- }
- }
-
- @Test
- def testLoadCache(): Unit = {
- val user1 = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, username)
- val acl1 = new AccessControlEntry(user1.toString, "host-1", READ, ALLOW)
- val acls = Set(acl1)
- addAcls(authorizer1, acls, resource)
-
- val user2 = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "bob")
- val resource1 = new ResourcePattern(TOPIC, "test-2", LITERAL)
- val acl2 = new AccessControlEntry(user2.toString, "host3", READ, DENY)
- val acls1 = Set(acl2)
- addAcls(authorizer1, acls1, resource1)
-
- zkClient.deleteAclChangeNotifications()
- val authorizer = new AclAuthorizer
- try {
- configureAclAuthorizer(authorizer, config.originals)
-
- assertEquals(acls, getAcls(authorizer, resource))
- assertEquals(acls1, getAcls(authorizer, resource1))
- } finally {
- authorizer.close()
- }
}
/**
- * Verify that there is no timing window between loading ACL cache and
setting
- * up ZK change listener. Cache must be loaded before creating change
listener
- * in the authorizer to avoid the timing window.
+ * Test ACL inheritance, as described in
#{org.apache.kafka.common.acl.AclOperation}
*/
- @Test
- def testChangeListenerTiming(): Unit = {
- val configureSemaphore = new Semaphore(0)
- val listenerSemaphore = new Semaphore(0)
- val executor = Executors.newSingleThreadExecutor
- val aclAuthorizer3 = new AclAuthorizer {
- override private[authorizer] def startZkChangeListeners(): Unit = {
- configureSemaphore.release()
- listenerSemaphore.acquireUninterruptibly()
- super.startZkChangeListeners()
- }
- }
- try {
- val future = executor.submit((() =>
aclAuthorizer3.configure(config.originals)): Runnable)
- configureSemaphore.acquire()
- val user1 = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, username)
- val acls = Set(new AccessControlEntry(user1.toString, "host-1", READ,
DENY))
- addAcls(authorizer1, acls, resource)
-
- listenerSemaphore.release()
- future.get(10, TimeUnit.SECONDS)
-
- assertEquals(acls, getAcls(aclAuthorizer3, resource))
- } finally {
- aclAuthorizer3.close()
- executor.shutdownNow()
- }
- }
-
- @Test
Review Comment:
Keep `testLocalConcurrentModificationOfResourceAcls`, but still remove other
test cases. It looks like `StandardAuthorizer` is not like `AclAuthorizer`. It
can't listen change on other `Authorizer` object, so we can't run test cases
like `testDistributedConcurrentModificationOfResourceAcls`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
