Fixed multipe problems in account cleanup process:

* don't try to delete the template when it's already removed
* no need to perform permission check when deleteFirewallRule is called by System (as a part of cleanupAccount process for instance)
This commit is contained in:
alena 2011-08-30 11:21:03 -07:00
parent 1b5fc8a5fa
commit 0100183d78
5 changed files with 30 additions and 18 deletions

View File

@ -2109,7 +2109,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag
@Override
@DB
public boolean destroyNetwork(long networkId, ReservationContext context) {
Account callerAccount = _accountMgr.getAccount(context.getCaller().getAccountId());
Account caller = _accountMgr.getAccount(context.getCaller().getAccountId());
NetworkVO network = _networksDao.findById(networkId);
if (network == null) {
@ -2146,7 +2146,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag
boolean success = true;
if (!cleanupNetworkResources(networkId, callerAccount, context.getCaller().getId())) {
if (!cleanupNetworkResources(networkId, caller, context.getCaller().getId())) {
s_logger.warn("Unable to delete network id=" + networkId + ": failed to cleanup network resources");
return false;
}

View File

@ -428,8 +428,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
throw new InvalidParameterValueException("Unable to find " + ruleId + " having purpose " + Purpose.Firewall);
}
_accountMgr.checkAccess(caller, null, rule);
_accountMgr.checkAccess(caller, null, rule);
revokeRule(rule, caller, userId, false);
@ -456,9 +455,6 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
@Override
@DB
public void revokeRule(FirewallRuleVO rule, Account caller, long userId, boolean needUsageEvent) {
if (caller != null) {
_accountMgr.checkAccess(caller, null, rule);
}
Transaction txn = Transaction.currentTxn();
boolean generateUsageEvent = false;

View File

@ -3077,16 +3077,14 @@ public class ManagementServerImpl implements ManagementServer {
}
}
{
// delete users which will also delete accounts and release resources for those accounts
SearchCriteria<AccountVO> sc = _accountDao.createSearchCriteria();
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
List<AccountVO> accounts = _accountDao.search(sc, null);
for (AccountVO account : accounts) {
success = (success && _accountMgr.deleteAccount(account, UserContext.current().getCallerUserId(), UserContext.current().getCaller()));
if (!success) {
s_logger.warn("Failed to cleanup account id=" + account.getId() + " as a part of domain cleanup");
}
// delete users which will also delete accounts and release resources for those accounts
SearchCriteria<AccountVO> sc = _accountDao.createSearchCriteria();
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
List<AccountVO> accounts = _accountDao.search(sc, null);
for (AccountVO account : accounts) {
success = (success && _accountMgr.deleteAccount(account, UserContext.current().getCallerUserId(), UserContext.current().getCaller()));
if (!success) {
s_logger.warn("Failed to cleanup account id=" + account.getId() + " as a part of domain cleanup");
}
}

View File

@ -635,7 +635,10 @@ public class TemplateManagerImpl implements TemplateManager, Manager, TemplateSe
public boolean delete(long userId, long templateId, Long zoneId) {
VMTemplateVO template = _tmpltDao.findById(templateId);
if (template == null || template.getRemoved() != null) {
throw new InvalidParameterValueException("Please specify a valid template.");
if (s_logger.isDebugEnabled()) {
s_logger.debug("The template id=" + templateId + " either doesn't exist or already removed, so not deleting it");
}
return true;
}
TemplateAdapter adapter = getAdapter(template.getHypervisorType());

View File

@ -894,6 +894,13 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
@Override
public void checkAccess(Account caller, Domain domain) throws PermissionDeniedException {
if (caller.getId() == Account.ACCOUNT_ID_SYSTEM) {
//no need to make permission checks if the system makes the call
if (s_logger.isTraceEnabled()) {
s_logger.trace("No need to make permission check for System account, returning true");
}
return;
}
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, domain)) {
if (s_logger.isDebugEnabled()) {
@ -910,6 +917,14 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
@Override
public void checkAccess(Account caller, AccessType accessType, ControlledEntity... entities) {
HashMap<Long, List<ControlledEntity>> domains = new HashMap<Long, List<ControlledEntity>>();
if (caller.getId() == Account.ACCOUNT_ID_SYSTEM) {
//no need to make permission checks if the system makes the call
if (s_logger.isTraceEnabled()) {
s_logger.trace("No need to make permission check for System account, returning true");
}
return;
}
for (ControlledEntity entity : entities) {
long domainId = entity.getDomainId();