From 5d545023fcc4ea525012232182a250669975f24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 1 Feb 2018 10:59:16 -0200 Subject: [PATCH] [CLOUDSTACK-9338] ACS not accounting resources of VMs with custom service offering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACS is accounting the resources properly when deploying VMs with custom service offerings. However, there are other methods (such as updateResourceCount) that do not execute the resource accounting properly, and these methods update the resource count for an account in the database. Therefore, if a user deploys VMs with custom service offerings, and later this user calls the “updateResourceCount” method, it (the method) will only account for VMs with normal service offerings, and update this as the number of resources used by the account. This will result in a smaller number of resources to be accounted for the given account than the real used value. The problem becomes worse because if the user starts to delete these VMs, it is possible to reach negative values of resources allocated (breaking all of the resource limiting for accounts). This is a very serious attack vector for public cloud providers! --- .../configuration/dao/ResourceCountDao.java | 14 ++++++ .../dao/ResourceCountDaoImpl.java | 42 ++++++++++++++++++ .../ResourceLimitManagerImpl.java | 44 +------------------ 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index d4695c3ff75..f5b76e3e7fc 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -57,4 +57,18 @@ public interface ResourceCountDao extends GenericDao { Set listRowsToUpdateForDomain(long domainId, ResourceType type); long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType); + + /** + * Counts the number of CPU cores allocated for the given account. + * + * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. + */ + long countCpuNumberAllocatedToAccount(long accountId); + + /** + * Counts the amount of memory allocated for the given account. + * + * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. + */ + long countMemoryAllocatedToAccount(long accountId); } diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index f7cd3cbf86f..3705418f98d 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -16,6 +16,9 @@ // under the License. package com.cloud.configuration.dao; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -42,6 +45,7 @@ import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.TransactionLegacy; +import com.cloud.utils.exception.CloudRuntimeException; @Component public class ResourceCountDaoImpl extends GenericDaoBase implements ResourceCountDao { @@ -248,4 +252,42 @@ public class ResourceCountDaoImpl extends GenericDaoBase return 0; } + private String baseSqlCountComputingResourceAllocatedToAccount = "Select " + + " SUM((CASE " + + " WHEN so.%s is not null THEN so.%s " + + " ELSE CONVERT(vmd.value, UNSIGNED INTEGER) " + + " END)) as total " + + " from vm_instance vm " + + " join service_offering_view so on so.id = vm.service_offering_id " + + " left join user_vm_details vmd on vmd.vm_id = vm.id and vmd.name = '%s' " + + " where vm.type = 'User' and state not in ('Destroyed', 'Error', 'Expunging') and display_vm = true and account_id = ? "; + + @Override + public long countCpuNumberAllocatedToAccount(long accountId) { + String sqlCountCpuNumberAllocatedToAccount = String.format(baseSqlCountComputingResourceAllocatedToAccount, ResourceType.cpu, ResourceType.cpu, "cpuNumber"); + return executeSqlCountComputingResourcesForAccount(accountId, sqlCountCpuNumberAllocatedToAccount); + } + + @Override + public long countMemoryAllocatedToAccount(long accountId) { + String serviceOfferingRamSizeField = "ram_size"; + String sqlCountCpuNumberAllocatedToAccount = String.format(baseSqlCountComputingResourceAllocatedToAccount, serviceOfferingRamSizeField, serviceOfferingRamSizeField, "memory"); + return executeSqlCountComputingResourcesForAccount(accountId, sqlCountCpuNumberAllocatedToAccount); + } + + private long executeSqlCountComputingResourcesForAccount(long accountId, String sqlCountComputingResourcesAllocatedToAccount) { + try (TransactionLegacy tx = TransactionLegacy.currentTxn()) { + PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlCountComputingResourcesAllocatedToAccount); + pstmt.setLong(1, accountId); + + ResultSet rs = pstmt.executeQuery(); + if (!rs.next()) { + throw new CloudRuntimeException(String.format("An unexpected case happened while counting allocated computing resources for account: " + accountId)); + } + return rs.getLong("total"); + } catch (SQLException e) { + throw new CloudRuntimeException(e); + } + } + } \ No newline at end of file diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 86fa46b6c26..df7276dcbe3 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -947,51 +947,11 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim } public long countCpusForAccount(long accountId) { - GenericSearchBuilder cpuSearch = _serviceOfferingDao.createSearchBuilder(SumCount.class); - cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu()); - SearchBuilder join1 = _userVmDao.createSearchBuilder(); - join1.and("accountId", join1.entity().getAccountId(), Op.EQ); - join1.and("type", join1.entity().getType(), Op.EQ); - join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); - join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - cpuSearch.join("offerings", join1, cpuSearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); - cpuSearch.done(); - - SearchCriteria sc = cpuSearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.User); - sc.setJoinParameters("offerings", "state", new Object[] {State.Destroyed, State.Error, State.Expunging}); - sc.setJoinParameters("offerings", "displayVm", 1); - List cpus = _serviceOfferingDao.customSearch(sc, null); - if (cpus != null) { - return cpus.get(0).sum; - } else { - return 0; - } + return _resourceCountDao.countCpuNumberAllocatedToAccount(accountId); } public long calculateMemoryForAccount(long accountId) { - GenericSearchBuilder memorySearch = _serviceOfferingDao.createSearchBuilder(SumCount.class); - memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize()); - SearchBuilder join1 = _userVmDao.createSearchBuilder(); - join1.and("accountId", join1.entity().getAccountId(), Op.EQ); - join1.and("type", join1.entity().getType(), Op.EQ); - join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); - join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - memorySearch.join("offerings", join1, memorySearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); - memorySearch.done(); - - SearchCriteria sc = memorySearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.User); - sc.setJoinParameters("offerings", "state", new Object[] {State.Destroyed, State.Error, State.Expunging}); - sc.setJoinParameters("offerings", "displayVm", 1); - List memory = _serviceOfferingDao.customSearch(sc, null); - if (memory != null) { - return memory.get(0).sum; - } else { - return 0; - } + return _resourceCountDao.countMemoryAllocatedToAccount(accountId); } public long calculateSecondaryStorageForAccount(long accountId) {