Skip to content

Commit e72a69a

Browse files
swillswill
committed
Merge pull request #1254 from shapeblue/master-9174
CLOUDSTACK-9174: A deleted account results in NPEWhen an account is deleted from cloudstack for which quota is still being calculated and if the quota reaches minimum threshold then quota service will try to alert the user. This results in NPE and is fixed by excluding such accounts from alerting and other quota related mechanisms. * pr/1254: CLOUDSTACK-9174: A deleted account results in NPE Signed-off-by: Will Stevens <williamstevens@gmail.com>
2 parents bb48d7f + 983dee7 commit e72a69a

6 files changed

Lines changed: 21 additions & 20 deletions

File tree

‎framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public void checkAndSendQuotaAlertEmails() {
153153
BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
154154
if (accountBalance != null) {
155155
AccountVO account = _accountDao.findById(quotaAccount.getId());
156+
if (account == null) continue; // the account is removed
156157
if (s_logger.isDebugEnabled()) {
157158
s_logger.debug("checkAndSendQuotaAlertEmails: Check id=" + account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", lockable=" + lockable);
158159
}

‎framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,10 +360,11 @@ public List<QuotaUsageVO> updateQuotaRunningVMUsage(UsageVO usageRecord, final B
360360
BigDecimal rawusage;
361361
// get service offering details
362362
ServiceOfferingVO serviceoffering = _serviceOfferingDao.findServiceOffering(usageRecord.getVmInstanceId(), usageRecord.getOfferingId());
363+
if (serviceoffering == null) return quotalist;
363364
rawusage = new BigDecimal(usageRecord.getRawUsage());
364365

365366
QuotaTariffVO tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.CPU_NUMBER, usageRecord.getEndDate());
366-
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) {
367+
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getCpu() != null) {
367368
BigDecimal cpu = new BigDecimal(serviceoffering.getCpu());
368369
onehourcostpercpu = tariff.getCurrencyValue().multiply(aggregationRatio);
369370
cpuquotausgage = rawusage.multiply(onehourcostpercpu).multiply(cpu);
@@ -373,7 +374,7 @@ public List<QuotaUsageVO> updateQuotaRunningVMUsage(UsageVO usageRecord, final B
373374
quotalist.add(quota_usage);
374375
}
375376
tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.CPU_CLOCK_RATE, usageRecord.getEndDate());
376-
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) {
377+
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getSpeed() != null) {
377378
BigDecimal speed = new BigDecimal(serviceoffering.getSpeed() / 100.00);
378379
onehourcostper100mhz = tariff.getCurrencyValue().multiply(aggregationRatio);
379380
speedquotausage = rawusage.multiply(onehourcostper100mhz).multiply(speed);
@@ -383,7 +384,7 @@ public List<QuotaUsageVO> updateQuotaRunningVMUsage(UsageVO usageRecord, final B
383384
quotalist.add(quota_usage);
384385
}
385386
tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.MEMORY, usageRecord.getEndDate());
386-
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) {
387+
if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getRamSize() != null) {
387388
BigDecimal memory = new BigDecimal(serviceoffering.getRamSize());
388389
onehourcostper1mb = tariff.getCurrencyValue().multiply(aggregationRatio);
389390
memoryquotausage = rawusage.multiply(onehourcostper1mb).multiply(memory);

‎framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,17 @@ public void sendStatement() {
122122
Date lastStatementDate = quotaAccount.getLastStatementDate();
123123
if (interval != null) {
124124
AccountVO account = _accountDao.findById(quotaAccount.getId());
125-
if (lastStatementDate == null || getDifferenceDays(lastStatementDate, new Date()) >= s_LAST_STATEMENT_SENT_DAYS + 1) {
126-
BigDecimal quotaUsage = _quotaUsage.findTotalQuotaUsage(account.getAccountId(), account.getDomainId(), null, interval[0].getTime(), interval[1].getTime());
127-
s_logger.info("For account=" + quotaAccount.getId() + ", quota used = " + quotaUsage);
128-
// send statement
129-
deferredQuotaEmailList.add(new DeferredQuotaEmail(account, quotaAccount, quotaUsage, QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT));
130-
} else {
131-
if (s_logger.isDebugEnabled()) {
132-
s_logger.debug("For " + quotaAccount.getId() + " the statement has been sent recently");
125+
if (account != null) {
126+
if (lastStatementDate == null || getDifferenceDays(lastStatementDate, new Date()) >= s_LAST_STATEMENT_SENT_DAYS + 1) {
127+
BigDecimal quotaUsage = _quotaUsage.findTotalQuotaUsage(account.getAccountId(), account.getDomainId(), null, interval[0].getTime(), interval[1].getTime());
128+
s_logger.info("For account=" + quotaAccount.getId() + ", quota used = " + quotaUsage);
129+
// send statement
130+
deferredQuotaEmailList.add(new DeferredQuotaEmail(account, quotaAccount, quotaUsage, QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT));
131+
} else {
132+
if (s_logger.isDebugEnabled()) {
133+
s_logger.debug("For " + quotaAccount.getId() + " the statement has been sent recently");
133134

135+
}
134136
}
135137
}
136138
} else if (lastStatementDate != null) {

‎plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public QuotaSummaryCmd() {
5959
public void execute() {
6060
Account caller = CallContext.current().getCallingAccount();
6161
List<QuotaSummaryResponse> responses;
62-
if (caller.getAccountId() <= 2) { //non root admin or system
62+
if (caller.getType() == Account.ACCOUNT_TYPE_ADMIN) { //admin account
6363
if (getAccountName() != null && getDomainId() != null)
6464
responses = _responseBuilder.createQuotaSummaryResponse(caller.getAccountName(), caller.getDomainId());
6565
else

‎plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ public List<QuotaSummaryResponse> createQuotaSummaryResponse(Boolean listAll) {
138138
} else {
139139
for (final QuotaAccountVO quotaAccount : _quotaAccountDao.listAllQuotaAccount()) {
140140
AccountVO account = _accountDao.findById(quotaAccount.getId());
141+
if (account == null) continue;
141142
QuotaSummaryResponse qr = getQuotaSummaryResponse(account);
142143
result.add(qr);
143144
}
@@ -167,7 +168,7 @@ private QuotaSummaryResponse getQuotaSummaryResponse(final Account account) {
167168
qr.setObjectName("summary");
168169
return qr;
169170
} else {
170-
throw new InvalidParameterValueException("Quota summary response for an account requires a valid account.");
171+
return new QuotaSummaryResponse();
171172
}
172173
}
173174

@@ -398,6 +399,9 @@ public QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Doubl
398399
QuotaCreditsVO result = _quotaCreditsDao.saveCredits(credits);
399400

400401
final AccountVO account = _accountDao.findById(accountId);
402+
if (account == null) {
403+
throw new InvalidParameterValueException("Account does not exist with account id " + accountId);
404+
}
401405
final boolean lockAccountEnforcement = "true".equalsIgnoreCase(QuotaConfig.QuotaEnableEnforcement.value());
402406
final BigDecimal currentAccountBalance = _quotaBalanceDao.lastQuotaBalance(accountId, domainId, startOfNextDay(new Date(despositedOn.getTime())));
403407
if (s_logger.isDebugEnabled()) {

‎ui/plugins/quota/quota.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -328,13 +328,6 @@
328328
});
329329
},
330330
detailView: {
331-
viewAll: [{
332-
path: 'quota.quotastatement',
333-
label: 'label.quota.statement.quota'
334-
},{
335-
path: 'quota.balancestatement',
336-
label: 'label.quota.statement.balance'
337-
}],
338331
actions: {
339332
add: {
340333
label: 'label.quota.add.credits',

0 commit comments

Comments
 (0)