Skip to content

Handle public IP race conditions #9234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sureshanaparti merged 3 commits into from
Jun 29, 2024
Merged

Handle public IP race conditions #9234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c40655
Lock public IP
Jun 11, 2024
86fe58c
Release IP if ID is not null
Jun 12, 2024
30b32e9
Fix NPEs
Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 43 additions & 34 deletions server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,50 +725,59 @@ protected boolean cleanupIpResources(long ipId, long userId, Account caller) {
@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;
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/com/cloud/network/NetworkModelImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,10 @@ public boolean checkIpForService(IpAddress userIp, Service service, Long network
}

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;
Expand Down
95 changes: 53 additions & 42 deletions server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,57 +194,54 @@ public FirewallRule createIngressFirewallRule(FirewallRule rule) throws NetworkR
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);
Expand All @@ -261,8 +258,12 @@ public FirewallRuleVO doInTransaction(TransactionStatus status) throws NetworkRu
CallContext.current().putContextParameter(FirewallRule.class, newRule.getId());

return newRule;
});
} finally {
if (ipAddrId != null) {
_ipAddressDao.releaseFromLockTable(ipAddrId);
}
});
}
}

@Override
Expand Down Expand Up @@ -668,9 +669,19 @@ public boolean applyIngressFwRules(long ipId, Account caller) throws ResourceUna
}

@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
Expand Down
Loading