This is an automated email from the ASF dual-hosted git repository. sureshanaparti pushed a commit to branch 4.19 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push: new d79735606bb Handle public IP race conditions (#9234) d79735606bb is described below commit d79735606bbe5e80ee1642bdb234028f9e7b5dcd Author: Henrique Sato <henriquesato2...@gmail.com> AuthorDate: Sat Jun 29 01:58:01 2024 -0300 Handle public IP race conditions (#9234) * Lock public IP * Release IP if ID is not null * Fix NPEs Co-authored-by: Henrique Sato <henrique.s...@scclouds.com.br> --- .../com/cloud/network/IpAddressManagerImpl.java | 77 +++--- .../java/com/cloud/network/NetworkModelImpl.java | 4 + .../network/firewall/FirewallManagerImpl.java | 95 ++++---- .../network/lb/LoadBalancingRulesManagerImpl.java | 94 +++----- .../com/cloud/network/rules/RulesManagerImpl.java | 262 ++++++++++----------- .../network/vpn/RemoteAccessVpnManagerImpl.java | 130 +++++----- 6 files changed, 336 insertions(+), 326 deletions(-) diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 0178236a21a..c8ac0c1016b 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -725,50 +725,59 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @Override @DB public boolean disassociatePublicIpAddress(long addrId, long userId, Account caller) { - boolean success = true; - IPAddressVO ipToBeDisassociated = _ipAddressDao.findById(addrId); - PublicIpQuarantine publicIpQuarantine = null; - // Cleanup all ip address resources - PF/LB/Static nat rules - if (!cleanupIpResources(addrId, userId, caller)) { - success = false; - s_logger.warn("Failed to release resources for ip address id=" + addrId); - } + try { + IPAddressVO ipToBeDisassociated = _ipAddressDao.acquireInLockTable(addrId); - IPAddressVO ip = markIpAsUnavailable(addrId); - if (ip == null) { - return true; - } + if (ipToBeDisassociated == null) { + s_logger.error(String.format("Unable to acquire lock on public IP %s.", addrId)); + throw new CloudRuntimeException("Unable to acquire lock on public IP."); + } - if (s_logger.isDebugEnabled()) { - s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " + ip.isSourceNat()); - } + PublicIpQuarantine publicIpQuarantine = null; + // Cleanup all ip address resources - PF/LB/Static nat rules + if (!cleanupIpResources(addrId, userId, caller)) { + success = false; + s_logger.warn("Failed to release resources for ip address id=" + addrId); + } - if (ip.getAssociatedWithNetworkId() != null) { - Network network = _networksDao.findById(ip.getAssociatedWithNetworkId()); - try { - if (!applyIpAssociations(network, rulesContinueOnErrFlag)) { - s_logger.warn("Unable to apply ip address associations for " + network); - success = false; + IPAddressVO ip = markIpAsUnavailable(addrId); + if (ip == null) { + return true; + } + + if (s_logger.isDebugEnabled()) { + s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " + ip.isSourceNat()); + } + + if (ip.getAssociatedWithNetworkId() != null) { + Network network = _networksDao.findById(ip.getAssociatedWithNetworkId()); + try { + if (!applyIpAssociations(network, rulesContinueOnErrFlag)) { + s_logger.warn("Unable to apply ip address associations for " + network); + success = false; + } + } catch (ResourceUnavailableException e) { + throw new CloudRuntimeException("We should never get to here because we used true when applyIpAssociations", e); } - } catch (ResourceUnavailableException e) { - throw new CloudRuntimeException("We should never get to here because we used true when applyIpAssociations", e); + } else if (ip.getState() == State.Releasing) { + publicIpQuarantine = addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId()); + _ipAddressDao.unassignIpAddress(ip.getId()); } - } else if (ip.getState() == State.Releasing) { - publicIpQuarantine = addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId()); - _ipAddressDao.unassignIpAddress(ip.getId()); - } - annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), ip.getUuid()); + annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), ip.getUuid()); - if (success) { - if (ip.isPortable()) { - releasePortableIpAddress(addrId); + if (success) { + if (ip.isPortable()) { + releasePortableIpAddress(addrId); + } + s_logger.debug("Released a public ip id=" + addrId); + } else if (publicIpQuarantine != null) { + removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP address removed from quarantine as there was an error while disassociating it."); } - s_logger.debug("Released a public ip id=" + addrId); - } else if (publicIpQuarantine != null) { - removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP address removed from quarantine as there was an error while disassociating it."); + } finally { + _ipAddressDao.releaseFromLockTable(addrId); } return success; diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index 2a604796d6e..4088e9539ea 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1612,6 +1612,10 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi } NetworkVO network = _networksDao.findById(networkId); + if (network == null) { + throw new CloudRuntimeException("Could not find network associated with public IP."); + } + NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); if (offering.getGuestType() != GuestType.Isolated) { return true; diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index b08df5a3d1b..4ed480ea68b 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -194,57 +194,54 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, return createFirewallRule(sourceIpAddressId, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), null, rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); } + //Destination CIDR capability is currently implemented for egress rules only. For others, the field is passed as null. @DB - protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, - final String protocol, final List<String> sourceCidrList, final List<String> destCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, - final FirewallRule.FirewallRuleType type, - final Long networkId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { - + protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, final String protocol, + final List<String> sourceCidrList, final List<String> destCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, + final FirewallRule.FirewallRuleType type, final Long networkId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { IPAddressVO ipAddress = null; - if (ipAddrId != null) { - // this for ingress firewall rule, for egress id is null - ipAddress = _ipAddressDao.findById(ipAddrId); - // Validate ip address - if (ipAddress == null && type == FirewallRule.FirewallRuleType.User) { - throw new InvalidParameterValueException("Unable to create firewall rule; " + "couldn't locate IP address by id in the system"); - } - _networkModel.checkIpForService(ipAddress, Service.Firewall, null); - } + try { + // Validate ip address + if (ipAddrId != null) { + // this for ingress firewall rule, for egress id is null + ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create firewall rule; " + "couldn't locate IP address by id in the system"); + } + _networkModel.checkIpForService(ipAddress, Service.Firewall, null); + } - validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type, networkId, trafficType); + validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type, networkId, trafficType); - // icmp code and icmp type can't be passed in for any other protocol rather than icmp - if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) { - throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only"); - } + // icmp code and icmp type can't be passed in for any other protocol rather than icmp + if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) { + throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only"); + } - if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { - throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP"); - } + if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { + throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP"); + } - Long accountId = null; - Long domainId = null; + Long accountId = null; + Long domainId = null; - if (ipAddress != null) { - //Ingress firewall rule - accountId = ipAddress.getAllocatedToAccountId(); - domainId = ipAddress.getAllocatedInDomainId(); - } else if (networkId != null) { - //egress firewall rule + if (ipAddress != null) { + //Ingress firewall rule + accountId = ipAddress.getAllocatedToAccountId(); + domainId = ipAddress.getAllocatedInDomainId(); + } else if (networkId != null) { + //egress firewall rule Network network = _networkModel.getNetwork(networkId); accountId = network.getAccountId(); domainId = network.getDomainId(); - } + } - final Long accountIdFinal = accountId; - final Long domainIdFinal = domainId; - return Transaction.execute(new TransactionCallbackWithException<FirewallRuleVO, NetworkRuleConflictException>() { - @Override - public FirewallRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { - FirewallRuleVO newRule = - new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, - sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); + final Long accountIdFinal = accountId; + final Long domainIdFinal = domainId; + return Transaction.execute((TransactionCallbackWithException<FirewallRuleVO, NetworkRuleConflictException>) status -> { + FirewallRuleVO newRule = new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, + sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); newRule.setType(type); if (forDisplay != null) { newRule.setDisplay(forDisplay); @@ -261,8 +258,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, CallContext.current().putContextParameter(FirewallRule.class, newRule.getId()); return newRule; + }); + } finally { + if (ipAddrId != null) { + _ipAddressDao.releaseFromLockTable(ipAddrId); } - }); + } } @Override @@ -668,9 +669,19 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, } @Override + @DB public boolean applyIngressFirewallRules(long ipId, Account caller) throws ResourceUnavailableException { - List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurpose(ipId, Purpose.Firewall); - return applyFirewallRules(rules, false, caller); + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipId); + if (ipAddress == null) { + s_logger.error(String.format("Unable to acquire lock for public IP [%s].", ipId)); + throw new CloudRuntimeException("Unable to acquire lock for public IP."); + } + List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurpose(ipId, Purpose.Firewall); + return applyFirewallRules(rules, false, caller); + } finally { + _ipAddressDao.releaseFromLockTable(ipId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 8cb8972e295..e0d8082c0d9 100644 --- a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -1814,13 +1814,12 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements } return cidr; } + @DB @Override - public LoadBalancer createPublicLoadBalancer(final String xId, final String name, final String description, final int srcPort, final int destPort, - final long sourceIpId, - final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, final Boolean forDisplay, String cidrList) - throws NetworkRuleConflictException { - + public LoadBalancer createPublicLoadBalancer(final String xId, final String name, final String description, final int srcPort, final int destPort, final long sourceIpId, + final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, + final Boolean forDisplay, String cidrList) throws NetworkRuleConflictException { if (!NetUtils.isValidPort(destPort)) { throw new InvalidParameterValueException("privatePort is an invalid value: " + destPort); } @@ -1829,55 +1828,41 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements throw new InvalidParameterValueException("Invalid algorithm: " + algorithm); } - final IPAddressVO ipAddr = _ipAddressDao.findById(sourceIpId); - // make sure ip address exists - if (ipAddr == null || !ipAddr.readyToUse()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule, invalid IP address id specified"); - if (ipAddr == null) { - ex.addProxyObject(String.valueOf(sourceIpId), "sourceIpId"); - } else { + try { + final IPAddressVO ipAddr = _ipAddressDao.acquireInLockTable(sourceIpId); + + // make sure ip address exists + if (ipAddr == null || !ipAddr.readyToUse()) { + InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule, invalid IP address id specified"); + if (ipAddr == null) { + ex.addProxyObject(String.valueOf(sourceIpId), "sourceIpId"); + } else { + ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + } + throw ex; + } else if (ipAddr.isOneToOneNat()) { + InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule; specified sourceip id has static nat enabled"); ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + throw ex; } - throw ex; - } else if (ipAddr.isOneToOneNat()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule; specified sourceip id has static nat enabled"); - ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); - throw ex; - } - _accountMgr.checkAccess(caller.getCallingAccount(), null, true, ipAddr); + _accountMgr.checkAccess(caller.getCallingAccount(), null, true, ipAddr); - final Long networkId = ipAddr.getAssociatedWithNetworkId(); - if (networkId == null) { - InvalidParameterValueException ex = - new InvalidParameterValueException("Unable to create load balancer rule ; specified sourceip id is not associated with any network"); - ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); - throw ex; - } - - // verify that lb service is supported by the network - isLbServiceSupportedInNetwork(networkId, Scheme.Public); - - _firewallMgr.validateFirewallRule(caller.getCallingAccount(), ipAddr, srcPort, srcPort, protocol, Purpose.LoadBalancing, FirewallRuleType.User, networkId, null); + final Long networkId = ipAddr.getAssociatedWithNetworkId(); + if (networkId == null) { + InvalidParameterValueException ex = + new InvalidParameterValueException("Unable to create load balancer rule ; specified sourceip id is not associated with any network"); + ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + throw ex; + } - LoadBalancerVO newRule = - new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), - ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList); + // verify that lb service is supported by the network + isLbServiceSupportedInNetwork(networkId, Scheme.Public); - // verify rule is supported by Lb provider of the network - Ip sourceIp = getSourceIp(newRule); - LoadBalancingRule loadBalancing = - new LoadBalancingRule(newRule, new ArrayList<LbDestination>(), new ArrayList<LbStickinessPolicy>(), new ArrayList<LbHealthCheckPolicy>(), sourceIp, null, - lbProtocol); - if (!validateLbRule(loadBalancing)) { - throw new InvalidParameterValueException("LB service provider cannot support this rule"); - } + _firewallMgr.validateFirewallRule(caller.getCallingAccount(), ipAddr, srcPort, srcPort, protocol, Purpose.LoadBalancing, FirewallRuleType.User, networkId, null); - return Transaction.execute(new TransactionCallbackWithException<LoadBalancerVO, NetworkRuleConflictException>() { - @Override - public LoadBalancerVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { - LoadBalancerVO newRule = - new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), + return Transaction.execute((TransactionCallbackWithException<LoadBalancerVO, NetworkRuleConflictException>) status -> { + LoadBalancerVO newRule = new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList); if (forDisplay != null) { @@ -1886,9 +1871,7 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements // verify rule is supported by Lb provider of the network Ip sourceIp = getSourceIp(newRule); - LoadBalancingRule loadBalancing = - new LoadBalancingRule(newRule, new ArrayList<LbDestination>(), new ArrayList<LbStickinessPolicy>(), new ArrayList<LbHealthCheckPolicy>(), sourceIp, - null, lbProtocol); + LoadBalancingRule loadBalancing = new LoadBalancingRule(newRule, new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), sourceIp, null, lbProtocol); if (!validateLbRule(loadBalancing)) { throw new InvalidParameterValueException("LB service provider cannot support this rule"); } @@ -1908,10 +1891,10 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements throw new CloudRuntimeException("Unable to update the state to add for " + newRule); } s_logger.debug("Load balancer " + newRule.getId() + " for Ip address id=" + sourceIpId + ", public port " + srcPort + ", private port " + destPort + - " is added successfully."); + " is added successfully."); CallContext.current().setEventDetails("Load balancer Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_LOAD_BALANCER_CREATE, ipAddr.getAllocatedToAccountId(), ipAddr.getDataCenterId(), newRule.getId(), - null, LoadBalancingRule.class.getName(), newRule.getUuid()); + null, LoadBalancingRule.class.getName(), newRule.getUuid()); return newRule; } catch (Exception e) { @@ -1926,9 +1909,10 @@ public class LoadBalancingRulesManagerImpl<Type> extends ManagerBase implements removeLBRule(newRule); } } - } - }); - + }); + } finally { + _ipAddressDao.releaseFromLockTable(sourceIpId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java index 624fbfb9d24..7686a8d7887 100644 --- a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java @@ -207,124 +207,122 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules final Long ipAddrId = rule.getSourceIpAddressId(); - IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); - - // Validate ip address - if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system"); - } else if (ipAddress.isOneToOneNat()) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); - } + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); - final Long networkId = rule.getNetworkId(); - Network network = _networkModel.getNetwork(networkId); - //associate ip address to network (if needed) - boolean performedIpAssoc = false; - Nic guestNic; - if (ipAddress.getAssociatedWithNetworkId() == null) { - boolean assignToVpcNtwk = network.getVpcId() != null && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); - if (assignToVpcNtwk) { - _networkModel.checkIpForService(ipAddress, Service.PortForwarding, networkId); + // Validate ip address + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system"); + } else if (ipAddress.isOneToOneNat()) { + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); + } - s_logger.debug("The ip is not associated with the VPC network id=" + networkId + ", so assigning"); - try { - ipAddress = _ipAddrMgr.associateIPToGuestNetwork(ipAddrId, networkId, false); - performedIpAssoc = true; - } catch (Exception ex) { - throw new CloudRuntimeException("Failed to associate ip to VPC network as " + "a part of port forwarding rule creation"); + final Long networkId = rule.getNetworkId(); + Network network = _networkModel.getNetwork(networkId); + //associate ip address to network (if needed) + boolean performedIpAssoc = false; + Nic guestNic; + if (ipAddress.getAssociatedWithNetworkId() == null) { + boolean assignToVpcNtwk = network.getVpcId() != null && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); + if (assignToVpcNtwk) { + _networkModel.checkIpForService(ipAddress, Service.PortForwarding, networkId); + + s_logger.debug("The ip is not associated with the VPC network id=" + networkId + ", so assigning"); + try { + ipAddress = _ipAddrMgr.associateIPToGuestNetwork(ipAddrId, networkId, false); + performedIpAssoc = true; + } catch (Exception ex) { + throw new CloudRuntimeException("Failed to associate ip to VPC network as " + "a part of port forwarding rule creation"); + } } + } else { + _networkModel.checkIpForService(ipAddress, Service.PortForwarding, null); } - } else { - _networkModel.checkIpForService(ipAddress, Service.PortForwarding, null); - } - if (ipAddress.getAssociatedWithNetworkId() == null) { - throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); - } + if (ipAddress.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); + } - try { - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, - FirewallRuleType.User, networkId, rule.getTrafficType()); + try { + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, + FirewallRuleType.User, networkId, rule.getTrafficType()); - final Long accountId = ipAddress.getAllocatedToAccountId(); - final Long domainId = ipAddress.getAllocatedInDomainId(); + final Long accountId = ipAddress.getAllocatedToAccountId(); + final Long domainId = ipAddress.getAllocatedInDomainId(); - // start port can't be bigger than end port - if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) { - throw new InvalidParameterValueException("Start port can't be bigger than end port"); - } + // start port can't be bigger than end port + if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) { + throw new InvalidParameterValueException("Start port can't be bigger than end port"); + } - // check that the port ranges are of equal size - if ((rule.getDestinationPortEnd() - rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - rule.getSourcePortStart())) { - throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes."); - } + // check that the port ranges are of equal size + if ((rule.getDestinationPortEnd() - rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - rule.getSourcePortStart())) { + throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes."); + } - // validate user VM exists - UserVm vm = _vmDao.findById(vmId); - if (vm == null) { - throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" + - vmId + ")."); - } else if (vm.getState() == VirtualMachine.State.Destroyed || vm.getState() == VirtualMachine.State.Expunging) { - throw new InvalidParameterValueException("Invalid user vm: " + vm.getId()); - } + // validate user VM exists + UserVm vm = _vmDao.findById(vmId); + if (vm == null) { + throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" + + vmId + ")."); + } else if (vm.getState() == VirtualMachine.State.Destroyed || vm.getState() == VirtualMachine.State.Expunging) { + throw new InvalidParameterValueException("Invalid user vm: " + vm.getId()); + } - // Verify that vm has nic in the network - Ip dstIp = rule.getDestinationIpAddress(); - guestNic = _networkModel.getNicInNetwork(vmId, networkId); - if (guestNic == null || guestNic.getIPv4Address() == null) { - throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); - } else { - dstIp = new Ip(guestNic.getIPv4Address()); - } + // Verify that vm has nic in the network + Ip dstIp = rule.getDestinationIpAddress(); + guestNic = _networkModel.getNicInNetwork(vmId, networkId); + if (guestNic == null || guestNic.getIPv4Address() == null) { + throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); + } else { + dstIp = new Ip(guestNic.getIPv4Address()); + } - if (vmIp != null) { - //vm ip is passed so it can be primary or secondary ip addreess. - if (!dstIp.equals(vmIp)) { - //the vm ip is secondary ip to the nic. - // is vmIp is secondary ip or not - NicSecondaryIp secondaryIp = _nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId()); - if (secondaryIp == null) { - throw new InvalidParameterValueException("IP Address is not in the VM nic's network "); + if (vmIp != null) { + //vm ip is passed so it can be primary or secondary ip addreess. + if (!dstIp.equals(vmIp)) { + //the vm ip is secondary ip to the nic. + // is vmIp is secondary ip or not + NicSecondaryIp secondaryIp = _nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId()); + if (secondaryIp == null) { + throw new InvalidParameterValueException("IP Address is not in the VM nic's network "); + } + dstIp = vmIp; } - dstIp = vmIp; } - } - - //if start port and end port are passed in, and they are not equal to each other, perform the validation - boolean validatePortRange = false; - if (rule.getSourcePortStart().intValue() != rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != rule.getDestinationPortEnd()) { - validatePortRange = true; - } - if (validatePortRange) { - //source start port and source dest port should be the same. The same applies to dest ports - if (rule.getSourcePortStart().intValue() != rule.getDestinationPortStart()) { - throw new InvalidParameterValueException("Private port start should be equal to public port start"); + //if start port and end port are passed in, and they are not equal to each other, perform the validation + boolean validatePortRange = false; + if (rule.getSourcePortStart().intValue() != rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != rule.getDestinationPortEnd()) { + validatePortRange = true; } - if (rule.getSourcePortEnd().intValue() != rule.getDestinationPortEnd()) { - throw new InvalidParameterValueException("Private port end should be equal to public port end"); + if (validatePortRange) { + //source start port and source dest port should be the same. The same applies to dest ports + if (rule.getSourcePortStart().intValue() != rule.getDestinationPortStart()) { + throw new InvalidParameterValueException("Private port start should be equal to public port start"); + } + + if (rule.getSourcePortEnd().intValue() != rule.getDestinationPortEnd()) { + throw new InvalidParameterValueException("Private port end should be equal to public port end"); + } } - } - final Ip dstIpFinal = dstIp; - final IPAddressVO ipAddressFinal = ipAddress; - return Transaction.execute(new TransactionCallbackWithException<PortForwardingRuleVO, NetworkRuleConflictException>() { - @Override - public PortForwardingRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + final Ip dstIpFinal = dstIp; + final IPAddressVO ipAddressFinal = ipAddress; + return Transaction.execute((TransactionCallbackWithException<PortForwardingRuleVO, NetworkRuleConflictException>) status -> { PortForwardingRuleVO newRule = - new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal, - rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); + new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal, + rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); if (forDisplay != null) { newRule.setDisplay(forDisplay); } newRule = _portForwardingDao.persist(newRule); - // create firewallRule for 0.0.0.0/0 cidr if (openFirewall) { _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, - newRule.getId(), networkId); + newRule.getId(), networkId); } try { @@ -334,7 +332,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, newRule.getAccountId(), ipAddressFinal.getDataCenterId(), newRule.getId(), null, - PortForwardingRule.class.getName(), newRule.getUuid()); + PortForwardingRule.class.getName(), newRule.getUuid()); return newRule; } catch (Exception e) { if (newRule != null) { @@ -349,16 +347,17 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules throw new CloudRuntimeException("Unable to add rule for the ip id=" + ipAddrId, e); } + }); + } finally { + // release ip address if ipassoc was perfored + if (performedIpAssoc) { + //if the rule is the last one for the ip address assigned to VPC, unassign it from the network + IpAddress ip = _ipAddressDao.findById(ipAddress.getId()); + _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId); } - }); - - } finally { - // release ip address if ipassoc was perfored - if (performedIpAssoc) { - //if the rule is the last one for the ip address assigned to VPC, unassign it from the network - IpAddress ip = _ipAddressDao.findById(ipAddress.getId()); - _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId); } + } finally { + _ipAddressDao.releaseFromLockTable(ipAddrId); } } @@ -370,46 +369,44 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules final Long ipAddrId = rule.getSourceIpAddressId(); - IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); - - // Validate ip address - if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create static nat rule; ip id=" + ipAddrId + " doesn't exist in the system"); - } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { - throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); - } + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, - FirewallRuleType.User, null, rule.getTrafficType()); + // Validate ip address + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create static nat rule; ip id=" + ipAddrId + " doesn't exist in the system"); + } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { + throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); + } - final Long networkId = ipAddress.getAssociatedWithNetworkId(); - final Long accountId = ipAddress.getAllocatedToAccountId(); - final Long domainId = ipAddress.getAllocatedInDomainId(); + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, + FirewallRuleType.User, null, rule.getTrafficType()); - _networkModel.checkIpForService(ipAddress, Service.StaticNat, null); + final Long networkId = ipAddress.getAssociatedWithNetworkId(); + final Long accountId = ipAddress.getAllocatedToAccountId(); + final Long domainId = ipAddress.getAllocatedInDomainId(); - Network network = _networkModel.getNetwork(networkId); - NetworkOffering off = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); - if (off.isElasticIp()) { - throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled"); - } + _networkModel.checkIpForService(ipAddress, Service.StaticNat, null); - //String dstIp = _networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); - final String dstIp = ipAddress.getVmIp(); - return Transaction.execute(new TransactionCallbackWithException<StaticNatRule, NetworkRuleConflictException>() { - @Override - public StaticNatRule doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + Network network = _networkModel.getNetwork(networkId); + NetworkOffering off = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); + if (off.isElasticIp()) { + throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled"); + } + //String dstIp = _networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); + final String dstIp = ipAddress.getVmIp(); + return Transaction.execute((TransactionCallbackWithException<StaticNatRule, NetworkRuleConflictException>) status -> { FirewallRuleVO newRule = - new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), - networkId, accountId, domainId, rule.getPurpose(), null, null, null, null, null); + new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), + networkId, accountId, domainId, rule.getPurpose(), null, null, null, null, null); newRule = _firewallDao.persist(newRule); // create firewallRule for 0.0.0.0/0 cidr if (openFirewall) { _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, - newRule.getId(), networkId); + newRule.getId(), networkId); } try { @@ -419,11 +416,9 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, newRule.getAccountId(), 0, newRule.getId(), null, FirewallRule.class.getName(), - newRule.getUuid()); + newRule.getUuid()); - StaticNatRule staticNatRule = new StaticNatRuleImpl(newRule, dstIp); - - return staticNatRule; + return new StaticNatRuleImpl(newRule, dstIp); } catch (Exception e) { if (newRule != null) { // no need to apply the rule as it wasn't programmed on the backend yet @@ -436,9 +431,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } throw new CloudRuntimeException("Unable to add static nat rule for the ip id=" + newRule.getSourceIpAddressId(), e); } - } - }); - + }); + } finally { + _ipAddressDao.releaseFromLockTable(ipAddrId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index 61d247d7b8a..9ff599b47c6 100644 --- a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -155,6 +155,7 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc return vpns; } + @Override @DB public RemoteAccessVpn createRemoteAccessVpn(final long publicIpId, String ipRange, boolean openFirewall, final Boolean forDisplay) throws NetworkRuleConflictException { @@ -172,92 +173,97 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc throw new InvalidParameterValueException("The Ip address is not ready to be used yet: " + ipAddr.getAddress()); } - IPAddressVO ipAddress = _ipAddressDao.findById(publicIpId); - - Long networkId = ipAddress.getAssociatedWithNetworkId(); - if (networkId != null) { - _networkMgr.checkIpForService(ipAddress, Service.Vpn, null); - } - - final Long vpcId = ipAddress.getVpcId(); - if (vpcId != null && ipAddress.isSourceNat()) { - assert networkId == null; - openFirewall = false; - } - - final boolean openFirewallFinal = openFirewall; - - if (networkId == null && vpcId == null) { - throw new InvalidParameterValueException("Unable to create remote access vpn for the ipAddress: " + ipAddr.getAddress().addr() + - " as ip is not associated with any network or VPC"); - } - - RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(publicIpId); - if (vpnVO != null) { - if (vpnVO.getState() == RemoteAccessVpn.State.Added) { - return vpnVO; + if (ipAddress == null) { + s_logger.error(String.format("Unable to acquire lock on public IP %s.", publicIpId)); + throw new CloudRuntimeException("Unable to acquire lock on public IP."); } - throw new InvalidParameterValueException(String.format("A remote Access VPN already exists for the public IP address [%s].", ipAddr.getAddress().toString())); - } + Long networkId = ipAddress.getAssociatedWithNetworkId(); + if (networkId != null) { + _networkMgr.checkIpForService(ipAddress, Service.Vpn, null); + } - if (ipRange == null) { - ipRange = RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId()); - } + final Long vpcId = ipAddress.getVpcId(); + if (vpcId != null && ipAddress.isSourceNat()) { + assert networkId == null; + openFirewall = false; + } - validateIpRange(ipRange, InvalidParameterValueException.class); + final boolean openFirewallFinal = openFirewall; - String[] range = ipRange.split("-"); + if (networkId == null && vpcId == null) { + throw new InvalidParameterValueException("Unable to create remote access vpn for the ipAddress: " + ipAddr.getAddress().addr() + + " as ip is not associated with any network or VPC"); + } - Pair<String, Integer> cidr = null; + RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); - if (networkId != null) { - long ipAddressOwner = ipAddr.getAccountId(); - vpnVO = _remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId); if (vpnVO != null) { if (vpnVO.getState() == RemoteAccessVpn.State.Added) { return vpnVO; } - throw new InvalidParameterValueException(String.format("A remote access VPN already exists for the account [%s].", ipAddressOwner)); + throw new InvalidParameterValueException(String.format("A remote Access VPN already exists for the public IP address [%s].", ipAddr.getAddress().toString())); } - Network network = _networkMgr.getNetwork(networkId); - if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.Vpn)) { - throw new InvalidParameterValueException("Vpn service is not supported in network id=" + ipAddr.getAssociatedWithNetworkId()); + + if (ipRange == null) { + ipRange = RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId()); } - cidr = NetUtils.getCidr(network.getCidr()); - } else { - Vpc vpc = _vpcDao.findById(vpcId); - cidr = NetUtils.getCidr(vpc.getCidr()); - } - String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), cidr.second()); - if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], guestIpRange[1])) { - throw new InvalidParameterValueException("Invalid ip range: " + ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + guestIpRange[1]); - } + validateIpRange(ipRange, InvalidParameterValueException.class); - long startIp = NetUtils.ip2Long(range[0]); - final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + range[1]; - final String sharedSecret = PasswordGenerator.generatePresharedKey(_pskLength); + String[] range = ipRange.split("-"); - return Transaction.execute(new TransactionCallbackWithException<RemoteAccessVpn, NetworkRuleConflictException>() { - @Override - public RemoteAccessVpn doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + Pair<String, Integer> cidr = null; + + if (networkId != null) { + long ipAddressOwner = ipAddr.getAccountId(); + vpnVO = _remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId); + if (vpnVO != null) { + if (vpnVO.getState() == RemoteAccessVpn.State.Added) { + return vpnVO; + } + + throw new InvalidParameterValueException(String.format("A remote access VPN already exists for the account [%s].", ipAddressOwner)); + } + Network network = _networkMgr.getNetwork(networkId); + if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.Vpn)) { + throw new InvalidParameterValueException("Vpn service is not supported in network id=" + ipAddr.getAssociatedWithNetworkId()); + } + cidr = NetUtils.getCidr(network.getCidr()); + } else { + Vpc vpc = _vpcDao.findById(vpcId); + cidr = NetUtils.getCidr(vpc.getCidr()); + } + + String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), cidr.second()); + if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], guestIpRange[1])) { + throw new InvalidParameterValueException("Invalid ip range: " + ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + guestIpRange[1]); + } + + long startIp = NetUtils.ip2Long(range[0]); + final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + range[1]; + final String sharedSecret = PasswordGenerator.generatePresharedKey(_pskLength); + + return Transaction.execute((TransactionCallbackWithException<RemoteAccessVpn, NetworkRuleConflictException>) status -> { if (vpcId == null) { _rulesMgr.reservePorts(ipAddr, NetUtils.UDP_PROTO, Purpose.Vpn, openFirewallFinal, caller, NetUtils.VPN_PORT, NetUtils.VPN_L2TP_PORT, - NetUtils.VPN_NATT_PORT); + NetUtils.VPN_NATT_PORT); } - RemoteAccessVpnVO vpnVO = - new RemoteAccessVpnVO(ipAddr.getAccountId(), ipAddr.getDomainId(), ipAddr.getAssociatedWithNetworkId(), publicIpId, vpcId, range[0], newIpRange, - sharedSecret); + RemoteAccessVpnVO remoteAccessVpnVO = new RemoteAccessVpnVO(ipAddr.getAccountId(), ipAddr.getDomainId(), ipAddr.getAssociatedWithNetworkId(), + publicIpId, vpcId, range[0], newIpRange, sharedSecret); if (forDisplay != null) { - vpnVO.setDisplay(forDisplay); + remoteAccessVpnVO.setDisplay(forDisplay); } - return _remoteAccessVpnDao.persist(vpnVO); - } - }); + return _remoteAccessVpnDao.persist(remoteAccessVpnVO); + }); + } finally { + _ipAddressDao.releaseFromLockTable(publicIpId); + } } private void validateRemoteAccessVpnConfiguration() throws ConfigurationException {