mirror of https://github.com/apache/cloudstack.git
bug 11462: 1) when delete PF rule, revoke corresponding firewall first (if exists) 2) never remove PF rule from the table when corresponding firewall rule wasn't removed yet
status 11462: resolved fixed Reviewed-by: edison@cloud.com
This commit is contained in:
parent
85f54b4d98
commit
5a89ba4dbb
|
|
@ -123,12 +123,13 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta
|
|||
this.setResponseObject(fwResponse);
|
||||
} finally {
|
||||
if (!result || rule == null) {
|
||||
_rulesService.revokeStaticNatRule(getEntityId(), true);
|
||||
|
||||
if (getOpenFirewall()) {
|
||||
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
|
||||
}
|
||||
|
||||
_rulesService.revokeStaticNatRule(getEntityId(), true);
|
||||
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Error in creating ip forwarding rule on the domr");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -170,13 +170,13 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCreateCmd /*implements
|
|||
lbResponse.setResponseName(getCommandName());
|
||||
} finally {
|
||||
if (!success || rule == null) {
|
||||
// no need to apply the rule on the backend as it exists in the db only
|
||||
_lbService.deleteLoadBalancerRule(getEntityId(), false);
|
||||
|
||||
if (getOpenFirewall()) {
|
||||
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
|
||||
}
|
||||
|
||||
// no need to apply the rule on the backend as it exists in the db only
|
||||
_lbService.deleteLoadBalancerRule(getEntityId(), false);
|
||||
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to create load balancer rule");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,7 +35,6 @@ import com.cloud.exception.InvalidParameterValueException;
|
|||
import com.cloud.exception.NetworkRuleConflictException;
|
||||
import com.cloud.exception.ResourceUnavailableException;
|
||||
import com.cloud.network.IpAddress;
|
||||
import com.cloud.network.rules.FirewallRule;
|
||||
import com.cloud.network.rules.PortForwardingRule;
|
||||
import com.cloud.user.Account;
|
||||
import com.cloud.user.UserContext;
|
||||
|
|
@ -145,12 +144,13 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
|
|||
fwResponse.setResponseName(getCommandName());
|
||||
} finally {
|
||||
if (!success || rule == null) {
|
||||
_rulesService.revokePortForwardingRule(getEntityId(), true);
|
||||
|
||||
if (getOpenFirewall()) {
|
||||
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
|
||||
}
|
||||
|
||||
_rulesService.revokePortForwardingRule(getEntityId(), true);
|
||||
|
||||
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to apply port forwarding rule");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -69,8 +69,8 @@ public class DeleteIpForwardingRuleCmd extends BaseAsyncCmd {
|
|||
@Override
|
||||
public void execute(){
|
||||
UserContext.current().setEventDetails("Rule Id: "+id);
|
||||
boolean result = _rulesService.revokeStaticNatRule(id, true);
|
||||
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
result = result && _rulesService.revokeStaticNatRule(id, true);
|
||||
|
||||
if (result) {
|
||||
SuccessResponse response = new SuccessResponse(getCommandName());
|
||||
|
|
|
|||
|
|
@ -83,8 +83,8 @@ public class DeleteLoadBalancerRuleCmd extends BaseAsyncCmd {
|
|||
@Override
|
||||
public void execute(){
|
||||
UserContext.current().setEventDetails("Load balancer Id: "+getId());
|
||||
boolean result = _lbService.deleteLoadBalancerRule(id, true);
|
||||
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
result = result && _lbService.deleteLoadBalancerRule(id, true);
|
||||
|
||||
if (result) {
|
||||
SuccessResponse response = new SuccessResponse(getCommandName());
|
||||
|
|
|
|||
|
|
@ -28,7 +28,6 @@ import com.cloud.api.ServerApiException;
|
|||
import com.cloud.api.response.SuccessResponse;
|
||||
import com.cloud.event.EventTypes;
|
||||
import com.cloud.exception.InvalidParameterValueException;
|
||||
import com.cloud.exception.ResourceUnavailableException;
|
||||
import com.cloud.network.rules.PortForwardingRule;
|
||||
import com.cloud.user.UserContext;
|
||||
|
||||
|
|
@ -90,9 +89,10 @@ public class DeletePortForwardingRuleCmd extends BaseAsyncCmd {
|
|||
@Override
|
||||
public void execute(){
|
||||
UserContext.current().setEventDetails("Rule Id: "+id);
|
||||
boolean result = _rulesService.revokePortForwardingRule(id, true);
|
||||
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
|
||||
//revoke corresponding firewall rule first
|
||||
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
|
||||
result = result && _rulesService.revokePortForwardingRule(id, true);
|
||||
|
||||
if (result) {
|
||||
SuccessResponse response = new SuccessResponse(getCommandName());
|
||||
this.setResponseObject(response);
|
||||
|
|
|
|||
|
|
@ -370,7 +370,12 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
|
|||
} else {
|
||||
for (FirewallRule rule : rules) {
|
||||
if (rule.getState() == FirewallRule.State.Revoke) {
|
||||
_firewallDao.remove(rule.getId());
|
||||
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");
|
||||
} else {
|
||||
_firewallDao.remove(rule.getId());
|
||||
}
|
||||
} else if (rule.getState() == FirewallRule.State.Add) {
|
||||
FirewallRuleVO ruleVO = _firewallDao.findById(rule.getId());
|
||||
ruleVO.setState(FirewallRule.State.Active);
|
||||
|
|
|
|||
|
|
@ -104,8 +104,6 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
@Inject
|
||||
IPAddressDao _ipAddressDao;
|
||||
@Inject
|
||||
FirewallRulesDao _rulesDao;
|
||||
@Inject
|
||||
LoadBalancerDao _lbDao;
|
||||
@Inject
|
||||
VlanDao _vlanDao;
|
||||
|
|
@ -351,7 +349,13 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
}
|
||||
}
|
||||
|
||||
_rulesDao.remove(lb.getId());
|
||||
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");
|
||||
} else {
|
||||
_firewallDao.remove(lb.getId());
|
||||
}
|
||||
|
||||
_elbMgr.handleDeleteLoadBalancerRule(lb, callerUserId, caller);
|
||||
s_logger.debug("Load balancer with id " + lb.getId() + " is removed successfully");
|
||||
return true;
|
||||
|
|
@ -431,7 +435,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
|
||||
try {
|
||||
_firewallMgr.detectRulesConflict(newRule, ipAddr);
|
||||
if (!_rulesDao.setStateToAdd(newRule)) {
|
||||
if (!_firewallDao.setStateToAdd(newRule)) {
|
||||
throw new CloudRuntimeException("Unable to update the state to add for " + newRule);
|
||||
}
|
||||
s_logger.debug("Load balancer " + newRule.getId() + " for Ip address id=" + ipId + ", public port " + srcPortStart + ", private port " + defPortStart + " is added successfully.");
|
||||
|
|
@ -454,8 +458,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
_firewallDao.remove(_firewallDao.findByRelatedId(newRule.getId()).getId());
|
||||
_lbDao.remove(newRule.getId());
|
||||
txn.commit();
|
||||
|
||||
_lbDao.remove(newRule.getId());
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -533,7 +536,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
|
||||
@Override
|
||||
public boolean removeAllLoadBalanacersForIp(long ipId, Account caller, long callerUserId) {
|
||||
List<FirewallRuleVO> rules = _rulesDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.LoadBalancing);
|
||||
List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.LoadBalancing);
|
||||
if (rules != null)
|
||||
s_logger.debug("Found " + rules.size() + " lb rules to cleanup");
|
||||
for (FirewallRule rule : rules) {
|
||||
|
|
@ -548,7 +551,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
|
|||
|
||||
@Override
|
||||
public boolean removeAllLoadBalanacersForNetwork(long networkId, Account caller, long callerUserId) {
|
||||
List<FirewallRuleVO> rules = _rulesDao.listByNetworkAndPurposeAndNotRevoked(networkId, Purpose.LoadBalancing);
|
||||
List<FirewallRuleVO> rules = _firewallDao.listByNetworkAndPurposeAndNotRevoked(networkId, Purpose.LoadBalancing);
|
||||
if (rules != null)
|
||||
s_logger.debug("Found " + rules.size() + " lb rules to cleanup");
|
||||
for (FirewallRule rule : rules) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue