You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by mm...@apache.org on 2021/12/14 04:38:50 UTC
[pulsar] branch branch-2.8 updated: [Broker] Remove tenant admin verification when listing partitioned topics (#13138)
This is an automated email from the ASF dual-hosted git repository.
mmarshall pushed a commit to branch branch-2.8
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.8 by this push:
new 755d193 [Broker] Remove tenant admin verification when listing partitioned topics (#13138)
755d193 is described below
commit 755d19339eb4b4f042b56ca426c9dfe53b1de85f
Author: Ruguo Yu <ji...@163.com>
AuthorDate: Tue Dec 14 12:25:48 2021 +0800
[Broker] Remove tenant admin verification when listing partitioned topics (#13138)
* [Broker] Remove tenant permission verification when list partitioned-topic
* Improve test
* Update annotation
(cherry picked from commit 498810886bc6fd5e1c1aa0e9ae1c3564df0fbdbc)
---
.../broker/admin/impl/PersistentTopicsBase.java | 1 -
.../pulsar/broker/admin/v1/PersistentTopics.java | 4 ++--
.../pulsar/broker/admin/v2/PersistentTopics.java | 4 ++--
.../api/AuthorizationProducerConsumerTest.java | 20 +++++++++++++++++++-
4 files changed, 23 insertions(+), 6 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 34e3fe7..fdd2397 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
@@ -182,7 +182,6 @@ public class PersistentTopicsBase extends AdminResource {
}
protected List<String> internalGetPartitionedTopicList() {
- validateAdminAccessForTenant(namespaceName.getTenant());
validateNamespaceOperation(namespaceName, NamespaceOperation.GET_TOPICS);
// Validate that namespace exists, throws 404 if it doesn't exist
try {
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
index a0309ac..112273feb 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
@@ -69,7 +69,7 @@ public class PersistentTopics extends PersistentTopicsBase {
@Path("/{property}/{cluster}/{namespace}")
@ApiOperation(hidden = true, value = "Get the list of topics under a namespace.",
response = String.class, responseContainer = "List")
- @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"),
@ApiResponse(code = 404, message = "Namespace doesn't exist")})
public void getList(@Suspended final AsyncResponse asyncResponse, @PathParam("property") String property,
@PathParam("cluster") String cluster, @PathParam("namespace") String namespace) {
@@ -87,7 +87,7 @@ public class PersistentTopics extends PersistentTopicsBase {
@Path("/{property}/{cluster}/{namespace}/partitioned")
@ApiOperation(hidden = true, value = "Get the list of partitioned topics under a namespace.",
response = String.class, responseContainer = "List")
- @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"),
@ApiResponse(code = 404, message = "Namespace doesn't exist")})
public List<String> getPartitionedTopicList(@PathParam("property") String property,
@PathParam("cluster") String cluster, @PathParam("namespace") String namespace) {
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
index 125e3f3..ca7e6cc 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
@@ -85,7 +85,7 @@ public class PersistentTopics extends PersistentTopicsBase {
response = String.class, responseContainer = "List")
@ApiResponses(value = {
@ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant"),
- @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"),
@ApiResponse(code = 404, message = "tenant/namespace/topic doesn't exit"),
@ApiResponse(code = 412, message = "Namespace name is not valid"),
@ApiResponse(code = 500, message = "Internal server error")})
@@ -111,7 +111,7 @@ public class PersistentTopics extends PersistentTopicsBase {
response = String.class, responseContainer = "List")
@ApiResponses(value = {
@ApiResponse(code = 401, message = "Don't have permission to administrate resources on this tenant"),
- @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 403, message = "Don't have admin or consume permission on namespace"),
@ApiResponse(code = 404, message = "tenant/namespace/topic doesn't exit"),
@ApiResponse(code = 412, message = "Namespace name is not valid"),
@ApiResponse(code = 500, message = "Internal server error")})
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java
index 78f71f2..8807f11 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthorizationProducerConsumerTest.java
@@ -24,6 +24,7 @@ import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
+import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import java.io.IOException;
@@ -329,6 +330,7 @@ public class AuthorizationProducerConsumerTest extends ProducerConsumerBase {
conf.setAuthorizationProvider(PulsarAuthorizationProvider.class.getName());
setup();
+ final String tenantRole = "tenant-role";
final String subscriptionRole = "sub-role";
final String subscriptionName = "sub1";
final String namespace = "my-property/my-ns-sub-auth";
@@ -341,6 +343,11 @@ public class AuthorizationProducerConsumerTest extends ProducerConsumerBase {
PulsarAdmin superAdmin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString())
.authentication(adminAuthentication).build());
+ Authentication tenantAdminAuthentication = new ClientAuthentication(tenantRole);
+ @Cleanup
+ PulsarAdmin tenantAdmin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString())
+ .authentication(tenantAdminAuthentication).build());
+
Authentication subAdminAuthentication = new ClientAuthentication(subscriptionRole);
@Cleanup
PulsarAdmin sub1Admin = spy(PulsarAdmin.builder().serviceHttpUrl(brokerUrl.toString())
@@ -349,9 +356,11 @@ public class AuthorizationProducerConsumerTest extends ProducerConsumerBase {
superAdmin.clusters().createCluster("test",
ClusterData.builder().serviceUrl(brokerUrl.toString()).build());
superAdmin.tenants().createTenant("my-property",
- new TenantInfoImpl(Sets.newHashSet(), Sets.newHashSet("test")));
+ new TenantInfoImpl(Sets.newHashSet(tenantRole), Sets.newHashSet("test")));
superAdmin.namespaces().createNamespace(namespace, Sets.newHashSet("test"));
superAdmin.topics().createPartitionedTopic(topicName, 1);
+ assertEquals(tenantAdmin.topics().getPartitionedTopicList(namespace),
+ Lists.newArrayList(topicName));
// grant topic consume&produce authorization to the subscriptionRole
superAdmin.topics().grantPermission(topicName, subscriptionRole,
@@ -381,6 +390,13 @@ public class AuthorizationProducerConsumerTest extends ProducerConsumerBase {
// subscriptionRole doesn't have namespace-level authorization, so it will fail to clear backlog
try {
+ sub1Admin.topics().getPartitionedTopicList(namespace);
+ fail("should have failed with authorization exception");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().startsWith(
+ "Unauthorized to validateNamespaceOperation for operation [GET_TOPICS]"));
+ }
+ try {
sub1Admin.namespaces().clearNamespaceBundleBacklog(namespace, "0x00000000_0xffffffff");
fail("should have failed with authorization exception");
} catch (Exception e) {
@@ -391,6 +407,8 @@ public class AuthorizationProducerConsumerTest extends ProducerConsumerBase {
superAdmin.namespaces().grantPermissionOnNamespace(namespace, subscriptionRole,
Sets.newHashSet(AuthAction.consume));
// now, subscriptionRole have consume authorization on namespace, so it will successfully clear backlog
+ assertEquals(sub1Admin.topics().getPartitionedTopicList(namespace),
+ Lists.newArrayList(topicName));
sub1Admin.namespaces().clearNamespaceBundleBacklog(namespace, "0x00000000_0xffffffff");
assertEquals(sub1Admin.topics().getStats(topicName + "-partition-0").getSubscriptions()
.get(subscriptionName).getMsgBacklog(), 0);