mirror of https://github.com/apache/cloudstack.git
bug 11537: revoke related FirewallRules when do vmExpunge and ipAddress release.
status 11537: resolved fixed Reviewed-by: edison@cloud.com Conflicts: server/src/com/cloud/network/firewall/FirewallManagerImpl.java server/src/com/cloud/vm/UserVmManagerImpl.java
This commit is contained in:
parent
63117b36df
commit
a1cab92ae1
|
|
@ -2849,8 +2849,19 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag
|
|||
|
||||
private boolean cleanupIpResources(long ipId, long userId, Account caller) {
|
||||
boolean success = true;
|
||||
|
||||
|
||||
//Revoke all firewall rules for the ip
|
||||
try {
|
||||
s_logger.debug("Revoking all " + Purpose.Firewall + "rules as a part of public IP id=" + ipId + " release...");
|
||||
if (!_firewallMgr.revokeFirewallRulesForIp(ipId, userId, caller)) {
|
||||
s_logger.warn("Unable to revoke all the firewall rules for ip id=" + ipId + " as a part of ip release");
|
||||
success = false;
|
||||
}
|
||||
} catch (ResourceUnavailableException e) {
|
||||
s_logger.warn("Unable to revoke all firewall rules for ip id=" + ipId + " as a part of ip release", e);
|
||||
success = false;
|
||||
}
|
||||
|
||||
//Revoke all PF/Static nat rules for the ip
|
||||
try {
|
||||
s_logger.debug("Revoking all " + Purpose.PortForwarding + "/" + Purpose.StaticNat + " rules as a part of public IP id=" + ipId + " release...");
|
||||
|
|
@ -2881,18 +2892,6 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag
|
|||
success = false;
|
||||
}
|
||||
|
||||
//Revoke all firewall rules for the ip
|
||||
try {
|
||||
s_logger.debug("Revoking all " + Purpose.Firewall + "rules as a part of public IP id=" + ipId + " release...");
|
||||
if (!_firewallMgr.revokeFirewallRulesForIp(ipId, userId, caller)) {
|
||||
s_logger.warn("Unable to revoke all the firewall rules for ip id=" + ipId + " as a part of ip release");
|
||||
success = false;
|
||||
}
|
||||
} catch (ResourceUnavailableException e) {
|
||||
s_logger.warn("Unable to revoke all firewall rules for ip id=" + ipId + " as a part of ip release", e);
|
||||
success = false;
|
||||
}
|
||||
|
||||
return success;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import java.util.Collection;
|
|||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import javax.ejb.Local;
|
||||
import javax.naming.ConfigurationException;
|
||||
|
|
@ -57,6 +58,8 @@ import com.cloud.network.rules.FirewallRule;
|
|||
import com.cloud.network.rules.FirewallRule.Purpose;
|
||||
import com.cloud.network.rules.FirewallRule.State;
|
||||
import com.cloud.network.rules.FirewallRuleVO;
|
||||
import com.cloud.network.rules.PortForwardingRuleVO;
|
||||
import com.cloud.network.rules.dao.PortForwardingRulesDao;
|
||||
import com.cloud.user.Account;
|
||||
import com.cloud.user.AccountManager;
|
||||
import com.cloud.user.DomainManager;
|
||||
|
|
@ -73,6 +76,8 @@ import com.cloud.utils.db.SearchCriteria.Op;
|
|||
import com.cloud.utils.db.Transaction;
|
||||
import com.cloud.utils.exception.CloudRuntimeException;
|
||||
import com.cloud.utils.net.NetUtils;
|
||||
import com.cloud.vm.UserVmVO;
|
||||
import com.cloud.vm.dao.UserVmDao;
|
||||
|
||||
@Local(value = { FirewallService.class, FirewallManager.class })
|
||||
public class FirewallManagerImpl implements FirewallService, FirewallManager, Manager{
|
||||
|
|
@ -99,6 +104,9 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
ConfigurationDao _configDao;
|
||||
@Inject
|
||||
DomainManager _domainMgr;
|
||||
PortForwardingRulesDao _pfRulesDao;
|
||||
@Inject
|
||||
UserVmDao _vmDao;
|
||||
|
||||
private boolean _elbEnabled=false;
|
||||
|
||||
|
|
@ -367,6 +375,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
|
||||
@Override
|
||||
public boolean applyRules(List<? extends FirewallRule> rules, boolean continueOnError) throws ResourceUnavailableException {
|
||||
boolean success = true;
|
||||
if (!_networkMgr.applyRules(rules, continueOnError)) {
|
||||
s_logger.warn("Rules are not completely applied");
|
||||
return false;
|
||||
|
|
@ -375,7 +384,8 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
if (rule.getState() == FirewallRule.State.Revoke) {
|
||||
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(rule.getId());
|
||||
if (relatedRule != null) {
|
||||
s_logger.debug("Not removing the firewall rule id=" + rule.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
|
||||
s_logger.warn("Can't remove the firewall rule id=" + rule.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
|
||||
success = false;
|
||||
} else {
|
||||
_firewallDao.remove(rule.getId());
|
||||
}
|
||||
|
|
@ -385,8 +395,9 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
_firewallDao.update(ruleVO.getId(), ruleVO);
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
return success;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -581,4 +592,61 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public boolean revokeFirewallRulesForVm(long vmId) {
|
||||
boolean success = true;
|
||||
UserVmVO vm = _vmDao.findByIdIncludingRemoved(vmId);
|
||||
if (vm == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
List<PortForwardingRuleVO> pfRules = _pfRulesDao.listByVm(vmId);
|
||||
List<FirewallRuleVO> staticNatRules = _firewallDao.listStaticNatByVmId(vm.getId());
|
||||
List<FirewallRuleVO> firewallRules = new ArrayList<FirewallRuleVO>();
|
||||
|
||||
//Make a list of firewall rules to reprogram
|
||||
for (PortForwardingRuleVO pfRule : pfRules) {
|
||||
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(pfRule.getId());
|
||||
if (relatedRule != null) {
|
||||
firewallRules.add(relatedRule);
|
||||
}
|
||||
}
|
||||
|
||||
for (FirewallRuleVO staticNatRule : staticNatRules) {
|
||||
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(staticNatRule.getId());
|
||||
if (relatedRule != null) {
|
||||
firewallRules.add(relatedRule);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Set<Long> ipsToReprogram = new HashSet<Long>();
|
||||
|
||||
if (firewallRules.isEmpty()) {
|
||||
s_logger.debug("No firewall rules are found for vm id=" + vmId);
|
||||
return true;
|
||||
} else {
|
||||
s_logger.debug("Found " + firewallRules.size() + " to cleanup for vm id=" + vmId);
|
||||
}
|
||||
|
||||
for (FirewallRuleVO rule : firewallRules) {
|
||||
// Mark firewall rules as Revoked, but don't revoke it yet (apply=false)
|
||||
revokeFirewallRule(rule.getId(), false, _accountMgr.getSystemAccount(), Account.ACCOUNT_ID_SYSTEM);
|
||||
ipsToReprogram.add(rule.getSourceIpAddressId());
|
||||
}
|
||||
|
||||
// apply rules for all ip addresses
|
||||
for (Long ipId : ipsToReprogram) {
|
||||
s_logger.debug("Applying firewall rules for ip address id=" + ipId + " as a part of vm expunge");
|
||||
try {
|
||||
success = success && applyFirewallRules(ipId,_accountMgr.getSystemAccount());
|
||||
} catch (ResourceUnavailableException ex) {
|
||||
s_logger.warn("Failed to apply port forwarding rules for ip id=" + ipId);
|
||||
success = false;
|
||||
}
|
||||
}
|
||||
|
||||
return success;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -309,6 +309,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
LoadBalancerVO lb = _lbDao.findById(loadBalancerId);
|
||||
Transaction txn = Transaction.currentTxn();
|
||||
boolean generateUsageEvent = false;
|
||||
boolean success = true;
|
||||
|
||||
txn.start();
|
||||
if (lb.getState() == FirewallRule.State.Staged) {
|
||||
|
|
@ -353,14 +354,18 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
|
||||
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(lb.getId());
|
||||
if (relatedRule != null) {
|
||||
s_logger.debug("Not removing the firewall rule id=" + lb.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
|
||||
s_logger.warn("Unable to remove firewall rule id=" + lb.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
|
||||
success = false;
|
||||
} else {
|
||||
_firewallDao.remove(lb.getId());
|
||||
}
|
||||
|
||||
_elbMgr.handleDeleteLoadBalancerRule(lb, callerUserId, caller);
|
||||
s_logger.debug("Load balancer with id " + lb.getId() + " is removed successfully");
|
||||
return true;
|
||||
if (success) {
|
||||
s_logger.debug("Load balancer with id " + lb.getId() + " is removed successfully");
|
||||
}
|
||||
|
||||
return success;
|
||||
}
|
||||
|
||||
@Override @DB
|
||||
|
|
|
|||
|
|
@ -73,4 +73,6 @@ public interface FirewallManager extends FirewallService{
|
|||
|
||||
boolean revokeAllFirewallRulesForNetwork(long networkId, long userId, Account caller) throws ResourceUnavailableException;
|
||||
|
||||
boolean revokeFirewallRulesForVm(long vmId);
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -483,7 +483,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
|
|||
|
||||
return success;
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public boolean revokeStaticNatRulesForVm(long vmId) {
|
||||
boolean success = true;
|
||||
|
|
|
|||
|
|
@ -129,6 +129,7 @@ import com.cloud.network.dao.LoadBalancerVMMapDao;
|
|||
import com.cloud.network.dao.NetworkDao;
|
||||
import com.cloud.network.lb.LoadBalancingRulesManager;
|
||||
import com.cloud.network.router.VirtualNetworkApplianceManager;
|
||||
import com.cloud.network.rules.FirewallManager;
|
||||
import com.cloud.network.rules.RulesManager;
|
||||
import com.cloud.network.security.SecurityGroup;
|
||||
import com.cloud.network.security.SecurityGroupManager;
|
||||
|
|
@ -340,6 +341,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
|
|||
protected VMInstanceDao _vmInstanceDao;
|
||||
@Inject
|
||||
protected ResourceLimitService _resourceLimitMgr;
|
||||
@Inject
|
||||
protected FirewallManager _firewallMgr;
|
||||
|
||||
protected ScheduledExecutorService _executor = null;
|
||||
protected int _expungeInterval;
|
||||
|
|
@ -1242,8 +1245,16 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
|
|||
|
||||
//Remove vm from instance group
|
||||
removeInstanceFromInstanceGroup(vmId);
|
||||
|
||||
//cleanup firewall rules
|
||||
if (_firewallMgr.revokeFirewallRulesForVm(vmId)) {
|
||||
s_logger.debug("Firewall rules are removed successfully as a part of vm id=" + vmId + " expunge");
|
||||
} else {
|
||||
success = false;
|
||||
s_logger.warn("Fail to remove firewall rules as a part of vm id=" + vmId + " expunge");
|
||||
}
|
||||
|
||||
// cleanup port forwarding rules
|
||||
//cleanup port forwarding rules
|
||||
if (_rulesMgr.revokePortForwardingRulesForVm(vmId)) {
|
||||
s_logger.debug("Port forwarding rules are removed successfully as a part of vm id=" + vmId + " expunge");
|
||||
} else {
|
||||
|
|
|
|||
Loading…
Reference in New Issue