You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by bo...@apache.org on 2022/12/01 10:48:37 UTC
[pulsar] branch branch-2.9 updated: [improve] clean the empty topicAuthenticationMap in zk when revoke permission (#16815)
This is an automated email from the ASF dual-hosted git repository.
bogong pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.9 by this push:
new 63096e6bcc1 [improve] clean the empty topicAuthenticationMap in zk when revoke permission (#16815)
63096e6bcc1 is described below
commit 63096e6bcc1470c5af56a197d7f68c202b593193
Author: ken <16...@qq.com>
AuthorDate: Sun Sep 4 12:06:55 2022 +0800
[improve] clean the empty topicAuthenticationMap in zk when revoke permission (#16815)
Co-authored-by: fanjianye <fa...@bigo.sg>
(cherry picked from commit d139d884bcf38d8d9f2ff99bb355591819b85ef5)
---
.../broker/admin/impl/PersistentTopicsBase.java | 9 +++--
.../broker/service/ReplicatorSubscriptionTest.java | 2 +-
.../api/AuthenticatedProducerConsumerTest.java | 41 ++++++++++++++++++++++
3 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
index 4444c69be0f..7b747975306 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
@@ -328,7 +328,13 @@ public class PersistentTopicsBase extends AdminResource {
try {
// Write the new policies to metadata store
namespaceResources().setPolicies(namespaceName, p -> {
- p.auth_policies.getTopicAuthentication().get(topicUri).remove(role);
+ p.auth_policies.getTopicAuthentication().computeIfPresent(topicUri, (k, roles) -> {
+ roles.remove(role);
+ if (roles.isEmpty()) {
+ return null;
+ }
+ return roles;
+ });
return p;
});
log.info("[{}] Successfully revoke access for role {} - topic {}", clientAppId(), role, topicUri);
@@ -336,7 +342,6 @@ public class PersistentTopicsBase extends AdminResource {
log.error("[{}] Failed to revoke permissions for topic {}", clientAppId(), topicUri, e);
throw new RestException(e);
}
-
}
protected void internalRevokePermissionsOnTopic(String role) {
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorSubscriptionTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorSubscriptionTest.java
index 175574ed828..186a2491a47 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorSubscriptionTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorSubscriptionTest.java
@@ -180,7 +180,7 @@ public class ReplicatorSubscriptionTest extends ReplicatorTestBase {
.getTopic(topicName, false).get().get();
ReplicatedSubscriptionsController rsc1 = t1.getReplicatedSubscriptionController().get();
// no snapshot should have been created before any messages are published
- assertTrue(rsc1.getLastCompletedSnapshotId().isEmpty());
+ assertFalse(rsc1.getLastCompletedSnapshotId().isPresent());
@Cleanup
PulsarClient client2 = PulsarClient.builder()
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticatedProducerConsumerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticatedProducerConsumerTest.java
index e0cc980991e..9be5076b374 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticatedProducerConsumerTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticatedProducerConsumerTest.java
@@ -404,4 +404,45 @@ public class AuthenticatedProducerConsumerTest extends ProducerConsumerBase {
admin.tenants().deleteTenant("p1");
admin.clusters().deleteCluster("test");
}
+
+ @Test
+ public void testCleanupEmptyTopicAuthenticationMap() throws Exception {
+ Map<String, String> authParams = new HashMap<>();
+ authParams.put("tlsCertFile", TLS_CLIENT_CERT_FILE_PATH);
+ authParams.put("tlsKeyFile", TLS_CLIENT_KEY_FILE_PATH);
+ Authentication authTls = new AuthenticationTls();
+ authTls.configure(authParams);
+ internalSetup(authTls);
+
+ admin.clusters().createCluster("test", ClusterData.builder().build());
+ admin.tenants().createTenant("p1",
+ new TenantInfoImpl(Collections.emptySet(), new HashSet<>(admin.clusters().getClusters())));
+ admin.namespaces().createNamespace("p1/ns1");
+
+ String topic = "persistent://p1/ns1/topic";
+ admin.topics().createNonPartitionedTopic(topic);
+ assertFalse(pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get("p1/ns1"))
+ .get().auth_policies.getTopicAuthentication().containsKey(topic));
+
+ // grant permission
+ admin.topics().grantPermission(topic, "test-user-1", EnumSet.of(AuthAction.consume));
+ Awaitility.await().untilAsserted(() -> {
+ assertTrue(pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get("p1/ns1"))
+ .get().auth_policies.getTopicAuthentication().containsKey(topic));
+ });
+
+ // revoke permission
+ admin.topics().revokePermissions(topic, "test-user-1");
+ Awaitility.await().untilAsserted(() -> {
+ assertFalse(pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get("p1/ns1"))
+ .get().auth_policies.getTopicAuthentication().containsKey(topic));
+ });
+
+ // grant permission again
+ admin.topics().grantPermission(topic, "test-user-1", EnumSet.of(AuthAction.consume));
+ Awaitility.await().untilAsserted(() -> {
+ assertTrue(pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get("p1/ns1"))
+ .get().auth_policies.getTopicAuthentication().containsKey(topic));
+ });
+ }
}