Dogface2k commented on code in PR #13766:
URL: https://github.com/apache/cloudstack/pull/13766#discussion_r4531521463
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -941,4 +988,341 @@ public List<Class<?>> getCommands() {
public boolean updateVpcSourceNatIp(Vpc vpc, IpAddress address) {
return nsxService.updateVpcSourceNatIp(vpc, address);
}
+
+ protected boolean isVpnProvidedByNsx(Vpc vpc) {
+ if (Objects.isNull(vpc)) {
+ return false;
+ }
+ return vpcManager.isProviderSupportServiceInVpc(vpc.getId(),
Network.Service.Vpn, Network.Provider.Nsx);
+ }
+
+ protected boolean isVpnProvidedByNsx(Vpc vpc, Site2SiteVpnGateway gateway)
{
+ return vpc != null && ownsVpnGateway(gateway);
+ }
+
+ private void validateVpnGatewayOwnedByNsx(Vpc vpc, Site2SiteVpnGateway
gateway) {
+ if (!isVpnProvidedByNsx(vpc, gateway)) {
+ throw new CloudRuntimeException(String.format(
+ "NSX does not own Site-to-Site VPN gateway %s of VPC %s",
gateway.getId(), vpc.getName()));
+ }
+ }
+
+ @Override
+ public IpAddress acquireVpnGatewayIp(Vpc vpc, IpAddress requestedIp) {
+ if (!isVpnProvidedByNsx(vpc)) {
+ return null;
+ }
+ IPAddressVO ip;
+ boolean autoAcquired = false;
+ boolean reconcileExistingService = false;
+ UserIpAddressDetailVO ownershipMarker = null;
+ if (Objects.nonNull(requestedIp)) {
+ ip = validateRequestedVpnGatewayIp(vpc, requestedIp);
+ ownershipMarker = userIpAddressDetailsDao.findDetail(ip.getId(),
NSX_VPN_GATEWAY_IP_DETAIL);
+ if (ownershipMarker != null) {
+ autoAcquired = isAutoAcquiredVpnGatewayIp(ownershipMarker, ip,
vpc);
+ reconcileExistingService = true;
+ }
+ } else {
+ Pair<IPAddressVO, UserIpAddressDetailVO> retainedIp =
findRetainedVpnGatewayIp(vpc);
+ if (retainedIp != null) {
+ ip = retainedIp.first();
+ ownershipMarker = retainedIp.second();
+ autoAcquired = isAutoAcquiredVpnGatewayIp(ownershipMarker, ip,
vpc);
+ reconcileExistingService = true;
+ } else {
+ ip = allocateVpnGatewayIp(vpc);
+ autoAcquired = true;
+ }
+ }
+ boolean requestedIpOwnershipMarkerAdded = false;
+ if (!autoAcquired && ownershipMarker == null) {
+ try {
+ userIpAddressDetailsDao.addDetail(ip.getId(),
NSX_VPN_GATEWAY_IP_DETAIL, "false", false);
+ requestedIpOwnershipMarkerAdded = true;
+ } catch (Exception e) {
+ throw new CloudRuntimeException(String.format(
+ "Failed to record NSX VPN ownership for requested IP
%s of VPC %s",
+ ip.getAddress(), vpc.getName()), e);
+ }
+ }
+ boolean endpointMayBeInUse = true;
+ try {
+ NsxVpnGatewayResult result = nsxService.createVpnGateway(vpc,
ip.getAddress().addr(), reconcileExistingService);
+ endpointMayBeInUse = result.isEndpointMayBeInUse();
+ if (!result.isSuccessful()) {
+ throw new CloudRuntimeException(String.format("The NSX VPN
gateway service for VPC %s was not created: the provider returned an
unsuccessful answer",
+ vpc.getName()));
+ }
+ } catch (Exception e) {
+ if (autoAcquired && !endpointMayBeInUse) {
+ try {
+ releaseAutoAcquiredVpnGatewayIp(ip);
+ } catch (Exception cleanupException) {
+ logger.warn("Failed to release the auto-acquired VPN
gateway IP {} of VPC {} after creation failed: {}",
+ ip.getAddress(), vpc.getName(),
cleanupException.getMessage());
+ }
+ } else if (autoAcquired) {
+ logger.warn("Retaining auto-acquired VPN gateway IP {} for VPC
{} because the NSX endpoint may still be using it",
+ ip.getAddress(), vpc.getName());
+ } else if (!endpointMayBeInUse && requestedIpOwnershipMarkerAdded)
{
+ try {
+ userIpAddressDetailsDao.removeDetail(ip.getId(),
NSX_VPN_GATEWAY_IP_DETAIL);
+ } catch (Exception cleanupException) {
+ logger.warn("Failed to remove the NSX VPN ownership marker
from requested IP {} of VPC {} after gateway creation failed: {}",
+ ip.getAddress(), vpc.getName(),
cleanupException.getMessage());
+ }
+ }
+ throw new CloudRuntimeException(String.format("Failed to create
the NSX VPN gateway for VPC %s: %s",
+ vpc.getName(), e.getMessage()), e);
+ }
+ return ip;
+ }
+
+ private Pair<IPAddressVO, UserIpAddressDetailVO>
findRetainedVpnGatewayIp(Vpc vpc) {
+ Pair<IPAddressVO, UserIpAddressDetailVO> retainedIp = null;
+ List<IPAddressVO> nonSourceNatIps =
ipAddressDao.listByAssociatedVpc(vpc.getId(), false);
+ if (CollectionUtils.isNullOrEmpty(nonSourceNatIps)) {
+ return null;
+ }
+ for (IPAddressVO candidate : nonSourceNatIps) {
+ UserIpAddressDetailVO ownershipMarker =
userIpAddressDetailsDao.findDetail(
+ candidate.getId(), NSX_VPN_GATEWAY_IP_DETAIL);
+ if (ownershipMarker == null) {
+ continue;
+ }
+ if (retainedIp != null) {
+ throw new CloudRuntimeException(String.format(
+ "VPC %s has more than one IP retained for an NSX VPN
gateway; refusing ambiguous recovery",
+ vpc.getName()));
+ }
+ retainedIp = new Pair<>(validateRequestedVpnGatewayIp(vpc,
candidate), ownershipMarker);
+ }
+ return retainedIp;
+ }
+
+ private boolean isAutoAcquiredVpnGatewayIp(UserIpAddressDetailVO
ownershipMarker, IPAddressVO ip, Vpc vpc) {
+ if ("true".equalsIgnoreCase(ownershipMarker.getValue())) {
+ return true;
+ }
+ if ("false".equalsIgnoreCase(ownershipMarker.getValue())) {
+ return false;
+ }
+ throw new CloudRuntimeException(String.format(
+ "IP %s retained for the NSX VPN gateway of VPC %s has invalid
ownership state '%s'",
+ ip.getAddress(), vpc == null ? ip.getVpcId() : vpc.getName(),
ownershipMarker.getValue()));
+ }
+
+ private IPAddressVO validateRequestedVpnGatewayIp(Vpc vpc, IpAddress
requestedIp) {
+ IPAddressVO ip = ipAddressDao.findById(requestedIp.getId());
+ if (Objects.isNull(ip) || !Objects.equals(ip.getVpcId(), vpc.getId())
+ || !ip.readyToUse() || ip.getRemoved() != null ||
ip.getAddress() == null) {
+ throw new InvalidParameterValueException(String.format(
+ "The requested IP id %s is not an allocated, active IP
associated to the VPC %s",
+ requestedIp.getId(), vpc.getName()));
+ }
+ if (ip.isSourceNat() || ip.isForSystemVms()) {
+ throw new InvalidParameterValueException(String.format(
+ "The requested IP %s cannot be used as the VPN gateway IP
as it is a source NAT or system IP", ip.getAddress().addr()));
+ }
+ if (ip.isOneToOneNat() ||
!firewallRulesDao.listByIpAndNotRevoked(ip.getId()).isEmpty()
+ ||
!portForwardingRulesDao.listByIpAndNotRevoked(ip.getId()).isEmpty()
+ || !loadBalancerDao.listByIpAddress(ip.getId()).isEmpty()) {
+ throw new InvalidParameterValueException(String.format(
+ "The requested IP %s cannot be used as the VPN gateway IP
as it is already in use by static NAT or network rules",
ip.getAddress().addr()));
+ }
+ return ip;
+ }
+
+ private IPAddressVO allocateVpnGatewayIp(Vpc vpc) {
+ Account owner = accountMgr.getAccount(vpc.getAccountId());
+ DataCenterVO zone = dataCenterDao.findById(vpc.getZoneId());
+ IpAddress allocatedIp = null;
+ try {
+ allocatedIp = ipAddressManager.allocateIp(owner, false,
CallContext.current().getCallingAccount(),
+ CallContext.current().getCallingUser(), zone, null, null);
+ vpcService.associateIPToVpc(allocatedIp.getId(), vpc.getId());
+ userIpAddressDetailsDao.addDetail(allocatedIp.getId(),
NSX_VPN_GATEWAY_IP_DETAIL, "true", false);
+ IPAddressVO ip = ipAddressDao.findById(allocatedIp.getId());
+ if (ip == null) {
+ throw new CloudRuntimeException(String.format("The allocated
VPN gateway IP %s could not be loaded after association",
+ allocatedIp.getId()));
+ }
+ if (ip.isSourceNat()) {
+ throw new CloudRuntimeException(String.format(
+ "The allocated IP %s became a source NAT IP when it
was associated to VPC %s; it cannot be used as a dedicated VPN endpoint",
+ ip.getAddress(), vpc.getName()));
+ }
+ return ip;
+ } catch (Exception e) {
+ // do not leak the IP when associating or tagging it fails after
allocation succeeded
+ if (Objects.nonNull(allocatedIp)) {
+ IPAddressVO ipToRelease =
ipAddressDao.findById(allocatedIp.getId());
+ if (Objects.nonNull(ipToRelease)) {
+ try {
+ releaseAutoAcquiredVpnGatewayIp(ipToRelease);
+ } catch (Exception releaseException) {
+ logger.warn("Failed to release the IP {} allocated for
the VPN gateway of VPC {}: {}",
+ ipToRelease.getAddress().addr(),
vpc.getName(), releaseException.getMessage());
+ }
+ } else {
+ try {
+
ipAddressManager.disassociatePublicIpAddress(allocatedIp,
CallContext.current().getCallingUserId(),
+ CallContext.current().getCallingAccount());
+ } catch (Exception releaseException) {
+ logger.warn("Failed to release allocated VPN gateway
IP {} of VPC {} after its database row disappeared: {}",
+ allocatedIp.getId(), vpc.getName(),
releaseException.getMessage());
+ }
+ }
+ }
+ throw new CloudRuntimeException(String.format("Failed to acquire
an IP for the VPN gateway of VPC %s: %s",
+ vpc.getName(), e.getMessage()), e);
+ }
+ }
+
+ private void releaseAutoAcquiredVpnGatewayIp(IPAddressVO ip) {
+ boolean disassociated =
ipAddressManager.disassociatePublicIpAddress(ip,
CallContext.current().getCallingUserId(),
+ CallContext.current().getCallingAccount());
+ if (!disassociated) {
+ throw new CloudRuntimeException(String.format("Failed to
disassociate auto-acquired VPN gateway IP %s", ip.getAddress()));
+ }
+ userIpAddressDetailsDao.removeDetail(ip.getId(),
NSX_VPN_GATEWAY_IP_DETAIL);
Review Comment:
Addressed in 6eee828. IPAddressDaoImpl.unassignIpAddress already removes the
IP details; the redundant second removal after disassociation is gone. Release
now locks and reloads the IP and checks the original VPC/account before
changing the allocation or marker. The gateway's provider is also retained
before cleanup, so a partial failure cannot redirect a later retry.
Operator-supplied IPs retain their allocation. Regression tests preserve both
auto- and operator-owned later allocations; removing the allocation guard makes
the test fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
