[20.3] Implement/fix limit validation for projects

DaanHoogland · Daan Hoogland · commit 1593944553fb · 2026-03-16T11:47:32.000+01:00
diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
@@ -36,6 +36,7 @@
 import javax.mail.MessagingException;
 import javax.naming.ConfigurationException;
 
+import com.cloud.resourcelimit.CheckedReservation;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.ProjectRole;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -47,6 +48,7 @@
 import org.apache.cloudstack.framework.messagebus.MessageBus;
 import org.apache.cloudstack.framework.messagebus.PublishScope;
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.reservation.dao.ReservationDao;
 import org.apache.cloudstack.utils.mailing.MailAddress;
 import org.apache.cloudstack.utils.mailing.SMTPMailProperties;
 import org.apache.cloudstack.utils.mailing.SMTPMailSender;
@@ -159,6 +161,8 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C
     private VpcManager _vpcMgr;
     @Inject
     MessageBus messageBus;
+    @Inject
+    private ReservationDao reservationDao;
 
     protected boolean _invitationRequired = false;
     protected long _invitationTimeOut = 86400000;
@@ -272,8 +276,7 @@ public Project createProject(final String name, final String displayText, String
             owner = _accountDao.findById(user.getAccountId());
         }
 
-        //do resource limit check
-        _resourceLimitMgr.checkResourceLimit(owner, ResourceType.project);
+        try (CheckedReservation projectReservation = new CheckedReservation(owner, ResourceType.project, null, null, 1L, reservationDao, _resourceLimitMgr)) {
 
         final Account ownerFinal = owner;
         User finalUser = user;
@@ -308,6 +311,7 @@ public Project doInTransaction(TransactionStatus status) {
         messageBus.publish(_name, ProjectManager.MESSAGE_CREATE_TUNGSTEN_PROJECT_EVENT, PublishScope.LOCAL, project);
 
         return project;
+        }
     }
 
     @Override
@@ -491,6 +495,9 @@ public Boolean doInTransaction(TransactionStatus status) {
         //remove account
         ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdAccountId(projectId, account.getId());
         success = _projectAccountDao.remove(projectAccount.getId());
+        if (projectAccount.getAccountRole() == Role.Admin) {
+            _resourceLimitMgr.decrementResourceCount(account.getId(), ResourceType.project);
+        }
 
         //remove all invitations for account
         if (success) {
@@ -594,12 +601,22 @@ public boolean addUserToProject(Long projectId, String username, String email, L
             if (username == null) {
                 throw new InvalidParameterValueException("User information (ID) is required to add user to the project");
             }
+
+            boolean shouldIncrementResourceCount = projectRole != null && Role.Admin == projectRole;
+            try (CheckedReservation cr = new CheckedReservation(userAccount, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) {
             if (assignUserToProject(project, user.getId(), user.getAccountId(), projectRole,
                     Optional.ofNullable(role).map(ProjectRole::getId).orElse(null)) != null) {
+                if (shouldIncrementResourceCount) {
+                    _resourceLimitMgr.incrementResourceCount(userAccount.getId(), ResourceType.project);
+                }
                 return true;
+            } else {
+                logger.warn("Failed to add user to project: {}", project);
+                return false;
+            }
+            } catch (ResourceAllocationException e) {
+                throw new RuntimeException(e);
             }
-            logger.warn("Failed to add user to project: {}", project);
-            return false;
         }
     }
 
@@ -652,14 +669,18 @@ public boolean canModifyProjectAccount(Account caller, long accountId) {
     }
 
     private void updateProjectAccount(ProjectAccountVO futureOwner, Role newAccRole, Long accountId) throws ResourceAllocationException {
-        _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(accountId), ResourceType.project);
+        Account account = _accountMgr.getAccount(accountId);
+        boolean shouldIncrementResourceCount = Role.Admin == newAccRole;
+
+        try (CheckedReservation checkedReservation = new CheckedReservation(account, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) {
         futureOwner.setAccountRole(newAccRole);
         _projectAccountDao.update(futureOwner.getId(), futureOwner);
-        if (newAccRole != null && Role.Admin == newAccRole) {
+        if (shouldIncrementResourceCount) {
             _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.project);
         } else {
             _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.project);
         }
+        }
     }
 
     @Override
@@ -701,7 +722,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                                     " doesn't belong to the project. Add it to the project first and then change the project's ownership");
                         }
 
-                        //do resource limit check
+                        try (CheckedReservation checkedReservation = new CheckedReservation(futureOwnerAccount, ResourceType.project, null, null, 1L, reservationDao, _resourceLimitMgr)) {
+
                         _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(futureOwnerAccount.getId()), ResourceType.project);
 
                         //unset the role for the old owner
@@ -714,7 +736,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                         futureOwner.setAccountRole(Role.Admin);
                         _projectAccountDao.update(futureOwner.getId(), futureOwner);
                         _resourceLimitMgr.incrementResourceCount(futureOwnerAccount.getId(), ResourceType.project);
-
+                        }
                     } else {
                         logger.trace("Future owner {}is already the owner of the project {}", newOwnerName, project);
                     }
@@ -857,13 +879,22 @@ public boolean addAccountToProject(long projectId, String accountName, String em
             if (account == null) {
                 throw new InvalidParameterValueException("Account information is required for assigning account to the project");
             }
+
+            boolean shouldIncrementResourceCount = projectRoleType != null && Role.Admin == projectRoleType;
+            try (CheckedReservation cr = new CheckedReservation(account, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) {
             if (assignAccountToProject(project, account.getId(), projectRoleType, null,
                     Optional.ofNullable(projectRole).map(ProjectRole::getId).orElse(null)) != null) {
+                if (shouldIncrementResourceCount) {
+                    _resourceLimitMgr.incrementResourceCount(account.getId(), ResourceType.project);
+                }
                 return true;
             } else {
                 logger.warn("Failed to add account {} to project {}", accountName, project);
                 return false;
             }
+            } catch (ResourceAllocationException e) {
+                throw new RuntimeException(e);
+            }
         }
     }
 
@@ -1042,7 +1073,9 @@ public Boolean doInTransaction(TransactionStatus status) {
                 boolean success = true;
                 ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdUserId(projectId, user.getAccountId(), user.getId());
                 success = _projectAccountDao.remove(projectAccount.getId());
-
+                if (projectAccount.getAccountRole() == Role.Admin) {
+                    _resourceLimitMgr.decrementResourceCount(user.getAccountId(), ResourceType.project);
+                }
                 if (success) {
                     logger.debug("Removed user {} from project. Removing any invite sent to the user", user);
                     ProjectInvitation invite = _projectInvitationDao.findByUserIdProjectId(user.getId(), user.getAccountId(),  projectId);
diff --git a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java
@@ -40,7 +40,7 @@
 import com.cloud.utils.exception.CloudRuntimeException;
 
 
-public class CheckedReservation  implements AutoCloseable {
+public class CheckedReservation implements Reserver {
     protected Logger logger = LogManager.getLogger(getClass());
 
     private static final int TRY_TO_GET_LOCK_TIME = 120;
@@ -174,7 +174,7 @@ protected void setQuotaLimitLock(GlobalLock quotaLimitLock) {
     }
 
     @Override
-    public void close() throws Exception {
+    public void close() {
         removeAllReservations();
     }
 
diff --git a/server/src/main/java/com/cloud/resourcelimit/Reserver.java b/server/src/main/java/com/cloud/resourcelimit/Reserver.java
@@ -0,0 +1,24 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.resourcelimit;
+
+public interface Reserver extends AutoCloseable {
+
+    void close();
+
+}
diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
@@ -1199,11 +1199,6 @@ public long recalculateDomainResourceCount(final long domainId, final ResourceTy
             long newResourceCount = 0L;
             ResourceCountVO domainRC = null;
 
-            // calculate project count here
-            if (type == ResourceType.project) {
-                newResourceCount += _projectDao.countProjectsForDomain(domainId);
-            }
-
             if (type == ResourceType.network) {
                 newResourceCount += networkDomainDao.listDomainNetworkMapByDomain(domainId).size();
             }

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@`
`40`	`40`	`import com.cloud.utils.exception.CloudRuntimeException;`
`41`	`41`
`42`	`42`
`43`		`-public class CheckedReservation implements AutoCloseable {`
	`43`	`+public class CheckedReservation implements Reserver {`
`44`	`44`	`protected Logger logger = LogManager.getLogger(getClass());`
`45`	`45`
`46`	`46`	`private static final int TRY_TO_GET_LOCK_TIME = 120;`
`@@ -174,7 +174,7 @@ protected void setQuotaLimitLock(GlobalLock quotaLimitLock) {`
`174`	`174`	`}`
`175`	`175`
`176`	`176`	`@Override`
`177`		`- public void close() throws Exception {`
	`177`	`+ public void close() {`
`178`	`178`	`removeAllReservations();`
`179`	`179`	`}`
`180`	`180`