You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by te...@apache.org on 2024/01/03 13:52:51 UTC
(pulsar) branch branch-3.1 updated: [improve][broker] cleanup the empty subscriptionAuthenticationMap in zk when revoke subscription permission (#21696)
This is an automated email from the ASF dual-hosted git repository.
technoboy pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-3.1 by this push:
new b7d3a9caa7a [improve][broker] cleanup the empty subscriptionAuthenticationMap in zk when revoke subscription permission (#21696)
b7d3a9caa7a is described below
commit b7d3a9caa7a8325c532be56a18b5d94cfa1aa0c7
Author: ken <16...@qq.com>
AuthorDate: Wed Dec 13 22:19:02 2023 +0800
[improve][broker] cleanup the empty subscriptionAuthenticationMap in zk when revoke subscription permission (#21696)
Co-authored-by: fanjianye <fa...@bigo.sg>
Co-authored-by: Jiwe Guo <te...@apache.org>
---
.../authorization/PulsarAuthorizationProvider.java | 3 ++
.../api/AuthenticatedProducerConsumerTest.java | 45 ++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
index ece22fe223b..acb6fce9b92 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
@@ -357,6 +357,9 @@ public class PulsarAuthorizationProvider implements AuthorizationProvider {
policies.auth_policies.getSubscriptionAuthentication().get(subscriptionName);
if (subscriptionAuth != null) {
subscriptionAuth.removeAll(roles);
+ if (subscriptionAuth.isEmpty()) {
+ policies.auth_policies.getSubscriptionAuthentication().remove(subscriptionName);
+ }
} else {
log.info("[{}] Couldn't find role {} while revoking for sub = {}", namespace,
roles, subscriptionName);
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 8189f8e86b5..3bd8b920a30 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
@@ -49,6 +49,7 @@ import org.apache.pulsar.common.naming.NamespaceName;
import org.apache.pulsar.common.naming.TopicName;
import org.apache.pulsar.common.policies.data.AuthAction;
import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.Policies;
import org.apache.pulsar.common.policies.data.TenantInfoImpl;
import org.apache.zookeeper.KeeperException.Code;
import org.awaitility.Awaitility;
@@ -497,4 +498,48 @@ public class AuthenticatedProducerConsumerTest extends ProducerConsumerBase {
.get().auth_policies.getTopicAuthentication().containsKey(topic));
});
}
+
+ @Test
+ public void testCleanupEmptySubscriptionAuthenticationMap() throws Exception {
+ Map<String, String> authParams = new HashMap<>();
+ authParams.put("tlsCertFile", getTlsFileForClient("admin.cert"));
+ authParams.put("tlsKeyFile", getTlsFileForClient("admin.key-pk8"));
+ 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())));
+ String namespace = "p1/ns1";
+ admin.namespaces().createNamespace("p1/ns1");
+
+ // grant permission1 and permission2
+ String subscription = "test-sub-1";
+ String role1 = "test-user-1";
+ String role2 = "test-user-2";
+ Set<String> roles = new HashSet<>();
+ roles.add(role1);
+ roles.add(role2);
+ admin.namespaces().grantPermissionOnSubscription(namespace, subscription, roles);
+ Optional<Policies> policies = pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get(namespace));
+ assertTrue(policies.isPresent());
+ assertTrue(policies.get().auth_policies.getSubscriptionAuthentication().containsKey(subscription));
+ assertTrue(policies.get().auth_policies.getSubscriptionAuthentication().get(subscription).contains(role1));
+ assertTrue(policies.get().auth_policies.getSubscriptionAuthentication().get(subscription).contains(role2));
+
+ // revoke permission1
+ admin.namespaces().revokePermissionOnSubscription(namespace, subscription, role1);
+ policies = pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get(namespace));
+ assertTrue(policies.isPresent());
+ assertTrue(policies.get().auth_policies.getSubscriptionAuthentication().containsKey(subscription));
+ assertFalse(policies.get().auth_policies.getSubscriptionAuthentication().get(subscription).contains(role1));
+ assertTrue(policies.get().auth_policies.getSubscriptionAuthentication().get(subscription).contains(role2));
+
+ // revoke permission2
+ admin.namespaces().revokePermissionOnSubscription(namespace, subscription, role2);
+ policies = pulsar.getPulsarResources().getNamespaceResources().getPolicies(NamespaceName.get(namespace));
+ assertTrue(policies.isPresent());
+ assertFalse(policies.get().auth_policies.getSubscriptionAuthentication().containsKey(subscription));
+ }
}