[pulsar] branch master updated: [fix] [admin] fix reach max tenants error if the tenant already exists (#15932)

[pe...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new 364b0c96a33 [fix] [admin] fix reach max tenants error if the tenant already exists (#15932)
364b0c96a33 is described below

commit 364b0c96a338b9630e317ed965a76138f9f19aef
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Mon Jun 6 14:20:58 2022 +0800

    [fix] [admin] fix reach max tenants error if the tenant already exists (#15932)
    
    ### Motivation
    When I use the Admin API to create a tenant, if the tenants are limited and I already have the max # of tenants, I get an error "412 Reached the max tenants [n] restriction." This is good, but if I'm trying to create a tenant that already exists, I get the same error. But, creating a tenant that already exists should be a no-op and would not actually push me over my limit.
    
    "409 Already exists" is a softer error than "412 Reached the max restriction." It might be a little cleaner if Pulsar checked if the tenant exists first, and returned that error, before throwing the 412 exception. Otherwise the responsibility to fetch the tenants and check if it already exists must be done by the user.
---
 .../pulsar/broker/admin/impl/TenantsBase.java      | 12 ++++----
 .../org/apache/pulsar/broker/admin/AdminTest.java  | 32 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java
index 045cafbfd53..c2b8bb7220b 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java
@@ -124,6 +124,12 @@ public class TenantsBase extends PulsarWebResource {
         validateSuperUserAccessAsync()
                 .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
                 .thenCompose(__ -> validateClustersAsync(tenantInfo))
+                .thenCompose(__ -> tenantResources().tenantExistsAsync(tenant))
+                .thenAccept(exist -> {
+                    if (exist) {
+                        throw new RestException(Status.CONFLICT, "Tenant already exist");
+                    }
+                })
                 .thenCompose(__ -> tenantResources().listTenantsAsync())
                 .thenAccept(tenants -> {
                     int maxTenants = pulsar().getConfiguration().getMaxTenants();
@@ -135,12 +141,6 @@ public class TenantsBase extends PulsarWebResource {
                         }
                     }
                 })
-                .thenCompose(__ -> tenantResources().tenantExistsAsync(tenant))
-                .thenAccept(exist -> {
-                    if (exist) {
-                        throw new RestException(Status.CONFLICT, "Tenant already exist");
-                    }
-                })
                 .thenCompose(__ -> tenantResources().createTenantAsync(tenant, tenantInfo))
                 .thenAccept(__ -> {
                     log.info("[{}] Created tenant {}", clientAppId, tenant);
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
index c6b1fbd65bf..0d2631b00a9 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
@@ -175,6 +175,12 @@ public class AdminTest extends MockedPulsarServiceBaseTest {
         schemasResource.setPulsar(pulsar);
     }
 
+    @Override
+    protected void doInitConf() throws Exception {
+        super.doInitConf();
+        conf.setMaxTenants(10);
+    }
+
     @Override
     @AfterMethod(alwaysRun = true)
     public void cleanup() throws Exception {
@@ -616,6 +622,32 @@ public class AdminTest extends MockedPulsarServiceBaseTest {
             assertEquals(e.getResponse().getStatus(), Status.PRECONDITION_FAILED.getStatusCode());
         }
 
+        // Check max tenant count
+        int maxTenants = pulsar.getConfiguration().getMaxTenants();
+        List<String> tenants = pulsar.getPulsarResources().getTenantResources().listTenants();
+
+        for(int tenantSize = tenants.size();tenantSize < maxTenants; tenantSize++ ){
+            final int tenantIndex = tenantSize;
+            Response obj = (Response) asyncRequests(ctx ->
+                    properties.createTenant(ctx, "test-tenant-" + tenantIndex, tenantInfo));
+            Assert.assertTrue(obj.getStatus() < 400 && obj.getStatus() >= 200);
+        }
+        try {
+            Response obj = (Response) asyncRequests(ctx ->
+                    properties.createTenant(ctx, "test-tenant-" +  maxTenants, tenantInfo));
+            fail("should have failed");
+        } catch (RestException e) {
+            assertEquals(e.getResponse().getStatus(), Status.PRECONDITION_FAILED.getStatusCode());
+        }
+
+        // Check creating existing property when tenant reach max count.
+        try {
+            response = asyncRequests(ctx -> properties.createTenant(ctx, "test-tenant-" +  (maxTenants-1), tenantInfo));
+            fail("should have failed");
+        } catch (RestException e) {
+            assertEquals(e.getResponse().getStatus(), Status.CONFLICT.getStatusCode());
+        }
+
         AsyncResponse response2 = mock(AsyncResponse.class);
         namespaces.deleteNamespace(response2, "my-tenant", "use", "my-namespace", false, false);
         ArgumentCaptor<Response> captor = ArgumentCaptor.forClass(Response.class);