From ca623530c86eaa19cdb7c05d8b00745706aae612 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 21 Jan 2026 11:39:09 -0300 Subject: [PATCH 1/2] Fix conserve mode for VPC Source NAT and extend rules for VPC tiers --- .../java/com/cloud/network/vpc/VpcOffering.java | 2 ++ .../org/apache/cloudstack/api/ApiConstants.java | 1 + .../user/firewall/CreatePortForwardingRuleCmd.java | 2 +- .../cloudstack/api/response/VpcResponse.java | 8 ++++++++ .../network/lb/LoadBalancingRulesManager.java | 2 +- .../java/com/cloud/network/vpc/VpcOfferingVO.java | 12 ++++++++++++ .../resources/META-INF/db/schema-42100to42200.sql | 2 ++ .../META-INF/db/views/cloud.vpc_offering_view.sql | 1 + .../cloud/network/lb/LoadBalanceRuleHandler.java | 2 +- .../main/java/com/cloud/api/ApiResponseHelper.java | 1 + .../com/cloud/api/query/vo/VpcOfferingJoinVO.java | 8 ++++++++ .../com/cloud/network/IpAddressManagerImpl.java | 14 +++++++++++++- .../network/firewall/FirewallManagerImpl.java | 10 ++++++++-- .../network/lb/LoadBalancingRulesManagerImpl.java | 6 +++--- ui/src/views/network/LoadBalancing.vue | 12 ++++++------ ui/src/views/network/PortForwarding.vue | 13 +++++-------- ui/src/views/network/PublicIpResource.vue | 12 +++++------- 17 files changed, 78 insertions(+), 30 deletions(-) diff --git a/api/src/main/java/com/cloud/network/vpc/VpcOffering.java b/api/src/main/java/com/cloud/network/vpc/VpcOffering.java index 17f49bb3652..f8460223215 100644 --- a/api/src/main/java/com/cloud/network/vpc/VpcOffering.java +++ b/api/src/main/java/com/cloud/network/vpc/VpcOffering.java @@ -84,4 +84,6 @@ public interface VpcOffering extends InternalIdentity, Identity { NetworkOffering.RoutingMode getRoutingMode(); Boolean isSpecifyAsNumber(); + + boolean isConserveMode(); } diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 896031806d5..f5b3205d881 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -980,6 +980,7 @@ public class ApiConstants { public static final String REGION_ID = "regionid"; public static final String VPC_OFF_ID = "vpcofferingid"; public static final String VPC_OFF_NAME = "vpcofferingname"; + public static final String VPC_OFFERING_CONSERVE_MODE = "vpcofferingconservemode"; public static final String NETWORK = "network"; public static final String VPC_ID = "vpcid"; public static final String VPC_NAME = "vpcname"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java index db6b788178a..de9fa741acc 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java @@ -280,7 +280,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P IpAddress ip = _entityMgr.findById(IpAddress.class, getIpAddressId()); Long ntwkId = null; - if (ip.getAssociatedWithNetworkId() != null) { + if (ip.getVpcId() == null && ip.getAssociatedWithNetworkId() != null) { ntwkId = ip.getAssociatedWithNetworkId(); } else { ntwkId = networkId; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/VpcResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/VpcResponse.java index 2648ba83678..acfabb11350 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/VpcResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/VpcResponse.java @@ -73,6 +73,10 @@ public class VpcResponse extends BaseResponseWithAnnotations implements Controll @Param(description = "VPC offering name the VPC is created from", since = "4.13.2") private String vpcOfferingName; + @SerializedName(ApiConstants.VPC_OFFERING_CONSERVE_MODE) + @Param(description = "true if VPC offering is ip conserve mode enabled", since = "4.23") + private Boolean vpcOfferingConserveMode; + @SerializedName(ApiConstants.CREATED) @Param(description = "The date this VPC was created") private Date created; @@ -197,6 +201,10 @@ public class VpcResponse extends BaseResponseWithAnnotations implements Controll this.displayText = displayText; } + public void setVpcOfferingConserveMode(Boolean vpcOfferingConserveMode) { + this.vpcOfferingConserveMode = vpcOfferingConserveMode; + } + public void setCreated(final Date created) { this.created = created; } diff --git a/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java b/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java index 669456cbdcc..d8011e9ade1 100644 --- a/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java @@ -39,7 +39,7 @@ import com.cloud.user.Account; public interface LoadBalancingRulesManager { LoadBalancer createPublicLoadBalancer(String xId, String name, String description, int srcPort, int destPort, long sourceIpId, String protocol, String algorithm, - boolean openFirewall, CallContext caller, String lbProtocol, Boolean forDisplay, String cidrList) throws NetworkRuleConflictException; + boolean openFirewall, CallContext caller, String lbProtocol, Boolean forDisplay, String cidrList, Long networkId) throws NetworkRuleConflictException; boolean removeAllLoadBalanacersForIp(long ipId, Account caller, long callerUserId); diff --git a/engine/schema/src/main/java/com/cloud/network/vpc/VpcOfferingVO.java b/engine/schema/src/main/java/com/cloud/network/vpc/VpcOfferingVO.java index 9320a37bc96..b913468384e 100644 --- a/engine/schema/src/main/java/com/cloud/network/vpc/VpcOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/network/vpc/VpcOfferingVO.java @@ -91,6 +91,9 @@ public class VpcOfferingVO implements VpcOffering { @Column(name = "specify_as_number") private Boolean specifyAsNumber = false; + @Column(name = "conserve_mode") + private boolean conserveMode; + public VpcOfferingVO() { this.uuid = UUID.randomUUID().toString(); } @@ -242,4 +245,13 @@ public class VpcOfferingVO implements VpcOffering { public void setSpecifyAsNumber(Boolean specifyAsNumber) { this.specifyAsNumber = specifyAsNumber; } + + @Override + public boolean isConserveMode() { + return conserveMode; + } + + public void setConserveMode(boolean conserveMode) { + this.conserveMode = conserveMode; + } } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42100to42200.sql b/engine/schema/src/main/resources/META-INF/db/schema-42100to42200.sql index b523016aa3d..4a5db563870 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42100to42200.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42100to42200.sql @@ -87,3 +87,5 @@ CALL `cloud`.`INSERT_EXTENSION_DETAIL_IF_NOT_EXISTS`('MaaS', 'orchestratorrequir CALL `cloud`.`IDEMPOTENT_DROP_UNIQUE_KEY`('counter', 'uc_counter__provider__source__value'); CALL `cloud`.`IDEMPOTENT_ADD_UNIQUE_KEY`('cloud.counter', 'uc_counter__provider__source__value__removed', '(provider, source, value, removed)'); + +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.vpc_offerings','conserve_mode', 'tinyint(1) unsigned NULL DEFAULT 1'); diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.vpc_offering_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.vpc_offering_view.sql index 751d8f91a25..3669bb10122 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.vpc_offering_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.vpc_offering_view.sql @@ -38,6 +38,7 @@ select `vpc_offerings`.`sort_key` AS `sort_key`, `vpc_offerings`.`routing_mode` AS `routing_mode`, `vpc_offerings`.`specify_as_number` AS `specify_as_number`, + `vpc_offerings`.`conserve_mode` AS `conserve_mode`, group_concat(distinct `domain`.`id` separator ',') AS `domain_id`, group_concat(distinct `domain`.`uuid` separator ',') AS `domain_uuid`, group_concat(distinct `domain`.`name` separator ',') AS `domain_name`, diff --git a/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java b/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java index 3df58470fc6..fc167b71c23 100644 --- a/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java +++ b/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java @@ -363,7 +363,7 @@ public class LoadBalanceRuleHandler { lb.setSourceIpAddressId(ipId); result = _lbMgr.createPublicLoadBalancer(lb.getXid(), lb.getName(), lb.getDescription(), lb.getSourcePortStart(), lb.getDefaultPortStart(), ipId.longValue(), - lb.getProtocol(), lb.getAlgorithm(), false, CallContext.current(), lb.getLbProtocol(), true, null); + lb.getProtocol(), lb.getAlgorithm(), false, CallContext.current(), lb.getLbProtocol(), true, null, networkId); } catch (final NetworkRuleConflictException e) { logger.warn("Failed to create LB rule, not continuing with ELB deployment"); if (newIp) { diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index ce794cf5388..ba4a5529b03 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -3499,6 +3499,7 @@ public class ApiResponseHelper implements ResponseGenerator { if (voff != null) { response.setVpcOfferingId(voff.getUuid()); response.setVpcOfferingName(voff.getName()); + response.setVpcOfferingConserveMode(voff.isConserveMode()); } response.setCidr(vpc.getCidr()); response.setRestartRequired(vpc.isRestartRequired()); diff --git a/server/src/main/java/com/cloud/api/query/vo/VpcOfferingJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/VpcOfferingJoinVO.java index 4e0707edf88..9d65c19479f 100644 --- a/server/src/main/java/com/cloud/api/query/vo/VpcOfferingJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/VpcOfferingJoinVO.java @@ -112,6 +112,9 @@ public class VpcOfferingJoinVO implements VpcOffering { @Column(name = "specify_as_number") private Boolean specifyAsNumber = false; + @Column(name = "conserve_mode") + private boolean conserveMode; + public VpcOfferingJoinVO() { } @@ -178,6 +181,11 @@ public class VpcOfferingJoinVO implements VpcOffering { return specifyAsNumber; } + @Override + public boolean isConserveMode() { + return conserveMode; + } + public void setSpecifyAsNumber(Boolean specifyAsNumber) { this.specifyAsNumber = specifyAsNumber; } diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 8d179735a4d..517ffb82c0d 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -1543,6 +1543,14 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage return ipaddr; } + protected IPAddressVO getExistingSourceNatInVPC(Long vpcId) { + List ips = _ipAddressDao.listByAssociatedVpc(vpcId, true); + if (CollectionUtils.isEmpty(ips)) { + return null; + } + return ips.get(0); + } + protected IPAddressVO getExistingSourceNatInNetwork(long ownerId, Long networkId) { List addrs; Network guestNetwork = _networksDao.findById(networkId); @@ -1723,7 +1731,11 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage NetworkOffering offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); boolean sharedSourceNat = offering.isSharedSourceNat(); boolean isSourceNat = false; - if (!sharedSourceNat) { + if (network.getVpcId() != null) { + // For VPCs: Check if the VPC Source NAT IP address is the same we are associating + IPAddressVO vpcSourceNatIpAddress = getExistingSourceNatInVPC(network.getVpcId()); + isSourceNat = vpcSourceNatIpAddress != null && vpcSourceNatIpAddress.getId() == ipToAssoc.getId(); + } else if (!sharedSourceNat) { if (getExistingSourceNatInNetwork(owner.getId(), network.getId()) == null) { if (network.getGuestType() == GuestType.Isolated && network.getVpcId() == null && !ipToAssoc.isPortable()) { isSourceNat = 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 4aee5fef48a..5232fbd7f66 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -395,6 +395,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, assert (rules.size() >= 1); } + NetworkVO newRuleNetwork = _networkDao.findById(newRule.getNetworkId()); + if (newRuleNetwork == null) { + throw new InvalidParameterValueException("Unable to create firewall rule as cannot find network by id=" + newRule.getNetworkId()); + } + boolean isNewRuleOnVpcNetwork = newRuleNetwork.getVpcId() != null; + for (FirewallRuleVO rule : rules) { if (rule.getId() == newRule.getId()) { continue; // Skips my own rule. @@ -442,8 +448,8 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, } } - // Checking if the rule applied is to the same network that is passed in the rule. - if (rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) { + // Checking if the rule applied is to the same network that is passed in the rule. (except for VPC networks) + if (!isNewRuleOnVpcNetwork && rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) { throw new NetworkRuleConflictException("New rule is for a different network than what's specified in rule " + rule.getXid()); } 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 c6aeeaf2db5..f6235fdc6a0 100644 --- a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -1761,7 +1761,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements } result = createPublicLoadBalancer(xId, name, description, srcPortStart, defPortStart, ipVO.getId(), protocol, algorithm, openFirewall, CallContext.current(), - lbProtocol, forDisplay, cidrString); + lbProtocol, forDisplay, cidrString, networkId); } catch (Exception ex) { logger.warn("Failed to create load balancer due to ", ex); if (ex instanceof NetworkRuleConflictException) { @@ -1824,7 +1824,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements @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 { + final Boolean forDisplay, String cidrList, Long networkIdParam) throws NetworkRuleConflictException { if (!NetUtils.isValidPort(destPort)) { throw new InvalidParameterValueException("privatePort is an invalid value: " + destPort); } @@ -1853,7 +1853,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements _accountMgr.checkAccess(caller.getCallingAccount(), null, true, ipAddr); - final Long networkId = ipAddr.getAssociatedWithNetworkId(); + final Long networkId = ipAddr.getVpcId() == null ? ipAddr.getAssociatedWithNetworkId() : networkIdParam; if (networkId == null) { InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule ; specified sourceip id is not associated with any network"); diff --git a/ui/src/views/network/LoadBalancing.vue b/ui/src/views/network/LoadBalancing.vue index ad091b218a8..aac72b94184 100644 --- a/ui/src/views/network/LoadBalancing.vue +++ b/ui/src/views/network/LoadBalancing.vue @@ -97,7 +97,7 @@ {{ $t('label.add') }} -
+
{{ $t('label.select.tier') }}
{{ $t('label.add') }} @@ -487,10 +487,10 @@ >
+ v-if="'vpcid' in resource"> {{ $t('label.select.tier') }} { if (!response || !response.listnicsresponse || !response.listnicsresponse.nic[0]) return const newItem = [] @@ -1850,7 +1850,7 @@ export default { this.vmCount = 0 this.vms = [] this.addVmModalLoading = true - const networkId = ('vpcid' in this.resource && !('associatednetworkid' in this.resource)) ? this.selectedTier : this.resource.associatednetworkid + const networkId = ('vpcid' in this.resource) ? this.selectedTier : this.resource.associatednetworkid if (!networkId) { this.addVmModalLoading = false return @@ -1999,7 +1999,7 @@ export default { } const networkId = this.selectedTierForAutoScaling != null ? this.selectedTierForAutoScaling - : ('vpcid' in this.resource && !('associatednetworkid' in this.resource)) ? this.selectedTier : this.resource.associatednetworkid + : ('vpcid' in this.resource) ? this.selectedTier : this.resource.associatednetworkid postAPI('createLoadBalancerRule', { openfirewall: false, networkid: networkId, diff --git a/ui/src/views/network/PortForwarding.vue b/ui/src/views/network/PortForwarding.vue index 8ab6559b12c..471f94d6995 100644 --- a/ui/src/views/network/PortForwarding.vue +++ b/ui/src/views/network/PortForwarding.vue @@ -216,10 +216,10 @@ @cancel="closeModal">
+ v-if="'vpcid' in resource"> {{ $t('label.select.tier') }} { if (!response.listnicsresponse.nic || response.listnicsresponse.nic.length < 1) return const nic = response.listnicsresponse.nic[0] @@ -808,7 +805,7 @@ export default { this.vmCount = 0 this.vms = [] this.addVmModalLoading = true - const networkId = ('vpcid' in this.resource && !('associatednetworkid' in this.resource)) ? this.selectedTier : this.resource.associatednetworkid + const networkId = ('vpcid' in this.resource) ? this.selectedTier : this.resource.associatednetworkid if (!networkId) { this.addVmModalLoading = false return diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 7c25e1c32ba..82511dfadf6 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -135,12 +135,6 @@ export default { return } if (this.resource && this.resource.vpcid) { - // VPC IPs with source nat have only VPN - if (this.resource.issourcenat) { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) - return - } - // VPC IPs with static nat have nothing if (this.resource.isstaticnat) { if (this.resource.virtualmachinetype === 'DomainRouter') { @@ -153,9 +147,13 @@ export default { let tabs = this.$route.meta.tabs.filter(tab => tab.name !== 'firewall') const network = await this.fetchNetwork() - if (network && network.networkofferingconservemode) { + if ((network && network.networkofferingconservemode) || !network && this.resource.issourcenat) { this.tabs = tabs return + } else if (this.resource.issourcenat) { + // VPC IPs with Source Nat have only VPN when conserve_mode = false + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + return } this.portFWRuleCount = await this.fetchPortFWRule() From 5ade2b303f41e064ab56394a3ed0318d5c7b194d Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 21 Jan 2026 15:57:33 -0300 Subject: [PATCH 2/2] Fix unit test --- .../com/cloud/network/firewall/FirewallManagerTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java index f94fd0c0c3c..3c663dde92d 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -24,6 +24,8 @@ import com.cloud.network.Network; import com.cloud.network.NetworkModel; import com.cloud.network.NetworkRuleApplier; import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkVO; import com.cloud.network.element.FirewallServiceProvider; import com.cloud.network.element.VirtualRouterElement; import com.cloud.network.element.VpcVirtualRouterElement; @@ -43,6 +45,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -76,6 +79,8 @@ public class FirewallManagerTest { IpAddressManager _ipAddrMgr; @Mock FirewallRulesDao _firewallDao; + @Mock + NetworkDao _networkDao; @Spy @InjectMocks @@ -196,6 +201,10 @@ public class FirewallManagerTest { FirewallRule newRule4 = new FirewallRuleVO("newRule4", 3L, 15, 25, "TCP", 1, 2, 1, Purpose.Firewall, sString, dString2, null, null, null, FirewallRule.TrafficType.Egress); + NetworkVO networkVO = Mockito.mock(NetworkVO.class); + when(firewallMgr._networkDao.findById(1L)).thenReturn(networkVO); + when(networkVO.getVpcId()).thenReturn(null); + try { firewallMgr.detectRulesConflict(newRule1); firewallMgr.detectRulesConflict(newRule2);