Dogface2k commented on code in PR #13766:
URL: https://github.com/apache/cloudstack/pull/13766#discussion_r3702473578
##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java:
##########
@@ -941,4 +987,283 @@ 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;
+ }
+ if (vpcManager != null) {
+ return vpcManager.isProviderSupportServiceInVpc(vpc.getId(),
Network.Service.Vpn, Network.Provider.Nsx);
+ }
+ return
Objects.nonNull(vpcOfferingServiceMapDao.findByServiceProviderAndOfferingId(
+ Network.Service.Vpn.getName(), Network.Provider.Nsx.getName(),
vpc.getVpcOfferingId()));
+ }
+
+ protected boolean isVpnProvidedByNsx(Vpc vpc, Site2SiteVpnGateway gateway)
{
+ return vpc != null && ownsVpnGateway(gateway);
+ }
+
+ @Override
+ public IpAddress acquireVpnGatewayIp(Vpc vpc, IpAddress requestedIp) {
+ if (!isVpnProvidedByNsx(vpc)) {
+ return null;
+ }
+ IPAddressVO ip;
+ boolean autoAcquired = false;
+ if (Objects.nonNull(requestedIp)) {
+ ip = validateRequestedVpnGatewayIp(vpc, requestedIp);
+ } else {
+ ip = allocateVpnGatewayIp(vpc);
+ autoAcquired = true;
+ }
+ if (!autoAcquired) {
+ try {
+ // Mark ownership first so ambiguous NSX responses remain
recoverable.
+ userIpAddressDetailsDao.addDetail(ip.getId(),
NSX_VPN_GATEWAY_IP_DETAIL, "false", false);
+ } 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());
+ 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) {
+ 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 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:
@DaanHoogland Thanks I traced that specific possibility. Reset, reconnect,
restart and stop operate only on the VPN connection/session and never call
`releaseVpnGatewayIp`. The release path is reached only during gateway-creation
rollback, explicit gateway deletion, or VPC teardown. The background VPC
teardown runs as a `ManagedContextRunnable` using the system user/account, and
`CallContext.current()` already registers a system context when none exists.
API-triggered deletion retains its propagated caller context. The ownership
marker is also removed only after disassociation succeeds, so a failed release
remains recoverable. I therefore don't think an additional fallback or
provider-SPI signature change is warranted here. (false positive)
--
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]
